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

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<lint::Level> [TRACKED],
         describe_lints: bool [UNTRACKED],
         output_types: OutputTypes [TRACKED],
-        search_paths: SearchPaths [UNTRACKED],
+        search_paths: Vec<SearchPath> [UNTRACKED],
         libs: Vec<(String, Option<String>, Option<cstore::NativeLibraryKind>)> [TRACKED],
         maybe_sysroot: Option<PathBuf> [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());

src/librustc/session/filesearch.rs

@@ -12,13 +12,12 @@
 
 pub use self::FileMatch::*;
 
-use rustc_data_structures::fx::FxHashSet;
 use std::borrow::Cow;
 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)]
@@ -30,31 +29,19 @@ pub enum FileMatch {
 // A module for searching for libraries
 
 pub struct FileSearch<'a> {
-    pub sysroot: &'a Path,
-    pub search_paths: &'a SearchPaths,
-    pub triple: &'a str,
-    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<F>(&self, mut f: F) where
-        F: FnMut(&Path, PathKind)
-    {
-        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());
-        }
-
-        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);
-        }
-
-        visited_dirs.insert(tlib_path);
+    pub fn search_paths(&self) -> impl Iterator<Item = &'a SearchPath> {
+        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 {
@@ -64,26 +51,20 @@ impl<'a> FileSearch<'a> {
     pub fn search<F>(&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) {
-                Ok(files) => files,
-                Err(..) => return,
-            };
-            let files = files.filter_map(|p| p.ok().map(|s| s.path()))
-                             .collect::<Vec<_>>();
+        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())
             }
             // 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, kind);
+                let maybe_picked = pick(path, search_path.kind);
                 match maybe_picked {
                     FileMatches => {
                         debug!("picked {}", path.display());
@@ -93,29 +74,30 @@ impl<'a> FileSearch<'a> {
                     }
                 }
             }
-        });
+        }
     }
 
     pub fn new(sysroot: &'a Path,
                triple: &'a str,
-               search_paths: &'a SearchPaths,
-               kind: PathKind) -> FileSearch<'a> {
+               search_paths: &'a Vec<SearchPath>,
+               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,
         }
     }
 
-    // Returns a list of directories where target-specific dylibs might be located.
-    pub fn get_dylib_search_paths(&self) -> Vec<PathBuf> {
-        let mut paths = Vec::new();
-        self.for_each_lib_search_path(|lib_search_path, _| {
-            paths.push(lib_search_path.to_path_buf());
-        });
-        paths
+    // Returns just the directories within the search paths.
+    pub fn search_path_dirs(&self) -> Vec<PathBuf> {
+        self.search_paths()
+            .map(|sp| sp.dir.to_path_buf())
+            .collect()
     }
 
     // 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
     p
 }
 
-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))
 }
 

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;
@@ -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};
@@ -64,12 +64,15 @@ 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)>>,
     pub plugin_registrar_fn: Once<Option<ast::NodeId>>,
     pub proc_macro_decls_static: Once<Option<ast::NodeId>>,
-    pub default_sysroot: Option<PathBuf>,
+    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<PathBuf>,
@@ -694,27 +697,22 @@ 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,
+            // 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,
         )
     }
     pub fn host_filesearch(&self, kind: PathKind) -> filesearch::FileSearch<'_> {
         filesearch::FileSearch::new(
-            self.sysroot(),
+            &self.sysroot,
             config::host_triple(),
             &self.opts.search_paths,
+            &self.host_tlib_path,
             kind,
         )
     }
@@ -1109,9 +1107,18 @@ 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 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();
@@ -1142,12 +1149,14 @@ 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(),
         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()),