Skip to content

Commit

Permalink
Auto merge of rust-lang#123854 - petrochenkov:searchdirs2, r=lqd
Browse files Browse the repository at this point in the history
linker: Remove laziness and caching from native search directory walks

It shouldn't be necessary for performance now.

Follow up to rust-lang#123827.
  • Loading branch information
bors committed Apr 13, 2024
2 parents 7106800 + ed62b57 commit 3a0db6c
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 122 deletions.
33 changes: 4 additions & 29 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ use regex::Regex;
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};
Expand All @@ -53,20 +52,6 @@ use std::path::{Path, PathBuf};
use std::process::{ExitStatus, Output, Stdio};
use std::{env, fmt, fs, io, mem, str};

#[derive(Default)]
pub struct SearchPaths(OnceCell<Vec<PathBuf>>);

impl SearchPaths {
pub(super) fn get(&self, sess: &Session) -> impl Iterator<Item = &Path> {
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 {
Expand Down Expand Up @@ -382,24 +367,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 })
});
Expand Down Expand Up @@ -2518,7 +2500,6 @@ fn add_native_libs_from_crate(
archive_builder_builder: &dyn ArchiveBuilderBuilder,
codegen_results: &CodegenResults,
tmpdir: &Path,
search_paths: &SearchPaths,
bundled_libs: &FxIndexSet<Symbol>,
cnum: CrateNum,
link_static: bool,
Expand Down Expand Up @@ -2581,7 +2562,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);
}
}
}
Expand All @@ -2595,7 +2576,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 {
Expand Down Expand Up @@ -2642,7 +2623,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;
Expand All @@ -2652,7 +2632,6 @@ fn add_local_native_libraries(
archive_builder_builder,
codegen_results,
tmpdir,
&search_paths,
&Default::default(),
LOCAL_CRATE,
link_static,
Expand Down Expand Up @@ -2684,7 +2663,6 @@ fn add_upstream_rust_crates(
.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.
Expand Down Expand Up @@ -2741,7 +2719,6 @@ fn add_upstream_rust_crates(
archive_builder_builder,
codegen_results,
tmpdir,
&search_paths,
&bundled_libs,
cnum,
link_static,
Expand All @@ -2759,7 +2736,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
Expand All @@ -2781,7 +2757,6 @@ fn add_upstream_native_libraries(
archive_builder_builder,
codegen_results,
tmpdir,
&search_paths,
&Default::default(),
cnum,
link_static,
Expand Down
87 changes: 12 additions & 75 deletions compiler/rustc_codegen_ssa/src/back/linker.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -482,13 +475,7 @@ impl<'a> Linker for GccLinker<'a> {
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 {
Expand All @@ -497,8 +484,7 @@ impl<'a> Linker for GccLinker<'a> {
// -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}"));
Expand Down Expand Up @@ -825,13 +811,7 @@ impl<'a> Linker for MsvcLinker<'a> {
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}"));
Expand Down Expand Up @@ -1064,13 +1044,7 @@ impl<'a> Linker for EmLinker<'a> {
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);
}

Expand Down Expand Up @@ -1243,13 +1217,7 @@ impl<'a> Linker for WasmLd<'a> {
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 {
Expand Down Expand Up @@ -1396,13 +1364,7 @@ impl<'a> Linker for L4Bender<'a> {
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}"));
Expand Down Expand Up @@ -1580,20 +1542,13 @@ impl<'a> Linker for AixLinker<'a> {
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);
}
}
Expand Down Expand Up @@ -1792,13 +1747,7 @@ impl<'a> Linker for PtxLinker<'a> {
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")
}

Expand Down Expand Up @@ -1880,13 +1829,7 @@ impl<'a> Linker for LlbcLinker<'a> {
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")
}

Expand Down Expand Up @@ -1977,13 +1920,7 @@ impl<'a> Linker for BpfLinker<'a> {
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")
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
18 changes: 6 additions & 12 deletions compiler/rustc_metadata/src/native_libs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,9 @@ use rustc_target::spec::abi::Abi;

use crate::errors;

use std::path::{Path, PathBuf};

pub fn find_native_static_library<'a>(
name: &str,
verbatim: bool,
search_paths: impl Iterator<Item = &'a Path>,
sess: &Session,
) -> PathBuf {
use std::path::PathBuf;

pub fn find_native_static_library(name: &str, verbatim: bool, sess: &Session) -> PathBuf {
let formats = if verbatim {
vec![("".into(), "".into())]
} else {
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_session/src/filesearch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ impl<'a> FileSearch<'a> {
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<Item = &'a Path> {
self.search_paths().map(|sp| &*sp.dir)
}
}

pub fn make_target_lib_path(sysroot: &Path, target_triple: &str) -> PathBuf {
Expand Down

0 comments on commit 3a0db6c

Please sign in to comment.