Skip to content

Commit a1746bd

Browse files
borsbors
committed
Auto merge of #56090 - nnethercote:filesearch, r=eddyb
Overhaul `FileSearch` and `SearchPaths` `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. We can do all the necessary traversals up front, when `Session` is created, and get the `Vec<PathBuf>`s then. This reduces instruction counts on several benchmarks by 1--5%. r? @alexcrichton CC @eddyb, @michaelwoerister, @nikomatsakis
2 parents 3a31213 + 4b70b01 commit a1746bd

File tree

12 files changed

+134
-154
lines changed

12 files changed

+134
-154
lines changed

‎src/librustc/session/config.rs

+26-25
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
use std::str::FromStr;
1515

1616
use session::{early_error, early_warn, Session};
17-
use session::search_paths::SearchPaths;
17+
use session::search_paths::SearchPath;
1818

1919
use rustc_target::spec::{LinkerFlavor, PanicStrategy, RelroLevel};
2020
use rustc_target::spec::{Target, TargetTriple};
@@ -374,7 +374,7 @@ top_level_options!(
374374
lint_cap: Option<lint::Level> [TRACKED],
375375
describe_lints: bool [UNTRACKED],
376376
output_types: OutputTypes [TRACKED],
377-
search_paths: SearchPaths [UNTRACKED],
377+
search_paths: Vec<SearchPath> [UNTRACKED],
378378
libs: Vec<(String, Option<String>, Option<cstore::NativeLibraryKind>)> [TRACKED],
379379
maybe_sysroot: Option<PathBuf> [TRACKED],
380380

@@ -593,7 +593,7 @@ impl Default for Options {
593593
lint_cap: None,
594594
describe_lints: false,
595595
output_types: OutputTypes(BTreeMap::new()),
596-
search_paths: SearchPaths::new(),
596+
search_paths: vec![],
597597
maybe_sysroot: None,
598598
target_triple: TargetTriple::from_triple(host_triple()),
599599
test: false,
@@ -2115,9 +2115,9 @@ pub fn build_session_options_and_crate_config(
21152115
}
21162116
};
21172117

2118-
let mut search_paths = SearchPaths::new();
2118+
let mut search_paths = vec![];
21192119
for s in &matches.opt_strs("L") {
2120-
search_paths.add_path(&s[..], error_format);
2120+
search_paths.push(SearchPath::from_cli_opt(&s[..], error_format));
21212121
}
21222122

21232123
let libs = matches
@@ -2535,6 +2535,7 @@ mod tests {
25352535
use session::config::{build_configuration, build_session_options_and_crate_config};
25362536
use session::config::{LtoCli, CrossLangLto};
25372537
use session::build_session;
2538+
use session::search_paths::SearchPath;
25382539
use std::collections::{BTreeMap, BTreeSet};
25392540
use std::iter::FromIterator;
25402541
use std::path::PathBuf;
@@ -2790,48 +2791,48 @@ mod tests {
27902791

27912792
// Reference
27922793
v1.search_paths
2793-
.add_path("native=abc", super::ErrorOutputType::Json(false));
2794+
.push(SearchPath::from_cli_opt("native=abc", super::ErrorOutputType::Json(false)));
27942795
v1.search_paths
2795-
.add_path("crate=def", super::ErrorOutputType::Json(false));
2796+
.push(SearchPath::from_cli_opt("crate=def", super::ErrorOutputType::Json(false)));
27962797
v1.search_paths
2797-
.add_path("dependency=ghi", super::ErrorOutputType::Json(false));
2798+
.push(SearchPath::from_cli_opt("dependency=ghi", super::ErrorOutputType::Json(false)));
27982799
v1.search_paths
2799-
.add_path("framework=jkl", super::ErrorOutputType::Json(false));
2800+
.push(SearchPath::from_cli_opt("framework=jkl", super::ErrorOutputType::Json(false)));
28002801
v1.search_paths
2801-
.add_path("all=mno", super::ErrorOutputType::Json(false));
2802+
.push(SearchPath::from_cli_opt("all=mno", super::ErrorOutputType::Json(false)));
28022803

28032804
v2.search_paths
2804-
.add_path("native=abc", super::ErrorOutputType::Json(false));
2805+
.push(SearchPath::from_cli_opt("native=abc", super::ErrorOutputType::Json(false)));
28052806
v2.search_paths
2806-
.add_path("dependency=ghi", super::ErrorOutputType::Json(false));
2807+
.push(SearchPath::from_cli_opt("dependency=ghi", super::ErrorOutputType::Json(false)));
28072808
v2.search_paths
2808-
.add_path("crate=def", super::ErrorOutputType::Json(false));
2809+
.push(SearchPath::from_cli_opt("crate=def", super::ErrorOutputType::Json(false)));
28092810
v2.search_paths
2810-
.add_path("framework=jkl", super::ErrorOutputType::Json(false));
2811+
.push(SearchPath::from_cli_opt("framework=jkl", super::ErrorOutputType::Json(false)));
28112812
v2.search_paths
2812-
.add_path("all=mno", super::ErrorOutputType::Json(false));
2813+
.push(SearchPath::from_cli_opt("all=mno", super::ErrorOutputType::Json(false)));
28132814

28142815
v3.search_paths
2815-
.add_path("crate=def", super::ErrorOutputType::Json(false));
2816+
.push(SearchPath::from_cli_opt("crate=def", super::ErrorOutputType::Json(false)));
28162817
v3.search_paths
2817-
.add_path("framework=jkl", super::ErrorOutputType::Json(false));
2818+
.push(SearchPath::from_cli_opt("framework=jkl", super::ErrorOutputType::Json(false)));
28182819
v3.search_paths
2819-
.add_path("native=abc", super::ErrorOutputType::Json(false));
2820+
.push(SearchPath::from_cli_opt("native=abc", super::ErrorOutputType::Json(false)));
28202821
v3.search_paths
2821-
.add_path("dependency=ghi", super::ErrorOutputType::Json(false));
2822+
.push(SearchPath::from_cli_opt("dependency=ghi", super::ErrorOutputType::Json(false)));
28222823
v3.search_paths
2823-
.add_path("all=mno", super::ErrorOutputType::Json(false));
2824+
.push(SearchPath::from_cli_opt("all=mno", super::ErrorOutputType::Json(false)));
28242825

28252826
v4.search_paths
2826-
.add_path("all=mno", super::ErrorOutputType::Json(false));
2827+
.push(SearchPath::from_cli_opt("all=mno", super::ErrorOutputType::Json(false)));
28272828
v4.search_paths
2828-
.add_path("native=abc", super::ErrorOutputType::Json(false));
2829+
.push(SearchPath::from_cli_opt("native=abc", super::ErrorOutputType::Json(false)));
28292830
v4.search_paths
2830-
.add_path("crate=def", super::ErrorOutputType::Json(false));
2831+
.push(SearchPath::from_cli_opt("crate=def", super::ErrorOutputType::Json(false)));
28312832
v4.search_paths
2832-
.add_path("dependency=ghi", super::ErrorOutputType::Json(false));
2833+
.push(SearchPath::from_cli_opt("dependency=ghi", super::ErrorOutputType::Json(false)));
28332834
v4.search_paths
2834-
.add_path("framework=jkl", super::ErrorOutputType::Json(false));
2835+
.push(SearchPath::from_cli_opt("framework=jkl", super::ErrorOutputType::Json(false)));
28352836

28362837
assert!(v1.dep_tracking_hash() == v2.dep_tracking_hash());
28372838
assert!(v1.dep_tracking_hash() == v3.dep_tracking_hash());

‎src/librustc/session/filesearch.rs

+29-48
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,12 @@
1212

1313
pub use self::FileMatch::*;
1414

15-
use rustc_data_structures::fx::FxHashSet;
1615
use std::borrow::Cow;
1716
use std::env;
1817
use std::fs;
1918
use std::path::{Path, PathBuf};
2019

21-
use session::search_paths::{SearchPaths, PathKind};
20+
use session::search_paths::{SearchPath, PathKind};
2221
use rustc_fs_util::fix_windows_verbatim_for_gcc;
2322

2423
#[derive(Copy, Clone)]
@@ -30,31 +29,19 @@ pub enum FileMatch {
3029
// A module for searching for libraries
3130

3231
pub struct FileSearch<'a> {
33-
pub sysroot: &'a Path,
34-
pub search_paths: &'a SearchPaths,
35-
pub triple: &'a str,
36-
pub kind: PathKind,
32+
sysroot: &'a Path,
33+
triple: &'a str,
34+
search_paths: &'a [SearchPath],
35+
tlib_path: &'a SearchPath,
36+
kind: PathKind,
3737
}
3838

3939
impl<'a> FileSearch<'a> {
40-
pub fn for_each_lib_search_path<F>(&self, mut f: F) where
41-
F: FnMut(&Path, PathKind)
42-
{
43-
let mut visited_dirs = FxHashSet::default();
44-
visited_dirs.reserve(self.search_paths.paths.len() + 1);
45-
for (path, kind) in self.search_paths.iter(self.kind) {
46-
f(path, kind);
47-
visited_dirs.insert(path.to_path_buf());
48-
}
49-
50-
debug!("filesearch: searching lib path");
51-
let tlib_path = make_target_lib_path(self.sysroot,
52-
self.triple);
53-
if !visited_dirs.contains(&tlib_path) {
54-
f(&tlib_path, PathKind::All);
55-
}
56-
57-
visited_dirs.insert(tlib_path);
40+
pub fn search_paths(&self) -> impl Iterator<Item = &'a SearchPath> {
41+
let kind = self.kind;
42+
self.search_paths.iter()
43+
.filter(move |sp| sp.kind.matches(kind))
44+
.chain(std::iter::once(self.tlib_path))
5845
}
5946

6047
pub fn get_lib_path(&self) -> PathBuf {
@@ -64,26 +51,20 @@ impl<'a> FileSearch<'a> {
6451
pub fn search<F>(&self, mut pick: F)
6552
where F: FnMut(&Path, PathKind) -> FileMatch
6653
{
67-
self.for_each_lib_search_path(|lib_search_path, kind| {
68-
debug!("searching {}", lib_search_path.display());
69-
let files = match fs::read_dir(lib_search_path) {
70-
Ok(files) => files,
71-
Err(..) => return,
72-
};
73-
let files = files.filter_map(|p| p.ok().map(|s| s.path()))
74-
.collect::<Vec<_>>();
54+
for search_path in self.search_paths() {
55+
debug!("searching {}", search_path.dir.display());
7556
fn is_rlib(p: &Path) -> bool {
7657
p.extension() == Some("rlib".as_ref())
7758
}
7859
// Reading metadata out of rlibs is faster, and if we find both
7960
// an rlib and a dylib we only read one of the files of
8061
// metadata, so in the name of speed, bring all rlib files to
8162
// the front of the search list.
82-
let files1 = files.iter().filter(|p| is_rlib(p));
83-
let files2 = files.iter().filter(|p| !is_rlib(p));
63+
let files1 = search_path.files.iter().filter(|p| is_rlib(p));
64+
let files2 = search_path.files.iter().filter(|p| !is_rlib(p));
8465
for path in files1.chain(files2) {
8566
debug!("testing {}", path.display());
86-
let maybe_picked = pick(path, kind);
67+
let maybe_picked = pick(path, search_path.kind);
8768
match maybe_picked {
8869
FileMatches => {
8970
debug!("picked {}", path.display());
@@ -93,29 +74,30 @@ impl<'a> FileSearch<'a> {
9374
}
9475
}
9576
}
96-
});
77+
}
9778
}
9879

9980
pub fn new(sysroot: &'a Path,
10081
triple: &'a str,
101-
search_paths: &'a SearchPaths,
102-
kind: PathKind) -> FileSearch<'a> {
82+
search_paths: &'a Vec<SearchPath>,
83+
tlib_path: &'a SearchPath,
84+
kind: PathKind)
85+
-> FileSearch<'a> {
10386
debug!("using sysroot = {}, triple = {}", sysroot.display(), triple);
10487
FileSearch {
10588
sysroot,
106-
search_paths,
10789
triple,
90+
search_paths,
91+
tlib_path,
10892
kind,
10993
}
11094
}
11195

112-
// Returns a list of directories where target-specific dylibs might be located.
113-
pub fn get_dylib_search_paths(&self) -> Vec<PathBuf> {
114-
let mut paths = Vec::new();
115-
self.for_each_lib_search_path(|lib_search_path, _| {
116-
paths.push(lib_search_path.to_path_buf());
117-
});
118-
paths
96+
// Returns just the directories within the search paths.
97+
pub fn search_path_dirs(&self) -> Vec<PathBuf> {
98+
self.search_paths()
99+
.map(|sp| sp.dir.to_path_buf())
100+
.collect()
119101
}
120102

121103
// Returns a list of directories where target-specific tool binaries are located.
@@ -138,8 +120,7 @@ pub fn relative_target_lib_path(sysroot: &Path, target_triple: &str) -> PathBuf
138120
p
139121
}
140122

141-
fn make_target_lib_path(sysroot: &Path,
142-
target_triple: &str) -> PathBuf {
123+
pub fn make_target_lib_path(sysroot: &Path, target_triple: &str) -> PathBuf {
143124
sysroot.join(&relative_target_lib_path(sysroot, target_triple))
144125
}
145126

‎src/librustc/session/mod.rs

+26-17
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ use lint;
1919
use lint::builtin::BuiltinLintDiagnostics;
2020
use middle::allocator::AllocatorKind;
2121
use middle::dependency_format;
22-
use session::search_paths::PathKind;
2322
use session::config::{OutputType, Lto};
23+
use session::search_paths::{PathKind, SearchPath};
2424
use util::nodemap::{FxHashMap, FxHashSet};
2525
use util::common::{duration_to_secs_str, ErrorReported};
2626
use util::common::ProfileQueriesMsg;
@@ -48,7 +48,7 @@ use std::cell::{self, Cell, RefCell};
4848
use std::env;
4949
use std::fmt;
5050
use std::io::Write;
51-
use std::path::{Path, PathBuf};
51+
use std::path::PathBuf;
5252
use std::time::Duration;
5353
use std::sync::mpsc;
5454
use std::sync::atomic::{AtomicUsize, Ordering};
@@ -64,12 +64,15 @@ pub struct Session {
6464
pub target: config::Config,
6565
pub host: Target,
6666
pub opts: config::Options,
67+
pub host_tlib_path: SearchPath,
68+
/// This is `None` if the host and target are the same.
69+
pub target_tlib_path: Option<SearchPath>,
6770
pub parse_sess: ParseSess,
6871
/// For a library crate, this is always none
6972
pub entry_fn: Once<Option<(NodeId, Span, config::EntryFnType)>>,
7073
pub plugin_registrar_fn: Once<Option<ast::NodeId>>,
7174
pub proc_macro_decls_static: Once<Option<ast::NodeId>>,
72-
pub default_sysroot: Option<PathBuf>,
75+
pub sysroot: PathBuf,
7376
/// The name of the root source file of the crate, in the local file system.
7477
/// `None` means that there is no source file.
7578
pub local_crate_source_file: Option<PathBuf>,
@@ -694,27 +697,22 @@ impl Session {
694697
)
695698
}
696699

697-
pub fn sysroot<'a>(&'a self) -> &'a Path {
698-
match self.opts.maybe_sysroot {
699-
Some(ref sysroot) => sysroot,
700-
None => self.default_sysroot
701-
.as_ref()
702-
.expect("missing sysroot and default_sysroot in Session"),
703-
}
704-
}
705700
pub fn target_filesearch(&self, kind: PathKind) -> filesearch::FileSearch<'_> {
706701
filesearch::FileSearch::new(
707-
self.sysroot(),
702+
&self.sysroot,
708703
self.opts.target_triple.triple(),
709704
&self.opts.search_paths,
705+
// target_tlib_path==None means it's the same as host_tlib_path.
706+
self.target_tlib_path.as_ref().unwrap_or(&self.host_tlib_path),
710707
kind,
711708
)
712709
}
713710
pub fn host_filesearch(&self, kind: PathKind) -> filesearch::FileSearch<'_> {
714711
filesearch::FileSearch::new(
715-
self.sysroot(),
712+
&self.sysroot,
716713
config::host_triple(),
717714
&self.opts.search_paths,
715+
&self.host_tlib_path,
718716
kind,
719717
)
720718
}
@@ -1109,9 +1107,18 @@ pub fn build_session_(
11091107
let target_cfg = config::build_target_config(&sopts, &span_diagnostic);
11101108

11111109
let p_s = parse::ParseSess::with_span_handler(span_diagnostic, source_map);
1112-
let default_sysroot = match sopts.maybe_sysroot {
1113-
Some(_) => None,
1114-
None => Some(filesearch::get_or_default_sysroot()),
1110+
let sysroot = match &sopts.maybe_sysroot {
1111+
Some(sysroot) => sysroot.clone(),
1112+
None => filesearch::get_or_default_sysroot(),
1113+
};
1114+
1115+
let host_triple = config::host_triple();
1116+
let target_triple = sopts.target_triple.triple();
1117+
let host_tlib_path = SearchPath::from_sysroot_and_triple(&sysroot, host_triple);
1118+
let target_tlib_path = if host_triple == target_triple {
1119+
None
1120+
} else {
1121+
Some(SearchPath::from_sysroot_and_triple(&sysroot, target_triple))
11151122
};
11161123

11171124
let file_path_mapping = sopts.file_path_mapping();
@@ -1142,12 +1149,14 @@ pub fn build_session_(
11421149
target: target_cfg,
11431150
host,
11441151
opts: sopts,
1152+
host_tlib_path,
1153+
target_tlib_path,
11451154
parse_sess: p_s,
11461155
// For a library crate, this is always none
11471156
entry_fn: Once::new(),
11481157
plugin_registrar_fn: Once::new(),
11491158
proc_macro_decls_static: Once::new(),
1150-
default_sysroot,
1159+
sysroot,
11511160
local_crate_source_file,
11521161
working_dir,
11531162
lint_store: RwLock::new(lint::LintStore::new()),

0 commit comments

Comments
 (0)