Skip to content

Commit

Permalink
Rollup merge of rust-lang#50558 - whitfin:issue-50500, r=michaelwoeri…
Browse files Browse the repository at this point in the history
…ster

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
  • Loading branch information
Mark-Simulacrum authored May 12, 2018
2 parents d7f5e1f + 8402a58 commit b9c9fac
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 63 deletions.
25 changes: 1 addition & 24 deletions src/librustc/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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> {
Expand All @@ -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> {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_incremental/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_incremental/persist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
21 changes: 10 additions & 11 deletions src/librustc_incremental/persist/save.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
19 changes: 10 additions & 9 deletions src/librustc_incremental/persist/work_product.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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) {
Expand Down
35 changes: 23 additions & 12 deletions src/librustc_trans/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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() {
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_trans/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;

Expand Down

0 comments on commit b9c9fac

Please sign in to comment.