Skip to content

Commit eea18ba

Browse files
committed
Cache file names in fill_todo
Background: While working with cargo, I've noticed that it takes ~30s to cargo clean -p with large enough target directory (~200GB). With a profiler, it turned out that most of the time was spent retrieving paths for removal in https://github.com/rust-lang/cargo/blob/eee4ea2f5a5fa1ae184a44675315548ec932a15c/src/cargo/ops/cargo_clean.rs#L319 (and not actually removing the files). Change description: In call to .sort_by, we repetitively parse the paths to obtain file names for comparison. This commit caches file names in PathWrapper object, akin to #135 that did so for dir info. For my use case, a cargo build using that branch takes ~14s to clean files instead of previous 30s (I've measured against main branch of this directory, to account for changes made since 0.3.1). Still not ideal, but hey, we're shaving 50% of time off for a bit heavier memory use.
1 parent 7a17f11 commit eea18ba

File tree

1 file changed

+23
-8
lines changed

1 file changed

+23
-8
lines changed

src/lib.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ doctest!("../README.md");
7272
use std::cmp;
7373
use std::cmp::Ordering;
7474
use std::error::Error;
75+
use std::ffi::OsString;
7576
use std::fmt;
7677
use std::fs;
7778
use std::fs::DirEntry;
@@ -329,10 +330,11 @@ impl fmt::Display for GlobError {
329330
struct PathWrapper {
330331
path: PathBuf,
331332
is_directory: bool,
333+
file_name: Option<OsString>,
332334
}
333335

334336
impl PathWrapper {
335-
fn from_dir_entry(path: PathBuf, e: DirEntry) -> Self {
337+
fn from_dir_entry(path: PathBuf, file_name: OsString, e: DirEntry) -> Self {
336338
let is_directory = e
337339
.file_type()
338340
.ok()
@@ -347,11 +349,20 @@ impl PathWrapper {
347349
})
348350
.or_else(|| fs::metadata(&path).map(|m| m.is_dir()).ok())
349351
.unwrap_or(false);
350-
Self { path, is_directory }
352+
Self {
353+
path,
354+
is_directory,
355+
file_name: Some(file_name),
356+
}
351357
}
352358
fn from_path(path: PathBuf) -> Self {
353359
let is_directory = fs::metadata(&path).map(|m| m.is_dir()).unwrap_or(false);
354-
Self { path, is_directory }
360+
let file_name = path.file_name().map(ToOwned::to_owned);
361+
Self {
362+
path,
363+
is_directory,
364+
file_name,
365+
}
355366
}
356367

357368
fn into_path(self) -> PathBuf {
@@ -930,12 +941,16 @@ fn fill_todo(
930941
let dirs = fs::read_dir(path).and_then(|d| {
931942
d.map(|e| {
932943
e.map(|e| {
933-
let path = if curdir {
934-
PathBuf::from(e.path().file_name().unwrap())
944+
let (path, file_name) = if curdir {
945+
let path = e.path();
946+
let file_name = path.file_name().unwrap();
947+
(PathBuf::from(file_name), file_name.to_owned())
935948
} else {
936-
e.path()
949+
let path = e.path();
950+
let file_name = path.file_name().unwrap().to_owned();
951+
(path, file_name)
937952
};
938-
PathWrapper::from_dir_entry(path, e)
953+
PathWrapper::from_dir_entry(path, file_name, e)
939954
})
940955
})
941956
.collect::<Result<Vec<_>, _>>()
@@ -946,7 +961,7 @@ fn fill_todo(
946961
children
947962
.retain(|x| !x.file_name().unwrap().to_str().unwrap().starts_with('.'));
948963
}
949-
children.sort_by(|p1, p2| p2.file_name().cmp(&p1.file_name()));
964+
children.sort_by(|p1, p2| p2.file_name.cmp(&p1.file_name));
950965
todo.extend(children.into_iter().map(|x| Ok((x, idx))));
951966

952967
// Matching the special directory entries . and .. that

0 commit comments

Comments
 (0)