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

Avoid no-op unlink+link dances in incr comp #128320

Merged
merged 2 commits into from
Mar 22, 2025
Merged
Changes from 1 commit
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
Next Next commit
Avoid no-op unlink+link dances in incr comp
  • Loading branch information
saethlin committed Feb 25, 2025
commit cae7c76d509386ea29a90c4001bb8dbc59c79d22
8 changes: 7 additions & 1 deletion compiler/rustc_codegen_cranelift/src/driver/aot.rs
Original file line number Diff line number Diff line change
@@ -103,12 +103,14 @@ impl OngoingCodegen {
("o", &module_regular.object.as_ref().unwrap()),
("asm.o", &module_global_asm.object.as_ref().unwrap()),
],
&[],
)
} else {
rustc_incremental::copy_cgu_workproduct_to_incr_comp_cache_dir(
sess,
&module_regular.name,
&[("o", &module_regular.object.as_ref().unwrap())],
&[],
)
};
if let Some((work_product_id, work_product)) = work_product {
@@ -381,6 +383,7 @@ fn emit_cgu(
bytecode: None,
assembly: None,
llvm_ir: None,
links_from_incr_cache: Vec::new(),
}),
existing_work_product: None,
})
@@ -437,6 +440,7 @@ fn emit_module(
bytecode: None,
assembly: None,
llvm_ir: None,
links_from_incr_cache: Vec::new(),
})
}

@@ -487,6 +491,7 @@ fn reuse_workproduct_for_cgu(
bytecode: None,
assembly: None,
llvm_ir: None,
links_from_incr_cache: Vec::new(),
},
module_global_asm: has_global_asm.then(|| CompiledModule {
name: cgu.name().to_string(),
@@ -496,6 +501,7 @@ fn reuse_workproduct_for_cgu(
bytecode: None,
assembly: None,
llvm_ir: None,
links_from_incr_cache: Vec::new(),
}),
existing_work_product: Some((cgu.work_product_id(), work_product)),
})
@@ -637,6 +643,7 @@ fn emit_metadata_module(tcx: TyCtxt<'_>, metadata: &EncodedMetadata) -> Compiled
bytecode: None,
assembly: None,
llvm_ir: None,
links_from_incr_cache: Vec::new(),
}
}

