Skip to content

Commit

Permalink
Auto merge of #57770 - Zoxc:no-hash-query, r=<try>
Browse files Browse the repository at this point in the history
Add a query type which is always marked as red if it runs

This is useful for queries which produce results which are very likely to change if their inputs do.

I also expect this to be useful for end to end queries because 1) we don't need `HashStable` impls and 2) we avoid the overhead of hashing the result of large results like the AST or the HIR map.

r? @michaelwoerister
  • Loading branch information
bors committed Jan 21, 2019
2 parents e730697 + ffee835 commit cb75345
Show file tree
Hide file tree
Showing 8 changed files with 104 additions and 59 deletions.
89 changes: 54 additions & 35 deletions src/librustc/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ struct DepGraphData {
loaded_from_cache: Lock<FxHashMap<DepNodeIndex, bool>>,
}

pub fn hash_result<R>(hcx: &mut StableHashingContext<'_>, result: &R) -> Option<Fingerprint>
where
R: for<'a> HashStable<StableHashingContext<'a>>,
{
let mut stable_hasher = StableHasher::new();
result.hash_stable(hcx, &mut stable_hasher);

Some(stable_hasher.finish())
}

impl DepGraph {

pub fn new(prev_graph: PreviousDepGraph,
Expand Down Expand Up @@ -169,14 +179,16 @@ impl DepGraph {
/// `arg` parameter.
///
/// [rustc guide]: https://rust-lang.github.io/rustc-guide/incremental-compilation.html
pub fn with_task<'gcx, C, A, R>(&self,
key: DepNode,
cx: C,
arg: A,
task: fn(C, A) -> R)
-> (R, DepNodeIndex)
where C: DepGraphSafe + StableHashingContextProvider<'gcx>,
R: HashStable<StableHashingContext<'gcx>>,
pub fn with_task<'a, C, A, R>(
&self,
key: DepNode,
cx: C,
arg: A,
task: fn(C, A) -> R,
hash_result: impl FnOnce(&mut StableHashingContext<'_>, &R) -> Option<Fingerprint>,
) -> (R, DepNodeIndex)
where
C: DepGraphSafe + StableHashingContextProvider<'a>,
{
self.with_task_impl(key, cx, arg, false, task,
|_key| Some(TaskDeps {
Expand All @@ -187,17 +199,18 @@ impl DepGraph {
}),
|data, key, fingerprint, task| {
data.borrow_mut().complete_task(key, task.unwrap(), fingerprint)
})
},
hash_result)
}

/// Creates a new dep-graph input with value `input`
pub fn input_task<'gcx, C, R>(&self,
pub fn input_task<'a, C, R>(&self,
key: DepNode,
cx: C,
input: R)
-> (R, DepNodeIndex)
where C: DepGraphSafe + StableHashingContextProvider<'gcx>,
R: HashStable<StableHashingContext<'gcx>>,
where C: DepGraphSafe + StableHashingContextProvider<'a>,
R: for<'b> HashStable<StableHashingContext<'b>>,
{
fn identity_fn<C, A>(_: C, arg: A) -> A {
arg
Expand All @@ -207,10 +220,11 @@ impl DepGraph {
|_| None,
|data, key, fingerprint, _| {
data.borrow_mut().alloc_node(key, SmallVec::new(), fingerprint)
})
},
hash_result::<R>)
}

