Skip to content
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.

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

Use the object crate for metadata reading #83640

Merged
merged 8 commits into from
May 14, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3700,6 +3700,7 @@ dependencies = [
"itertools 0.9.0",
"jobserver",
"libc",
"object",
"pathdiff",
"rustc_apfloat",
"rustc_ast",
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl CodegenBackend for CraneliftCodegenBackend {
}

fn metadata_loader(&self) -> Box<dyn MetadataLoader + Sync> {
Box::new(crate::metadata::CraneliftMetadataLoader)
Box::new(rustc_codegen_ssa::back::metadata::DefaultMetadataLoader)
}

fn provide(&self, _providers: &mut Providers) {}
Expand Down
66 changes: 1 addition & 65 deletions compiler/rustc_codegen_cranelift/src/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,73 +1,9 @@
//! Reading and writing of the rustc metadata for rlibs and dylibs
//! Writing of the rustc metadata for dylibs

use std::fs::File;
use std::path::Path;

use rustc_codegen_ssa::METADATA_FILENAME;
use rustc_data_structures::memmap::Mmap;
use rustc_data_structures::owning_ref::OwningRef;
use rustc_data_structures::rustc_erase_owner;
use rustc_data_structures::sync::MetadataRef;
use rustc_middle::middle::cstore::MetadataLoader;
use rustc_middle::ty::TyCtxt;
use rustc_target::spec::Target;

use crate::backend::WriteMetadata;

/// The metadata loader used by cg_clif.
///
/// The metadata is stored in the same format as cg_llvm.
///
/// # Metadata location
///
/// <dl>
/// <dt>rlib</dt>
/// <dd>The metadata can be found in the `lib.rmeta` file inside of the ar archive.</dd>
/// <dt>dylib</dt>
/// <dd>The metadata can be found in the `.rustc` section of the shared library.</dd>
/// </dl>
pub(crate) struct CraneliftMetadataLoader;

fn load_metadata_with(
path: &Path,
f: impl for<'a> FnOnce(&'a [u8]) -> Result<&'a [u8], String>,
) -> Result<MetadataRef, String> {
let file = File::open(path).map_err(|e| format!("{:?}", e))?;
let data = unsafe { Mmap::map(file) }.map_err(|e| format!("{:?}", e))?;
let metadata = OwningRef::new(data).try_map(f)?;
return Ok(rustc_erase_owner!(metadata.map_owner_box()));
}

impl MetadataLoader for CraneliftMetadataLoader {
fn get_rlib_metadata(&self, _target: &Target, path: &Path) -> Result<MetadataRef, String> {
load_metadata_with(path, |data| {
let archive = object::read::archive::ArchiveFile::parse(&*data)
.map_err(|e| format!("{:?}", e))?;

for entry_result in archive.members() {
let entry = entry_result.map_err(|e| format!("{:?}", e))?;
if entry.name() == METADATA_FILENAME.as_bytes() {
return Ok(entry.data());
}
}

Err("couldn't find metadata entry".to_string())
})
}

fn get_dylib_metadata(&self, _target: &Target, path: &Path) -> Result<MetadataRef, String> {
use object::{Object, ObjectSection};

load_metadata_with(path, |data| {
let file = object::File::parse(&data).map_err(|e| format!("parse: {:?}", e))?;
file.section_by_name(".rustc")
.ok_or("no .rustc section")?
.data()
.map_err(|e| format!("failed to read .rustc section: {:?}", e))
})
}
}

