Skip to content

Commit 3b0f3ea

Browse files
committed
Auto merge of #128320 - saethlin:link-me-maybe, r=<try>
Avoid no-op unlink+link dances in incr comp Incremental compilation scales quite poorly with the number of CGUs. This PR improves one reason for that. The incr comp process hard-links all the files from an old session into a new one, then it runs the backend, which may just hard-link the new session files into the output directory. Then codegen hard-links all the output files back to the new session directory. This PR (perhaps unimaginatively) fixes the silliness that ensues in the last step. The old `link_or_copy` implementation would be passed pairs of paths which are already the same inode, then it would blindly delete the destination and re-create the hard-link that it just deleted. This PR lets us skip both those operations. We don't skip the other two hard-links. `cargo +stage1 b && touch crates/core/main.rs && strace -cfw -elink,linkat,unlink,unlinkat cargo +stage1 b` before and then after on `ripgrep-13.0.0`: ``` % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 52.56 0.024950 25 978 485 unlink 34.38 0.016318 22 727 linkat 13.06 0.006200 24 249 unlinkat ------ ----------- ----------- --------- --------- ---------------- 100.00 0.047467 24 1954 485 total ``` ``` % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 42.83 0.014521 57 252 unlink 38.41 0.013021 26 486 linkat 18.77 0.006362 25 249 unlinkat ------ ----------- ----------- --------- --------- ---------------- 100.00 0.033904 34 987 total ``` r? `@ghost`
2 parents 5d3c6ee + 6f5792c commit 3b0f3ea

File tree

6 files changed

+52
-21
lines changed

6 files changed

+52
-21
lines changed

compiler/rustc_codegen_cranelift/src/driver/aot.rs

+7
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,14 @@ impl OngoingCodegen {
9797
("o", &module_regular.object.as_ref().unwrap()),
9898
("asm.o", &module_global_asm.object.as_ref().unwrap()),
9999
],
100+
&[],
100101
)
101102
} else {
102103
rustc_incremental::copy_cgu_workproduct_to_incr_comp_cache_dir(
103104
sess,
104105
&module_regular.name,
105106
&[("o", &module_regular.object.as_ref().unwrap())],
107+
&[],
106108
)
107109
};
108110
if let Some((work_product_id, work_product)) = work_product {
@@ -372,6 +374,7 @@ fn emit_cgu(
372374
bytecode: None,
373375
assembly: None,
374376
llvm_ir: None,
377+
links_from_incr_cache: Vec::new(),
375378
}),
376379
existing_work_product: None,
377380
})
@@ -422,6 +425,7 @@ fn emit_module(
422425
bytecode: None,
423426
assembly: None,
424427
llvm_ir: None,
428+
links_from_incr_cache: Vec::new(),
425429
})
426430
}
427431

