From ed4ba22d1503d67b38aca2fa7ff2407f5fe5e304 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 7 Sep 2017 16:11:58 +0200 Subject: [PATCH] incr.comp.: Move result fingerprinting to DepGraph::with_task(). This makes sure that we don't introduce strange cases where we have nodes outside the query system that could break red/green tracking and it will allow to keep red/green neatly encapsulated within the DepGraph implementation. --- src/librustc/dep_graph/graph.rs | 36 +++++++++++++------ src/librustc/dep_graph/safe.rs | 7 ++++ src/librustc/ich/hcx.rs | 10 ++++-- src/librustc/ty/maps.rs | 17 +-------- src/librustc/ty/mod.rs | 4 +++ src/librustc_data_structures/stable_hasher.rs | 28 +++++++++++++-- src/librustc_trans/base.rs | 36 +++++++++++++++++-- src/librustc_trans/context.rs | 13 +++++++ src/librustc_typeck/variance/constraints.rs | 20 ++++++++--- 9 files changed, 132 insertions(+), 39 deletions(-) diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs index c487fc963ca14..7e82d8c97e4be 100644 --- a/src/librustc/dep_graph/graph.rs +++ b/src/librustc/dep_graph/graph.rs @@ -9,11 +9,15 @@ // except according to those terms. use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::stable_hasher::{HashStable, StableHasher, + StableHashingContextProvider}; use session::config::OutputType; use std::cell::{Ref, RefCell}; use std::rc::Rc; use util::common::{ProfileQueriesMsg, profq_msg}; +use ich::Fingerprint; + use super::dep_node::{DepNode, DepKind, WorkProductId}; use super::query::DepGraphQuery; use super::raii; @@ -71,10 +75,6 @@ impl DepGraph { self.data.as_ref().map(|data| raii::IgnoreTask::new(&data.edges)) } - pub fn in_task<'graph>(&'graph self, key: DepNode) -> Option> { - self.data.as_ref().map(|data| raii::DepTask::new(&data.edges, key)) - } - pub fn with_ignore(&self, op: OP) -> R where OP: FnOnce() -> R { @@ -109,24 +109,38 @@ impl DepGraph { /// `arg` parameter. /// /// [README]: README.md - pub fn with_task(&self, - key: DepNode, - cx: C, - arg: A, - task: fn(C, A) -> R) - -> (R, DepNodeIndex) - where C: DepGraphSafe + pub fn with_task(&self, + key: DepNode, + cx: C, + arg: A, + task: fn(C, A) -> R) + -> (R, DepNodeIndex) + where C: DepGraphSafe + StableHashingContextProvider, + R: HashStable, { if let Some(ref data) = self.data { data.edges.borrow_mut().push_task(key); if cfg!(debug_assertions) { profq_msg(ProfileQueriesMsg::TaskBegin(key.clone())) }; + + // In incremental mode, hash the result of the task. We don't + // do anything with the hash yet, but we are computing it + // anyway so that + // - we make sure that the infrastructure works and + // - we can get an idea of the runtime cost. + let mut hcx = cx.create_stable_hashing_context(); + let result = task(cx, arg); if cfg!(debug_assertions) { profq_msg(ProfileQueriesMsg::TaskEnd) }; let dep_node_index = data.edges.borrow_mut().pop_task(key); + + let mut stable_hasher = StableHasher::new(); + result.hash_stable(&mut hcx, &mut stable_hasher); + let _: Fingerprint = stable_hasher.finish(); + (result, dep_node_index) } else { (task(cx, arg), DepNodeIndex::INVALID) diff --git a/src/librustc/dep_graph/safe.rs b/src/librustc/dep_graph/safe.rs index bf224f89f0ddb..f82bf9be03390 100644 --- a/src/librustc/dep_graph/safe.rs +++ b/src/librustc/dep_graph/safe.rs @@ -58,6 +58,13 @@ impl<'a, A> DepGraphSafe for &'a A { } +/// Mut ref to dep-graph-safe stuff should still be dep-graph-safe. +impl<'a, A> DepGraphSafe for &'a mut A + where A: DepGraphSafe, +{ +} + + /// No data here! :) impl DepGraphSafe for () { } diff --git a/src/librustc/ich/hcx.rs b/src/librustc/ich/hcx.rs index 5a0f2a88af48d..18ce6fb84118f 100644 --- a/src/librustc/ich/hcx.rs +++ b/src/librustc/ich/hcx.rs @@ -26,8 +26,8 @@ use syntax::ext::hygiene::SyntaxContext; use syntax::symbol::Symbol; use syntax_pos::Span; -use rustc_data_structures::stable_hasher::{HashStable, StableHasher, - StableHasherResult}; +use rustc_data_structures::stable_hasher::{HashStable, StableHashingContextProvider, + StableHasher, StableHasherResult}; use rustc_data_structures::accumulate_vec::AccumulateVec; /// This is the context state available during incr. comp. hashing. It contains @@ -196,6 +196,12 @@ impl<'a, 'gcx, 'tcx> StableHashingContext<'a, 'gcx, 'tcx> { } } +impl<'a, 'gcx, 'lcx> StableHashingContextProvider for ty::TyCtxt<'a, 'gcx, 'lcx> { + type ContextType = StableHashingContext<'a, 'gcx, 'lcx>; + fn create_stable_hashing_context(&self) -> Self::ContextType { + StableHashingContext::new(*self) + } +} impl<'a, 'gcx, 'tcx> HashStable> for ast::NodeId { fn hash_stable(&self, diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index 5ad8ca6c504cf..dd89324323375 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -13,7 +13,6 @@ use errors::{Diagnostic, DiagnosticBuilder}; use hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use hir::def::{Def, Export}; use hir::{self, TraitCandidate, HirId}; -use ich::{Fingerprint, StableHashingContext}; use lint; use middle::const_val; use middle::cstore::{ExternCrate, LinkagePreference}; @@ -37,7 +36,7 @@ use rustc_data_structures::indexed_set::IdxSetBuf; use rustc_data_structures::indexed_vec::IndexVec; use rustc_data_structures::fx::FxHashMap; use std::cell::{RefCell, RefMut, Cell}; -use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; + use std::fmt::Debug; use std::hash::Hash; use std::marker::PhantomData; @@ -732,20 +731,6 @@ macro_rules! define_maps { tcx.dep_graph.read_index(dep_node_index); - // In incremental mode, hash the result of the query. We don't - // do anything with the hash yet, but we are computing it - // anyway so that - // - we make sure that the infrastructure works and - // - we can get an idea of the runtime cost. - if !dep_node.kind.is_anon() && tcx.sess.opts.incremental.is_some() { - let mut hcx = StableHashingContext::new(tcx); - let mut hasher = StableHasher::new(); - - result.hash_stable(&mut hcx, &mut hasher); - - let _: Fingerprint = hasher.finish(); - } - let value = QueryValue { value: result, index: dep_node_index, diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 1851e1b8d34bb..492ebb5bbea2b 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2597,6 +2597,10 @@ pub struct SymbolName { pub name: InternedString } +impl_stable_hash_for!(struct self::SymbolName { + name +}); + impl Deref for SymbolName { type Target = str; diff --git a/src/librustc_data_structures/stable_hasher.rs b/src/librustc_data_structures/stable_hasher.rs index 0d0cf67248944..f80cbfadf2e7f 100644 --- a/src/librustc_data_structures/stable_hasher.rs +++ b/src/librustc_data_structures/stable_hasher.rs @@ -192,6 +192,28 @@ impl Hasher for StableHasher { } +/// Something that can provide a stable hashing context. +pub trait StableHashingContextProvider { + type ContextType; + fn create_stable_hashing_context(&self) -> Self::ContextType; +} + +impl<'a, T: StableHashingContextProvider> StableHashingContextProvider for &'a T { + type ContextType = T::ContextType; + + fn create_stable_hashing_context(&self) -> Self::ContextType { + (**self).create_stable_hashing_context() + } +} + +impl<'a, T: StableHashingContextProvider> StableHashingContextProvider for &'a mut T { + type ContextType = T::ContextType; + + fn create_stable_hashing_context(&self) -> Self::ContextType { + (**self).create_stable_hashing_context() + } +} + /// Something that implements `HashStable` can be hashed in a way that is /// stable across multiple compilation sessions. pub trait HashStable { @@ -292,7 +314,7 @@ impl, CTX> HashStable for Vec { } } -impl, CTX> HashStable for Box { +impl, CTX> HashStable for Box { #[inline] fn hash_stable(&self, ctx: &mut CTX, @@ -301,7 +323,7 @@ impl, CTX> HashStable for Box { } } -impl, CTX> HashStable for ::std::rc::Rc { +impl, CTX> HashStable for ::std::rc::Rc { #[inline] fn hash_stable(&self, ctx: &mut CTX, @@ -310,7 +332,7 @@ impl, CTX> HashStable for ::std::rc::Rc { } } -impl, CTX> HashStable for ::std::sync::Arc { +impl, CTX> HashStable for ::std::sync::Arc { #[inline] fn hash_stable(&self, ctx: &mut CTX, diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index a6c6b0efcfa8c..0c551d86969db 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -1116,7 +1116,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let dep_node = cgu.work_product_dep_node(); let ((stats, module), _) = tcx.dep_graph.with_task(dep_node, - AssertDepGraphSafe(&shared_ccx), + &shared_ccx, AssertDepGraphSafe((cgu, translation_items.clone(), exported_symbols.clone())), @@ -1159,14 +1159,13 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } fn module_translation<'a, 'tcx>( - scx: AssertDepGraphSafe<&SharedCrateContext<'a, 'tcx>>, + scx: &SharedCrateContext<'a, 'tcx>, args: AssertDepGraphSafe<(CodegenUnit<'tcx>, Arc>>, Arc)>) -> (Stats, ModuleTranslation) { // FIXME(#40304): We ought to be using the id as a key and some queries, I think. - let AssertDepGraphSafe(scx) = scx; let AssertDepGraphSafe((cgu, crate_trans_items, exported_symbols)) = args; let cgu_name = String::from(cgu.name()); @@ -1502,3 +1501,34 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a (translation_items, codegen_units) } + +// FIXME(mw): Anything that is produced via DepGraph::with_task() must implement +// the HashStable trait. Normally DepGraph::with_task() calls are +// hidden behind queries, but CGU creation is a special case in two +// ways: (1) it's not a query and (2) CGU are output nodes, so their +// Fingerprints are not actually needed. It remains to be clarified +// how exactly this case will be handled in the red/green system but +// for now we content ourselves with providing a no-op HashStable +// implementation for CGUs. +mod temp_stable_hash_impls { + use rustc_data_structures::stable_hasher::{StableHasherResult, StableHasher, + HashStable}; + use context::Stats; + use ModuleTranslation; + + impl HashStable for Stats { + fn hash_stable(&self, + _: &mut HCX, + _: &mut StableHasher) { + // do nothing + } + } + + impl HashStable for ModuleTranslation { + fn hash_stable(&self, + _: &mut HCX, + _: &mut StableHasher) { + // do nothing + } + } +} diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index f2b07cf6a5831..943137eb43549 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -13,6 +13,7 @@ use llvm::{ContextRef, ModuleRef, ValueRef}; use rustc::dep_graph::{DepGraph, DepGraphSafe}; use rustc::hir; use rustc::hir::def_id::DefId; +use rustc::ich::StableHashingContext; use rustc::traits; use debuginfo; use callee; @@ -25,6 +26,7 @@ use partitioning::CodegenUnit; use trans_item::TransItem; use type_::Type; use rustc_data_structures::base_n; +use rustc_data_structures::stable_hasher::StableHashingContextProvider; use rustc::session::config::{self, NoDebugInfo, OutputFilenames}; use rustc::session::Session; use rustc::ty::{self, Ty, TyCtxt}; @@ -174,6 +176,17 @@ impl<'a, 'tcx> CrateContext<'a, 'tcx> { impl<'a, 'tcx> DepGraphSafe for CrateContext<'a, 'tcx> { } +impl<'a, 'tcx> DepGraphSafe for SharedCrateContext<'a, 'tcx> { +} + +impl<'a, 'tcx> StableHashingContextProvider for SharedCrateContext<'a, 'tcx> { + type ContextType = StableHashingContext<'a, 'tcx, 'tcx>; + + fn create_stable_hashing_context(&self) -> Self::ContextType { + StableHashingContext::new(self.tcx) + } +} + pub fn get_reloc_model(sess: &Session) -> llvm::RelocMode { let reloc_model_arg = match sess.opts.cg.relocation_model { Some(ref s) => &s[..], diff --git a/src/librustc_typeck/variance/constraints.rs b/src/librustc_typeck/variance/constraints.rs index 70b989b6ab21c..4918dd78b3793 100644 --- a/src/librustc_typeck/variance/constraints.rs +++ b/src/librustc_typeck/variance/constraints.rs @@ -14,7 +14,8 @@ //! We walk the set of items and, for each member, generate new constraints. use hir::def_id::DefId; -use rustc::dep_graph::{AssertDepGraphSafe, DepKind}; +use rustc::dep_graph::{DepGraphSafe, DepKind}; +use rustc::ich::StableHashingContext; use rustc::ty::subst::Substs; use rustc::ty::{self, Ty, TyCtxt}; use syntax::ast; @@ -22,6 +23,7 @@ use rustc::hir; use rustc::hir::itemlikevisit::ItemLikeVisitor; use rustc_data_structures::transitive_relation::TransitiveRelation; +use rustc_data_structures::stable_hasher::StableHashingContextProvider; use super::terms::*; use super::terms::VarianceTerm::*; @@ -138,6 +140,16 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for ConstraintContext<'a, 'tcx> { } } +impl<'a, 'tcx> StableHashingContextProvider for ConstraintContext<'a, 'tcx> { + type ContextType = StableHashingContext<'a, 'tcx, 'tcx>; + + fn create_stable_hashing_context(&self) -> Self::ContextType { + StableHashingContext::new(self.terms_cx.tcx) + } +} + +impl<'a, 'tcx> DepGraphSafe for ConstraintContext<'a, 'tcx> {} + impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { fn visit_node_helper(&mut self, id: ast::NodeId) { let tcx = self.terms_cx.tcx; @@ -151,14 +163,14 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { // on dep-graph management. let dep_node = def_id.to_dep_node(tcx, DepKind::ItemVarianceConstraints); tcx.dep_graph.with_task(dep_node, - AssertDepGraphSafe(self), + self, def_id, visit_item_task); - fn visit_item_task<'a, 'tcx>(ccx: AssertDepGraphSafe<&mut ConstraintContext<'a, 'tcx>>, + fn visit_item_task<'a, 'tcx>(ccx: &mut ConstraintContext<'a, 'tcx>, def_id: DefId) { - ccx.0.build_constraints_for_item(def_id); + ccx.build_constraints_for_item(def_id); } }