Skip to content

Commit

Permalink
ignore: partially revert symlink loop check optimization
Browse files Browse the repository at this point in the history
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
  • Loading branch information
BurntSushi committed Oct 22, 2017
1 parent 311ccb1 commit 5714dbd
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 23 deletions.
22 changes: 0 additions & 22 deletions ignore/src/dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use std::sync::{Arc, RwLock};
use gitignore::{self, Gitignore, GitignoreBuilder};
use pathutil::{is_hidden, strip_prefix};
use overrides::{self, Override};
use same_file::Handle;
use types::{self, Types};
use {Error, Match, PartialErrorBuilder};

Expand Down Expand Up @@ -96,9 +95,6 @@ struct IgnoreInner {
compiled: Arc<RwLock<HashMap<OsString, Ignore>>>,
/// The path to the directory that this matcher was built from.
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).
overrides: Arc<Override>,
/// A file type matcher.
Expand Down Expand Up @@ -135,11 +131,6 @@ impl Ignore {
&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.
pub fn is_root(&self) -> bool {
self.0.parent.is_none()
Expand Down Expand Up @@ -246,17 +237,9 @@ impl Ignore {
errs.maybe_push(err);
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 {
compiled: self.0.compiled.clone(),
dir: dir.to_path_buf(),
handle: Arc::new(handle),
overrides: self.0.overrides.clone(),
types: self.0.types.clone(),
parent: Some(self.clone()),
Expand Down Expand Up @@ -467,14 +450,9 @@ impl IgnoreBuilder {
}
gi
};
let handle = match Handle::from_path(&self.dir) {
Ok(handle) => Some(handle),
Err(_) => None,
};
Ignore(Arc::new(IgnoreInner {
compiled: Arc::new(RwLock::new(HashMap::new())),
dir: self.dir.clone(),
handle: Arc::new(handle),
overrides: self.overrides.clone(),
types: self.types.clone(),
parent: None,
Expand Down
5 changes: 4 additions & 1 deletion ignore/src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1312,7 +1312,10 @@ fn check_symlink_loop(
Error::from(err).with_path(child_path).with_depth(child_depth)
})?;
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 {
ancestor: ig.path().to_path_buf(),
child: child_path.to_path_buf(),
Expand Down

0 comments on commit 5714dbd

Please sign in to comment.