fn with_task_impl<'gcx, C, A, R>(
fn with_task_impl<'a, C, A, R>(
&self,
key: DepNode,
cx: C,
Expand All @@ -221,11 +235,11 @@ impl DepGraph {
finish_task_and_alloc_depnode: fn(&Lock<CurrentDepGraph>,
DepNode,
Fingerprint,
Option<TaskDeps>) -> DepNodeIndex
Option<TaskDeps>) -> DepNodeIndex,
hash_result: impl FnOnce(&mut StableHashingContext<'_>, &R) -> Option<Fingerprint>,
) -> (R, DepNodeIndex)
where
C: DepGraphSafe + StableHashingContextProvider<'gcx>,
R: HashStable<StableHashingContext<'gcx>>,
C: DepGraphSafe + StableHashingContextProvider<'a>,
{
if let Some(ref data) = self.data {
let task_deps = create_task(key).map(|deps| Lock::new(deps));
Expand Down Expand Up @@ -260,31 +274,33 @@ impl DepGraph {
profq_msg(hcx.sess(), ProfileQueriesMsg::TaskEnd)
};

let mut stable_hasher = StableHasher::new();
result.hash_stable(&mut hcx, &mut stable_hasher);

let current_fingerprint = stable_hasher.finish();
let current_fingerprint = hash_result(&mut hcx, &result);

let dep_node_index = finish_task_and_alloc_depnode(
&data.current,
key,
current_fingerprint,
current_fingerprint.unwrap_or(Fingerprint::ZERO),
task_deps.map(|lock| lock.into_inner()),
);

// Determine the color of the new DepNode.
if let Some(prev_index) = data.previous.node_to_index_opt(&key) {
let prev_fingerprint = data.previous.fingerprint_by_index(prev_index);

let color = if current_fingerprint == prev_fingerprint {
DepNodeColor::Green(dep_node_index)
let color = if let Some(current_fingerprint) = current_fingerprint {
if current_fingerprint == prev_fingerprint {
DepNodeColor::Green(dep_node_index)
} else {
DepNodeColor::Red
}
} else {
// Mark the node as Red if we can't hash the result
DepNodeColor::Red
};

debug_assert!(data.colors.get(prev_index).is_none(),
"DepGraph::with_task() - Duplicate DepNodeColor \
insertion for {:?}", key);
"DepGraph::with_task() - Duplicate DepNodeColor \
insertion for {:?}", key);

data.colors.insert(prev_index, color);
}
Expand Down Expand Up @@ -333,14 +349,16 @@ impl DepGraph {

/// Execute something within an "eval-always" task which is a task
// that runs whenever anything changes.
pub fn with_eval_always_task<'gcx, C, A, R>(&self,
key: DepNode,
cx: C,
arg: A,
task: fn(C, A) -> R)
-> (R, DepNodeIndex)
where C: DepGraphSafe + StableHashingContextProvider<'gcx>,
R: HashStable<StableHashingContext<'gcx>>,
pub fn with_eval_always_task<'a, C, A, R>(
&self,
key: DepNode,
cx: C,
arg: A,
task: fn(C, A) -> R,
hash_result: impl FnOnce(&mut StableHashingContext<'_>, &R) -> Option<Fingerprint>,
) -> (R, DepNodeIndex)
where
C: DepGraphSafe + StableHashingContextProvider<'a>,
{
self.with_task_impl(key, cx, arg, false, task,
|_| None,
Expand All @@ -350,7 +368,8 @@ impl DepGraph {
&DepNode::new_no_params(DepKind::Krate)
];
current.alloc_node(key, smallvec![krate_idx], fingerprint)
})
},
hash_result)
}

#[inline]
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/dep_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub mod cgu_reuse_tracker;

pub use self::dep_tracking_map::{DepTrackingMap, DepTrackingMapConfig};
pub use self::dep_node::{DepNode, DepKind, DepConstructor, WorkProductId, label_strs};
pub use self::graph::{DepGraph, WorkProduct, DepNodeIndex, DepNodeColor, TaskDeps};
pub use self::graph::{DepGraph, WorkProduct, DepNodeIndex, DepNodeColor, TaskDeps, hash_result};
pub use self::graph::WorkProductFileKind;
pub use self::prev::PreviousDepGraph;
pub use self::query::DepGraphQuery;
Expand Down
14 changes: 7 additions & 7 deletions src/librustc/hir/map/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ pub(super) struct NodeCollector<'a, 'hir> {
hir_body_nodes: Vec<(DefPathHash, Fingerprint)>,
}

fn input_dep_node_and_hash<'a, I>(
fn input_dep_node_and_hash<I>(
dep_graph: &DepGraph,
hcx: &mut StableHashingContext<'a>,
hcx: &mut StableHashingContext<'_>,
dep_node: DepNode,
input: I,
) -> (DepNodeIndex, Fingerprint)
where
I: HashStable<StableHashingContext<'a>>,
I: for<'a> HashStable<StableHashingContext<'a>>,
{
let dep_node_index = dep_graph.input_task(dep_node, &mut *hcx, &input).1;

Expand All @@ -67,15 +67,15 @@ where
(dep_node_index, hash)
}

