From ed62b57c86a7d2985a4eae817bbce807f1ca957e Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 12 Apr 2024 17:01:46 +0300 Subject: [PATCH] linker: Remove laziness and caching from native search directory walks It shouldn't be necessary for performance now. --- compiler/rustc_codegen_ssa/src/back/link.rs | 33 +------ compiler/rustc_codegen_ssa/src/back/linker.rs | 87 +++---------------- compiler/rustc_interface/src/passes.rs | 2 +- compiler/rustc_metadata/src/native_libs.rs | 16 ++-- compiler/rustc_session/src/filesearch.rs | 5 -- 5 files changed, 22 insertions(+), 121 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index f4374392b9b..5dbf31e5d6e 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -42,7 +42,6 @@ use tempfile::Builder as TempFileBuilder; use itertools::Itertools; -use std::cell::OnceCell; use std::collections::BTreeSet; use std::ffi::{OsStr, OsString}; use std::fs::{read, File, OpenOptions}; @@ -52,20 +51,6 @@ use std::process::{ExitStatus, Output, Stdio}; use std::{env, fmt, fs, io, mem, str}; -#[derive(Default)] -pub struct SearchPaths(OnceCell>); - -impl SearchPaths { - pub(super) fn get(&self, sess: &Session) -> impl Iterator { - let native_search_paths = || { - Vec::from_iter( - sess.target_filesearch(PathKind::Native).search_path_dirs().map(|p| p.to_owned()), - ) - }; - self.0.get_or_init(native_search_paths).iter().map(|p| &**p) - } -} - pub fn ensure_removed(dcx: &DiagCtxt, path: &Path) { if let Err(e) = fs::remove_file(path) { if e.kind() != io::ErrorKind::NotFound { @@ -381,24 +366,21 @@ fn link_rlib<'a>( // feature then we'll need to figure out how to record what objects were // loaded from the libraries found here and then encode that into the // metadata of the rlib we're generating somehow. - let search_paths = SearchPaths::default(); for lib in codegen_results.crate_info.used_libraries.iter() { let NativeLibKind::Static { bundle: None | Some(true), .. } = lib.kind else { continue; }; - let search_paths = search_paths.get(sess); if flavor == RlibFlavor::Normal && let Some(filename) = lib.filename { - let path = find_native_static_library(filename.as_str(), true, search_paths, sess); + let path = find_native_static_library(filename.as_str(), true, sess); let src = read(path) .map_err(|e| sess.dcx().emit_fatal(errors::ReadFileError { message: e }))?; let (data, _) = create_wrapper_file(sess, ".bundled_lib".to_string(), &src); let wrapper_file = emit_wrapper_file(sess, &data, tmpdir, filename.as_str()); packed_bundled_libs.push(wrapper_file); } else { - let path = - find_native_static_library(lib.name.as_str(), lib.verbatim, search_paths, sess); + let path = find_native_static_library(lib.name.as_str(), lib.verbatim, sess); ab.add_archive(&path, Box::new(|_| false)).unwrap_or_else(|error| { sess.dcx().emit_fatal(errors::AddNativeLibrary { library_path: path, error }) }); @@ -2497,7 +2479,6 @@ fn add_native_libs_from_crate( archive_builder_builder: &dyn ArchiveBuilderBuilder, codegen_results: &CodegenResults, tmpdir: &Path, - search_paths: &SearchPaths, bundled_libs: &FxIndexSet, cnum: CrateNum, link_static: bool, @@ -2560,7 +2541,7 @@ fn add_native_libs_from_crate( cmd.link_staticlib_by_path(&path, whole_archive); } } else { - cmd.link_staticlib_by_name(name, verbatim, whole_archive, search_paths); + cmd.link_staticlib_by_name(name, verbatim, whole_archive); } } } @@ -2574,7 +2555,7 @@ fn add_native_libs_from_crate( // link kind is unspecified. if !link_output_kind.can_link_dylib() && !sess.target.crt_static_allows_dylibs { if link_static { - cmd.link_staticlib_by_name(name, verbatim, false, search_paths); + cmd.link_staticlib_by_name(name, verbatim, false); } } else { if link_dynamic { @@ -2621,7 +2602,6 @@ fn add_local_native_libraries( } } - let search_paths = SearchPaths::default(); // All static and dynamic native library dependencies are linked to the local crate. let link_static = true; let link_dynamic = true; @@ -2631,7 +2611,6 @@ fn add_local_native_libraries( archive_builder_builder, codegen_results, tmpdir, - &search_paths, &Default::default(), LOCAL_CRATE, link_static, @@ -2663,7 +2642,6 @@ fn add_upstream_rust_crates<'a>( .find(|(ty, _)| *ty == crate_type) .expect("failed to find crate type in dependency format list"); - let search_paths = SearchPaths::default(); for &cnum in &codegen_results.crate_info.used_crates { // We may not pass all crates through to the linker. Some crates may appear statically in // an existing dylib, meaning we'll pick up all the symbols from the dylib. @@ -2720,7 +2698,6 @@ fn add_upstream_rust_crates<'a>( archive_builder_builder, codegen_results, tmpdir, - &search_paths, &bundled_libs, cnum, link_static, @@ -2738,7 +2715,6 @@ fn add_upstream_native_libraries( tmpdir: &Path, link_output_kind: LinkOutputKind, ) { - let search_paths = SearchPaths::default(); for &cnum in &codegen_results.crate_info.used_crates { // Static libraries are not linked here, they are linked in `add_upstream_rust_crates`. // FIXME: Merge this function to `add_upstream_rust_crates` so that all native libraries @@ -2760,7 +2736,6 @@ fn add_upstream_native_libraries( archive_builder_builder, codegen_results, tmpdir, - &search_paths, &Default::default(), cnum, link_static, diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index f5640ea26bc..fad6f439441 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -1,6 +1,5 @@ use super::command::Command; use super::symbol_export; -use crate::back::link::SearchPaths; use crate::errors; use rustc_span::symbol::sym; @@ -172,13 +171,7 @@ pub trait Linker { fn link_framework_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { bug!("framework linked with unsupported linker") } - fn link_staticlib_by_name( - &mut self, - name: &str, - verbatim: bool, - whole_archive: bool, - search_paths: &SearchPaths, - ); + fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool, whole_archive: bool); fn link_staticlib_by_path(&mut self, path: &Path, whole_archive: bool); fn include_path(&mut self, path: &Path); fn framework_path(&mut self, path: &Path); @@ -482,13 +475,7 @@ fn link_framework_by_name(&mut self, name: &str, _verbatim: bool, as_needed: boo self.cmd.arg("-framework").arg(name); } - fn link_staticlib_by_name( - &mut self, - name: &str, - verbatim: bool, - whole_archive: bool, - search_paths: &SearchPaths, - ) { + fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool, whole_archive: bool) { self.hint_static(); let colon = if verbatim && self.is_gnu { ":" } else { "" }; if !whole_archive { @@ -497,8 +484,7 @@ fn link_staticlib_by_name( // -force_load is the macOS equivalent of --whole-archive, but it // involves passing the full path to the library to link. self.linker_arg("-force_load"); - let search_paths = search_paths.get(self.sess); - self.linker_arg(find_native_static_library(name, verbatim, search_paths, self.sess)); + self.linker_arg(find_native_static_library(name, verbatim, self.sess)); } else { self.linker_arg("--whole-archive"); self.cmd.arg(format!("-l{colon}{name}")); @@ -825,13 +811,7 @@ fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, _as_needed: bool) { self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); } - fn link_staticlib_by_name( - &mut self, - name: &str, - verbatim: bool, - whole_archive: bool, - _search_paths: &SearchPaths, - ) { + fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool, whole_archive: bool) { let prefix = if whole_archive { "/WHOLEARCHIVE:" } else { "" }; let suffix = if verbatim { "" } else { ".lib" }; self.cmd.arg(format!("{prefix}{name}{suffix}")); @@ -1064,13 +1044,7 @@ fn link_dylib_by_name(&mut self, name: &str, _verbatim: bool, _as_needed: bool) self.cmd.arg("-l").arg(name); } - fn link_staticlib_by_name( - &mut self, - name: &str, - _verbatim: bool, - _whole_archive: bool, - _search_paths: &SearchPaths, - ) { + fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool, _whole_archive: bool) { self.cmd.arg("-l").arg(name); } @@ -1243,13 +1217,7 @@ fn link_dylib_by_name(&mut self, name: &str, _verbatim: bool, _as_needed: bool) self.cmd.arg("-l").arg(name); } - fn link_staticlib_by_name( - &mut self, - name: &str, - _verbatim: bool, - whole_archive: bool, - _search_paths: &SearchPaths, - ) { + fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool, whole_archive: bool) { if !whole_archive { self.cmd.arg("-l").arg(name); } else { @@ -1396,13 +1364,7 @@ fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) bug!("dylibs are not supported on L4Re"); } - fn link_staticlib_by_name( - &mut self, - name: &str, - _verbatim: bool, - whole_archive: bool, - _search_paths: &SearchPaths, - ) { + fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool, whole_archive: bool) { self.hint_static(); if !whole_archive { self.cmd.arg(format!("-PC{name}")); @@ -1580,20 +1542,13 @@ fn link_dylib_by_name(&mut self, name: &str, _verbatim: bool, _as_needed: bool) self.cmd.arg(format!("-l{name}")); } - fn link_staticlib_by_name( - &mut self, - name: &str, - verbatim: bool, - whole_archive: bool, - search_paths: &SearchPaths, - ) { + fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool, whole_archive: bool) { self.hint_static(); if !whole_archive { self.cmd.arg(format!("-l{name}")); } else { let mut arg = OsString::from("-bkeepfile:"); - let search_path = search_paths.get(self.sess); - arg.push(find_native_static_library(name, verbatim, search_path, self.sess)); + arg.push(find_native_static_library(name, verbatim, self.sess)); self.cmd.arg(arg); } } @@ -1792,13 +1747,7 @@ fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) panic!("external dylibs not supported") } - fn link_staticlib_by_name( - &mut self, - _name: &str, - _verbatim: bool, - _whole_archive: bool, - _search_paths: &SearchPaths, - ) { + fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool, _whole_archive: bool) { panic!("staticlibs not supported") } @@ -1880,13 +1829,7 @@ fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) panic!("external dylibs not supported") } - fn link_staticlib_by_name( - &mut self, - _name: &str, - _verbatim: bool, - _whole_archive: bool, - _search_paths: &SearchPaths, - ) { + fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool, _whole_archive: bool) { panic!("staticlibs not supported") } @@ -1977,13 +1920,7 @@ fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) panic!("external dylibs not supported") } - fn link_staticlib_by_name( - &mut self, - _name: &str, - _verbatim: bool, - _whole_archive: bool, - _search_paths: &SearchPaths, - ) { + fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool, _whole_archive: bool) { panic!("staticlibs not supported") } diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index fe546d05ff2..91cef02c7d1 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -171,7 +171,7 @@ fn configure_and_expand( if cfg!(windows) { old_path = env::var_os("PATH").unwrap_or(old_path); let mut new_path = Vec::from_iter( - sess.host_filesearch(PathKind::All).search_path_dirs().map(|p| p.to_owned()), + sess.host_filesearch(PathKind::All).search_paths().map(|p| p.dir.clone()), ); for path in env::split_paths(&old_path) { if !new_path.contains(&path) { diff --git a/compiler/rustc_metadata/src/native_libs.rs b/compiler/rustc_metadata/src/native_libs.rs index c43b730eccf..58760be921a 100644 --- a/compiler/rustc_metadata/src/native_libs.rs +++ b/compiler/rustc_metadata/src/native_libs.rs @@ -17,14 +17,9 @@ use crate::errors; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; -pub fn find_native_static_library<'a>( - name: &str, - verbatim: bool, - search_paths: impl Iterator, - sess: &Session, -) -> PathBuf { +pub fn find_native_static_library(name: &str, verbatim: bool, sess: &Session) -> PathBuf { let formats = if verbatim { vec![("".into(), "".into())] } else { @@ -35,9 +30,9 @@ pub fn find_native_static_library<'a>( if os == unix { vec![os] } else { vec![os, unix] } }; - for path in search_paths { + for path in sess.target_filesearch(PathKind::Native).search_paths() { for (prefix, suffix) in &formats { - let test = path.join(format!("{prefix}{name}{suffix}")); + let test = path.dir.join(format!("{prefix}{name}{suffix}")); if test.exists() { return test; } @@ -60,8 +55,7 @@ fn find_bundled_library( && (sess.opts.unstable_opts.packed_bundled_libs || has_cfg || whole_archive == Some(true)) { let verbatim = verbatim.unwrap_or(false); - let search_paths = sess.target_filesearch(PathKind::Native).search_path_dirs(); - return find_native_static_library(name.as_str(), verbatim, search_paths, sess) + return find_native_static_library(name.as_str(), verbatim, sess) .file_name() .and_then(|s| s.to_str()) .map(Symbol::intern); diff --git a/compiler/rustc_session/src/filesearch.rs b/compiler/rustc_session/src/filesearch.rs index 6120900d9f4..aecf5954c4c 100644 --- a/compiler/rustc_session/src/filesearch.rs +++ b/compiler/rustc_session/src/filesearch.rs @@ -45,11 +45,6 @@ pub fn new( debug!("using sysroot = {}, triple = {}", sysroot.display(), triple); FileSearch { sysroot, triple, search_paths, tlib_path, kind } } - - /// Returns just the directories within the search paths. - pub fn search_path_dirs(&self) -> impl Iterator { - self.search_paths().map(|sp| &*sp.dir) - } } pub fn make_target_lib_path(sysroot: &Path, target_triple: &str) -> PathBuf {