Skip to content

Commit

Permalink
Auto merge of #70837 - nnethercote:speed-up-find_library_crate, r=pet…
Browse files Browse the repository at this point in the history
…rochenkov

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.
  • Loading branch information
bors committed Apr 7, 2020
2 parents 209b2be + a932616 commit 39b6253
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 21 deletions.
14 changes: 6 additions & 8 deletions src/librustc_metadata/locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -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;
Expand Down
22 changes: 11 additions & 11 deletions src/librustc_session/filesearch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -43,28 +43,28 @@ impl<'a> FileSearch<'a> {

pub fn search<F>(&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());
}
}
}
Expand Down
30 changes: 28 additions & 2 deletions src/librustc_session/search_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,31 @@ use std::path::{Path, PathBuf};
pub struct SearchPath {
pub kind: PathKind,
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)]
Expand Down Expand Up @@ -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::<Vec<_>>(),
Ok(files) => files
.filter_map(|e| e.ok().map(|e| SearchPathFile::new(e.path())))
.collect::<Vec<_>>(),
Err(..) => vec![],
};

Expand Down

0 comments on commit 39b6253

Please sign in to comment.