Avoid no-op unlink+link dances in incr comp

saethlin · saethlin · commit 3d55891763cf · 2024-08-16T12:07:12.000-04:00
diff --git a/compiler/rustc_codegen_cranelift/src/driver/aot.rs b/compiler/rustc_codegen_cranelift/src/driver/aot.rs
@@ -96,12 +96,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 {
@@ -371,6 +373,7 @@ fn emit_cgu(
             bytecode: None,
             assembly: None,
             llvm_ir: None,
+            links_from_incr_cache: Vec::new(),
         }),
         existing_work_product: None,
     })
@@ -416,6 +419,7 @@ fn emit_module(
         bytecode: None,
         assembly: None,
         llvm_ir: None,
+        links_from_incr_cache: Vec::new(),
     })
 }
 
@@ -466,6 +470,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(),
@@ -475,6 +480,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)),
     })
@@ -712,6 +718,7 @@ pub(crate) fn run_aot(
             bytecode: None,
             assembly: None,
             llvm_ir: None,
+            links_from_incr_cache: Vec::new(),
         })
     } else {
         None
diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs
@@ -18,7 +18,7 @@ use rustc_errors::{
     Diag, DiagArgMap, DiagCtxt, DiagMessage, ErrCode, FatalError, FluentBundle, Level, MultiSpan,
     Style,
 };
-use rustc_fs_util::link_or_copy;
+use rustc_fs_util::{link_or_copy, LinkOrCopy};
 use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
 use rustc_incremental::{
     copy_cgu_workproduct_to_incr_comp_cache_dir, in_incr_comp_dir, in_incr_comp_dir_sess,
@@ -546,9 +546,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);
         }
     }
@@ -940,7 +943,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 {}",
@@ -949,7 +954,11 @@ 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(LinkOrCopy::Copy) => Some(output_path),
+            Ok(LinkOrCopy::Link) => {
+                links_from_incr_cache.push(source_file);
+                Some(output_path)
+            }
             Err(error) => {
                 cgcx.create_dcx().handle().emit_err(errors::CopyPathBuf {
                     source_file,
@@ -972,7 +981,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));
@@ -992,6 +1001,7 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
     }
 
     WorkItemResult::Finished(CompiledModule {
+        links_from_incr_cache,
         name: module.name,
         kind: ModuleKind::Regular,
         object,
diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs
@@ -626,6 +626,7 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
                 bytecode: None,
                 assembly: None,
                 llvm_ir: None,
+                links_from_incr_cache: Vec::new(),
             }
         })
     });
diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs
@@ -94,6 +94,7 @@ impl<M> ModuleCodegen<M> {
             bytecode,
             assembly,
             llvm_ir,
+            links_from_incr_cache: Vec::new(),
         }
     }
 }
@@ -107,6 +108,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 {
diff --git a/compiler/rustc_fs_util/src/lib.rs b/compiler/rustc_fs_util/src/lib.rs
@@ -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(unix)]
diff --git a/compiler/rustc_incremental/src/persist/work_product.rs b/compiler/rustc_incremental/src/persist/work_product.rs
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -626,6 +626,7 @@ pub fn codegen_crate<B: ExtraBackendMethods>(`
`626`	`626`	`bytecode: None,`
`627`	`627`	`assembly: None,`
`628`	`628`	`llvm_ir: None,`
	`629`	`+ links_from_incr_cache: Vec::new(),`
`629`	`630`	`}`
`630`	`631`	`})`
`631`	`632`	`});`
Original file line number	Diff line number	Diff line change
`@@ -94,6 +94,7 @@ impl<M> ModuleCodegen<M> {`
`94`	`94`	`bytecode,`
`95`	`95`	`assembly,`
`96`	`96`	`llvm_ir,`
	`97`	`+ links_from_incr_cache: Vec::new(),`
`97`	`98`	`}`
`98`	`99`	`}`
`99`	`100`	`}`
`@@ -107,6 +108,7 @@ pub struct CompiledModule {`
`107`	`108`	`pub bytecode: Option<PathBuf>,`
`108`	`109`	`pub assembly: Option<PathBuf>, // --emit=asm`
`109`	`110`	`pub llvm_ir: Option<PathBuf>, // --emit=llvm-ir, llvm-bc is in bytecode`
	`111`	`+ pub links_from_incr_cache: Vec<PathBuf>,`
`110`	`112`	`}`
`111`	`113`
`112`	`114`	`impl CompiledModule {`