From 4e5256e186c5505f3dab9409c873d8eb25b92d3f Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Thu, 25 Aug 2022 15:33:07 +0800 Subject: [PATCH 1/2] add `with_hash_task` to generate `DepNode` deterministically --- compiler/rustc_query_system/src/cache.rs | 8 +++- .../rustc_query_system/src/dep_graph/graph.rs | 46 +++++++++++++------ .../src/traits/select/mod.rs | 41 ++++++++++++++--- 3 files changed, 73 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_query_system/src/cache.rs b/compiler/rustc_query_system/src/cache.rs index 7cc885be2ba6a..83dc2f11361b5 100644 --- a/compiler/rustc_query_system/src/cache.rs +++ b/compiler/rustc_query_system/src/cache.rs @@ -3,7 +3,7 @@ use crate::dep_graph::{DepContext, DepNodeIndex}; use rustc_data_structures::fx::FxHashMap; -use rustc_data_structures::sync::Lock; +use rustc_data_structures::sync::{HashMapExt, Lock}; use std::hash::Hash; @@ -35,6 +35,12 @@ impl Cache { } } +impl Cache { + pub fn insert_same(&self, key: Key, dep_node: DepNodeIndex, value: Value) { + self.hashmap.borrow_mut().insert_same(key, WithDepNode::new(dep_node, value)); + } +} + #[derive(Clone, Eq, PartialEq)] pub struct WithDepNode { dep_node: DepNodeIndex, diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 0b1ff5d709fe9..c986700dfc7f8 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -377,14 +377,39 @@ impl DepGraph { /// Executes something within an "anonymous" task, that is, a task the /// `DepNode` of which is determined by the list of inputs it read from. - pub fn with_anon_task, OP, R>( - &self, + #[inline] + pub fn with_anon_task, OP, R>( &self, cx: Tcx, dep_kind: K, op: OP, ) -> (R, DepNodeIndex) where OP: FnOnce() -> R, + { + self.with_hash_task(cx, dep_kind, op, |task_deps| { + // The dep node indices are hashed here instead of hashing the dep nodes of the + // dependencies. These indices may refer to different nodes per session, but this isn't + // a problem here because we that ensure the final dep node hash is per session only by + // combining it with the per session random number `anon_id_seed`. This hash only need + // to map the dependencies to a single value on a per session basis. + let mut hasher = StableHasher::new(); + task_deps.reads.hash(&mut hasher); + hasher.finish() + }) + } + + /// Executes something within an "anonymous" task. The `hash` is used for + /// generating the `DepNode`. + pub fn with_hash_task, OP, R, H>( + &self, + cx: Ctxt, + dep_kind: K, + op: OP, + hash: H, + ) -> (R, DepNodeIndex) + where + OP: FnOnce() -> R, + H: FnOnce(&TaskDeps) -> Fingerprint, { debug_assert!(!cx.is_eval_always(dep_kind)); @@ -392,9 +417,8 @@ impl DepGraph { let task_deps = Lock::new(TaskDeps::default()); let result = K::with_deps(TaskDepsRef::Allow(&task_deps), op); let task_deps = task_deps.into_inner(); - let task_deps = task_deps.reads; - let dep_node_index = match task_deps.len() { + let dep_node_index = match task_deps.reads.len() { 0 => { // Because the dep-node id of anon nodes is computed from the sets of its // dependencies we already know what the ID of this dependency-less node is @@ -405,29 +429,23 @@ impl DepGraph { } 1 => { // When there is only one dependency, don't bother creating a node. - task_deps[0] + task_deps.reads[0] } _ => { - // The dep node indices are hashed here instead of hashing the dep nodes of the - // dependencies. These indices may refer to different nodes per session, but this isn't - // a problem here because we that ensure the final dep node hash is per session only by - // combining it with the per session random number `anon_id_seed`. This hash only need - // to map the dependencies to a single value on a per session basis. - let mut hasher = StableHasher::new(); - task_deps.hash(&mut hasher); + let hash_result = hash(&task_deps); let target_dep_node = DepNode { kind: dep_kind, // Fingerprint::combine() is faster than sending Fingerprint // through the StableHasher (at least as long as StableHasher // is so slow). - hash: data.current.anon_id_seed.combine(hasher.finish()).into(), + hash: data.current.anon_id_seed.combine(hash_result).into(), }; data.current.intern_new_node( cx.profiler(), target_dep_node, - task_deps, + task_deps.reads, Fingerprint::ZERO, ) } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 760b4585f4e19..b8529e51a16a8 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -49,13 +49,17 @@ use rustc_middle::ty::{self, EarlyBinder, PolyProjectionPredicate, ToPolyTraitRe use rustc_middle::ty::{Ty, TyCtxt, TypeFoldable, TypeVisitable}; use rustc_span::symbol::sym; +use rustc_data_structures::fingerprint::Fingerprint; +use rustc_data_structures::stable_hasher::StableHasher; use std::cell::{Cell, RefCell}; use std::cmp; use std::fmt::{self, Display}; +use std::hash::Hash; use std::iter; pub use rustc_middle::traits::select::*; use rustc_middle::ty::print::with_no_trimmed_paths; +use rustc_query_system::dep_graph::TaskDeps; mod candidate_assembly; mod confirmation; @@ -1017,7 +1021,19 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { return Ok(cycle_result); } - let (result, dep_node) = self.in_task(|this| this.evaluate_stack(&stack)); + let (result, dep_node) = if cfg!(parallel_compiler) { + self.in_task_with_hash( + |this| this.evaluate_stack(&stack), + |_| { + let mut hasher = StableHasher::new(); + (param_env, fresh_trait_pred).hash(&mut hasher); + hasher.finish() + }, + ) + } else { + self.in_task(|this| this.evaluate_stack(&stack)) + }; + let result = result?; if !result.must_apply_modulo_regions() { @@ -1263,17 +1279,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { if self.can_use_global_caches(param_env) { if !trait_pred.needs_infer() { debug!(?trait_pred, ?result, "insert_evaluation_cache global"); - // This may overwrite the cache with the same value - // FIXME: Due to #50507 this overwrites the different values - // This should be changed to use HashMapExt::insert_same - // when that is fixed - self.tcx().evaluation_cache.insert((param_env, trait_pred), dep_node, result); + self.tcx().evaluation_cache.insert_same((param_env, trait_pred), dep_node, result); return; } } debug!(?trait_pred, ?result, "insert_evaluation_cache"); - self.infcx.evaluation_cache.insert((param_env, trait_pred), dep_node, result); + self.infcx.evaluation_cache.insert_same((param_env, trait_pred), dep_node, result); } /// For various reasons, it's possible for a subobligation @@ -1344,6 +1356,21 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { (result, dep_node) } + fn in_task_with_hash(&mut self, op: OP, hash: H) -> (R, DepNodeIndex) + where + OP: FnOnce(&mut Self) -> R, + H: FnOnce(&TaskDeps) -> Fingerprint, + { + let (result, dep_node) = self.tcx().dep_graph.with_hash_task( + self.tcx(), + DepKind::TraitSelect, + || op(self), + hash, + ); + self.tcx().dep_graph.read_index(dep_node); + (result, dep_node) + } + /// filter_impls filters constant trait obligations and candidates that have a positive impl /// for a negative goal and a negative impl for a positive goal #[instrument(level = "debug", skip(self, candidates))] From a4b16633175bccb811390752381274d4d93c7df4 Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Thu, 5 Jan 2023 18:52:19 +0800 Subject: [PATCH 2/2] avoid hash in client code --- .../rustc_query_system/src/dep_graph/graph.rs | 39 ++++++++++--------- .../src/traits/select/mod.rs | 22 ++--------- 2 files changed, 25 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index c986700dfc7f8..7cf1afb73e979 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -378,7 +378,8 @@ impl DepGraph { /// Executes something within an "anonymous" task, that is, a task the /// `DepNode` of which is determined by the list of inputs it read from. #[inline] - pub fn with_anon_task, OP, R>( &self, + pub fn with_anon_task, OP, R>( + &self, cx: Tcx, dep_kind: K, op: OP, @@ -386,16 +387,7 @@ impl DepGraph { where OP: FnOnce() -> R, { - self.with_hash_task(cx, dep_kind, op, |task_deps| { - // The dep node indices are hashed here instead of hashing the dep nodes of the - // dependencies. These indices may refer to different nodes per session, but this isn't - // a problem here because we that ensure the final dep node hash is per session only by - // combining it with the per session random number `anon_id_seed`. This hash only need - // to map the dependencies to a single value on a per session basis. - let mut hasher = StableHasher::new(); - task_deps.reads.hash(&mut hasher); - hasher.finish() - }) + self.with_hash_task(cx, dep_kind, op, None::<()>) } /// Executes something within an "anonymous" task. The `hash` is used for @@ -405,11 +397,11 @@ impl DepGraph { cx: Ctxt, dep_kind: K, op: OP, - hash: H, + hash: Option, ) -> (R, DepNodeIndex) where OP: FnOnce() -> R, - H: FnOnce(&TaskDeps) -> Fingerprint, + H: Hash, { debug_assert!(!cx.is_eval_always(dep_kind)); @@ -417,8 +409,9 @@ impl DepGraph { let task_deps = Lock::new(TaskDeps::default()); let result = K::with_deps(TaskDepsRef::Allow(&task_deps), op); let task_deps = task_deps.into_inner(); + let task_deps = task_deps.reads; - let dep_node_index = match task_deps.reads.len() { + let dep_node_index = match task_deps.len() { 0 => { // Because the dep-node id of anon nodes is computed from the sets of its // dependencies we already know what the ID of this dependency-less node is @@ -429,23 +422,33 @@ impl DepGraph { } 1 => { // When there is only one dependency, don't bother creating a node. - task_deps.reads[0] + task_deps[0] } _ => { - let hash_result = hash(&task_deps); + let mut hasher = StableHasher::new(); + if let Some(hash) = hash { + hash.hash(&mut hasher); + } else { + // The dep node indices are hashed here instead of hashing the dep nodes of the + // dependencies. These indices may refer to different nodes per session, but this isn't + // a problem here because we that ensure the final dep node hash is per session only by + // combining it with the per session random number `anon_id_seed`. This hash only need + // to map the dependencies to a single value on a per session basis. + task_deps.hash(&mut hasher); + } let target_dep_node = DepNode { kind: dep_kind, // Fingerprint::combine() is faster than sending Fingerprint // through the StableHasher (at least as long as StableHasher // is so slow). - hash: data.current.anon_id_seed.combine(hash_result).into(), + hash: data.current.anon_id_seed.combine(hasher.finish()).into(), }; data.current.intern_new_node( cx.profiler(), target_dep_node, - task_deps.reads, + task_deps, Fingerprint::ZERO, ) } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index b8529e51a16a8..ff395a9c2604a 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -49,8 +49,6 @@ use rustc_middle::ty::{self, EarlyBinder, PolyProjectionPredicate, ToPolyTraitRe use rustc_middle::ty::{Ty, TyCtxt, TypeFoldable, TypeVisitable}; use rustc_span::symbol::sym; -use rustc_data_structures::fingerprint::Fingerprint; -use rustc_data_structures::stable_hasher::StableHasher; use std::cell::{Cell, RefCell}; use std::cmp; use std::fmt::{self, Display}; @@ -59,8 +57,6 @@ use std::iter; pub use rustc_middle::traits::select::*; use rustc_middle::ty::print::with_no_trimmed_paths; -use rustc_query_system::dep_graph::TaskDeps; - mod candidate_assembly; mod confirmation; @@ -1021,18 +1017,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { return Ok(cycle_result); } - let (result, dep_node) = if cfg!(parallel_compiler) { - self.in_task_with_hash( - |this| this.evaluate_stack(&stack), - |_| { - let mut hasher = StableHasher::new(); - (param_env, fresh_trait_pred).hash(&mut hasher); - hasher.finish() - }, - ) - } else { - self.in_task(|this| this.evaluate_stack(&stack)) - }; + let (result, dep_node) = self + .in_task_with_hash(|this| this.evaluate_stack(&stack), &(param_env, fresh_trait_pred)); let result = result?; @@ -1359,13 +1345,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { fn in_task_with_hash(&mut self, op: OP, hash: H) -> (R, DepNodeIndex) where OP: FnOnce(&mut Self) -> R, - H: FnOnce(&TaskDeps) -> Fingerprint, + H: Hash, { let (result, dep_node) = self.tcx().dep_graph.with_hash_task( self.tcx(), DepKind::TraitSelect, || op(self), - hash, + Some(hash), ); self.tcx().dep_graph.read_index(dep_node); (result, dep_node)