Rollup merge of rust-lang#50558 - whitfin:issue-50500, r=michaelwoerister

Remove all reference to DepGraph::work_products

This is an attempt at fixing rust-lang#50500. It will remove the `work_products` key from `DepGraphData` completely, in favour of just passing the relevant data around. I went in a little blindly; everything appears to work just fine but I'd appreciate any additional advice people.

I didn't want to remove too much of what was already there, so I kept the structure pretty much the same (aside from some naming tweaks) - if anyone has suggestions on how to streamline it a little better, happy to follow up.

r? @michaelwoerister

Author: kennytm

src/librustc/dep_graph/graph.rs

@@ -13,7 +13,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::indexed_vec::{Idx, IndexVec};
 use rustc_data_structures::small_vec::SmallVec;
-use rustc_data_structures::sync::{Lrc, RwLock, ReadGuard, Lock};
+use rustc_data_structures::sync::{Lrc, Lock};
 use std::env;
 use std::hash::Hash;
 use ty::{self, TyCtxt};
@@ -80,9 +80,6 @@ struct DepGraphData {
     /// this map. We can later look for and extract that data.
     previous_work_products: FxHashMap<WorkProductId, WorkProduct>,
 
-    /// Work-products that we generate in this run.
-    work_products: RwLock<FxHashMap<WorkProductId, WorkProduct>>,
-
     dep_node_debug: Lock<FxHashMap<DepNode, String>>,
 
     // Used for testing, only populated when -Zquery-dep-graph is specified.
@@ -103,7 +100,6 @@ impl DepGraph {
         DepGraph {
             data: Some(Lrc::new(DepGraphData {
                 previous_work_products: prev_work_products,
-                work_products: RwLock::new(FxHashMap()),
                 dep_node_debug: Lock::new(FxHashMap()),
                 current: Lock::new(CurrentDepGraph::new()),
                 previous: prev_graph,
@@ -462,19 +458,6 @@ impl DepGraph {
         self.data.as_ref().unwrap().previous.node_to_index(dep_node)
     }
 
-    /// Indicates that we created the given work-product in this run
-    /// for `v`. This record will be preserved and loaded in the next
-    /// run.
-    pub fn insert_work_product(&self, v: &WorkProductId, data: WorkProduct) {
-        debug!("insert_work_product({:?}, {:?})", v, data);
-        self.data
-            .as_ref()
-            .unwrap()
-            .work_products
-            .borrow_mut()
-            .insert(v.clone(), data);
-    }
-
     /// Check whether a previous work product exists for `v` and, if
     /// so, return the path that leads to it. Used to skip doing work.
     pub fn previous_work_product(&self, v: &WorkProductId) -> Option<WorkProduct> {
@@ -485,12 +468,6 @@ impl DepGraph {
             })
     }
 
-    /// Access the map of work-products created during this run. Only
-    /// used during saving of the dep-graph.
-    pub fn work_products(&self) -> ReadGuard<FxHashMap<WorkProductId, WorkProduct>> {
-        self.data.as_ref().unwrap().work_products.borrow()
-    }
-
     /// Access the map of work-products created during the cached run. Only
     /// used during saving of the dep-graph.
     pub fn previous_work_products(&self) -> &FxHashMap<WorkProductId, WorkProduct> {

src/librustc_incremental/lib.rs

@@ -36,9 +36,9 @@ pub use persist::dep_graph_tcx_init;
 pub use persist::load_dep_graph;
 pub use persist::load_query_result_cache;
 pub use persist::LoadResult;
+pub use persist::copy_cgu_workproducts_to_incr_comp_cache_dir;
 pub use persist::save_dep_graph;
-pub use persist::save_trans_partition;
-pub use persist::save_work_products;
+pub use persist::save_work_product_index;
 pub use persist::in_incr_comp_dir;
 pub use persist::prepare_session_directory;
 pub use persist::finalize_session_directory;

src/librustc_incremental/persist/mod.rs

@@ -29,6 +29,6 @@ pub use self::load::load_dep_graph;
 pub use self::load::load_query_result_cache;
 pub use self::load::LoadResult;
 pub use self::save::save_dep_graph;
-pub use self::save::save_work_products;
-pub use self::work_product::save_trans_partition;
+pub use self::save::save_work_product_index;
+pub use self::work_product::copy_cgu_workproducts_to_incr_comp_cache_dir;
 pub use self::work_product::delete_workproduct_files;

src/librustc_incremental/persist/save.rs

@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use rustc::dep_graph::{DepGraph, DepKind};
+use rustc::dep_graph::{DepGraph, DepKind, WorkProduct, WorkProductId};
 use rustc::session::Session;
 use rustc::ty::TyCtxt;
 use rustc::util::common::time;
@@ -55,22 +55,22 @@ pub fn save_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
     })
 }
 
-pub fn save_work_products(sess: &Session, dep_graph: &DepGraph) {
+pub fn save_work_product_index(sess: &Session,
+                               dep_graph: &DepGraph,
+                               new_work_products: FxHashMap<WorkProductId, WorkProduct>) {
     if sess.opts.incremental.is_none() {
         return;
     }
 
-    debug!("save_work_products()");
+    debug!("save_work_product_index()");
     dep_graph.assert_ignored();
     let path = work_products_path(sess);
-    save_in(sess, path, |e| encode_work_products(dep_graph, e));
+    save_in(sess, path, |e| encode_work_product_index(&new_work_products, e));
 
     // We also need to clean out old work-products, as not all of them are
     // deleted during invalidation. Some object files don't change their
     // content, they are just not needed anymore.
-    let new_work_products = dep_graph.work_products();
     let previous_work_products = dep_graph.previous_work_products();
-
     for (id, wp) in previous_work_products.iter() {
         if !new_work_products.contains_key(id) {
             work_product::delete_workproduct_files(sess, wp);
@@ -234,10 +234,9 @@ fn encode_dep_graph(tcx: TyCtxt,
     Ok(())
 }
 
-fn encode_work_products(dep_graph: &DepGraph,
-                        encoder: &mut Encoder) -> io::Result<()> {
-    let work_products: Vec<_> = dep_graph
-        .work_products()
+fn encode_work_product_index(work_products: &FxHashMap<WorkProductId, WorkProduct>,
+                             encoder: &mut Encoder) -> io::Result<()> {
+    let serialized_products: Vec<_> = work_products
         .iter()
         .map(|(id, work_product)| {
             SerializedWorkProduct {
@@ -247,7 +246,7 @@ fn encode_work_products(dep_graph: &DepGraph,
         })
         .collect();
 
-    work_products.encode(encoder)
+    serialized_products.encode(encoder)
 }
 
 fn encode_query_cache(tcx: TyCtxt,

src/librustc_incremental/persist/work_product.rs

@@ -11,21 +11,22 @@
 //! This module contains files for saving intermediate work-products.
 
 use persist::fs::*;
-use rustc::dep_graph::{WorkProduct, WorkProductId, DepGraph, WorkProductFileKind};
+use rustc::dep_graph::{WorkProduct, WorkProductId, WorkProductFileKind};
 use rustc::session::Session;
 use rustc::util::fs::link_or_copy;
 use std::path::PathBuf;
 use std::fs as std_fs;
 
-pub fn save_trans_partition(sess: &Session,
-                            dep_graph: &DepGraph,
-                            cgu_name: &str,
-                            files: &[(WorkProductFileKind, PathBuf)]) {
-    debug!("save_trans_partition({:?},{:?})",
+pub fn copy_cgu_workproducts_to_incr_comp_cache_dir(
+    sess: &Session,
+    cgu_name: &str,
+    files: &[(WorkProductFileKind, PathBuf)]
+) -> Option<(WorkProductId, WorkProduct)> {
+    debug!("copy_cgu_workproducts_to_incr_comp_cache_dir({:?},{:?})",
            cgu_name,
            files);
     if sess.opts.incremental.is_none() {
-        return
+        return None
     }
     let work_product_id = WorkProductId::from_cgu_name(cgu_name);
 
@@ -53,16 +54,16 @@ pub fn save_trans_partition(sess: &Session,
              })
              .collect();
     let saved_files = match saved_files {
+        None => return None,
         Some(v) => v,
-        None => return,
     };
 
     let work_product = WorkProduct {
         cgu_name: cgu_name.to_string(),
         saved_files,
     };
 
-    dep_graph.insert_work_product(&work_product_id, work_product);
+    Some((work_product_id, work_product))
 }
 
 pub fn delete_workproduct_files(sess: &Session, work_product: &WorkProduct) {

src/librustc_trans/back/write.rs

@@ -17,8 +17,8 @@ use back::linker::LinkerInfo;
 use back::symbol_export::ExportedSymbols;
 use base;
 use consts;
-use rustc_incremental::{save_trans_partition, in_incr_comp_dir};
-use rustc::dep_graph::{DepGraph, WorkProductFileKind};
+use rustc_incremental::{copy_cgu_workproducts_to_incr_comp_cache_dir, in_incr_comp_dir};
+use rustc::dep_graph::{WorkProduct, WorkProductId, WorkProductFileKind};
 use rustc::middle::cstore::{LinkMeta, EncodedMetadata};
 use rustc::session::config::{self, OutputFilenames, OutputType, Passes, SomePasses,
                              AllPasses, Sanitizer, Lto};
@@ -1021,11 +1021,14 @@ pub fn start_async_translation(tcx: TyCtxt,
     }
 }
 
-fn copy_module_artifacts_into_incr_comp_cache(sess: &Session,
-                                              dep_graph: &DepGraph,
-                                              compiled_modules: &CompiledModules) {
+fn copy_all_cgu_workproducts_to_incr_comp_cache_dir(
+    sess: &Session,
+    compiled_modules: &CompiledModules
+) -> FxHashMap<WorkProductId, WorkProduct> {
+    let mut work_products = FxHashMap::default();
+
     if sess.opts.incremental.is_none() {
-        return;
+        return work_products;
     }
 
     for module in compiled_modules.modules.iter() {
@@ -1041,8 +1044,13 @@ fn copy_module_artifacts_into_incr_comp_cache(sess: &Session,
             files.push((WorkProductFileKind::BytecodeCompressed, path.clone()));
         }
 
-        save_trans_partition(sess, dep_graph, &module.name, &files);
+        if let Some((id, product)) =
+                copy_cgu_workproducts_to_incr_comp_cache_dir(sess, &module.name, &files) {
+            work_products.insert(id, product);
+        }
     }
+
+    work_products
 }
 
 fn produce_final_output_artifacts(sess: &Session,
@@ -2236,7 +2244,10 @@ pub struct OngoingCrateTranslation {
 }
 
 impl OngoingCrateTranslation {
-    pub(crate) fn join(self, sess: &Session, dep_graph: &DepGraph) -> CrateTranslation {
+    pub(crate) fn join(
+        self,
+        sess: &Session
+    ) -> (CrateTranslation, FxHashMap<WorkProductId, WorkProduct>) {
         self.shared_emitter_main.check(sess, true);
         let compiled_modules = match self.future.join() {
             Ok(Ok(compiled_modules)) => compiled_modules,
@@ -2255,9 +2266,9 @@ impl OngoingCrateTranslation {
             time_graph.dump(&format!("{}-timings", self.crate_name));
         }
 
-        copy_module_artifacts_into_incr_comp_cache(sess,
-                                                   dep_graph,
-                                                   &compiled_modules);
+        let work_products = copy_all_cgu_workproducts_to_incr_comp_cache_dir(sess,
+                                                                             &compiled_modules);
+
         produce_final_output_artifacts(sess,
                                        &compiled_modules,
                                        &self.output_filenames);
@@ -2281,7 +2292,7 @@ impl OngoingCrateTranslation {
             metadata_module: compiled_modules.metadata_module,
         };
 
-        trans
+        (trans, work_products)
     }
 
     pub(crate) fn submit_pre_translated_module_to_llvm(&self,

src/librustc_trans/lib.rs

@@ -212,16 +212,16 @@ impl TransCrate for LlvmTransCrate {
         outputs: &OutputFilenames,
     ) -> Result<(), CompileIncomplete>{
         use rustc::util::common::time;
-        let trans = trans.downcast::<::back::write::OngoingCrateTranslation>()
+        let (trans, work_products) = trans.downcast::<::back::write::OngoingCrateTranslation>()
             .expect("Expected LlvmTransCrate's OngoingCrateTranslation, found Box<Any>")
-            .join(sess, dep_graph);
+            .join(sess);
         if sess.opts.debugging_opts.incremental_info {
             back::write::dump_incremental_data(&trans);
         }
 
         time(sess,
              "serialize work products",
-             move || rustc_incremental::save_work_products(sess, &dep_graph));
+             move || rustc_incremental::save_work_product_index(sess, &dep_graph, work_products));
 
         sess.compile_status()?;