Skip to content

rustc_codegen_ssa: Refactor ArchiveEntry to include entry kind #157263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
petrochenkov wants to merge 1 commit into
base: main
Choose a base branch
from
+60 −42
Open

rustc_codegen_ssa: Refactor ArchiveEntry to include entry kind #157263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 41 additions & 21 deletions compiler/rustc_codegen_ssa/src/back/archive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use rustc_target::spec::Arch;
use tracing::trace;

use super::metadata::{create_compressed_metadata_file, search_for_section};
use super::rmeta_link::{self, RmetaLink};
use super::rmeta_link;
use crate::common;
// Public for ArchiveBuilderBuilder::extract_bundled_libs
pub use crate::errors::ExtractBundledLibsError;
Expand Down Expand Up @@ -309,12 +309,12 @@ fn find_binutils_dlltool(sess: &Session) -> OsString {
}

pub trait ArchiveBuilder {
fn add_file(&mut self, path: &Path);
fn add_file(&mut self, path: &Path, kind: ArchiveEntryKind);

fn add_archive(
&mut self,
archive: &Path,
skip: Option<Box<dyn FnMut(&str, Option<&RmetaLink>) -> bool + 'static>>,
skip: Option<Box<dyn FnMut(&str, ArchiveEntryKind) -> bool + 'static>>,
) -> io::Result<()>;

fn build(self: Box<Self>, output: &Path) -> bool;
Expand Down Expand Up @@ -383,12 +383,27 @@ pub struct ArArchiveBuilder<'a> {
entries: Vec<(Vec<u8>, ArchiveEntry)>,
}

#[derive(Clone, Copy, PartialEq, Debug)]
pub enum ArchiveEntryKind {
/// Object file produced from Rust code.
RustObj,
/// Anything else, introduce new variants as needed.
Other,
}