// Adapted from https://github.com/rust-lang/rust/blob/da573206f87b5510de4b0ee1a9c044127e409bd3/src/librustc_codegen_llvm/base.rs#L47-L112
pub(crate) fn write_metadata<O: WriteMetadata>(tcx: TyCtxt<'_>, object: &mut O) {
use snap::write::FrameEncoder;
Expand Down
17 changes: 15 additions & 2 deletions compiler/rustc_codegen_llvm/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use crate::builder::Builder;
use crate::common;
use crate::context::CodegenCx;
use crate::llvm;
use crate::metadata;
use crate::value::Value;

use rustc_codegen_ssa::base::maybe_create_entry_wrapper;
Expand Down Expand Up @@ -47,6 +46,21 @@ pub fn write_compressed_metadata<'tcx>(
use snap::write::FrameEncoder;
use std::io::Write;

// Historical note:
//
// When using link.exe it was seen that the section name `.note.rustc`
// was getting shortened to `.note.ru`, and according to the PE and COFF
// specification:
//
// > Executable images do not use a string table and do not support
// > section names longer than 8 characters
//
// https://docs.microsoft.com/en-us/windows/win32/debug/pe-format
//
// As a result, we choose a slightly shorter name! As to why
// `.note.rustc` works on MinGW, that's another good question...
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
let section_name = if tcx.sess.target.is_like_osx { "__DATA,.rustc" } else { ".rustc" };

let (metadata_llcx, metadata_llmod) = (&*llvm_module.llcx, llvm_module.llmod());
let mut compressed = tcx.metadata_encoding_version();
FrameEncoder::new(&mut compressed).write_all(&metadata.raw_data).unwrap();
Expand All @@ -59,7 +73,6 @@ pub fn write_compressed_metadata<'tcx>(
unsafe { llvm::LLVMAddGlobal(metadata_llmod, common::val_ty(llconst), buf.as_ptr()) };
unsafe {
llvm::LLVMSetInitializer(llglobal, llconst);
let section_name = metadata::metadata_section_name(&tcx.sess.target);
let name = SmallCStr::new(section_name);
llvm::LLVMSetSection(llglobal, name.as_ptr());

Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_llvm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ pub mod llvm {
}

mod llvm_util;
mod metadata;
mod mono_item;
mod type_;
mod type_of;
Expand Down Expand Up @@ -250,7 +249,7 @@ impl CodegenBackend for LlvmCodegenBackend {
}

fn metadata_loader(&self) -> Box<MetadataLoaderDyn> {
Box::new(metadata::LlvmMetadataLoader)
Box::new(rustc_codegen_ssa::back::metadata::DefaultMetadataLoader)
}

fn provide(&self, providers: &mut ty::query::Providers) {
Expand Down
112 changes: 0 additions & 112 deletions compiler/rustc_codegen_llvm/src/metadata.rs

This file was deleted.

5 changes: 5 additions & 0 deletions compiler/rustc_codegen_ssa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,8 @@ rustc_index = { path = "../rustc_index" }
rustc_macros = { path = "../rustc_macros" }
rustc_target = { path = "../rustc_target" }
rustc_session = { path = "../rustc_session" }

[dependencies.object]
version = "0.22.0"
default-features = false
features = ["read_core", "elf", "macho", "pe", "unaligned", "archive"]
71 changes: 71 additions & 0 deletions compiler/rustc_codegen_ssa/src/back/metadata.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
//! Reading of the rustc metadata for rlibs and dylibs

use std::fs::File;
use std::path::Path;

use rustc_data_structures::memmap::Mmap;
use rustc_data_structures::owning_ref::OwningRef;
use rustc_data_structures::rustc_erase_owner;
use rustc_data_structures::sync::MetadataRef;
use rustc_middle::middle::cstore::MetadataLoader;
use rustc_target::spec::Target;

use crate::METADATA_FILENAME;

/// The default metadata loader. This is used by cg_llvm and cg_clif.
///
/// # Metadata location
///
/// <dl>
/// <dt>rlib</dt>
/// <dd>The metadata can be found in the `lib.rmeta` file inside of the ar archive.</dd>
/// <dt>dylib</dt>
/// <dd>The metadata can be found in the `.rustc` section of the shared library.</dd>
/// </dl>
pub struct DefaultMetadataLoader;

fn load_metadata_with(
path: &Path,
f: impl for<'a> FnOnce(&'a [u8]) -> Result<&'a [u8], String>,
) -> Result<MetadataRef, String> {
let file =
File::open(path).map_err(|e| format!("failed to open file '{}': {}", path.display(), e))?;
let data = unsafe { Mmap::map(file) }
.map_err(|e| format!("failed to mmap file '{}': {}", path.display(), e))?;
let metadata = OwningRef::new(data).try_map(f)?;
return Ok(rustc_erase_owner!(metadata.map_owner_box()));
}

impl MetadataLoader for DefaultMetadataLoader {
fn get_rlib_metadata(&self, _target: &Target, path: &Path) -> Result<MetadataRef, String> {
load_metadata_with(path, |data| {
let archive = object::read::archive::ArchiveFile::parse(&*data)
.map_err(|e| format!("failed to parse rlib '{}': {}", path.display(), e))?;

for entry_result in archive.members() {
Copy link
Member

Choose a reason for hiding this comment

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

So, I think this (and use of mmap) may be one plausible reason for the slight performance regression. If my memory of the ar format serves me well, obtaining the list of members in an ar has to process the file effectively as if it was a ump list. I suspect that such a read pattern may be a pathological for mmap based I/O: kernel would try loading more data (page?) into memory only for us to inspect the file name and length before we jump to the next entry(-ies), discarding the rest of the data that kernel spent time loading in.

Without digging into LLVM's ArchiveRO implementation I can imagine that more precise reads could be more effective here.

Copy link
Member Author

Choose a reason for hiding this comment

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

ArchiveRO::open also uses mmap for header reading I think:

ErrorOr<std::unique_ptr<MemoryBuffer>> BufOr =
MemoryBufferRef doesn't export any method allowing read calls on the mapped file.

let entry = entry_result
.map_err(|e| format!("failed to parse rlib '{}': {}", path.display(), e))?;
if entry.name() == METADATA_FILENAME.as_bytes() {
return Ok(entry.data());
}
}

Err(format!("metadata not found in rlib '{}'", path.display()))
})
}

fn get_dylib_metadata(&self, _target: &Target, path: &Path) -> Result<MetadataRef, String> {
use object::{Object, ObjectSection};

load_metadata_with(path, |data| {
let file = object::File::parse(&data)
.map_err(|e| format!("failed to parse dylib '{}': {}", path.display(), e))?;
file.section_by_name(".rustc")
.ok_or_else(|| format!("no .rustc section in '{}'", path.display()))?
.data()
.map_err(|e| {
format!("failed to read .rustc section in '{}': {}", path.display(), e)
})
})
}
}
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/back/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pub mod command;
pub mod link;
pub mod linker;
pub mod lto;
pub mod metadata;
pub mod rpath;
pub mod symbol_export;
pub mod write;