@@ -745,7 +752,6 @@ pub(crate) fn run_aot(

let metadata_module =
if need_metadata_module { Some(emit_metadata_module(tcx, &metadata)) } else { None };

Box::new(OngoingCodegen {
modules,
allocator_module,
21 changes: 15 additions & 6 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
@@ -540,9 +540,12 @@ fn copy_all_cgu_workproducts_to_incr_comp_cache_dir(
if let Some(path) = &module.bytecode {
files.push((OutputType::Bitcode.extension(), path.as_path()));
}
if let Some((id, product)) =
copy_cgu_workproduct_to_incr_comp_cache_dir(sess, &module.name, files.as_slice())
{
if let Some((id, product)) = copy_cgu_workproduct_to_incr_comp_cache_dir(
sess,
&module.name,
files.as_slice(),
&module.links_from_incr_cache,
) {
work_products.insert(id, product);
}
}
@@ -934,7 +937,9 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
) -> WorkItemResult<B> {
let incr_comp_session_dir = cgcx.incr_comp_session_dir.as_ref().unwrap();

let load_from_incr_comp_dir = |output_path: PathBuf, saved_path: &str| {
let mut links_from_incr_cache = Vec::new();

let mut load_from_incr_comp_dir = |output_path: PathBuf, saved_path: &str| {
let source_file = in_incr_comp_dir(incr_comp_session_dir, saved_path);
debug!(
"copying preexisting module `{}` from {:?} to {}",
@@ -943,7 +948,10 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
output_path.display()
);
match link_or_copy(&source_file, &output_path) {
Ok(_) => Some(output_path),
Ok(_) => {
links_from_incr_cache.push(source_file);
Some(output_path)
}
Err(error) => {
cgcx.create_dcx().handle().emit_err(errors::CopyPathBuf {
source_file,
@@ -966,7 +974,7 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
load_from_incr_comp_dir(dwarf_obj_out, saved_dwarf_object_file)
});

let load_from_incr_cache = |perform, output_type: OutputType| {
let mut load_from_incr_cache = |perform, output_type: OutputType| {
if perform {
let saved_file = module.source.saved_files.get(output_type.extension())?;
let output_path = cgcx.output_filenames.temp_path(output_type, Some(&module.name));
@@ -986,6 +994,7 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
}

WorkItemResult::Finished(CompiledModule {
links_from_incr_cache,
name: module.name,
kind: ModuleKind::Regular,
object,
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
@@ -656,6 +656,7 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
bytecode: None,
assembly: None,
llvm_ir: None,
links_from_incr_cache: Vec::new(),
}
})
});
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_ssa/src/lib.rs
Original file line number Diff line number Diff line change
@@ -102,6 +102,7 @@ impl<M> ModuleCodegen<M> {
bytecode,
assembly,
llvm_ir,
links_from_incr_cache: Vec::new(),
}
}
}
@@ -115,6 +116,7 @@ pub struct CompiledModule {
pub bytecode: Option<PathBuf>,
pub assembly: Option<PathBuf>, // --emit=asm
pub llvm_ir: Option<PathBuf>, // --emit=llvm-ir, llvm-bc is in bytecode
pub links_from_incr_cache: Vec<PathBuf>,
}

impl CompiledModule {
32 changes: 19 additions & 13 deletions compiler/rustc_fs_util/src/lib.rs
Original file line number Diff line number Diff line change
@@ -55,25 +55,31 @@ pub enum LinkOrCopy {
Copy,
}

/// Copies `p` into `q`, preferring to use hard-linking if possible. If
/// `q` already exists, it is removed first.
/// Copies `p` into `q`, preferring to use hard-linking if possible.
/// The result indicates which of the two operations has been performed.
pub fn link_or_copy<P: AsRef<Path>, Q: AsRef<Path>>(p: P, q: Q) -> io::Result<LinkOrCopy> {
// Creating a hard-link will fail if the destination path already exists. We could defensively
// call remove_file in this function, but that pessimizes callers who can avoid such calls.
// Incremental compilation calls this function a lot, and is able to avoid calls that
// would fail the first hard_link attempt.

let p = p.as_ref();
let q = q.as_ref();
match fs::remove_file(q) {
Ok(()) => (),
Err(err) if err.kind() == io::ErrorKind::NotFound => (),
Err(err) => return Err(err),
}

match fs::hard_link(p, q) {
Ok(()) => Ok(LinkOrCopy::Link),
Err(_) => match fs::copy(p, q) {
Ok(_) => Ok(LinkOrCopy::Copy),
Err(e) => Err(e),
},
let err = match fs::hard_link(p, q) {
Ok(()) => return Ok(LinkOrCopy::Link),
Err(err) => err,
};

if err.kind() == io::ErrorKind::AlreadyExists {
fs::remove_file(q)?;
if fs::hard_link(p, q).is_ok() {
return Ok(LinkOrCopy::Link);
}
}

// Hard linking failed, fall back to copying.
fs::copy(p, q).map(|_| LinkOrCopy::Copy)
}

#[cfg(any(unix, all(target_os = "wasi", target_env = "p1")))]
7 changes: 6 additions & 1 deletion compiler/rustc_incremental/src/persist/work_product.rs
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@
//! [work products]: WorkProduct

use std::fs as std_fs;
use std::path::Path;
use std::path::{Path, PathBuf};

use rustc_data_structures::unord::UnordMap;
use rustc_fs_util::link_or_copy;
@@ -20,6 +20,7 @@ pub fn copy_cgu_workproduct_to_incr_comp_cache_dir(
sess: &Session,
cgu_name: &str,
files: &[(&'static str, &Path)],
known_links: &[PathBuf],
) -> Option<(WorkProductId, WorkProduct)> {
debug!(?cgu_name, ?files);
sess.opts.incremental.as_ref()?;
@@ -28,6 +29,10 @@ pub fn copy_cgu_workproduct_to_incr_comp_cache_dir(
for (ext, path) in files {
let file_name = format!("{cgu_name}.{ext}");
let path_in_incr_dir = in_incr_comp_dir_sess(sess, &file_name);
if known_links.contains(&path_in_incr_dir) {
let _ = saved_files.insert(ext.to_string(), file_name);
continue;
}
match link_or_copy(path, &path_in_incr_dir) {
Ok(_) => {
let _ = saved_files.insert(ext.to_string(), file_name);
Loading