From a932616cf1426484f4a530ef6d2d654b7d1accaf Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 6 Apr 2020 17:44:30 +1000 Subject: [PATCH] Speed up path searching with `find_library_crate`. By doing prefix and suffix checking on a `String` copy of each relevant `PathBuf`, rather than the `PathBuf` itself. --- src/librustc_metadata/locator.rs | 14 ++++++------- src/librustc_session/filesearch.rs | 22 ++++++++++---------- src/librustc_session/search_paths.rs | 30 ++++++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 21 deletions(-) diff --git a/src/librustc_metadata/locator.rs b/src/librustc_metadata/locator.rs index d56c8490ef5ee..fc63cfd143eb3 100644 --- a/src/librustc_metadata/locator.rs +++ b/src/librustc_metadata/locator.rs @@ -543,8 +543,8 @@ impl<'a> CrateLocator<'a> { // of the crate id (path/name/id). // // The goal of this step is to look at as little metadata as possible. - self.filesearch.search(|path, kind| { - let file = match path.file_name().and_then(|s| s.to_str()) { + self.filesearch.search(|spf, kind| { + let file = match &spf.file_name_str { None => return FileDoesntMatch, Some(file) => file, }; @@ -556,20 +556,18 @@ impl<'a> CrateLocator<'a> { (&file[(dylib_prefix.len())..(file.len() - dypair.1.len())], CrateFlavor::Dylib) } else { if file.starts_with(&staticlib_prefix) && file.ends_with(&staticpair.1) { - staticlibs.push(CrateMismatch { - path: path.to_path_buf(), - got: "static".to_string(), - }); + staticlibs + .push(CrateMismatch { path: spf.path.clone(), got: "static".to_string() }); } return FileDoesntMatch; }; - info!("lib candidate: {}", path.display()); + info!("lib candidate: {}", spf.path.display()); let hash_str = hash.to_string(); let slot = candidates.entry(hash_str).or_default(); let (ref mut rlibs, ref mut rmetas, ref mut dylibs) = *slot; - fs::canonicalize(path) + fs::canonicalize(&spf.path) .map(|p| { if seen_paths.contains(&p) { return FileDoesntMatch; diff --git a/src/librustc_session/filesearch.rs b/src/librustc_session/filesearch.rs index 4347512eda01c..e98746231fb30 100644 --- a/src/librustc_session/filesearch.rs +++ b/src/librustc_session/filesearch.rs @@ -7,7 +7,7 @@ use std::env; use std::fs; use std::path::{Path, PathBuf}; -use crate::search_paths::{PathKind, SearchPath}; +use crate::search_paths::{PathKind, SearchPath, SearchPathFile}; use log::debug; use rustc_fs_util::fix_windows_verbatim_for_gcc; @@ -43,28 +43,28 @@ impl<'a> FileSearch<'a> { pub fn search(&self, mut pick: F) where - F: FnMut(&Path, PathKind) -> FileMatch, + F: FnMut(&SearchPathFile, PathKind) -> FileMatch, { for search_path in self.search_paths() { debug!("searching {}", search_path.dir.display()); - fn is_rlib(p: &Path) -> bool { - p.extension() == Some("rlib".as_ref()) + fn is_rlib(spf: &SearchPathFile) -> bool { + if let Some(f) = &spf.file_name_str { f.ends_with(".rlib") } else { false } } // Reading metadata out of rlibs is faster, and if we find both // an rlib and a dylib we only read one of the files of // metadata, so in the name of speed, bring all rlib files to // the front of the search list. - let files1 = search_path.files.iter().filter(|p| is_rlib(p)); - let files2 = search_path.files.iter().filter(|p| !is_rlib(p)); - for path in files1.chain(files2) { - debug!("testing {}", path.display()); - let maybe_picked = pick(path, search_path.kind); + let files1 = search_path.files.iter().filter(|spf| is_rlib(&spf)); + let files2 = search_path.files.iter().filter(|spf| !is_rlib(&spf)); + for spf in files1.chain(files2) { + debug!("testing {}", spf.path.display()); + let maybe_picked = pick(spf, search_path.kind); match maybe_picked { FileMatches => { - debug!("picked {}", path.display()); + debug!("picked {}", spf.path.display()); } FileDoesntMatch => { - debug!("rejected {}", path.display()); + debug!("rejected {}", spf.path.display()); } } } diff --git a/src/librustc_session/search_paths.rs b/src/librustc_session/search_paths.rs index 06f408b4a8d64..4ff06acaa1fd4 100644 --- a/src/librustc_session/search_paths.rs +++ b/src/librustc_session/search_paths.rs @@ -6,7 +6,31 @@ use std::path::{Path, PathBuf}; pub struct SearchPath { pub kind: PathKind, pub dir: PathBuf, - pub files: Vec, + pub files: Vec, +} + +// The obvious implementation of `SearchPath::files` is a `Vec`. But +// it is searched repeatedly by `find_library_crate`, and the searches involve +// checking the prefix and suffix of the filename of each `PathBuf`. This is +// doable, but very slow, because it involves calls to `file_name` and +// `extension` that are themselves slow. +// +// This type augments the `PathBuf` with an `Option` containing the +// `PathBuf`'s filename. The prefix and suffix checking is much faster on the +// `Option` than the `PathBuf`. (It's an `Option` because +// `Path::file_name` can fail; if that happens then all subsequent checking +// will also fail, which is fine.) +#[derive(Clone, Debug)] +pub struct SearchPathFile { + pub path: PathBuf, + pub file_name_str: Option, +} + +impl SearchPathFile { + fn new(path: PathBuf) -> SearchPathFile { + let file_name_str = path.file_name().and_then(|f| f.to_str()).map(|s| s.to_string()); + SearchPathFile { path, file_name_str } + } } #[derive(PartialEq, Clone, Copy, Debug, Hash, Eq, RustcEncodable, RustcDecodable)] @@ -60,7 +84,9 @@ impl SearchPath { fn new(kind: PathKind, dir: PathBuf) -> Self { // Get the files within the directory. let files = match std::fs::read_dir(&dir) { - Ok(files) => files.filter_map(|p| p.ok().map(|s| s.path())).collect::>(), + Ok(files) => files + .filter_map(|e| e.ok().map(|e| SearchPathFile::new(e.path()))) + .collect::>(), Err(..) => vec![], };