ignore: partially revert symlink loop check optimization

This optimization wasn't tested too carefully, and it seems to result
in a massive amount of file handles open simultaneously. This is likely
a result of the parallel iterator, where many directories are being
traversed simultaneously.

Fixes #648
This commit is contained in:
Andrew Gallant 2017-10-22 10:26:50 -04:00
parent 311ccb1f6b
commit 5714dbde09
No known key found for this signature in database
GPG Key ID: B2E3A4923F8B0D44
2 changed files with 4 additions and 23 deletions

View File

@ -21,7 +21,6 @@ use std::sync::{Arc, RwLock};
use gitignore::{self, Gitignore, GitignoreBuilder}; use gitignore::{self, Gitignore, GitignoreBuilder};
use pathutil::{is_hidden, strip_prefix}; use pathutil::{is_hidden, strip_prefix};
use overrides::{self, Override}; use overrides::{self, Override};
use same_file::Handle;
use types::{self, Types}; use types::{self, Types};
use {Error, Match, PartialErrorBuilder}; use {Error, Match, PartialErrorBuilder};
@ -96,9 +95,6 @@ struct IgnoreInner {
compiled: Arc<RwLock<HashMap<OsString, Ignore>>>, compiled: Arc<RwLock<HashMap<OsString, Ignore>>>,
/// The path to the directory that this matcher was built from. /// The path to the directory that this matcher was built from.
dir: PathBuf, dir: PathBuf,
/// An open handle to the directory, for checking symlink loops in the
/// parallel iterator.
handle: Arc<Option<Handle>>,
/// An override matcher (default is empty). /// An override matcher (default is empty).
overrides: Arc<Override>, overrides: Arc<Override>,
/// A file type matcher. /// A file type matcher.
@ -135,11 +131,6 @@ impl Ignore {
&self.0.dir &self.0.dir
} }
/// Return a handle to the directory of this matcher.
pub fn handle(&self) -> Option<&Handle> {
(*self.0.handle).as_ref()
}
/// Return true if this matcher has no parent. /// Return true if this matcher has no parent.
pub fn is_root(&self) -> bool { pub fn is_root(&self) -> bool {
self.0.parent.is_none() self.0.parent.is_none()
@ -246,17 +237,9 @@ impl Ignore {
errs.maybe_push(err); errs.maybe_push(err);
m m
}; };
let handle = match Handle::from_path(dir) {
Ok(handle) => Some(handle),
Err(err) => {
errs.push(Error::from(err).with_path(dir));
None
}
};
let ig = IgnoreInner { let ig = IgnoreInner {
compiled: self.0.compiled.clone(), compiled: self.0.compiled.clone(),
dir: dir.to_path_buf(), dir: dir.to_path_buf(),
handle: Arc::new(handle),
overrides: self.0.overrides.clone(), overrides: self.0.overrides.clone(),
types: self.0.types.clone(), types: self.0.types.clone(),
parent: Some(self.clone()), parent: Some(self.clone()),
@ -467,14 +450,9 @@ impl IgnoreBuilder {
} }
gi gi
}; };
let handle = match Handle::from_path(&self.dir) {
Ok(handle) => Some(handle),
Err(_) => None,
};
Ignore(Arc::new(IgnoreInner { Ignore(Arc::new(IgnoreInner {
compiled: Arc::new(RwLock::new(HashMap::new())), compiled: Arc::new(RwLock::new(HashMap::new())),
dir: self.dir.clone(), dir: self.dir.clone(),
handle: Arc::new(handle),
overrides: self.overrides.clone(), overrides: self.overrides.clone(),
types: self.types.clone(), types: self.types.clone(),
parent: None, parent: None,

View File

@ -1312,7 +1312,10 @@ fn check_symlink_loop(
Error::from(err).with_path(child_path).with_depth(child_depth) Error::from(err).with_path(child_path).with_depth(child_depth)
})?; })?;
for ig in ig_parent.parents().take_while(|ig| !ig.is_absolute_parent()) { for ig in ig_parent.parents().take_while(|ig| !ig.is_absolute_parent()) {
if ig.handle().map_or(true, |parent| parent == &hchild) { let h = Handle::from_path(ig.path()).map_err(|err| {
Error::from(err).with_path(child_path).with_depth(child_depth)
})?;
if hchild == h {
return Err(Error::Loop { return Err(Error::Loop {
ancestor: ig.path().to_path_buf(), ancestor: ig.path().to_path_buf(),
child: child_path.to_path_buf(), child: child_path.to_path_buf(),