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.
This commit is contained in:
Nicholas Nethercote 2020-04-06 17:44:30 +10:00
parent 83f8c02eb9
commit a932616cf1
3 changed files with 45 additions and 21 deletions

View File

@ -543,8 +543,8 @@ impl<'a> CrateLocator<'a> {
// of the crate id (path/name/id). // of the crate id (path/name/id).
// //
// The goal of this step is to look at as little metadata as possible. // The goal of this step is to look at as little metadata as possible.
self.filesearch.search(|path, kind| { self.filesearch.search(|spf, kind| {
let file = match path.file_name().and_then(|s| s.to_str()) { let file = match &spf.file_name_str {
None => return FileDoesntMatch, None => return FileDoesntMatch,
Some(file) => file, Some(file) => file,
}; };
@ -556,20 +556,18 @@ impl<'a> CrateLocator<'a> {
(&file[(dylib_prefix.len())..(file.len() - dypair.1.len())], CrateFlavor::Dylib) (&file[(dylib_prefix.len())..(file.len() - dypair.1.len())], CrateFlavor::Dylib)
} else { } else {
if file.starts_with(&staticlib_prefix) && file.ends_with(&staticpair.1) { if file.starts_with(&staticlib_prefix) && file.ends_with(&staticpair.1) {
staticlibs.push(CrateMismatch { staticlibs
path: path.to_path_buf(), .push(CrateMismatch { path: spf.path.clone(), got: "static".to_string() });
got: "static".to_string(),
});
} }
return FileDoesntMatch; return FileDoesntMatch;
}; };
info!("lib candidate: {}", path.display()); info!("lib candidate: {}", spf.path.display());
let hash_str = hash.to_string(); let hash_str = hash.to_string();
let slot = candidates.entry(hash_str).or_default(); let slot = candidates.entry(hash_str).or_default();
let (ref mut rlibs, ref mut rmetas, ref mut dylibs) = *slot; let (ref mut rlibs, ref mut rmetas, ref mut dylibs) = *slot;
fs::canonicalize(path) fs::canonicalize(&spf.path)
.map(|p| { .map(|p| {
if seen_paths.contains(&p) { if seen_paths.contains(&p) {
return FileDoesntMatch; return FileDoesntMatch;

View File

@ -7,7 +7,7 @@ use std::env;
use std::fs; use std::fs;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use crate::search_paths::{PathKind, SearchPath}; use crate::search_paths::{PathKind, SearchPath, SearchPathFile};
use log::debug; use log::debug;
use rustc_fs_util::fix_windows_verbatim_for_gcc; use rustc_fs_util::fix_windows_verbatim_for_gcc;
@ -43,28 +43,28 @@ impl<'a> FileSearch<'a> {
pub fn search<F>(&self, mut pick: F) pub fn search<F>(&self, mut pick: F)
where where
F: FnMut(&Path, PathKind) -> FileMatch, F: FnMut(&SearchPathFile, PathKind) -> FileMatch,
{ {
for search_path in self.search_paths() { for search_path in self.search_paths() {
debug!("searching {}", search_path.dir.display()); debug!("searching {}", search_path.dir.display());
fn is_rlib(p: &Path) -> bool { fn is_rlib(spf: &SearchPathFile) -> bool {
p.extension() == Some("rlib".as_ref()) 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 // 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 // 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 // metadata, so in the name of speed, bring all rlib files to
// the front of the search list. // the front of the search list.
let files1 = search_path.files.iter().filter(|p| is_rlib(p)); let files1 = search_path.files.iter().filter(|spf| is_rlib(&spf));
let files2 = search_path.files.iter().filter(|p| !is_rlib(p)); let files2 = search_path.files.iter().filter(|spf| !is_rlib(&spf));
for path in files1.chain(files2) { for spf in files1.chain(files2) {
debug!("testing {}", path.display()); debug!("testing {}", spf.path.display());
let maybe_picked = pick(path, search_path.kind); let maybe_picked = pick(spf, search_path.kind);
match maybe_picked { match maybe_picked {
FileMatches => { FileMatches => {
debug!("picked {}", path.display()); debug!("picked {}", spf.path.display());
} }
FileDoesntMatch => { FileDoesntMatch => {
debug!("rejected {}", path.display()); debug!("rejected {}", spf.path.display());
} }
} }
} }

View File

@ -6,7 +6,31 @@ use std::path::{Path, PathBuf};
pub struct SearchPath { pub struct SearchPath {
pub kind: PathKind, pub kind: PathKind,
pub dir: PathBuf, pub dir: PathBuf,
pub files: Vec<PathBuf>, pub files: Vec<SearchPathFile>,
}
// The obvious implementation of `SearchPath::files` is a `Vec<PathBuf>`. 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<String>` containing the
// `PathBuf`'s filename. The prefix and suffix checking is much faster on the
// `Option<String>` 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<String>,
}
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)] #[derive(PartialEq, Clone, Copy, Debug, Hash, Eq, RustcEncodable, RustcDecodable)]
@ -60,7 +84,9 @@ impl SearchPath {
fn new(kind: PathKind, dir: PathBuf) -> Self { fn new(kind: PathKind, dir: PathBuf) -> Self {
// Get the files within the directory. // Get the files within the directory.
let files = match std::fs::read_dir(&dir) { let files = match std::fs::read_dir(&dir) {
Ok(files) => files.filter_map(|p| p.ok().map(|s| s.path())).collect::<Vec<_>>(), Ok(files) => files
.filter_map(|e| e.ok().map(|e| SearchPathFile::new(e.path())))
.collect::<Vec<_>>(),
Err(..) => vec![], Err(..) => vec![],
}; };