Skip to content

Commit

Permalink
incr.comp.: Move result fingerprinting to DepGraph::with_task().
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
michaelwoerister committed Sep 7, 2017
1 parent 0cf6000 commit ed4ba22
Show file tree
Hide file tree
Showing 9 changed files with 132 additions and 39 deletions.
36 changes: 25 additions & 11 deletions src/librustc/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<raii::DepTask<'graph>> {
self.data.as_ref().map(|data| raii::DepTask::new(&data.edges, key))
}

pub fn with_ignore<OP,R>(&self, op: OP) -> R
where OP: FnOnce() -> R
{
Expand Down Expand Up @@ -109,24 +109,38 @@ impl DepGraph {
/// `arg` parameter.
///
/// [README]: README.md
pub fn with_task<C, A, R>(&self,
key: DepNode,
cx: C,
arg: A,
task: fn(C, A) -> R)
-> (R, DepNodeIndex)
where C: DepGraphSafe
pub fn with_task<C, A, R, HCX>(&self,
key: DepNode,
cx: C,
arg: A,
task: fn(C, A) -> R)
-> (R, DepNodeIndex)
where C: DepGraphSafe + StableHashingContextProvider<ContextType=HCX>,
R: HashStable<HCX>,
{
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)
Expand Down
7 changes: 7 additions & 0 deletions src/librustc/dep_graph/safe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
}
Expand Down
10 changes: 8 additions & 2 deletions src/librustc/ich/hcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<StableHashingContext<'a, 'gcx, 'tcx>> for ast::NodeId {
fn hash_stable<W: StableHasherResult>(&self,
Expand Down
17 changes: 1 addition & 16 deletions src/librustc/ty/maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
28 changes: 25 additions & 3 deletions src/librustc_data_structures/stable_hasher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,28 @@ impl<W> Hasher for StableHasher<W> {
}


/// 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<CTX>` can be hashed in a way that is
/// stable across multiple compilation sessions.
pub trait HashStable<CTX> {
Expand Down Expand Up @@ -292,7 +314,7 @@ impl<T: HashStable<CTX>, CTX> HashStable<CTX> for Vec<T> {
}
}

impl<T: HashStable<CTX>, CTX> HashStable<CTX> for Box<T> {
impl<T: ?Sized + HashStable<CTX>, CTX> HashStable<CTX> for Box<T> {
#[inline]
fn hash_stable<W: StableHasherResult>(&self,
ctx: &mut CTX,
Expand All @@ -301,7 +323,7 @@ impl<T: HashStable<CTX>, CTX> HashStable<CTX> for Box<T> {
}
}

impl<T: HashStable<CTX>, CTX> HashStable<CTX> for ::std::rc::Rc<T> {
impl<T: ?Sized + HashStable<CTX>, CTX> HashStable<CTX> for ::std::rc::Rc<T> {
#[inline]
fn hash_stable<W: StableHasherResult>(&self,
ctx: &mut CTX,
Expand All @@ -310,7 +332,7 @@ impl<T: HashStable<CTX>, CTX> HashStable<CTX> for ::std::rc::Rc<T> {
}
}

impl<T: HashStable<CTX>, CTX> HashStable<CTX> for ::std::sync::Arc<T> {
impl<T: ?Sized + HashStable<CTX>, CTX> HashStable<CTX> for ::std::sync::Arc<T> {
#[inline]
fn hash_stable<W: StableHasherResult>(&self,
ctx: &mut CTX,
Expand Down
36 changes: 33 additions & 3 deletions src/librustc_trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())),
Expand Down Expand Up @@ -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<FxHashSet<TransItem<'tcx>>>,
Arc<ExportedSymbols>)>)
-> (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());
Expand Down Expand Up @@ -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<HCX> HashStable<HCX> for Stats {
fn hash_stable<W: StableHasherResult>(&self,
_: &mut HCX,
_: &mut StableHasher<W>) {
// do nothing
}
}

impl<HCX> HashStable<HCX> for ModuleTranslation {
fn hash_stable<W: StableHasherResult>(&self,
_: &mut HCX,
_: &mut StableHasher<W>) {
// do nothing
}
}
}
13 changes: 13 additions & 0 deletions src/librustc_trans/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand Down Expand Up @@ -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[..],
Expand Down
20 changes: 16 additions & 4 deletions src/librustc_typeck/variance/constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@
//! 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;
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::*;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
}

Expand Down

0 comments on commit ed4ba22

Please sign in to comment.