Skip to content

Commit 7ad61f9

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 49ee1e9 commit 7ad61f9

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
@@ -71,6 +71,7 @@ doctest!("../README.md");
7171

7272
use std::cmp;
7373
use std::error::Error;
74+
use std::ffi::OsString;
7475
use std::fmt;
7576
use std::fs;
7677
use std::fs::DirEntry;
@@ -330,10 +331,11 @@ impl fmt::Display for GlobError {
330331
struct PathWrapper {
331332
path: PathBuf,
332333
is_directory: bool,
334+
file_name: Option<OsString>,
333335
}
334336

335337
impl PathWrapper {
336-
fn from_dir_entry(path: PathBuf, e: DirEntry) -> Self {
338+
fn from_dir_entry(path: PathBuf, file_name: OsString, e: DirEntry) -> Self {
337339
let is_directory = e
338340
.file_type()
339341
.ok()
@@ -348,11 +350,20 @@ impl PathWrapper {
348350
})
349351
.or_else(|| fs::metadata(&path).map(|m| m.is_dir()).ok())
350352
.unwrap_or(false);
351-
Self { path, is_directory }
353+
Self {
354+
path,
355+
is_directory,
356+
file_name: Some(file_name),
357+
}
352358
}
353359
fn from_path(path: PathBuf) -> Self {
354360
let is_directory = fs::metadata(&path).map(|m| m.is_dir()).unwrap_or(false);
355-
Self { path, is_directory }
361+
let file_name = path.file_name().map(ToOwned::to_owned);
362+
Self {
363+
path,
364+
is_directory,
365+
file_name,
366+
}
356367
}
357368

358369
fn into_path(self) -> PathBuf {
@@ -927,12 +938,16 @@ fn fill_todo(
927938
let dirs = fs::read_dir(path).and_then(|d| {
928939
d.map(|e| {
929940
e.map(|e| {
930-
let path = if curdir {
931-
PathBuf::from(e.path().file_name().unwrap())
941+
let (path, file_name) = if curdir {
942+
let path = e.path();
943+
let file_name = path.file_name().unwrap();
944+
(PathBuf::from(file_name), file_name.to_owned())
932945
} else {
933-
e.path()
946+
let path = e.path();
947+
let file_name = path.file_name().unwrap().to_owned();
948+
(path, file_name)
934949
};
935-
PathWrapper::from_dir_entry(path, e)
950+
PathWrapper::from_dir_entry(path, file_name, e)
936951
})
937952
})
938953
.collect::<Result<Vec<_>, _>>()
@@ -943,7 +958,7 @@ fn fill_todo(
943958
children
944959
.retain(|x| !x.file_name().unwrap().to_str().unwrap().starts_with("."));
945960
}
946-
children.sort_by(|p1, p2| p2.file_name().cmp(&p1.file_name()));
961+
children.sort_by(|p1, p2| p2.file_name.cmp(&p1.file_name));
947962
todo.extend(children.into_iter().map(|x| Ok((x, idx))));
948963

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

0 commit comments

Comments
 (0)