From 85136dbe56dbbd7cdbcb27886a03807ea265ecd1 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 13 May 2022 10:29:35 +0000 Subject: [PATCH 1/5] Remove unnecessary cgu name length hash This is a tiny optimization --- compiler/rustc_query_system/src/dep_graph/dep_node.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_query_system/src/dep_graph/dep_node.rs b/compiler/rustc_query_system/src/dep_graph/dep_node.rs index c274c2cc26c15..bb2179a24953b 100644 --- a/compiler/rustc_query_system/src/dep_graph/dep_node.rs +++ b/compiler/rustc_query_system/src/dep_graph/dep_node.rs @@ -164,7 +164,6 @@ pub struct WorkProductId { impl WorkProductId { pub fn from_cgu_name(cgu_name: &str) -> WorkProductId { let mut hasher = StableHasher::new(); - cgu_name.len().hash(&mut hasher); cgu_name.hash(&mut hasher); WorkProductId { hash: hasher.finish() } } From 02162c4163f5d1a0e3dc8b552aafaa92d652b279 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 13 May 2022 10:32:03 +0000 Subject: [PATCH 2/5] Rename CodegenUnit::work_product to previous_work_product It returns the previous work product or panics if there is none. This rename makes the purpose of this method clearer. --- compiler/rustc_codegen_cranelift/src/driver/aot.rs | 2 +- compiler/rustc_codegen_ssa/src/base.rs | 4 ++-- compiler/rustc_middle/src/mir/mono.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/driver/aot.rs b/compiler/rustc_codegen_cranelift/src/driver/aot.rs index 5e1e1c81d26ea..0faf1221c8399 100644 --- a/compiler/rustc_codegen_cranelift/src/driver/aot.rs +++ b/compiler/rustc_codegen_cranelift/src/driver/aot.rs @@ -85,7 +85,7 @@ fn reuse_workproduct_for_cgu( work_products: &mut FxHashMap, ) -> CompiledModule { let mut object = None; - let work_product = cgu.work_product(tcx); + let work_product = cgu.previous_work_product(tcx); if let Some(saved_file) = &work_product.saved_file { let obj_out = tcx.output_filenames(()).temp_path(OutputType::Object, Some(cgu.name().as_str())); diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index 420adec456f61..c52a908e90f45 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -716,7 +716,7 @@ pub fn codegen_crate( &ongoing_codegen.coordinator_send, CachedModuleCodegen { name: cgu.name().to_string(), - source: cgu.work_product(tcx), + source: cgu.previous_work_product(tcx), }, ); true @@ -727,7 +727,7 @@ pub fn codegen_crate( &ongoing_codegen.coordinator_send, CachedModuleCodegen { name: cgu.name().to_string(), - source: cgu.work_product(tcx), + source: cgu.previous_work_product(tcx), }, ); true diff --git a/compiler/rustc_middle/src/mir/mono.rs b/compiler/rustc_middle/src/mir/mono.rs index d389fa8c0ebdd..021f278273646 100644 --- a/compiler/rustc_middle/src/mir/mono.rs +++ b/compiler/rustc_middle/src/mir/mono.rs @@ -336,7 +336,7 @@ impl<'tcx> CodegenUnit<'tcx> { WorkProductId::from_cgu_name(self.name().as_str()) } - pub fn work_product(&self, tcx: TyCtxt<'_>) -> WorkProduct { + pub fn previous_work_product(&self, tcx: TyCtxt<'_>) -> WorkProduct { let work_product_id = self.work_product_id(); tcx.dep_graph .previous_work_product(&work_product_id) From 065e202b563b7f690b7cc79b8fb5a61150c2976e Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 13 May 2022 12:18:13 +0000 Subject: [PATCH 3/5] Avoid an unnecessary clone for copy_cgu_workproduct_to_incr_comp_cache_dir calls --- compiler/rustc_codegen_cranelift/src/driver/aot.rs | 2 +- compiler/rustc_codegen_ssa/src/back/write.rs | 4 ++-- compiler/rustc_incremental/src/persist/work_product.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/driver/aot.rs b/compiler/rustc_codegen_cranelift/src/driver/aot.rs index 0faf1221c8399..ef8fc14a5a00b 100644 --- a/compiler/rustc_codegen_cranelift/src/driver/aot.rs +++ b/compiler/rustc_codegen_cranelift/src/driver/aot.rs @@ -69,7 +69,7 @@ fn emit_module( rustc_incremental::copy_cgu_workproduct_to_incr_comp_cache_dir( tcx.sess, &name, - &Some(tmp_file.clone()), + Some(&tmp_file), ) }; diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 88293dec01cac..7cd062e5e4bea 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -494,10 +494,10 @@ fn copy_all_cgu_workproducts_to_incr_comp_cache_dir( let _timer = sess.timer("copy_all_cgu_workproducts_to_incr_comp_cache_dir"); for module in compiled_modules.modules.iter().filter(|m| m.kind == ModuleKind::Regular) { - let path = module.object.as_ref().cloned(); + let path = module.object.as_deref(); if let Some((id, product)) = - copy_cgu_workproduct_to_incr_comp_cache_dir(sess, &module.name, &path) + copy_cgu_workproduct_to_incr_comp_cache_dir(sess, &module.name, path) { work_products.insert(id, product); } diff --git a/compiler/rustc_incremental/src/persist/work_product.rs b/compiler/rustc_incremental/src/persist/work_product.rs index 85b44ed753192..54b06e56d2418 100644 --- a/compiler/rustc_incremental/src/persist/work_product.rs +++ b/compiler/rustc_incremental/src/persist/work_product.rs @@ -7,13 +7,13 @@ use rustc_fs_util::link_or_copy; use rustc_middle::dep_graph::{WorkProduct, WorkProductId}; use rustc_session::Session; use std::fs as std_fs; -use std::path::PathBuf; +use std::path::Path; /// Copies a CGU work product to the incremental compilation directory, so next compilation can find and reuse it. pub fn copy_cgu_workproduct_to_incr_comp_cache_dir( sess: &Session, cgu_name: &str, - path: &Option, + path: Option<&Path>, ) -> Option<(WorkProductId, WorkProduct)> { debug!("copy_cgu_workproduct_to_incr_comp_cache_dir({:?},{:?})", cgu_name, path); sess.opts.incremental.as_ref()?; From 906b85157cc7928d86fd186a255f3fd89543aca8 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 13 May 2022 12:20:32 +0000 Subject: [PATCH 4/5] Factor Option out of copy_cgu_workproduct_to_incr_comp_cache_dir call This improves clarity of the code a bit --- .../rustc_codegen_cranelift/src/driver/aot.rs | 6 +--- compiler/rustc_codegen_ssa/src/back/write.rs | 12 ++++---- .../src/persist/work_product.rs | 30 ++++++++----------- 3 files changed, 20 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/driver/aot.rs b/compiler/rustc_codegen_cranelift/src/driver/aot.rs index ef8fc14a5a00b..b652b58cb65a8 100644 --- a/compiler/rustc_codegen_cranelift/src/driver/aot.rs +++ b/compiler/rustc_codegen_cranelift/src/driver/aot.rs @@ -66,11 +66,7 @@ fn emit_module( let work_product = if backend_config.disable_incr_cache { None } else { - rustc_incremental::copy_cgu_workproduct_to_incr_comp_cache_dir( - tcx.sess, - &name, - Some(&tmp_file), - ) + rustc_incremental::copy_cgu_workproduct_to_incr_comp_cache_dir(tcx.sess, &name, &tmp_file) }; ModuleCodegenResult( diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 7cd062e5e4bea..36e5906652412 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -494,12 +494,12 @@ fn copy_all_cgu_workproducts_to_incr_comp_cache_dir( let _timer = sess.timer("copy_all_cgu_workproducts_to_incr_comp_cache_dir"); for module in compiled_modules.modules.iter().filter(|m| m.kind == ModuleKind::Regular) { - let path = module.object.as_deref(); - - if let Some((id, product)) = - copy_cgu_workproduct_to_incr_comp_cache_dir(sess, &module.name, path) - { - work_products.insert(id, product); + if let Some(path) = &module.object { + if let Some((id, product)) = + copy_cgu_workproduct_to_incr_comp_cache_dir(sess, &module.name, path) + { + work_products.insert(id, product); + } } } diff --git a/compiler/rustc_incremental/src/persist/work_product.rs b/compiler/rustc_incremental/src/persist/work_product.rs index 54b06e56d2418..2d5cbc28d6eac 100644 --- a/compiler/rustc_incremental/src/persist/work_product.rs +++ b/compiler/rustc_incremental/src/persist/work_product.rs @@ -13,28 +13,24 @@ use std::path::Path; pub fn copy_cgu_workproduct_to_incr_comp_cache_dir( sess: &Session, cgu_name: &str, - path: Option<&Path>, + path: &Path, ) -> Option<(WorkProductId, WorkProduct)> { debug!("copy_cgu_workproduct_to_incr_comp_cache_dir({:?},{:?})", cgu_name, path); sess.opts.incremental.as_ref()?; - let saved_file = if let Some(path) = path { - let file_name = format!("{}.o", cgu_name); - let path_in_incr_dir = in_incr_comp_dir_sess(sess, &file_name); - match link_or_copy(path, &path_in_incr_dir) { - Ok(_) => Some(file_name), - Err(err) => { - sess.warn(&format!( - "error copying object file `{}` to incremental directory as `{}`: {}", - path.display(), - path_in_incr_dir.display(), - err - )); - return None; - } + let file_name = format!("{}.o", cgu_name); + let path_in_incr_dir = in_incr_comp_dir_sess(sess, &file_name); + let saved_file = match link_or_copy(path, &path_in_incr_dir) { + Ok(_) => Some(file_name), + Err(err) => { + sess.warn(&format!( + "error copying object file `{}` to incremental directory as `{}`: {}", + path.display(), + path_in_incr_dir.display(), + err + )); + return None; } - } else { - None }; let work_product = WorkProduct { cgu_name: cgu_name.to_string(), saved_file }; From e16c3b4a44214add436e9834d2090a7288eba3a3 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sun, 15 May 2022 11:31:28 +0000 Subject: [PATCH 5/5] Make saved_file field of WorkProduct non-optional A WorkProduct without a saved file is useless --- .../rustc_codegen_cranelift/src/driver/aot.rs | 25 +++++------ compiler/rustc_codegen_ssa/src/back/write.rs | 42 +++++++++---------- .../rustc_incremental/src/persist/load.rs | 20 ++++----- .../rustc_incremental/src/persist/save.rs | 9 +--- .../src/persist/work_product.rs | 22 +++++----- .../rustc_query_system/src/dep_graph/graph.rs | 2 +- 6 files changed, 51 insertions(+), 69 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/driver/aot.rs b/compiler/rustc_codegen_cranelift/src/driver/aot.rs index b652b58cb65a8..05457ce15e9a7 100644 --- a/compiler/rustc_codegen_cranelift/src/driver/aot.rs +++ b/compiler/rustc_codegen_cranelift/src/driver/aot.rs @@ -80,21 +80,16 @@ fn reuse_workproduct_for_cgu( cgu: &CodegenUnit<'_>, work_products: &mut FxHashMap, ) -> CompiledModule { - let mut object = None; let work_product = cgu.previous_work_product(tcx); - if let Some(saved_file) = &work_product.saved_file { - let obj_out = - tcx.output_filenames(()).temp_path(OutputType::Object, Some(cgu.name().as_str())); - object = Some(obj_out.clone()); - let source_file = rustc_incremental::in_incr_comp_dir_sess(&tcx.sess, &saved_file); - if let Err(err) = rustc_fs_util::link_or_copy(&source_file, &obj_out) { - tcx.sess.err(&format!( - "unable to copy {} to {}: {}", - source_file.display(), - obj_out.display(), - err - )); - } + let obj_out = tcx.output_filenames(()).temp_path(OutputType::Object, Some(cgu.name().as_str())); + let source_file = rustc_incremental::in_incr_comp_dir_sess(&tcx.sess, &work_product.saved_file); + if let Err(err) = rustc_fs_util::link_or_copy(&source_file, &obj_out) { + tcx.sess.err(&format!( + "unable to copy {} to {}: {}", + source_file.display(), + obj_out.display(), + err + )); } work_products.insert(cgu.work_product_id(), work_product); @@ -102,7 +97,7 @@ fn reuse_workproduct_for_cgu( CompiledModule { name: cgu.name().to_string(), kind: ModuleKind::Regular, - object, + object: Some(obj_out), dwarf_object: None, bytecode: None, } diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 36e5906652412..02c7c1a435fae 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -853,35 +853,31 @@ fn execute_copy_from_cache_work_item( module: CachedModuleCodegen, module_config: &ModuleConfig, ) -> WorkItemResult { + assert!(module_config.emit_obj != EmitObj::None); + let incr_comp_session_dir = cgcx.incr_comp_session_dir.as_ref().unwrap(); - let mut object = None; - if let Some(saved_file) = module.source.saved_file { - let obj_out = cgcx.output_filenames.temp_path(OutputType::Object, Some(&module.name)); - object = Some(obj_out.clone()); - let source_file = in_incr_comp_dir(&incr_comp_session_dir, &saved_file); - debug!( - "copying pre-existing module `{}` from {:?} to {}", - module.name, - source_file, - obj_out.display() - ); - if let Err(err) = link_or_copy(&source_file, &obj_out) { - let diag_handler = cgcx.create_diag_handler(); - diag_handler.err(&format!( - "unable to copy {} to {}: {}", - source_file.display(), - obj_out.display(), - err - )); - } + let obj_out = cgcx.output_filenames.temp_path(OutputType::Object, Some(&module.name)); + let source_file = in_incr_comp_dir(&incr_comp_session_dir, &module.source.saved_file); + debug!( + "copying pre-existing module `{}` from {:?} to {}", + module.name, + source_file, + obj_out.display() + ); + if let Err(err) = link_or_copy(&source_file, &obj_out) { + let diag_handler = cgcx.create_diag_handler(); + diag_handler.err(&format!( + "unable to copy {} to {}: {}", + source_file.display(), + obj_out.display(), + err + )); } - assert_eq!(object.is_some(), module_config.emit_obj != EmitObj::None); - WorkItemResult::Compiled(CompiledModule { name: module.name, kind: ModuleKind::Regular, - object, + object: Some(obj_out), dwarf_object: None, bytecode: None, }) diff --git a/compiler/rustc_incremental/src/persist/load.rs b/compiler/rustc_incremental/src/persist/load.rs index 908a936142475..9de14950aa8d3 100644 --- a/compiler/rustc_incremental/src/persist/load.rs +++ b/compiler/rustc_incremental/src/persist/load.rs @@ -162,18 +162,16 @@ pub fn load_dep_graph(sess: &Session) -> DepGraphFuture { for swp in work_products { let mut all_files_exist = true; - if let Some(ref file_name) = swp.work_product.saved_file { - let path = in_incr_comp_dir_sess(sess, file_name); - if !path.exists() { - all_files_exist = false; - - if sess.opts.debugging_opts.incremental_info { - eprintln!( - "incremental: could not find file for work \ + let path = in_incr_comp_dir_sess(sess, &swp.work_product.saved_file); + if !path.exists() { + all_files_exist = false; + + if sess.opts.debugging_opts.incremental_info { + eprintln!( + "incremental: could not find file for work \ product: {}", - path.display() - ); - } + path.display() + ); } } diff --git a/compiler/rustc_incremental/src/persist/save.rs b/compiler/rustc_incremental/src/persist/save.rs index 20ffde90064bf..0223976b08a5b 100644 --- a/compiler/rustc_incremental/src/persist/save.rs +++ b/compiler/rustc_incremental/src/persist/save.rs @@ -107,11 +107,7 @@ pub fn save_work_product_index( for (id, wp) in previous_work_products.iter() { if !new_work_products.contains_key(id) { work_product::delete_workproduct_files(sess, wp); - debug_assert!( - wp.saved_file.as_ref().map_or(true, |file_name| { - !in_incr_comp_dir_sess(sess, &file_name).exists() - }) - ); + debug_assert!(!in_incr_comp_dir_sess(sess, &wp.saved_file).exists()); } } @@ -119,8 +115,7 @@ pub fn save_work_product_index( debug_assert!({ new_work_products .iter() - .flat_map(|(_, wp)| wp.saved_file.iter()) - .map(|name| in_incr_comp_dir_sess(sess, name)) + .map(|(_, wp)| in_incr_comp_dir_sess(sess, &wp.saved_file)) .all(|path| path.exists()) }); } diff --git a/compiler/rustc_incremental/src/persist/work_product.rs b/compiler/rustc_incremental/src/persist/work_product.rs index 2d5cbc28d6eac..4789c0f581fdb 100644 --- a/compiler/rustc_incremental/src/persist/work_product.rs +++ b/compiler/rustc_incremental/src/persist/work_product.rs @@ -21,7 +21,7 @@ pub fn copy_cgu_workproduct_to_incr_comp_cache_dir( let file_name = format!("{}.o", cgu_name); let path_in_incr_dir = in_incr_comp_dir_sess(sess, &file_name); let saved_file = match link_or_copy(path, &path_in_incr_dir) { - Ok(_) => Some(file_name), + Ok(_) => file_name, Err(err) => { sess.warn(&format!( "error copying object file `{}` to incremental directory as `{}`: {}", @@ -41,17 +41,15 @@ pub fn copy_cgu_workproduct_to_incr_comp_cache_dir( /// Removes files for a given work product. pub fn delete_workproduct_files(sess: &Session, work_product: &WorkProduct) { - if let Some(ref file_name) = work_product.saved_file { - let path = in_incr_comp_dir_sess(sess, file_name); - match std_fs::remove_file(&path) { - Ok(()) => {} - Err(err) => { - sess.warn(&format!( - "file-system error deleting outdated file `{}`: {}", - path.display(), - err - )); - } + let path = in_incr_comp_dir_sess(sess, &work_product.saved_file); + match std_fs::remove_file(&path) { + Ok(()) => {} + Err(err) => { + sess.warn(&format!( + "file-system error deleting outdated file `{}`: {}", + path.display(), + err + )); } } } diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index f7655e55d3485..f6d06e4362c53 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -887,7 +887,7 @@ impl DepGraph { pub struct WorkProduct { pub cgu_name: String, /// Saved file associated with this CGU. - pub saved_file: Option, + pub saved_file: String, } // Index type for `DepNodeData`'s edges.