Skip to content

Commit

Permalink
Avoid regenerating the Vec<PathBuf> in FileSearch::search().
Browse files Browse the repository at this point in the history
`FileSearch::search()` traverses one or more directories. For each
directory it generates a `Vec<PathBuf>` containing one element per file
in that directory.

In some benchmarks this occurs enough that the allocations done for the
`PathBuf`s are significant, and in practice a small number of
directories are being traversed over and over again. For example, when
compiling the `tokio-webpush-simple` benchmark, two directories are
traversed 58 times each. Each of these directories have more than 100
files.

This commit changes things so that all the `Vec<PathBuf>`s that will be
needed by a `Session` are precomputed when that `Session` is created;
they are stored in `SearchPath`. `FileSearch` gets a reference to the
necessary `SearchPath`s. This reduces instruction counts on several
benchmarks by 1--5%.

The commit also removes the barely-used `visited_dirs` hash in
`for_each_lib_searchPath`. It only detects if `tlib_path` is the same as
one of the previously seen paths, which is unlikely.
  • Loading branch information
nnethercote committed Dec 11, 2018
1 parent c435605 commit b77fe07
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 25 deletions.
34 changes: 11 additions & 23 deletions src/librustc/session/filesearch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

pub use self::FileMatch::*;

use rustc_data_structures::fx::FxHashSet;
use std::borrow::Cow;
use std::env;
use std::fs;
Expand All @@ -31,29 +30,22 @@ pub enum FileMatch {

pub struct FileSearch<'a> {
pub sysroot: &'a Path,
pub search_paths: &'a [SearchPath],
pub triple: &'a str,
pub search_paths: &'a [SearchPath],
pub tlib_path: &'a SearchPath,
pub kind: PathKind,
}

impl<'a> FileSearch<'a> {
pub fn for_each_lib_search_path<F>(&self, mut f: F) where
F: FnMut(&SearchPath)
{
let mut visited_dirs = FxHashSet::default();
visited_dirs.reserve(self.search_paths.len() + 1);
let iter = self.search_paths.iter().filter(|sp| sp.kind.matches(self.kind));
for search_path in iter {
f(search_path);
visited_dirs.insert(search_path.dir.to_path_buf());
}

debug!("filesearch: searching lib path");
let tlib_path = make_target_lib_path(self.sysroot,
self.triple);
if !visited_dirs.contains(&tlib_path) {
f(&SearchPath { kind: PathKind::All, dir: tlib_path });
}
f(self.tlib_path);
}

pub fn get_lib_path(&self) -> PathBuf {
Expand All @@ -65,21 +57,15 @@ impl<'a> FileSearch<'a> {
{
self.for_each_lib_search_path(|search_path| {
debug!("searching {}", search_path.dir.display());
let files = match fs::read_dir(&search_path.dir) {
Ok(files) => files,
Err(..) => return,
};
let files = files.filter_map(|p| p.ok().map(|s| s.path()))
.collect::<Vec<_>>();
fn is_rlib(p: &Path) -> bool {
p.extension() == Some("rlib".as_ref())
}
// 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 = files.iter().filter(|p| is_rlib(p));
let files2 = files.iter().filter(|p| !is_rlib(p));
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);
Expand All @@ -98,12 +84,15 @@ impl<'a> FileSearch<'a> {
pub fn new(sysroot: &'a Path,
triple: &'a str,
search_paths: &'a Vec<SearchPath>,
kind: PathKind) -> FileSearch<'a> {
tlib_path: &'a SearchPath,
kind: PathKind)
-> FileSearch<'a> {
debug!("using sysroot = {}, triple = {}", sysroot.display(), triple);
FileSearch {
sysroot,
search_paths,
triple,
search_paths,
tlib_path,
kind,
}
}
Expand Down Expand Up @@ -137,8 +126,7 @@ pub fn relative_target_lib_path(sysroot: &Path, target_triple: &str) -> PathBuf
p
}

pub fn make_target_lib_path(sysroot: &Path,
target_triple: &str) -> PathBuf {
pub fn make_target_lib_path(sysroot: &Path, target_triple: &str) -> PathBuf {
sysroot.join(&relative_target_lib_path(sysroot, target_triple))
}

Expand Down
19 changes: 18 additions & 1 deletion src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ use lint;
use lint::builtin::BuiltinLintDiagnostics;
use middle::allocator::AllocatorKind;
use middle::dependency_format;
use session::search_paths::PathKind;
use session::config::{OutputType, Lto};
use session::search_paths::{PathKind, SearchPath};
use util::nodemap::{FxHashMap, FxHashSet};
use util::common::{duration_to_secs_str, ErrorReported};
use util::common::ProfileQueriesMsg;
Expand Down Expand Up @@ -64,6 +64,9 @@ pub struct Session {
pub target: config::Config,
pub host: Target,
pub opts: config::Options,
pub host_tlib_path: SearchPath,
/// This is `None` if the host and target are the same.
pub target_tlib_path: Option<SearchPath>,
pub parse_sess: ParseSess,
/// For a library crate, this is always none
pub entry_fn: Once<Option<(NodeId, Span, config::EntryFnType)>>,
Expand Down Expand Up @@ -699,6 +702,8 @@ impl Session {
&self.sysroot,
self.opts.target_triple.triple(),
&self.opts.search_paths,
// target_tlib_path==None means it's the same as host_tlib_path.
self.target_tlib_path.as_ref().unwrap_or(&self.host_tlib_path),
kind,
)
}
Expand All @@ -707,6 +712,7 @@ impl Session {
&self.sysroot,
config::host_triple(),
&self.opts.search_paths,
&self.host_tlib_path,
kind,
)
}
Expand Down Expand Up @@ -1106,6 +1112,15 @@ pub fn build_session_(
None => filesearch::get_or_default_sysroot(),
};

let host_triple = config::host_triple();
let target_triple = sopts.target_triple.triple();
let host_tlib_path = SearchPath::from_sysroot_and_triple(&sysroot, host_triple);
let target_tlib_path = if host_triple == target_triple {
None
} else {
Some(SearchPath::from_sysroot_and_triple(&sysroot, target_triple))
};

let file_path_mapping = sopts.file_path_mapping();

let local_crate_source_file =
Expand Down Expand Up @@ -1134,6 +1149,8 @@ pub fn build_session_(
target: target_cfg,
host,
opts: sopts,
host_tlib_path,
target_tlib_path,
parse_sess: p_s,
// For a library crate, this is always none
entry_fn: Once::new(),
Expand Down
14 changes: 13 additions & 1 deletion src/librustc/session/search_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use session::filesearch::make_target_lib_path;
pub struct SearchPath {
pub kind: PathKind,
pub dir: PathBuf,
pub files: Vec<PathBuf>,
}

#[derive(Eq, PartialEq, Clone, Copy, Debug, PartialOrd, Ord, Hash)]
Expand Down Expand Up @@ -65,7 +66,18 @@ impl SearchPath {
}

fn new(kind: PathKind, dir: PathBuf) -> Self {
SearchPath { kind, dir }
// 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<_>>()
}
Err(..) => vec![],
};

SearchPath { kind, dir, files }
}
}

0 comments on commit b77fe07

Please sign in to comment.