Skip to content

Commit e7d90c6

Browse files
committed
Auto merge of rust-lang#153131 - Kobzol:filesearch-opt, r=nnethercote
Optimize dependency file search I tried to look into the slowdown reported in rust-lang/cargo#16665. I created a Rust hello world program, and used this Python script to create a directory containing 200k files: ```python from pathlib import Path dir = Path("deps") dir.mkdir(parents=True, exist_ok=True) for i in range(200000): path = dir / f"file{i:07}.o" with open(path, "w") as f: f.write("\n") ``` Then I tried to do various small microoptimalizations and simplifications to the code that iterates the search directories. Each individual commit improved performance, with the third one having the biggest effect. Here are the results on `main` vs the last commit with the stage1 compiler on Linux, using `hyperfine "rustc +stage1 src/main.rs -L deps" -r 30` (there's IO involved, so it's good to let it run for a while): ```bash Benchmark 1: rustc +stage1 src/main.rs -L deps Time (mean ± σ): 299.4 ms ± 2.7 ms [User: 161.9 ms, System: 144.9 ms] Range (min … max): 294.8 ms … 307.1 ms 30 runs Benchmark 1: rustc +stage1 src/main.rs -L deps Time (mean ± σ): 208.1 ms ± 4.5 ms [User: 87.3 ms, System: 128.7 ms] Range (min … max): 202.4 ms … 219.6 ms 30 runs ``` Would be cool if someone could try this on macOS (maybe @ehuss - not sure if you have macOS or you only commented about its behavior on the Cargo issue :) ). I also tried to prefilter the paths (not in this PR); right now we load everything and then we filter files with given prefixes, that's wasteful. Filtering just files starting with `lib` would get us down to ~150ms here. (The baseline without `-L` is ~80ms on my PC). The rest of the 70ms is essentially allocations from iterating the directory entries and sorting. That would be very hard to change - iterating the directory entries (de)allocates a lot of intermediate paths :( We'd have to implement the iteration by hand with either arena allocation, or at least some better management of memory. r? @nnethercote
2 parents 8038127 + f450289 commit e7d90c6

File tree

2 files changed

+26
-26
lines changed

2 files changed

+26
-26
lines changed

