Rollup merge of #93608 - nnethercote:speed-up-find_library_crate, r=p…

…etrochenkov

Clean up `find_library_crate`

Some clean-ups.

r? `@petrochenkov`

compiler/rustc_metadata/src/locator.rs

@@ -223,7 +223,7 @@ use rustc_data_structures::sync::MetadataRef;
 use rustc_errors::{struct_span_err, FatalError};
 use rustc_session::config::{self, CrateType};
 use rustc_session::cstore::{CrateSource, MetadataLoader};
-use rustc_session::filesearch::{FileDoesntMatch, FileMatches, FileSearch};
+use rustc_session::filesearch::FileSearch;
 use rustc_session::search_paths::PathKind;
 use rustc_session::utils::CanonicalizedPath;
 use rustc_session::Session;
@@ -371,15 +371,20 @@ impl<'a> CrateLocator<'a> {
         extra_prefix: &str,
         seen_paths: &mut FxHashSet<PathBuf>,
     ) -> Result<Option<Library>, CrateError> {
-        // want: crate_name.dir_part() + prefix + crate_name.file_part + "-"
-        let dylib_prefix = format!("{}{}{}", self.target.dll_prefix, self.crate_name, extra_prefix);
-        let rlib_prefix = format!("lib{}{}", self.crate_name, extra_prefix);
+        let rmeta_prefix = &format!("lib{}{}", self.crate_name, extra_prefix);
+        let rlib_prefix = rmeta_prefix;
+        let dylib_prefix =
+            &format!("{}{}{}", self.target.dll_prefix, self.crate_name, extra_prefix);
         let staticlib_prefix =
-            format!("{}{}{}", self.target.staticlib_prefix, self.crate_name, extra_prefix);
+            &format!("{}{}{}", self.target.staticlib_prefix, self.crate_name, extra_prefix);
+
+        let rmeta_suffix = ".rmeta";
+        let rlib_suffix = ".rlib";
+        let dylib_suffix = &self.target.dll_suffix;
+        let staticlib_suffix = &self.target.staticlib_suffix;
 
         let mut candidates: FxHashMap<_, (FxHashMap<_, _>, FxHashMap<_, _>, FxHashMap<_, _>)> =
             Default::default();
-        let mut staticlibs = vec![];
 
         // First, find all possible candidate rlibs and dylibs purely based on
         // the name of the files themselves. We're trying to match against an
@@ -394,46 +399,50 @@ 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(|spf, kind| {
-            let file = match &spf.file_name_str {
-                None => return FileDoesntMatch,
-                Some(file) => file,
-            };
-            let (hash, found_kind) = if file.starts_with(&rlib_prefix) && file.ends_with(".rlib") {
-                (&file[(rlib_prefix.len())..(file.len() - ".rlib".len())], CrateFlavor::Rlib)
-            } else if file.starts_with(&rlib_prefix) && file.ends_with(".rmeta") {
-                (&file[(rlib_prefix.len())..(file.len() - ".rmeta".len())], CrateFlavor::Rmeta)
-            } else if file.starts_with(&dylib_prefix) && file.ends_with(&self.target.dll_suffix) {
-                (
-                    &file[(dylib_prefix.len())..(file.len() - self.target.dll_suffix.len())],
-                    CrateFlavor::Dylib,
-                )
-            } else {
-                if file.starts_with(&staticlib_prefix)
-                    && file.ends_with(&self.target.staticlib_suffix)
-                {
-                    staticlibs
-                        .push(CrateMismatch { path: spf.path.clone(), got: "static".to_string() });
-                }
-                return FileDoesntMatch;
-            };
+        // Unfortunately, the prefix-based matching sometimes is over-eager.
+        // E.g. if `rlib_suffix` is `libstd` it'll match the file
+        // `libstd_detect-8d6701fb958915ad.rlib` (incorrect) as well as
+        // `libstd-f3ab5b1dea981f17.rlib` (correct). But this is hard to avoid
+        // given that `extra_filename` comes from the `-C extra-filename`
+        // option and thus can be anything, and the incorrect match will be
+        // handled safely in `extract_one`.
+        for search_path in self.filesearch.search_paths() {
+            debug!("searching {}", search_path.dir.display());
+            for spf in search_path.files.iter() {
+                debug!("testing {}", spf.path.display());
+
+                let f = &spf.file_name_str;
+                let (hash, kind) = if f.starts_with(rlib_prefix) && f.ends_with(rlib_suffix) {
+                    (&f[rlib_prefix.len()..(f.len() - rlib_suffix.len())], CrateFlavor::Rlib)
+                } else if f.starts_with(rmeta_prefix) && f.ends_with(rmeta_suffix) {
+                    (&f[rmeta_prefix.len()..(f.len() - rmeta_suffix.len())], CrateFlavor::Rmeta)
+                } else if f.starts_with(dylib_prefix) && f.ends_with(dylib_suffix) {
+                    (&f[dylib_prefix.len()..(f.len() - dylib_suffix.len())], CrateFlavor::Dylib)
+                } else {
+                    if f.starts_with(staticlib_prefix) && f.ends_with(staticlib_suffix) {
+                        self.crate_rejections.via_kind.push(CrateMismatch {
+                            path: spf.path.clone(),
+                            got: "static".to_string(),
+                        });
+                    }
+                    continue;
+                };
 
-            info!("lib candidate: {}", spf.path.display());
+                info!("lib candidate: {}", spf.path.display());
 
-            let (rlibs, rmetas, dylibs) = candidates.entry(hash.to_string()).or_default();
-            let path = fs::canonicalize(&spf.path).unwrap_or_else(|_| spf.path.clone());
-            if seen_paths.contains(&path) {
-                return FileDoesntMatch;
-            };
-            seen_paths.insert(path.clone());
-            match found_kind {
-                CrateFlavor::Rlib => rlibs.insert(path, kind),
-                CrateFlavor::Rmeta => rmetas.insert(path, kind),
-                CrateFlavor::Dylib => dylibs.insert(path, kind),
-            };
-            FileMatches
-        });
-        self.crate_rejections.via_kind.extend(staticlibs);
+                let (rlibs, rmetas, dylibs) = candidates.entry(hash.to_string()).or_default();
+                let path = fs::canonicalize(&spf.path).unwrap_or_else(|_| spf.path.clone());
+                if seen_paths.contains(&path) {
+                    continue;
+                };
+                seen_paths.insert(path.clone());
+                match kind {
+                    CrateFlavor::Rlib => rlibs.insert(path, search_path.kind),
+                    CrateFlavor::Rmeta => rmetas.insert(path, search_path.kind),
+                    CrateFlavor::Dylib => dylibs.insert(path, search_path.kind),
+                };
+            }
+        }
 
         // We have now collected all known libraries into a set of candidates
         // keyed of the filename hash listed. For each filename, we also have a

compiler/rustc_session/src/filesearch.rs

@@ -1,13 +1,11 @@
 //! A module for searching for libraries
 
-pub use self::FileMatch::*;
-
 use std::env;
 use std::fs;
 use std::iter::FromIterator;
 use std::path::{Path, PathBuf};
 
-use crate::search_paths::{PathKind, SearchPath, SearchPathFile};
+use crate::search_paths::{PathKind, SearchPath};
 use rustc_fs_util::fix_windows_verbatim_for_gcc;
 use tracing::debug;
 
@@ -43,36 +41,6 @@ impl<'a> FileSearch<'a> {
         self.get_lib_path().join("self-contained")
     }
 
-    pub fn search<F>(&self, mut pick: F)
-    where
-        F: FnMut(&SearchPathFile, PathKind) -> FileMatch,
-    {
-        for search_path in self.search_paths() {
-            debug!("searching {}", search_path.dir.display());
-            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(|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 {}", spf.path.display());
-                    }
-                    FileDoesntMatch => {
-                        debug!("rejected {}", spf.path.display());
-                    }
-                }
-            }
-        }
-    }
-
     pub fn new(
         sysroot: &'a Path,
         triple: &'a str,

compiler/rustc_session/src/search_paths.rs

@@ -15,22 +15,15 @@ pub struct SearchPath {
 /// 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
+/// This type augments the `PathBuf` with an `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.)
+/// `String` than the `PathBuf`. (The filename must be valid UTF-8. If it's
+/// not, the entry should be skipped, because all Rust output files are valid
+/// UTF-8, and so a non-UTF-8 filename couldn't be one we're looking for.)
 #[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 }
-    }
+    pub file_name_str: String,
 }
 
 #[derive(PartialEq, Clone, Copy, Debug, Hash, Eq, Encodable, Decodable)]
@@ -85,7 +78,14 @@ impl SearchPath {
         // Get the files within the directory.
         let files = match std::fs::read_dir(&dir) {
             Ok(files) => files
-                .filter_map(|e| e.ok().map(|e| SearchPathFile::new(e.path())))
+                .filter_map(|e| {
+                    e.ok().and_then(|e| {
+                        e.file_name().to_str().map(|s| SearchPathFile {
+                            path: e.path(),
+                            file_name_str: s.to_string(),
+                        })
+                    })
+                })
                 .collect::<Vec<_>>(),
             Err(..) => vec![],
         };