Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit a712ff3

Browse files
committedJul 28, 2024
Auto merge of rust-lang#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 2cbbe8b + 26319ff commit a712ff3

File tree

6 files changed

+52
-19
lines changed

6 files changed

+52
-19
lines changed
 

‎compiler/rustc_codegen_cranelift/src/driver/aot.rs

+7
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,14 @@ impl OngoingCodegen {
9494
("o", &module_regular.object.as_ref().unwrap()),
9595
("asm.o", &module_global_asm.object.as_ref().unwrap()),
9696
],
97+
&[],
9798
)
9899
} else {
99100
rustc_incremental::copy_cgu_workproduct_to_incr_comp_cache_dir(
100101
sess,
101102
&module_regular.name,
102103
&[("o", &module_regular.object.as_ref().unwrap())],
104+
&[],
103105
)
104106
};
105107
if let Some((work_product_id, work_product)) = work_product {
@@ -369,6 +371,7 @@ fn emit_cgu(
369371
bytecode: None,
370372
assembly: None,
371373
llvm_ir: None,
374+
links_from_incr_cache: Vec::new(),
372375
}),
373376
existing_work_product: None,
374377
})
@@ -414,6 +417,7 @@ fn emit_module(
414417
bytecode: None,
415418
assembly: None,
416419
llvm_ir: None,
420+
links_from_incr_cache: Vec::new(),
417421
})
418422
}
419423

@@ -464,6 +468,7 @@ fn reuse_workproduct_for_cgu(
464468
bytecode: None,
465469
assembly: None,
466470
llvm_ir: None,
471+
links_from_incr_cache: Vec::new(),
467472
},
468473
module_global_asm: has_global_asm.then(|| CompiledModule {
469474
name: cgu.name().to_string(),
@@ -473,6 +478,7 @@ fn reuse_workproduct_for_cgu(
473478
bytecode: None,
474479
assembly: None,
475480
llvm_ir: None,
481+
links_from_incr_cache: Vec::new(),
476482
}),
477483
existing_work_product: Some((cgu.work_product_id(), work_product)),
478484
})
@@ -710,6 +716,7 @@ pub(crate) fn run_aot(
710716
bytecode: None,
711717
assembly: None,
712718
llvm_ir: None,
719+
links_from_incr_cache: Vec::new(),
713720
})
714721
} else {
715722
None

‎compiler/rustc_codegen_ssa/src/back/write.rs

+17-6
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use rustc_errors::{
2020
Style,
2121
};
2222
use rustc_fs_util::link_or_copy;
23+
use rustc_fs_util::LinkOrCopy;
2324
use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
2425
use rustc_incremental::{
2526
copy_cgu_workproduct_to_incr_comp_cache_dir, in_incr_comp_dir, in_incr_comp_dir_sess,
@@ -548,9 +549,12 @@ fn copy_all_cgu_workproducts_to_incr_comp_cache_dir(
548549
if let Some(path) = &module.bytecode {
549550
files.push((OutputType::Bitcode.extension(), path.as_path()));
550551
}
551-
if let Some((id, product)) =
552-
copy_cgu_workproduct_to_incr_comp_cache_dir(sess, &module.name, files.as_slice())
553-
{
552+
if let Some((id, product)) = copy_cgu_workproduct_to_incr_comp_cache_dir(
553+
sess,
554+
&module.name,
555+
files.as_slice(),
556+
&module.links_from_incr_cache,
557+
) {
554558
work_products.insert(id, product);
555559
}
556560
}
@@ -942,7 +946,9 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
942946
) -> WorkItemResult<B> {
943947
let incr_comp_session_dir = cgcx.incr_comp_session_dir.as_ref().unwrap();
944948

945-
let load_from_incr_comp_dir = |output_path: PathBuf, saved_path: &str| {
949+
let mut links_from_incr_cache = Vec::new();
950+
951+
let mut load_from_incr_comp_dir = |output_path: PathBuf, saved_path: &str| {
946952
let source_file = in_incr_comp_dir(incr_comp_session_dir, saved_path);
947953
debug!(
948954
"copying preexisting module `{}` from {:?} to {}",
@@ -951,7 +957,11 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
951957
output_path.display()
952958
);
953959
match link_or_copy(&source_file, &output_path) {
954-
Ok(_) => Some(output_path),
960+
Ok(LinkOrCopy::Copy) => Some(output_path),
961+
Ok(LinkOrCopy::Link) => {
962+
links_from_incr_cache.push(source_file);
963+
Some(output_path)
964+
}
955965
Err(error) => {
956966
cgcx.create_dcx().handle().emit_err(errors::CopyPathBuf {
957967
source_file,
@@ -974,7 +984,7 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
974984
load_from_incr_comp_dir(dwarf_obj_out, saved_dwarf_object_file)
975985
});
976986

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

9961006
WorkItemResult::Finished(CompiledModule {
1007+
links_from_incr_cache,
9971008
name: module.name,
9981009
kind: ModuleKind::Regular,
9991010
object,

‎compiler/rustc_codegen_ssa/src/base.rs

+1
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,7 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
629629
bytecode: None,
630630
assembly: None,
631631
llvm_ir: None,
632+
links_from_incr_cache: Vec::new(),
632633
}
633634
})
634635
});

‎compiler/rustc_codegen_ssa/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ impl<M> ModuleCodegen<M> {
9393
bytecode,
9494
assembly,
9595
llvm_ir,
96+
links_from_incr_cache: Vec::new(),
9697
}
9798
}
9899
}
@@ -106,6 +107,7 @@ pub struct CompiledModule {
106107
pub bytecode: Option<PathBuf>,
107108
pub assembly: Option<PathBuf>, // --emit=asm
108109
pub llvm_ir: Option<PathBuf>, // --emit=llvm-ir, llvm-bc is in bytecode
110+
pub links_from_incr_cache: Vec<PathBuf>,
109111
}
110112

111113
impl CompiledModule {

‎compiler/rustc_fs_util/src/lib.rs

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

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

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

8086
#[cfg(unix)]

‎compiler/rustc_incremental/src/persist/work_product.rs

+6
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use rustc_middle::dep_graph::{WorkProduct, WorkProductId};
1010
use rustc_session::Session;
1111
use std::fs as std_fs;
1212
use std::path::Path;
13+
use std::path::PathBuf;
1314
use tracing::debug;
1415

1516
/// Copies a CGU work product to the incremental compilation directory, so next compilation can
@@ -18,6 +19,7 @@ pub fn copy_cgu_workproduct_to_incr_comp_cache_dir(
1819
sess: &Session,
1920
cgu_name: &str,
2021
files: &[(&'static str, &Path)],
22+
known_links: &[PathBuf],
2123
) -> Option<(WorkProductId, WorkProduct)> {
2224
debug!(?cgu_name, ?files);
2325
sess.opts.incremental.as_ref()?;
@@ -26,6 +28,10 @@ pub fn copy_cgu_workproduct_to_incr_comp_cache_dir(
2628
for (ext, path) in files {
2729
let file_name = format!("{cgu_name}.{ext}");
2830
let path_in_incr_dir = in_incr_comp_dir_sess(sess, &file_name);
31+
if known_links.contains(&path_in_incr_dir) {
32+
let _ = saved_files.insert(ext.to_string(), file_name);
33+
continue;
34+
}
2935
match link_or_copy(path, &path_in_incr_dir) {
3036
Ok(_) => {
3137
let _ = saved_files.insert(ext.to_string(), file_name);

0 commit comments

Comments
 (0)
Failed to load comments.