fn alloc_hir_dep_nodes<'a, I>(
fn alloc_hir_dep_nodes<I>(
dep_graph: &DepGraph,
hcx: &mut StableHashingContext<'a>,
hcx: &mut StableHashingContext<'_>,
def_path_hash: DefPathHash,
item_like: I,
hir_body_nodes: &mut Vec<(DefPathHash, Fingerprint)>,
) -> (DepNodeIndex, DepNodeIndex)
where
I: HashStable<StableHashingContext<'a>>,
I: for<'a> HashStable<StableHashingContext<'a>>,
{
let sig = dep_graph.input_task(
def_path_hash.to_dep_node(DepKind::Hir),
Expand Down Expand Up @@ -280,7 +280,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
self.parent_node = parent_node;
}

fn with_dep_node_owner<T: HashStable<StableHashingContext<'a>>,
fn with_dep_node_owner<T: for<'b> HashStable<StableHashingContext<'b>>,
F: FnOnce(&mut Self)>(&mut self,
dep_node_owner: DefIndex,
item_like: &T,
Expand Down
5 changes: 3 additions & 2 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! type context book-keeping
use dep_graph::DepGraph;
use dep_graph::{DepNode, DepConstructor};
use dep_graph::{self, DepNode, DepConstructor};
use errors::DiagnosticBuilder;
use session::Session;
use session::config::{BorrowckMode, OutputFilenames};
Expand Down Expand Up @@ -1414,7 +1414,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self.dep_graph.with_task(dep_node,
self,
crate_hash,
|_, x| x // No transformation needed
|_, x| x, // No transformation needed
dep_graph::hash_result,
);
}
}
Expand Down
9 changes: 7 additions & 2 deletions src/librustc/ty/query/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::hash::Hash;
use std::fmt::Debug;
use syntax_pos::symbol::InternedString;
use rustc_data_structures::sync::Lock;
use rustc_data_structures::stable_hasher::HashStable;
use rustc_data_structures::fingerprint::Fingerprint;
use ich::StableHashingContext;

// Query configuration and description traits.
Expand All @@ -30,7 +30,7 @@ pub trait QueryConfig<'tcx> {
const CATEGORY: ProfileCategory;

type Key: Eq + Hash + Clone + Debug;
type Value: Clone + for<'a> HashStable<StableHashingContext<'a>>;
type Value: Clone;
}