@@ -472,6 +476,7 @@ fn reuse_workproduct_for_cgu(
472476
bytecode: None,
473477
assembly: None,
474478
llvm_ir: None,
479+
links_from_incr_cache: Vec::new(),
475480
},
476481
module_global_asm: has_global_asm.then(|| CompiledModule {
477482
name: cgu.name().to_string(),
@@ -481,6 +486,7 @@ fn reuse_workproduct_for_cgu(
481486
bytecode: None,
482487
assembly: None,
483488
llvm_ir: None,
489+
links_from_incr_cache: Vec::new(),
484490
}),
485491
existing_work_product: Some((cgu.work_product_id(), work_product)),
486492
})
@@ -718,6 +724,7 @@ pub(crate) fn run_aot(
718724
bytecode: None,
719725
assembly: None,
720726
llvm_ir: None,
727+
links_from_incr_cache: Vec::new(),
721728
})
722729
} else {
723730
None

compiler/rustc_codegen_ssa/src/back/write.rs

+17-7
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use rustc_errors::{
1717
Diag, DiagArgMap, DiagCtxt, DiagMessage, ErrCode, FatalError, FluentBundle, Level, MultiSpan,
1818
Style, Suggestions,
1919
};
20-
use rustc_fs_util::link_or_copy;
20+
use rustc_fs_util::{link_or_copy, LinkOrCopy};
2121
use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
2222
use rustc_incremental::{
2323
copy_cgu_workproduct_to_incr_comp_cache_dir, in_incr_comp_dir, in_incr_comp_dir_sess,
@@ -532,9 +532,12 @@ fn copy_all_cgu_workproducts_to_incr_comp_cache_dir(
532532
if let Some(path) = &module.bytecode {
533533
files.push((OutputType::Bitcode.extension(), path.as_path()));
534534
}
535-
if let Some((id, product)) =
536-
copy_cgu_workproduct_to_incr_comp_cache_dir(sess, &module.name, files.as_slice())
537-
{
535+
if let Some((id, product)) = copy_cgu_workproduct_to_incr_comp_cache_dir(
536+
sess,
537+
&module.name,
538+
files.as_slice(),
539+
&module.links_from_incr_cache,
540+
) {
538541
work_products.insert(id, product);
539542
}
540543
}
@@ -926,7 +929,9 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
926929
) -> WorkItemResult<B> {
927930
let incr_comp_session_dir = cgcx.incr_comp_session_dir.as_ref().unwrap();
928931

929-
let load_from_incr_comp_dir = |output_path: PathBuf, saved_path: &str| {
932+
let mut links_from_incr_cache = Vec::new();
933+
934+
let mut load_from_incr_comp_dir = |output_path: PathBuf, saved_path: &str| {
930935
let source_file = in_incr_comp_dir(incr_comp_session_dir, saved_path);
931936
debug!(
932937
"copying preexisting module `{}` from {:?} to {}",
@@ -935,7 +940,11 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
935940
output_path.display()
936941
);
937942
match link_or_copy(&source_file, &output_path) {
938-
Ok(_) => Some(output_path),
943+
Ok(LinkOrCopy::Copy) => Some(output_path),
944+
Ok(LinkOrCopy::Link) => {
945+
links_from_incr_cache.push(source_file);
946+
Some(output_path)
947+
}
939948
Err(error) => {
940949
cgcx.create_dcx().handle().emit_err(errors::CopyPathBuf {
941950
source_file,
@@ -958,7 +967,7 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
958967
load_from_incr_comp_dir(dwarf_obj_out, saved_dwarf_object_file)
959968
});
960969

961-
let load_from_incr_cache = |perform, output_type: OutputType| {
970+
let mut load_from_incr_cache = |perform, output_type: OutputType| {
962971
if perform {
963972
let saved_file = module.source.saved_files.get(output_type.extension())?;
964973
let output_path = cgcx.output_filenames.temp_path(output_type, Some(&module.name));
@@ -978,6 +987,7 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
978987
}
979988

980989
WorkItemResult::Finished(CompiledModule {
990+
links_from_incr_cache,
981991
name: module.name,
982992
kind: ModuleKind::Regular,
983993
object,

compiler/rustc_codegen_ssa/src/base.rs

+1
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,7 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
657657
bytecode: None,
658658
assembly: None,
659659
llvm_ir: None,
660+
links_from_incr_cache: Vec::new(),
660661
}
661662
})
662663
});

compiler/rustc_codegen_ssa/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ impl<M> ModuleCodegen<M> {
9797
bytecode,
9898
assembly,
9999
llvm_ir,
100+
links_from_incr_cache: Vec::new(),
100101
}
101102
}
102103
}
@@ -110,6 +111,7 @@ pub struct CompiledModule {
110111
pub bytecode: Option<PathBuf>,
111112
pub assembly: Option<PathBuf>, // --emit=asm
112113
pub llvm_ir: Option<PathBuf>, // --emit=llvm-ir, llvm-bc is in bytecode
114+
pub links_from_incr_cache: Vec<PathBuf>,
113115
}
114116

115117
impl CompiledModule {

compiler/rustc_fs_util/src/lib.rs

+19-13
Original file line numberDiff line numberDiff line change
@@ -55,25 +55,31 @@ pub enum LinkOrCopy {
5555
Copy,
5656
}
5757

58-
/// Copies `p` into `q`, preferring to use hard-linking if possible. If
59-
/// `q` already exists, it is removed first.
58+
/// Copies `p` into `q`, preferring to use hard-linking if possible.
6059
/// The result indicates which of the two operations has been performed.
6160
pub fn link_or_copy<P: AsRef<Path>, Q: AsRef<Path>>(p: P, q: Q) -> io::Result<LinkOrCopy> {
61+
// Creating a hard-link will fail if the destination path already exists. We could defensively
62+
// call remove_file in this function, but that pessimizes callers who can avoid such calls.
63+
// Incremental compilation calls this function a lot, and is able to avoid calls that
64+
// would fail the first hard_link attempt.
65+
6266
let p = p.as_ref();
6367
let q = q.as_ref();
64-
match fs::remove_file(q) {
65-
Ok(()) => (),
66-
Err(err) if err.kind() == io::ErrorKind::NotFound => (),
67-
Err(err) => return Err(err),
68-
}
6968

70-
match fs::hard_link(p, q) {
71-
Ok(()) => Ok(LinkOrCopy::Link),
72-
Err(_) => match fs::copy(p, q) {
73-
Ok(_) => Ok(LinkOrCopy::Copy),
74-
Err(e) => Err(e),
75-
},
69+
let err = match fs::hard_link(p, q) {
70+
Ok(()) => return Ok(LinkOrCopy::Link),
71+
Err(err) => err,
72+
};
73+
74+
if err.kind() == io::ErrorKind::AlreadyExists {
75+
fs::remove_file(q)?;
76+
if fs::hard_link(p, q).is_ok() {
77+
return Ok(LinkOrCopy::Link);
78+
}
7679
}
80+
81+
// Hard linking failed, fall back to copying.
82+
fs::copy(p, q).map(|_| LinkOrCopy::Copy)
7783
}
7884

7985
#[cfg(any(unix, all(target_os = "wasi", target_env = "p1")))]

compiler/rustc_incremental/src/persist/work_product.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! [work products]: WorkProduct
44
55
use std::fs as std_fs;
6-
use std::path::Path;
6+
use std::path::{Path, PathBuf};
77

88
use rustc_data_structures::unord::UnordMap;
99
use rustc_fs_util::link_or_copy;
@@ -20,6 +20,7 @@ pub fn copy_cgu_workproduct_to_incr_comp_cache_dir(
2020
sess: &Session,
2121
cgu_name: &str,
2222
files: &[(&'static str, &Path)],
23+
known_links: &[PathBuf],
2324
) -> Option<(WorkProductId, WorkProduct)> {
2425
debug!(?cgu_name, ?files);
2526
sess.opts.incremental.as_ref()?;
@@ -28,6 +29,10 @@ pub fn copy_cgu_workproduct_to_incr_comp_cache_dir(
2829
for (ext, path) in files {
2930
let file_name = format!("{cgu_name}.{ext}");
3031
let path_in_incr_dir = in_incr_comp_dir_sess(sess, &file_name);
32+
if known_links.contains(&path_in_incr_dir) {
33+
let _ = saved_files.insert(ext.to_string(), file_name);
34+
continue;
35+
}
3136
match link_or_copy(path, &path_in_incr_dir) {
3237
Ok(_) => {
3338
let _ = saved_files.insert(ext.to_string(), file_name);

0 commit comments

Comments
 (0)