about summary refs log tree commit diff
path: root/src/libstd
diff options
context:
space:
mode:
authorAaron Turon <aturon@mozilla.com>2014-04-23 17:09:58 -0700
committerAaron Turon <aturon@mozilla.com>2014-04-24 10:34:13 -0700
commitb536d2bb763d478dbf96c035dbd5b68b5ff639b9 (patch)
tree2819a98060d80a93b68fc455bcb40c824f98d7ae /src/libstd
parent3157c3e95b3f8d217e4a1e1e7f93939866e74472 (diff)
downloadrust-b536d2bb763d478dbf96c035dbd5b68b5ff639b9.tar.gz
rust-b536d2bb763d478dbf96c035dbd5b68b5ff639b9.zip
fix O(n^2) perf bug for std::io::fs::walk_dir
The `walk_dir` iterator was simulating a queue using a vector (in particular, using `shift`),
leading to O(n^2) performance. Since the order was not well-specified (see issue #13411),
the simplest fix is to use the vector as a stack (and thus yield a depth-first traversal).
This patch does exactly that.  It leaves the order as originally specified -- "some top-down
order" -- and adds a test to ensure a top-down traversal.

Note that the underlying `readdir` function does not specify any particular order, nor
does the system call it uses.

Closes #13411.
Diffstat (limited to 'src/libstd')
-rw-r--r--src/libstd/io/fs.rs31
1 files changed, 29 insertions, 2 deletions
diff --git a/src/libstd/io/fs.rs b/src/libstd/io/fs.rs
index 6bc32d0ed7b..a9c493c284d 100644
--- a/src/libstd/io/fs.rs
+++ b/src/libstd/io/fs.rs
@@ -491,7 +491,8 @@ pub fn readdir(path: &Path) -> IoResult<Vec<Path>> {
 
 /// Returns an iterator which will recursively walk the directory structure
 /// rooted at `path`. The path given will not be iterated over, and this will
-/// perform iteration in a top-down order.
+/// perform iteration in some top-down order.  The contents of unreadable
+/// subdirectories are ignored.
 pub fn walk_dir(path: &Path) -> IoResult<Directories> {
     Ok(Directories { stack: try!(readdir(path)) })
 }
@@ -503,7 +504,7 @@ pub struct Directories {
 
 impl Iterator<Path> for Directories {
     fn next(&mut self) -> Option<Path> {
-        match self.stack.shift() {
+        match self.stack.pop() {
             Some(path) => {
                 if path.is_dir() {
                     match readdir(&path) {
@@ -970,6 +971,32 @@ mod test {
         check!(rmdir(dir));
     })
 
+    iotest!(fn file_test_walk_dir() {
+        let tmpdir = tmpdir();
+        let dir = &tmpdir.join("walk_dir");
+        check!(mkdir(dir, io::UserRWX));
+
+        let dir1 = &dir.join("01/02/03");
+        check!(mkdir_recursive(dir1, io::UserRWX));
+        check!(File::create(&dir1.join("04")));
+
+        let dir2 = &dir.join("11/12/13");
+        check!(mkdir_recursive(dir2, io::UserRWX));
+        check!(File::create(&dir2.join("14")));
+
+        let mut files = check!(walk_dir(dir));
+        let mut cur = [0u8, .. 2];
+        for f in files {
+            let stem = f.filestem_str().unwrap();
+            let root = stem[0] - ('0' as u8);
+            let name = stem[1] - ('0' as u8);
+            assert!(cur[root as uint] < name);
+            cur[root as uint] = name;
+        }
+
+        check!(rmdir_recursive(dir));
+    })
+
     iotest!(fn recursive_mkdir() {
         let tmpdir = tmpdir();
         let dir = tmpdir.join("d1/d2");