#[derive(Debug)]
enum ArchiveEntry {
FromArchive { archive_index: usize, file_range: (u64, u64) },
enum ArchiveEntrySource {
Archive { archive_index: usize, file_range: (u64, u64) },
File(PathBuf),
}

#[derive(Debug)]
struct ArchiveEntry {
source: ArchiveEntrySource,
#[expect(dead_code)] // used in #155338
kind: ArchiveEntryKind,
}

impl<'a> ArArchiveBuilder<'a> {
pub fn new(sess: &'a Session, object_reader: &'static ObjectReader) -> ArArchiveBuilder<'a> {
ArArchiveBuilder { sess, object_reader, src_archives: vec![], entries: vec![] }
Expand Down Expand Up @@ -446,7 +461,7 @@ impl<'a> ArchiveBuilder for ArArchiveBuilder<'a> {
fn add_archive(
&mut self,
archive_path: &Path,
mut skip: Option<Box<dyn FnMut(&str, Option<&RmetaLink>) -> bool + 'static>>,
mut skip: Option<Box<dyn FnMut(&str, ArchiveEntryKind) -> bool + 'static>>,
) -> io::Result<()> {
let mut archive_path = archive_path.to_path_buf();
if self.sess.target.llvm_target.contains("-apple-macosx")
Expand All @@ -462,8 +477,7 @@ impl<'a> ArchiveBuilder for ArArchiveBuilder<'a> {
let archive_map = unsafe { Mmap::map(File::open(&archive_path)?)? };
let archive = ArchiveFile::parse(&*archive_map)
.map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?;
let metadata_link =
skip.as_ref().and_then(|_| rmeta_link::read(&archive, &archive_map, &archive_path));
let metadata_link = rmeta_link::read(&archive, &archive_map, &archive_path);
Copy link
Copy Markdown
Contributor Author

@petrochenkov petrochenkov Jun 1, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We always try to read the link-metadata now.

Right now skip happens to be always Some for Rust rlibs and always None for native libs, so the previous optimization happened to be correct.
We could potentially reinstate the optimization and add a comment saying that skip must always be Some for Rust rlibs, but it's probably not that useful for performance in practice.

View changes since the review

let archive_index = self.src_archives.len();

if let Some(expected_kind) =
Expand All @@ -483,17 +497,23 @@ impl<'a> ArchiveBuilder for ArArchiveBuilder<'a> {
let entry = entry.map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?;
let file_name = String::from_utf8(entry.name().to_vec())
.map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?;
let drop = skip.as_mut().is_some_and(|f| f(&file_name, metadata_link.as_ref()));
let kind = if metadata_link
.as_ref()
.is_some_and(|m| m.rust_object_files.iter().any(|f| f == &file_name))
{
ArchiveEntryKind::RustObj
} else {
ArchiveEntryKind::Other
};
let drop = skip.as_mut().is_some_and(|f| f(&file_name, kind));
if !drop {
if entry.is_thin() {
let source = if entry.is_thin() {
let member_path = archive_path.parent().unwrap().join(Path::new(&file_name));
self.entries.push((file_name.into_bytes(), ArchiveEntry::File(member_path)));
ArchiveEntrySource::File(member_path)
} else {
self.entries.push((
file_name.into_bytes(),
ArchiveEntry::FromArchive { archive_index, file_range: entry.file_range() },
));
}
ArchiveEntrySource::Archive { archive_index, file_range: entry.file_range() }
};
self.entries.push((file_name.into_bytes(), ArchiveEntry { source, kind }));
}
}

Expand All @@ -502,10 +522,10 @@ impl<'a> ArchiveBuilder for ArArchiveBuilder<'a> {
}

/// Adds an arbitrary file to this archive
fn add_file(&mut self, file: &Path) {
fn add_file(&mut self, file: &Path, kind: ArchiveEntryKind) {
self.entries.push((
file.file_name().unwrap().to_str().unwrap().to_string().into_bytes(),
ArchiveEntry::File(file.to_owned()),
ArchiveEntry { source: ArchiveEntrySource::File(file.to_owned()), kind },
));
}

Expand Down Expand Up @@ -539,8 +559,8 @@ impl<'a> ArArchiveBuilder<'a> {

for (entry_name, entry) in self.entries {
let data =
match entry {
ArchiveEntry::FromArchive { archive_index, file_range } => {
match entry.source {
ArchiveEntrySource::Archive { archive_index, file_range } => {
let src_archive = &self.src_archives[archive_index];
let archive_data = &src_archive.1;
let start = file_range.0 as usize;
Expand All @@ -563,7 +583,7 @@ impl<'a> ArArchiveBuilder<'a> {

Box::new(data) as Box<dyn AsRef<[u8]>>
}
ArchiveEntry::File(file) => unsafe {
ArchiveEntrySource::File(file) => unsafe {
Box::new(
Mmap::map(File::open(file).map_err(|err| {
io_error_context("failed to open object file", err)
Expand Down
40 changes: 19 additions & 21 deletions compiler/rustc_codegen_ssa/src/back/link.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ use rustc_target::spec::{
};
use tracing::{debug, info, warn};

use super::archive::{ArchiveBuilder, ArchiveBuilderBuilder};
use super::archive::{ArchiveBuilder, ArchiveBuilderBuilder, ArchiveEntryKind};
use super::command::Command;
use super::linker::{self, Linker};
use super::metadata::{MetadataPosition, create_wrapper_file};
Expand Down Expand Up @@ -342,11 +342,11 @@ fn link_rlib<'a>(
// normal linkers for the platform. Sometimes this is not possible however.
// If it is possible however, placing the metadata object first improves
// performance of getting metadata from rlibs.
ab.add_file(&metadata);
ab.add_file(&metadata, ArchiveEntryKind::Other);
// Place the rmeta-link member immediately after metadata so consumers
// can find it without iterating the whole archive.
if let Some(file) = &metadata_link_file {
ab.add_file(file);
ab.add_file(file, ArchiveEntryKind::Other);
}
None
}
Expand All @@ -359,15 +359,15 @@ fn link_rlib<'a>(

for m in &compiled_modules.modules {
if let Some(obj) = m.object.as_ref() {
ab.add_file(obj);
ab.add_file(obj, ArchiveEntryKind::RustObj);
}

if let Some(obj) = m.global_asm_object.as_ref() {
ab.add_file(obj);
ab.add_file(obj, ArchiveEntryKind::RustObj);
}

if let Some(dwarf_obj) = m.dwarf_object.as_ref() {
ab.add_file(dwarf_obj);
ab.add_file(dwarf_obj, ArchiveEntryKind::Other);
}
}

Expand All @@ -376,10 +376,10 @@ fn link_rlib<'a>(
RlibFlavor::StaticlibBase => {
if let Some(m) = &compiled_modules.allocator_module {
if let Some(obj) = &m.object {
ab.add_file(obj);
ab.add_file(obj, ArchiveEntryKind::RustObj);
}
if let Some(obj) = &m.global_asm_object {
ab.add_file(obj);
ab.add_file(obj, ArchiveEntryKind::RustObj);
}
}
}
Expand Down Expand Up @@ -469,18 +469,18 @@ fn link_rlib<'a>(
//
// Basically, all this means is that this code should not move above the
// code above.
ab.add_file(&trailing_metadata);
ab.add_file(&trailing_metadata, ArchiveEntryKind::Other);
// Place the rmeta-link member immediately after metadata so consumers can
// find it without iterating the whole archive.
if let Some(file) = &metadata_link_file {
ab.add_file(file);
ab.add_file(file, ArchiveEntryKind::Other);
}
}

// Add all bundled static native library dependencies.
// Archives added to the end of .rlib archive, see comment above for the reason.
for lib in packed_bundled_libs {
ab.add_file(&lib)
ab.add_file(&lib, ArchiveEntryKind::Other)
}

ab
Expand Down Expand Up @@ -529,16 +529,14 @@ fn link_staticlib(
let bundled_libs: FxIndexSet<_> = native_libs.filter_map(|lib| lib.filename).collect();
ab.add_archive(
path,
Some(Box::new(move |fname: &str, metadata_link| {
Some(Box::new(move |fname: &str, entry_kind| {
// Ignore metadata and rmeta-link files.
if fname == METADATA_FILENAME || fname == rmeta_link::FILENAME {
return true;
}

// Don't include Rust objects if LTO is enabled.
if lto
&& metadata_link.is_some_and(|m| m.rust_object_files.iter().any(|f| f == fname))
{
if lto && entry_kind == ArchiveEntryKind::RustObj {
return true;
}

Expand Down Expand Up @@ -1266,7 +1264,7 @@ fn link_natively(

if should_archive {
let mut ab = archive_builder_builder.new_archive_builder(sess);
ab.add_file(temp_filename);
ab.add_file(temp_filename, ArchiveEntryKind::Other);
ab.build(out_filename);
}
}
Expand Down Expand Up @@ -3242,19 +3240,19 @@ fn add_static_crate(
let mut archive = archive_builder_builder.new_archive_builder(sess);
if let Err(error) = archive.add_archive(
cratepath,
Some(Box::new(move |f, metadata_link| {
Some(Box::new(move |f, entry_kind| {
if f == METADATA_FILENAME || f == rmeta_link::FILENAME {
return true;
}

let is_rust_object =
metadata_link.is_some_and(|m| m.rust_object_files.iter().any(|rf| rf == f));

// If we're performing LTO and this is a rust-generated object
// file, then we don't need the object file as it's part of the
// LTO module. Note that `#![no_builtins]` is excluded from LTO,
// though, so we let that object file slide.
if upstream_rust_objects_already_included && is_rust_object && is_builtins {
if upstream_rust_objects_already_included
&& entry_kind == ArchiveEntryKind::RustObj
&& is_builtins
{
return true;
}

Expand Down
Loading