From 0238bcc60d943878244ef7221d338f13a35e2a45 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 21 Nov 2018 22:03:59 +1100 Subject: [PATCH 1/6] Avoid a useless `FxHashSet::insert` in `FileSearch::for_each_lib_search_path`. --- src/librustc/session/filesearch.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/librustc/session/filesearch.rs b/src/librustc/session/filesearch.rs index 8159c65a8bc2d..f5dfde7d5189d 100644 --- a/src/librustc/session/filesearch.rs +++ b/src/librustc/session/filesearch.rs @@ -53,8 +53,6 @@ impl<'a> FileSearch<'a> { if !visited_dirs.contains(&tlib_path) { f(&tlib_path, PathKind::All); } - - visited_dirs.insert(tlib_path); } pub fn get_lib_path(&self) -> PathBuf { From 2640da7d13a1051dddcf93c06f9ae90508fce6fb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 22 Nov 2018 15:49:48 +1100 Subject: [PATCH 2/6] Remove `Session::sysroot()`. Instead of maybe storing its own sysroot and maybe deferring to the one in `Session::opts`, just clone the latter when necessary so one is always directly available. This removes the need for the getter. --- src/librustc/session/mod.rs | 24 ++++++++---------------- src/librustc_codegen_llvm/back/link.rs | 3 +-- src/librustc_codegen_ssa/back/linker.rs | 3 +-- src/librustc_driver/lib.rs | 2 +- src/librustc_metadata/locator.rs | 2 +- 5 files changed, 12 insertions(+), 22 deletions(-) diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index d1dd745add9a4..56a57b5375fd8 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -48,7 +48,7 @@ use std::cell::{self, Cell, RefCell}; use std::env; use std::fmt; use std::io::Write; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::time::Duration; use std::sync::mpsc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -69,7 +69,7 @@ pub struct Session { pub entry_fn: Once>, pub plugin_registrar_fn: Once>, pub proc_macro_decls_static: Once>, - pub default_sysroot: Option, + pub sysroot: PathBuf, /// The name of the root source file of the crate, in the local file system. /// `None` means that there is no source file. pub local_crate_source_file: Option, @@ -694,17 +694,9 @@ impl Session { ) } - pub fn sysroot<'a>(&'a self) -> &'a Path { - match self.opts.maybe_sysroot { - Some(ref sysroot) => sysroot, - None => self.default_sysroot - .as_ref() - .expect("missing sysroot and default_sysroot in Session"), - } - } pub fn target_filesearch(&self, kind: PathKind) -> filesearch::FileSearch<'_> { filesearch::FileSearch::new( - self.sysroot(), + &self.sysroot, self.opts.target_triple.triple(), &self.opts.search_paths, kind, @@ -712,7 +704,7 @@ impl Session { } pub fn host_filesearch(&self, kind: PathKind) -> filesearch::FileSearch<'_> { filesearch::FileSearch::new( - self.sysroot(), + &self.sysroot, config::host_triple(), &self.opts.search_paths, kind, @@ -1109,9 +1101,9 @@ pub fn build_session_( let target_cfg = config::build_target_config(&sopts, &span_diagnostic); let p_s = parse::ParseSess::with_span_handler(span_diagnostic, source_map); - let default_sysroot = match sopts.maybe_sysroot { - Some(_) => None, - None => Some(filesearch::get_or_default_sysroot()), + let sysroot = match &sopts.maybe_sysroot { + Some(sysroot) => sysroot.clone(), + None => filesearch::get_or_default_sysroot(), }; let file_path_mapping = sopts.file_path_mapping(); @@ -1147,7 +1139,7 @@ pub fn build_session_( entry_fn: Once::new(), plugin_registrar_fn: Once::new(), proc_macro_decls_static: Once::new(), - default_sysroot, + sysroot, local_crate_source_file, working_dir, lint_store: RwLock::new(lint::LintStore::new()), diff --git a/src/librustc_codegen_llvm/back/link.rs b/src/librustc_codegen_llvm/back/link.rs index 9e100a1427fc4..20a7bf9a1a959 100644 --- a/src/librustc_codegen_llvm/back/link.rs +++ b/src/librustc_codegen_llvm/back/link.rs @@ -1024,11 +1024,10 @@ fn link_args(cmd: &mut dyn Linker, // where extern libraries might live, based on the // addl_lib_search_paths if sess.opts.cg.rpath { - let sysroot = sess.sysroot(); let target_triple = sess.opts.target_triple.triple(); let mut get_install_prefix_lib_path = || { let install_prefix = option_env!("CFG_PREFIX").expect("CFG_PREFIX"); - let tlib = filesearch::relative_target_lib_path(sysroot, target_triple); + let tlib = filesearch::relative_target_lib_path(&sess.sysroot, target_triple); let mut path = PathBuf::from(install_prefix); path.push(&tlib); diff --git a/src/librustc_codegen_ssa/back/linker.rs b/src/librustc_codegen_ssa/back/linker.rs index 2f92c427f657c..4960c8922b9f9 100644 --- a/src/librustc_codegen_ssa/back/linker.rs +++ b/src/librustc_codegen_ssa/back/linker.rs @@ -606,8 +606,7 @@ impl<'a> Linker for MsvcLinker<'a> { self.cmd.arg("/DEBUG"); // This will cause the Microsoft linker to embed .natvis info into the PDB file - let sysroot = self.sess.sysroot(); - let natvis_dir_path = sysroot.join("lib\\rustlib\\etc"); + let natvis_dir_path = self.sess.sysroot.join("lib\\rustlib\\etc"); if let Ok(natvis_dir) = fs::read_dir(&natvis_dir_path) { // LLVM 5.0.0's lld-link frontend doesn't yet recognize, and chokes // on, the /NATVIS:... flags. LLVM 6 (or earlier) should at worst ignore diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 39777e0a65b50..41c9b22afe06f 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -1042,7 +1042,7 @@ impl RustcDefaultCalls { targets.sort(); println!("{}", targets.join("\n")); }, - Sysroot => println!("{}", sess.sysroot().display()), + Sysroot => println!("{}", sess.sysroot.display()), TargetSpec => println!("{}", sess.target.target.to_json().pretty()), FileNames | CrateName => { let input = input.unwrap_or_else(|| diff --git a/src/librustc_metadata/locator.rs b/src/librustc_metadata/locator.rs index 1f298f6d2d3a5..f01ed9e3ceef7 100644 --- a/src/librustc_metadata/locator.rs +++ b/src/librustc_metadata/locator.rs @@ -678,7 +678,7 @@ impl<'a> Context<'a> { // candidates are all canonicalized, so we canonicalize the sysroot // as well. if let Some((ref prev, _)) = ret { - let sysroot = self.sess.sysroot(); + let sysroot = &self.sess.sysroot; let sysroot = sysroot.canonicalize() .unwrap_or_else(|_| sysroot.to_path_buf()); if prev.starts_with(&sysroot) { From f13006182c9df451e7703307467fc1717239cf6e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 22 Nov 2018 16:33:07 +1100 Subject: [PATCH 3/6] Introduce `SearchPath` and replace `SearchPaths` with `Vec`. It's more idiomatic, makes the code shorter, and will help with the next commit. --- src/librustc/session/config.rs | 51 +++++++++++------------ src/librustc/session/filesearch.rs | 33 +++++++-------- src/librustc/session/search_paths.rs | 56 +++++++++----------------- src/librustc_codegen_llvm/back/link.rs | 12 +++--- src/librustdoc/config.rs | 11 +++-- src/librustdoc/core.rs | 2 +- src/librustdoc/test.rs | 8 ++-- 7 files changed, 79 insertions(+), 94 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 750b7f151f585..7ea7e44798764 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -14,7 +14,7 @@ use std::str::FromStr; use session::{early_error, early_warn, Session}; -use session::search_paths::SearchPaths; +use session::search_paths::SearchPath; use rustc_target::spec::{LinkerFlavor, PanicStrategy, RelroLevel}; use rustc_target::spec::{Target, TargetTriple}; @@ -374,7 +374,7 @@ top_level_options!( lint_cap: Option [TRACKED], describe_lints: bool [UNTRACKED], output_types: OutputTypes [TRACKED], - search_paths: SearchPaths [UNTRACKED], + search_paths: Vec [UNTRACKED], libs: Vec<(String, Option, Option)> [TRACKED], maybe_sysroot: Option [TRACKED], @@ -593,7 +593,7 @@ impl Default for Options { lint_cap: None, describe_lints: false, output_types: OutputTypes(BTreeMap::new()), - search_paths: SearchPaths::new(), + search_paths: vec![], maybe_sysroot: None, target_triple: TargetTriple::from_triple(host_triple()), test: false, @@ -2115,9 +2115,9 @@ pub fn build_session_options_and_crate_config( } }; - let mut search_paths = SearchPaths::new(); + let mut search_paths = vec![]; for s in &matches.opt_strs("L") { - search_paths.add_path(&s[..], error_format); + search_paths.push(SearchPath::from_cli_opt(&s[..], error_format)); } let libs = matches @@ -2535,6 +2535,7 @@ mod tests { use session::config::{build_configuration, build_session_options_and_crate_config}; use session::config::{LtoCli, CrossLangLto}; use session::build_session; + use session::search_paths::SearchPath; use std::collections::{BTreeMap, BTreeSet}; use std::iter::FromIterator; use std::path::PathBuf; @@ -2790,48 +2791,48 @@ mod tests { // Reference v1.search_paths - .add_path("native=abc", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("native=abc", super::ErrorOutputType::Json(false))); v1.search_paths - .add_path("crate=def", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("crate=def", super::ErrorOutputType::Json(false))); v1.search_paths - .add_path("dependency=ghi", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("dependency=ghi", super::ErrorOutputType::Json(false))); v1.search_paths - .add_path("framework=jkl", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("framework=jkl", super::ErrorOutputType::Json(false))); v1.search_paths - .add_path("all=mno", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("all=mno", super::ErrorOutputType::Json(false))); v2.search_paths - .add_path("native=abc", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("native=abc", super::ErrorOutputType::Json(false))); v2.search_paths - .add_path("dependency=ghi", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("dependency=ghi", super::ErrorOutputType::Json(false))); v2.search_paths - .add_path("crate=def", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("crate=def", super::ErrorOutputType::Json(false))); v2.search_paths - .add_path("framework=jkl", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("framework=jkl", super::ErrorOutputType::Json(false))); v2.search_paths - .add_path("all=mno", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("all=mno", super::ErrorOutputType::Json(false))); v3.search_paths - .add_path("crate=def", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("crate=def", super::ErrorOutputType::Json(false))); v3.search_paths - .add_path("framework=jkl", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("framework=jkl", super::ErrorOutputType::Json(false))); v3.search_paths - .add_path("native=abc", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("native=abc", super::ErrorOutputType::Json(false))); v3.search_paths - .add_path("dependency=ghi", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("dependency=ghi", super::ErrorOutputType::Json(false))); v3.search_paths - .add_path("all=mno", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("all=mno", super::ErrorOutputType::Json(false))); v4.search_paths - .add_path("all=mno", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("all=mno", super::ErrorOutputType::Json(false))); v4.search_paths - .add_path("native=abc", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("native=abc", super::ErrorOutputType::Json(false))); v4.search_paths - .add_path("crate=def", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("crate=def", super::ErrorOutputType::Json(false))); v4.search_paths - .add_path("dependency=ghi", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("dependency=ghi", super::ErrorOutputType::Json(false))); v4.search_paths - .add_path("framework=jkl", super::ErrorOutputType::Json(false)); + .push(SearchPath::from_cli_opt("framework=jkl", super::ErrorOutputType::Json(false))); assert!(v1.dep_tracking_hash() == v2.dep_tracking_hash()); assert!(v1.dep_tracking_hash() == v3.dep_tracking_hash()); diff --git a/src/librustc/session/filesearch.rs b/src/librustc/session/filesearch.rs index f5dfde7d5189d..6f52d28535343 100644 --- a/src/librustc/session/filesearch.rs +++ b/src/librustc/session/filesearch.rs @@ -18,7 +18,7 @@ use std::env; use std::fs; use std::path::{Path, PathBuf}; -use session::search_paths::{SearchPaths, PathKind}; +use session::search_paths::{SearchPath, PathKind}; use rustc_fs_util::fix_windows_verbatim_for_gcc; #[derive(Copy, Clone)] @@ -31,27 +31,28 @@ pub enum FileMatch { pub struct FileSearch<'a> { pub sysroot: &'a Path, - pub search_paths: &'a SearchPaths, + pub search_paths: &'a [SearchPath], pub triple: &'a str, pub kind: PathKind, } impl<'a> FileSearch<'a> { pub fn for_each_lib_search_path(&self, mut f: F) where - F: FnMut(&Path, PathKind) + F: FnMut(&SearchPath) { let mut visited_dirs = FxHashSet::default(); - visited_dirs.reserve(self.search_paths.paths.len() + 1); - for (path, kind) in self.search_paths.iter(self.kind) { - f(path, kind); - visited_dirs.insert(path.to_path_buf()); + 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(&tlib_path, PathKind::All); + f(&SearchPath { kind: PathKind::All, dir: tlib_path }); } } @@ -62,9 +63,9 @@ impl<'a> FileSearch<'a> { pub fn search(&self, mut pick: F) where F: FnMut(&Path, PathKind) -> FileMatch { - self.for_each_lib_search_path(|lib_search_path, kind| { - debug!("searching {}", lib_search_path.display()); - let files = match fs::read_dir(lib_search_path) { + 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, }; @@ -81,7 +82,7 @@ impl<'a> FileSearch<'a> { let files2 = files.iter().filter(|p| !is_rlib(p)); for path in files1.chain(files2) { debug!("testing {}", path.display()); - let maybe_picked = pick(path, kind); + let maybe_picked = pick(path, search_path.kind); match maybe_picked { FileMatches => { debug!("picked {}", path.display()); @@ -96,7 +97,7 @@ impl<'a> FileSearch<'a> { pub fn new(sysroot: &'a Path, triple: &'a str, - search_paths: &'a SearchPaths, + search_paths: &'a Vec, kind: PathKind) -> FileSearch<'a> { debug!("using sysroot = {}, triple = {}", sysroot.display(), triple); FileSearch { @@ -110,8 +111,8 @@ impl<'a> FileSearch<'a> { // Returns a list of directories where target-specific dylibs might be located. pub fn get_dylib_search_paths(&self) -> Vec { let mut paths = Vec::new(); - self.for_each_lib_search_path(|lib_search_path, _| { - paths.push(lib_search_path.to_path_buf()); + self.for_each_lib_search_path(|search_path| { + paths.push(search_path.dir.to_path_buf()); }); paths } @@ -136,7 +137,7 @@ pub fn relative_target_lib_path(sysroot: &Path, target_triple: &str) -> PathBuf p } -fn make_target_lib_path(sysroot: &Path, +pub fn make_target_lib_path(sysroot: &Path, target_triple: &str) -> PathBuf { sysroot.join(&relative_target_lib_path(sysroot, target_triple)) } diff --git a/src/librustc/session/search_paths.rs b/src/librustc/session/search_paths.rs index 6b0a8a0af2b9d..7a0bd2ed439b2 100644 --- a/src/librustc/session/search_paths.rs +++ b/src/librustc/session/search_paths.rs @@ -8,18 +8,14 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::slice; use std::path::{Path, PathBuf}; use session::{early_error, config}; +use session::filesearch::make_target_lib_path; #[derive(Clone, Debug)] -pub struct SearchPaths { - crate paths: Vec<(PathKind, PathBuf)>, -} - -pub struct Iter<'a> { - kind: PathKind, - iter: slice::Iter<'a, (PathKind, PathBuf)>, +pub struct SearchPath { + pub kind: PathKind, + pub dir: PathBuf, } #[derive(Eq, PartialEq, Clone, Copy, Debug, PartialOrd, Ord, Hash)] @@ -32,12 +28,17 @@ pub enum PathKind { All, } -impl SearchPaths { - pub fn new() -> SearchPaths { - SearchPaths { paths: Vec::new() } +impl PathKind { + pub fn matches(&self, kind: PathKind) -> bool { + match (self, kind) { + (PathKind::All, _) | (_, PathKind::All) => true, + _ => *self == kind, + } } +} - pub fn add_path(&mut self, path: &str, output: config::ErrorOutputType) { +impl SearchPath { + pub fn from_cli_opt(path: &str, output: config::ErrorOutputType) -> Self { let (kind, path) = if path.starts_with("native=") { (PathKind::Native, &path["native=".len()..]) } else if path.starts_with("crate=") { @@ -54,34 +55,17 @@ impl SearchPaths { if path.is_empty() { early_error(output, "empty search path given via `-L`"); } - self.paths.push((kind, PathBuf::from(path))); - } - pub fn iter(&self, kind: PathKind) -> Iter<'_> { - Iter { kind: kind, iter: self.paths.iter() } + let dir = PathBuf::from(path); + Self::new(kind, dir) } -} - -impl<'a> Iterator for Iter<'a> { - type Item = (&'a Path, PathKind); - fn next(&mut self) -> Option<(&'a Path, PathKind)> { - loop { - match *self.iter.next()? { - (kind, ref p) if self.kind == PathKind::All || - kind == PathKind::All || - kind == self.kind => { - return Some((p, kind)) - } - _ => {} - } - } + pub fn from_sysroot_and_triple(sysroot: &Path, triple: &str) -> Self { + Self::new(PathKind::All, make_target_lib_path(sysroot, triple)) } - fn size_hint(&self) -> (usize, Option) { - // This iterator will never return more elements than the base iterator; - // but it can ignore all the remaining elements. - let (_, upper) = self.iter.size_hint(); - (0, upper) + fn new(kind: PathKind, dir: PathBuf) -> Self { + SearchPath { kind, dir } } } + diff --git a/src/librustc_codegen_llvm/back/link.rs b/src/librustc_codegen_llvm/back/link.rs index 20a7bf9a1a959..da5ad39ad2664 100644 --- a/src/librustc_codegen_llvm/back/link.rs +++ b/src/librustc_codegen_llvm/back/link.rs @@ -213,8 +213,8 @@ fn link_binary_output(sess: &Session, fn archive_search_paths(sess: &Session) -> Vec { let mut search = Vec::new(); - sess.target_filesearch(PathKind::Native).for_each_lib_search_path(|path, _| { - search.push(path.to_path_buf()); + sess.target_filesearch(PathKind::Native).for_each_lib_search_path(|search_path| { + search.push(search_path.dir.to_path_buf()); }); search @@ -1067,10 +1067,10 @@ fn link_args(cmd: &mut dyn Linker, fn add_local_native_libraries(cmd: &mut dyn Linker, sess: &Session, codegen_results: &CodegenResults) { - sess.target_filesearch(PathKind::All).for_each_lib_search_path(|path, k| { - match k { - PathKind::Framework => { cmd.framework_path(path); } - _ => { cmd.include_path(&fix_windows_verbatim_for_gcc(path)); } + sess.target_filesearch(PathKind::All).for_each_lib_search_path(|search_path| { + match search_path.kind { + PathKind::Framework => { cmd.framework_path(&search_path.dir); } + _ => { cmd.include_path(&fix_windows_verbatim_for_gcc(&search_path.dir)); } } }); diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index f4d05c6dbd65c..b421f07ddafa2 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -20,7 +20,7 @@ use rustc::session::early_error; use rustc::session::config::{CodegenOptions, DebuggingOptions, ErrorOutputType, Externs}; use rustc::session::config::{nightly_options, build_codegen_options, build_debugging_options, get_cmd_lint_options}; -use rustc::session::search_paths::SearchPaths; +use rustc::session::search_paths::SearchPath; use rustc_driver; use rustc_target::spec::TargetTriple; use syntax::edition::Edition; @@ -46,7 +46,7 @@ pub struct Options { /// How to format errors and warnings. pub error_format: ErrorOutputType, /// Library search paths to hand to the compiler. - pub libs: SearchPaths, + pub libs: Vec, /// The list of external crates to link against. pub externs: Externs, /// List of `cfg` flags to hand to the compiler. Always includes `rustdoc`. @@ -295,10 +295,9 @@ impl Options { } let input = PathBuf::from(&matches.free[0]); - let mut libs = SearchPaths::new(); - for s in &matches.opt_strs("L") { - libs.add_path(s, error_format); - } + let libs = matches.opt_strs("L").iter() + .map(|s| SearchPath::from_cli_opt(s, error_format)) + .collect(); let externs = match parse_externs(&matches) { Ok(ex) => ex, Err(err) => { diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 047d35579444f..b85342f631181 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -51,7 +51,7 @@ use html::render::RenderInfo; use passes; pub use rustc::session::config::{Input, Options, CodegenOptions}; -pub use rustc::session::search_paths::SearchPaths; +pub use rustc::session::search_paths::SearchPath; pub type ExternalPaths = FxHashMap, clean::TypeKind)>; diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index be9327ced26e9..50acde64cf023 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -21,7 +21,7 @@ use rustc::hir; use rustc::hir::intravisit; use rustc::session::{self, CompileIncomplete, config}; use rustc::session::config::{OutputType, OutputTypes, Externs, CodegenOptions}; -use rustc::session::search_paths::{SearchPaths, PathKind}; +use rustc::session::search_paths::{SearchPath, PathKind}; use syntax::ast; use syntax::source_map::SourceMap; use syntax::edition::Edition; @@ -187,7 +187,7 @@ fn scrape_test_config(krate: &::rustc::hir::Crate) -> TestOptions { } fn run_test(test: &str, cratename: &str, filename: &FileName, line: usize, - cfgs: Vec, libs: SearchPaths, + cfgs: Vec, libs: Vec, cg: CodegenOptions, externs: Externs, should_panic: bool, no_run: bool, as_test_harness: bool, compile_fail: bool, mut error_codes: Vec, opts: &TestOptions, @@ -556,7 +556,7 @@ pub struct Collector { names: Vec, cfgs: Vec, - libs: SearchPaths, + libs: Vec, cg: CodegenOptions, externs: Externs, use_headers: bool, @@ -571,7 +571,7 @@ pub struct Collector { } impl Collector { - pub fn new(cratename: String, cfgs: Vec, libs: SearchPaths, cg: CodegenOptions, + pub fn new(cratename: String, cfgs: Vec, libs: Vec, cg: CodegenOptions, externs: Externs, use_headers: bool, opts: TestOptions, maybe_sysroot: Option, source_map: Option>, filename: Option, linker: Option, edition: Edition) -> Collector { From 2bfe32cc9301d404c98d896efbabe8f04361d5bf Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 20 Nov 2018 11:06:45 +1100 Subject: [PATCH 4/6] Avoid regenerating the `Vec` in `FileSearch::search()`. `FileSearch::search()` traverses one or more directories. For each directory it generates a `Vec` 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`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. --- src/librustc/session/filesearch.rs | 34 +++++++++------------------- src/librustc/session/mod.rs | 19 +++++++++++++++- src/librustc/session/search_paths.rs | 14 +++++++++++- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/librustc/session/filesearch.rs b/src/librustc/session/filesearch.rs index 6f52d28535343..6bfed4fbf5a25 100644 --- a/src/librustc/session/filesearch.rs +++ b/src/librustc/session/filesearch.rs @@ -12,7 +12,6 @@ pub use self::FileMatch::*; -use rustc_data_structures::fx::FxHashSet; use std::borrow::Cow; use std::env; use std::fs; @@ -31,8 +30,9 @@ 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, } @@ -40,20 +40,12 @@ impl<'a> FileSearch<'a> { pub fn for_each_lib_search_path(&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 { @@ -65,12 +57,6 @@ 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::>(); fn is_rlib(p: &Path) -> bool { p.extension() == Some("rlib".as_ref()) } @@ -78,8 +64,8 @@ impl<'a> FileSearch<'a> { // 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); @@ -98,12 +84,15 @@ impl<'a> FileSearch<'a> { pub fn new(sysroot: &'a Path, triple: &'a str, search_paths: &'a Vec, - 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, } } @@ -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)) } diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 56a57b5375fd8..12b5646e7f1d1 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -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; @@ -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, pub parse_sess: ParseSess, /// For a library crate, this is always none pub entry_fn: Once>, @@ -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, ) } @@ -707,6 +712,7 @@ impl Session { &self.sysroot, config::host_triple(), &self.opts.search_paths, + &self.host_tlib_path, kind, ) } @@ -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 = @@ -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(), diff --git a/src/librustc/session/search_paths.rs b/src/librustc/session/search_paths.rs index 7a0bd2ed439b2..5c44a07f84341 100644 --- a/src/librustc/session/search_paths.rs +++ b/src/librustc/session/search_paths.rs @@ -16,6 +16,7 @@ use session::filesearch::make_target_lib_path; pub struct SearchPath { pub kind: PathKind, pub dir: PathBuf, + pub files: Vec, } #[derive(Eq, PartialEq, Clone, Copy, Debug, PartialOrd, Ord, Hash)] @@ -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::>() + } + Err(..) => vec![], + }; + + SearchPath { kind, dir, files } } } From 95a6262df18d8eb474efab273717dee25e79738e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 23 Nov 2018 13:36:41 +1100 Subject: [PATCH 5/6] Replace `FileSearch::for_each_lib_search_path` with `search_paths`. Returning an iterator leads to nicer code all around. --- src/librustc/session/filesearch.rs | 40 +++++++++++--------------- src/librustc_codegen_llvm/back/link.rs | 12 +++----- src/librustc_driver/driver.rs | 2 +- 3 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/librustc/session/filesearch.rs b/src/librustc/session/filesearch.rs index 6bfed4fbf5a25..c204556d517a8 100644 --- a/src/librustc/session/filesearch.rs +++ b/src/librustc/session/filesearch.rs @@ -29,23 +29,19 @@ pub enum FileMatch { // A module for searching for libraries pub struct FileSearch<'a> { - pub sysroot: &'a Path, - pub triple: &'a str, - pub search_paths: &'a [SearchPath], - pub tlib_path: &'a SearchPath, - pub kind: PathKind, + sysroot: &'a Path, + triple: &'a str, + search_paths: &'a [SearchPath], + tlib_path: &'a SearchPath, + kind: PathKind, } impl<'a> FileSearch<'a> { - pub fn for_each_lib_search_path(&self, mut f: F) where - F: FnMut(&SearchPath) - { - let iter = self.search_paths.iter().filter(|sp| sp.kind.matches(self.kind)); - for search_path in iter { - f(search_path); - } - - f(self.tlib_path); + pub fn search_paths(&self) -> impl Iterator { + let kind = self.kind; + self.search_paths.iter() + .filter(move |sp| sp.kind.matches(kind)) + .chain(std::iter::once(self.tlib_path)) } pub fn get_lib_path(&self) -> PathBuf { @@ -55,7 +51,7 @@ impl<'a> FileSearch<'a> { pub fn search(&self, mut pick: F) where F: FnMut(&Path, PathKind) -> FileMatch { - self.for_each_lib_search_path(|search_path| { + 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()) @@ -78,7 +74,7 @@ impl<'a> FileSearch<'a> { } } } - }); + } } pub fn new(sysroot: &'a Path, @@ -97,13 +93,11 @@ impl<'a> FileSearch<'a> { } } - // Returns a list of directories where target-specific dylibs might be located. - pub fn get_dylib_search_paths(&self) -> Vec { - let mut paths = Vec::new(); - self.for_each_lib_search_path(|search_path| { - paths.push(search_path.dir.to_path_buf()); - }); - paths + // Returns just the directories within the search paths. + pub fn search_path_dirs(&self) -> Vec { + self.search_paths() + .map(|sp| sp.dir.to_path_buf()) + .collect() } // Returns a list of directories where target-specific tool binaries are located. diff --git a/src/librustc_codegen_llvm/back/link.rs b/src/librustc_codegen_llvm/back/link.rs index da5ad39ad2664..f1c0464f5f25b 100644 --- a/src/librustc_codegen_llvm/back/link.rs +++ b/src/librustc_codegen_llvm/back/link.rs @@ -212,12 +212,7 @@ fn link_binary_output(sess: &Session, } fn archive_search_paths(sess: &Session) -> Vec { - let mut search = Vec::new(); - sess.target_filesearch(PathKind::Native).for_each_lib_search_path(|search_path| { - search.push(search_path.dir.to_path_buf()); - }); - - search + sess.target_filesearch(PathKind::Native).search_path_dirs() } fn archive_config<'a>(sess: &'a Session, @@ -1067,12 +1062,13 @@ fn link_args(cmd: &mut dyn Linker, fn add_local_native_libraries(cmd: &mut dyn Linker, sess: &Session, codegen_results: &CodegenResults) { - sess.target_filesearch(PathKind::All).for_each_lib_search_path(|search_path| { + let filesearch = sess.target_filesearch(PathKind::All); + for search_path in filesearch.search_paths() { match search_path.kind { PathKind::Framework => { cmd.framework_path(&search_path.dir); } _ => { cmd.include_path(&fix_windows_verbatim_for_gcc(&search_path.dir)); } } - }); + } let relevant_libs = codegen_results.crate_info.used_libraries.iter().filter(|l| { relevant_lib(sess, l) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index f2edcdc1bac35..b26d81b9c6729 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -975,7 +975,7 @@ where let mut old_path = OsString::new(); if cfg!(windows) { old_path = env::var_os("PATH").unwrap_or(old_path); - let mut new_path = sess.host_filesearch(PathKind::All).get_dylib_search_paths(); + let mut new_path = sess.host_filesearch(PathKind::All).search_path_dirs(); for path in env::split_paths(&old_path) { if !new_path.contains(&path) { new_path.push(path); From 209240dc267075d69eb8d1dba23be2a8c80c6427 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 12 Dec 2018 16:28:43 +1100 Subject: [PATCH 6/6] Remove some env vars for rustdoc invocations. In an attempt to avoid "thread '' panicked at 'failed to acquire jobserver token: Bad file descriptor" errors. --- src/bootstrap/builder.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 7dc64db70b8fc..32f3e573d6845 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -684,6 +684,11 @@ impl<'a> Builder<'a> { .env("RUSTDOC_REAL", self.rustdoc(host)) .env("RUSTDOC_CRATE_VERSION", self.rust_version()) .env("RUSTC_BOOTSTRAP", "1"); + + // Remove make-related flags that can cause jobserver problems. + cmd.env_remove("MAKEFLAGS"); + cmd.env_remove("MFLAGS"); + if let Some(linker) = self.linker(host) { cmd.env("RUSTC_TARGET_LINKER", linker); }