‎compiler/rustc_metadata/src/locator.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,8 @@ impl<'a> CrateLocator<'a> {
438438
}
439439
if let Some(matches) = spf.query(prefix, suffix) {
440440
for (hash, spf) in matches {
441-
info!("lib candidate: {}", spf.path.display());
441+
let spf_path = spf.path(&search_path.dir);
442+
info!("lib candidate: {}", spf_path.display());
442443

443444
let (rlibs, rmetas, dylibs, interfaces) =
444445
candidates.entry(hash).or_default();
@@ -447,21 +448,20 @@ impl<'a> CrateLocator<'a> {
447448
// ones we've already seen. This allows us to ignore crates
448449
// we know are exactual equal to ones we've already found.
449450
// Going to the same crate through different symlinks does not change the result.
450-
let path = try_canonicalize(&spf.path)
451-
.unwrap_or_else(|_| spf.path.to_path_buf());
451+
let path =
452+
try_canonicalize(&spf_path).unwrap_or_else(|_| spf_path.clone());
452453
if seen_paths.contains(&path) {
453454
continue;
454455
};
455456
seen_paths.insert(path);
456457
}
457458
// Use the original path (potentially with unresolved symlinks),
458459
// filesystem code should not care, but this is nicer for diagnostics.
459-
let path = spf.path.to_path_buf();
460460
match kind {
461-
CrateFlavor::Rlib => rlibs.insert(path),
462-
CrateFlavor::Rmeta => rmetas.insert(path),
463-
CrateFlavor::Dylib => dylibs.insert(path),
464-
CrateFlavor::SDylib => interfaces.insert(path),
461+
CrateFlavor::Rlib => rlibs.insert(spf_path),
462+
CrateFlavor::Rmeta => rmetas.insert(spf_path),
463+
CrateFlavor::Dylib => dylibs.insert(spf_path),
464+
CrateFlavor::SDylib => interfaces.insert(spf_path),
465465
};
466466
}
467467
}
@@ -472,7 +472,7 @@ impl<'a> CrateLocator<'a> {
472472
{
473473
for (_, spf) in static_matches {
474474
crate_rejections.via_kind.push(CrateMismatch {
475-
path: spf.path.to_path_buf(),
475+
path: spf.path(&search_path.dir),
476476
got: "static".to_string(),
477477
});
478478
}

‎compiler/rustc_session/src/search_paths.rs‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ pub struct SearchPath {
1616

1717
/// [FilesIndex] contains paths that can be efficiently looked up with (prefix, suffix) pairs.
1818
#[derive(Clone, Debug)]
19-
pub struct FilesIndex(Vec<(Arc<str>, SearchPathFile)>);
19+
pub struct FilesIndex(Vec<SearchPathFile>);
2020

2121
impl FilesIndex {
2222
/// Look up [SearchPathFile] by (prefix, suffix) pair.
@@ -25,15 +25,15 @@ impl FilesIndex {
2525
prefix: &str,
2626
suffix: &str,
2727
) -> Option<impl Iterator<Item = (String, &'s SearchPathFile)>> {
28-
let start = self.0.partition_point(|(k, _)| **k < *prefix);
28+
let start = self.0.partition_point(|v| *v.file_name_str < *prefix);
2929
if start == self.0.len() {
3030
return None;
3131
}
32-
let end = self.0[start..].partition_point(|(k, _)| k.starts_with(prefix));
32+
let end = self.0[start..].partition_point(|v| v.file_name_str.starts_with(prefix));
3333
let prefixed_items = &self.0[start..][..end];
3434

35-
let ret = prefixed_items.into_iter().filter_map(move |(k, v)| {
36-
k.ends_with(suffix).then(|| {
35+
let ret = prefixed_items.into_iter().filter_map(move |v| {
36+
v.file_name_str.ends_with(suffix).then(|| {
3737
(
3838
String::from(
3939
&v.file_name_str[prefix.len()..v.file_name_str.len() - suffix.len()],
@@ -45,7 +45,7 @@ impl FilesIndex {
4545
Some(ret)
4646
}
4747
pub fn retain(&mut self, prefixes: &[&str]) {
48-
self.0.retain(|(k, _)| prefixes.iter().any(|prefix| k.starts_with(prefix)));
48+
self.0.retain(|v| prefixes.iter().any(|prefix| v.file_name_str.starts_with(prefix)));
4949
}
5050
}
5151
/// The obvious implementation of `SearchPath::files` is a `Vec<PathBuf>`. But
@@ -61,8 +61,14 @@ impl FilesIndex {
6161
/// UTF-8, and so a non-UTF-8 filename couldn't be one we're looking for.)
6262
#[derive(Clone, Debug)]
6363
pub struct SearchPathFile {
64-
pub path: Arc<Path>,
65-
pub file_name_str: Arc<str>,
64+
file_name_str: Arc<str>,
65+
}
66+
67+
impl SearchPathFile {
68+
/// Constructs the full path to the file.
69+
pub fn path(&self, dir: &Path) -> PathBuf {
70+
dir.join(&*self.file_name_str)
71+
}
6672
}
6773

6874
#[derive(PartialEq, Clone, Copy, Debug, Hash, Eq, Encodable, Decodable, HashStable_Generic)]
@@ -134,20 +140,14 @@ impl SearchPath {
134140
Ok(files) => files
135141
.filter_map(|e| {
136142
e.ok().and_then(|e| {
137-
e.file_name().to_str().map(|s| {
138-
let file_name_str: Arc<str> = s.into();
139-
(
140-
Arc::clone(&file_name_str),
141-
SearchPathFile { path: e.path().into(), file_name_str },
142-
)
143-
})
143+
e.file_name().to_str().map(|s| SearchPathFile { file_name_str: s.into() })
144144
})
145145
})
146-
.collect::<Vec<_>>(),
146+
.collect::<Vec<SearchPathFile>>(),
147147

148148
Err(..) => Default::default(),
149149
};
150-
files.sort_by(|(lhs, _), (rhs, _)| lhs.cmp(rhs));
150+
files.sort_unstable_by(|lhs, rhs| lhs.file_name_str.cmp(&rhs.file_name_str));
151151
let files = FilesIndex(files);
152152
SearchPath { kind, dir, files }
153153
}

0 commit comments

Comments
 (0)