pub(super) trait QueryAccessors<'tcx>: QueryConfig<'tcx> {
Expand All @@ -44,6 +44,11 @@ pub(super) trait QueryAccessors<'tcx>: QueryConfig<'tcx> {
// Don't use this method to compute query results, instead use the methods on TyCtxt
fn compute(tcx: TyCtxt<'_, 'tcx, '_>, key: Self::Key) -> Self::Value;

fn hash_result(
hcx: &mut StableHashingContext<'_>,
result: &Self::Value
) -> Option<Fingerprint>;

fn handle_cycle_error(tcx: TyCtxt<'_, 'tcx, '_>) -> Self::Value;
}

Expand Down
9 changes: 5 additions & 4 deletions src/librustc/ty/query/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use dep_graph::{DepConstructor, DepNode};
use dep_graph::{self, DepConstructor, DepNode};
use errors::DiagnosticBuilder;
use hir::def_id::{CrateNum, DefId, DefIndex};
use hir::def::{Def, Export};
Expand Down Expand Up @@ -49,6 +49,7 @@ use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::stable_hasher::StableVec;
use rustc_data_structures::sync::Lrc;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_target::spec::PanicStrategy;

use std::borrow::Cow;
Expand Down Expand Up @@ -227,15 +228,15 @@ define_queries! { <'tcx>

/// Fetch the MIR for a given def-id right after it's built - this includes
/// unreachable code.
[] fn mir_built: MirBuilt(DefId) -> &'tcx Steal<mir::Mir<'tcx>>,
[no_hash] fn mir_built: MirBuilt(DefId) -> &'tcx Steal<mir::Mir<'tcx>>,

/// Fetch the MIR for a given def-id up till the point where it is
/// ready for const evaluation.
///
/// See the README for the `mir` module for details.
[] fn mir_const: MirConst(DefId) -> &'tcx Steal<mir::Mir<'tcx>>,
[no_hash] fn mir_const: MirConst(DefId) -> &'tcx Steal<mir::Mir<'tcx>>,

[] fn mir_validated: MirValidated(DefId) -> &'tcx Steal<mir::Mir<'tcx>>,
[no_hash] fn mir_validated: MirValidated(DefId) -> &'tcx Steal<mir::Mir<'tcx>>,

/// MIR after our optimization passes have run. This is MIR that is ready
/// for codegen. This is also the only query that can fetch non-local MIR, at present.
Expand Down
31 changes: 24 additions & 7 deletions src/librustc/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
dep_node: &DepNode,
dep_node_index: DepNodeIndex,
) {
use rustc_data_structures::stable_hasher::{StableHasher, HashStable};
use ich::Fingerprint;

assert!(Some(self.dep_graph.fingerprint_of(dep_node_index)) ==
Expand All @@ -509,11 +508,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {

debug!("BEGIN verify_ich({:?})", dep_node);
let mut hcx = self.create_stable_hashing_context();
let mut hasher = StableHasher::new();

result.hash_stable(&mut hcx, &mut hasher);

let new_hash: Fingerprint = hasher.finish();
let new_hash = Q::hash_result(&mut hcx, result).unwrap_or(Fingerprint::ZERO);
debug!("END verify_ich({:?})", dep_node);

let old_hash = self.dep_graph.fingerprint_of(dep_node_index);
Expand Down Expand Up @@ -549,12 +545,14 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
tcx.dep_graph.with_eval_always_task(dep_node,
tcx,
key,
Q::compute)
Q::compute,
Q::hash_result)
} else {
tcx.dep_graph.with_task(dep_node,
tcx,
key,
Q::compute)
Q::compute,
Q::hash_result)
}
})
});
Expand Down Expand Up @@ -679,6 +677,18 @@ macro_rules! handle_cycle_error {
};
}

macro_rules! hash_result {
([][$hcx:expr, $result:expr]) => {{
dep_graph::hash_result($hcx, &$result)
}};
([no_hash$(, $modifiers:ident)*][$hcx:expr, $result:expr]) => {{
None
}};
([$other:ident$(, $modifiers:ident)*][$($args:tt)*]) => {
hash_result!([$($modifiers),*][$($args)*])
};
}

macro_rules! define_queries {
(<$tcx:tt> $($category:tt {
$($(#[$attr:meta])* [$($modifiers:tt)*] fn $name:ident: $node:ident($K:ty) -> $V:ty,)*
Expand Down Expand Up @@ -966,6 +976,13 @@ macro_rules! define_queries_inner {
})
}

fn hash_result(
_hcx: &mut StableHashingContext<'_>,
_result: &Self::Value
) -> Option<Fingerprint> {
hash_result!([$($modifiers)*][_hcx, _result])
}

fn handle_cycle_error(tcx: TyCtxt<'_, 'tcx, '_>) -> Self::Value {
handle_cycle_error!([$($modifiers)*][tcx])
}
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_codegen_llvm/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use super::LlvmCodegenBackend;

use llvm;
use metadata;
use rustc::dep_graph;
use rustc::mir::mono::{Linkage, Visibility, Stats};
use rustc::middle::cstore::{EncodedMetadata};
use rustc::ty::TyCtxt;
Expand Down Expand Up @@ -145,7 +146,8 @@ pub fn compile_codegen_unit<'ll, 'tcx>(tcx: TyCtxt<'ll, 'tcx, 'tcx>,
let ((stats, module), _) = tcx.dep_graph.with_task(dep_node,
tcx,
cgu_name,
module_codegen);
module_codegen,
dep_graph::hash_result);
let time_to_codegen = start_time.elapsed();

// We assume that the cost to run LLVM on a CGU is proportional to
Expand Down

0 comments on commit cb75345

Please sign in to comment.