From 3abd4753b7c9fa9a3855d4b41b557f81b3e06aab Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 Mar 2020 18:36:24 +0100 Subject: [PATCH] Make QueryCache parameters associated types. --- src/librustc/ty/query/caches.rs | 45 +++++--- src/librustc/ty/query/config.rs | 7 +- src/librustc/ty/query/plumbing.rs | 118 +++++++++++---------- src/librustc/ty/query/profiling_support.rs | 8 +- src/librustc/ty/query/stats.rs | 17 ++- 5 files changed, 101 insertions(+), 94 deletions(-) diff --git a/src/librustc/ty/query/caches.rs b/src/librustc/ty/query/caches.rs index 17639545d12a1..71523ea39ca3e 100644 --- a/src/librustc/ty/query/caches.rs +++ b/src/librustc/ty/query/caches.rs @@ -6,12 +6,15 @@ use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sharded::Sharded; use std::default::Default; use std::hash::Hash; +use std::marker::PhantomData; pub(crate) trait CacheSelector { - type Cache: QueryCache; + type Cache: QueryCache; } -pub(crate) trait QueryCache: Default { +pub(crate) trait QueryCache: Default { + type Key; + type Value; type Sharded: Default; /// Checks if the query is already computed and in the cache. @@ -20,25 +23,26 @@ pub(crate) trait QueryCache: Default { /// to compute it. fn lookup<'tcx, R, GetCache, OnHit, OnMiss>( &self, - state: &'tcx QueryStateImpl<'tcx, K, V, Self>, + state: &'tcx QueryStateImpl<'tcx, Self>, get_cache: GetCache, - key: K, + key: Self::Key, // `on_hit` can be called while holding a lock to the query state shard. on_hit: OnHit, on_miss: OnMiss, ) -> R where - GetCache: - for<'a> Fn(&'a mut QueryStateShard<'tcx, K, Self::Sharded>) -> &'a mut Self::Sharded, - OnHit: FnOnce(&V, DepNodeIndex) -> R, - OnMiss: FnOnce(K, QueryLookup<'tcx, K, Self::Sharded>) -> R; + GetCache: for<'a> Fn( + &'a mut QueryStateShard<'tcx, Self::Key, Self::Sharded>, + ) -> &'a mut Self::Sharded, + OnHit: FnOnce(&Self::Value, DepNodeIndex) -> R, + OnMiss: FnOnce(Self::Key, QueryLookup<'tcx, Self::Key, Self::Sharded>) -> R; fn complete( &self, tcx: TyCtxt<'tcx>, lock_sharded_storage: &mut Self::Sharded, - key: K, - value: V, + key: Self::Key, + value: Self::Value, index: DepNodeIndex, ); @@ -46,26 +50,35 @@ pub(crate) trait QueryCache: Default { &self, shards: &Sharded, get_shard: impl Fn(&mut L) -> &mut Self::Sharded, - f: impl for<'a> FnOnce(Box + 'a>) -> R, + f: impl for<'a> FnOnce( + Box + 'a>, + ) -> R, ) -> R; } pub struct DefaultCacheSelector; impl CacheSelector for DefaultCacheSelector { - type Cache = DefaultCache; + type Cache = DefaultCache; } -#[derive(Default)] -pub struct DefaultCache; +pub struct DefaultCache(PhantomData<(K, V)>); + +impl Default for DefaultCache { + fn default() -> Self { + DefaultCache(PhantomData) + } +} -impl QueryCache for DefaultCache { +impl QueryCache for DefaultCache { + type Key = K; + type Value = V; type Sharded = FxHashMap; #[inline(always)] fn lookup<'tcx, R, GetCache, OnHit, OnMiss>( &self, - state: &'tcx QueryStateImpl<'tcx, K, V, Self>, + state: &'tcx QueryStateImpl<'tcx, Self>, get_cache: GetCache, key: K, on_hit: OnHit, diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs index 29b9e0c3b40ef..5b0653bd599e1 100644 --- a/src/librustc/ty/query/config.rs +++ b/src/librustc/ty/query/config.rs @@ -30,7 +30,7 @@ pub(crate) trait QueryAccessors<'tcx>: QueryConfig<'tcx> { const EVAL_ALWAYS: bool; const DEP_KIND: DepKind; - type Cache: QueryCache; + type Cache: QueryCache; // Don't use this method to access query results, instead use the methods on TyCtxt fn query_state<'a>(tcx: TyCtxt<'tcx>) -> &'a QueryState<'tcx, Self>; @@ -59,10 +59,7 @@ pub(crate) trait QueryDescription<'tcx>: QueryAccessors<'tcx> { } } -impl<'tcx, M: QueryAccessors<'tcx, Key = DefId>> QueryDescription<'tcx> for M -where - >::Cache: QueryCache>::Value>, -{ +impl<'tcx, M: QueryAccessors<'tcx, Key = DefId>> QueryDescription<'tcx> for M { default fn describe(tcx: TyCtxt<'_>, def_id: DefId) -> Cow<'static, str> { if !tcx.sess.verbose() { format!("processing `{}`", tcx.def_path_str(def_id)).into() diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 467b1a7e4a177..c2740733977e8 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -4,7 +4,7 @@ use crate::dep_graph::{DepKind, DepNode, DepNodeIndex, SerializedDepNodeIndex}; use crate::ty::query::caches::QueryCache; -use crate::ty::query::config::{QueryAccessors, QueryConfig, QueryDescription}; +use crate::ty::query::config::{QueryAccessors, QueryDescription}; use crate::ty::query::job::{QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryShardJobId}; use crate::ty::query::Query; use crate::ty::tls; @@ -49,22 +49,20 @@ impl<'tcx, K, C: Default> Default for QueryStateShard<'tcx, K, C> { } } -pub(crate) type QueryState<'tcx, Q> = QueryStateImpl< - 'tcx, - >::Key, - >::Value, - >::Cache, ->; +pub(crate) type QueryState<'tcx, Q> = QueryStateImpl<'tcx, >::Cache>; -pub(crate) struct QueryStateImpl<'tcx, K, V, C: QueryCache> { +pub(crate) struct QueryStateImpl<'tcx, C: QueryCache> { pub(super) cache: C, - pub(super) shards: Sharded>, + pub(super) shards: Sharded>, #[cfg(debug_assertions)] pub(super) cache_hits: AtomicUsize, } -impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { - pub(super) fn get_lookup(&'tcx self, key: &K2) -> QueryLookup<'tcx, K, C::Sharded> { +impl<'tcx, C: QueryCache> QueryStateImpl<'tcx, C> { + pub(super) fn get_lookup( + &'tcx self, + key: &K2, + ) -> QueryLookup<'tcx, C::Key, C::Sharded> { // We compute the key's hash once and then use it for both the // shard lookup and the hashmap lookup. This relies on the fact // that both of them use `FxHasher`. @@ -88,10 +86,12 @@ pub(super) enum QueryResult<'tcx> { Poisoned, } -impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { +impl<'tcx, C: QueryCache> QueryStateImpl<'tcx, C> { pub fn iter_results( &self, - f: impl for<'a> FnOnce(Box + 'a>) -> R, + f: impl for<'a> FnOnce( + Box + 'a>, + ) -> R, ) -> R { self.cache.iter(&self.shards, |shard| &mut shard.cache, f) } @@ -103,11 +103,11 @@ impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { pub(super) fn try_collect_active_jobs( &self, kind: DepKind, - make_query: fn(K) -> Query<'tcx>, + make_query: fn(C::Key) -> Query<'tcx>, jobs: &mut FxHashMap>, ) -> Option<()> where - K: Clone, + C::Key: Clone, { // We use try_lock_shards here since we are called from the // deadlock handler, and this shouldn't be locked. @@ -130,8 +130,8 @@ impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { } } -impl<'tcx, K, V, C: QueryCache> Default for QueryStateImpl<'tcx, K, V, C> { - fn default() -> QueryStateImpl<'tcx, K, V, C> { +impl<'tcx, C: QueryCache> Default for QueryStateImpl<'tcx, C> { + fn default() -> QueryStateImpl<'tcx, C> { QueryStateImpl { cache: C::default(), shards: Default::default(), @@ -150,27 +150,22 @@ pub(crate) struct QueryLookup<'tcx, K, C> { /// A type representing the responsibility to execute the job in the `job` field. /// This will poison the relevant query if dropped. -pub(super) type JobOwner<'tcx, Q> = JobOwnerImpl< - 'tcx, - >::Key, - >::Value, - >::Cache, ->; - -pub(super) struct JobOwnerImpl<'tcx, K, V, C: QueryCache> +pub(super) struct JobOwner<'tcx, C> where - K: Eq + Hash + Clone + Debug, - V: Clone, + C: QueryCache, + C::Key: Eq + Hash + Clone + Debug, + C::Value: Clone, { - state: &'tcx QueryStateImpl<'tcx, K, V, C>, - key: K, + state: &'tcx QueryStateImpl<'tcx, C>, + key: C::Key, id: QueryJobId, } -impl<'tcx, K, V, C: QueryCache> JobOwnerImpl<'tcx, K, V, C> +impl<'tcx, C: QueryCache> JobOwner<'tcx, C> where - K: Eq + Hash + Clone + Debug, - V: Clone, + C: QueryCache, + C::Key: Eq + Hash + Clone + Debug, + C::Value: Clone, { /// Either gets a `JobOwner` corresponding the query, allowing us to /// start executing the query, or returns with the result of the query. @@ -184,13 +179,11 @@ where pub(super) fn try_start( tcx: TyCtxt<'tcx>, span: Span, - key: &K, - mut lookup: QueryLookup<'tcx, K, C::Sharded>, - ) -> TryGetJob<'tcx, Q> + key: &C::Key, + mut lookup: QueryLookup<'tcx, C::Key, C::Sharded>, + ) -> TryGetJob<'tcx, C> where - K: Eq + Hash + Clone + Debug, - V: Clone, - Q: QueryDescription<'tcx, Key = K, Value = V, Cache = C> + 'tcx, + Q: QueryDescription<'tcx, Key = C::Key, Value = C::Value, Cache = C>, { let lock = &mut *lookup.lock; @@ -230,7 +223,7 @@ where entry.insert(QueryResult::Started(job)); let owner = - JobOwnerImpl { state: Q::query_state(tcx), id: global_id, key: (*key).clone() }; + JobOwner { state: Q::query_state(tcx), id: global_id, key: (*key).clone() }; return TryGetJob::NotYetStarted(owner); } }; @@ -271,7 +264,12 @@ where /// Completes the query by updating the query cache with the `result`, /// signals the waiter and forgets the JobOwner, so it won't poison the query #[inline(always)] - pub(super) fn complete(self, tcx: TyCtxt<'tcx>, result: &V, dep_node_index: DepNodeIndex) { + pub(super) fn complete( + self, + tcx: TyCtxt<'tcx>, + result: &C::Value, + dep_node_index: DepNodeIndex, + ) { // We can move out of `self` here because we `mem::forget` it below let key = unsafe { ptr::read(&self.key) }; let state = self.state; @@ -304,10 +302,10 @@ where (result, diagnostics.into_inner()) } -impl<'tcx, K, V, C: QueryCache> Drop for JobOwnerImpl<'tcx, K, V, C> +impl<'tcx, C: QueryCache> Drop for JobOwner<'tcx, C> where - K: Eq + Hash + Clone + Debug, - V: Clone, + C::Key: Eq + Hash + Clone + Debug, + C::Value: Clone, { #[inline(never)] #[cold] @@ -338,18 +336,22 @@ pub struct CycleError<'tcx> { } /// The result of `try_start`. -pub(super) enum TryGetJob<'tcx, D: QueryDescription<'tcx>> { +pub(super) enum TryGetJob<'tcx, C: QueryCache> +where + C::Key: Eq + Hash + Clone + Debug, + C::Value: Clone, +{ /// The query is not yet started. Contains a guard to the cache eventually used to start it. - NotYetStarted(JobOwner<'tcx, D>), + NotYetStarted(JobOwner<'tcx, C>), /// The query was already completed. /// Returns the result of the query and its dep-node index /// if it succeeded or a cycle error if it failed. #[cfg(parallel_compiler)] - JobCompleted((D::Value, DepNodeIndex)), + JobCompleted((C::Value, DepNodeIndex)), /// Trying to execute the query resulted in a cycle. - Cycle(D::Value), + Cycle(C::Value), } impl<'tcx> TyCtxt<'tcx> { @@ -478,22 +480,22 @@ impl<'tcx> TyCtxt<'tcx> { /// which will be used if the query is not in the cache and we need /// to compute it. #[inline(always)] - fn try_get_cached( + fn try_get_cached( self, - state: &'tcx QueryStateImpl<'tcx, K, V, C>, - key: K, + state: &'tcx QueryStateImpl<'tcx, C>, + key: C::Key, // `on_hit` can be called while holding a lock to the query cache on_hit: OnHit, on_miss: OnMiss, ) -> R where - C: QueryCache, - OnHit: FnOnce(&V, DepNodeIndex) -> R, - OnMiss: FnOnce(K, QueryLookup<'tcx, K, C::Sharded>) -> R, + C: QueryCache, + OnHit: FnOnce(&C::Value, DepNodeIndex) -> R, + OnMiss: FnOnce(C::Key, QueryLookup<'tcx, C::Key, C::Sharded>) -> R, { state.cache.lookup( state, - QueryStateShard::::get_cache, + QueryStateShard::::get_cache, key, |value, index| { if unlikely!(self.prof.enabled()) { @@ -533,9 +535,9 @@ impl<'tcx> TyCtxt<'tcx> { self, span: Span, key: Q::Key, - lookup: QueryLookup<'tcx, Q::Key, >::Sharded>, + lookup: QueryLookup<'tcx, Q::Key, ::Sharded>, ) -> Q::Value { - let job = match JobOwnerImpl::try_start::(self, span, &key, lookup) { + let job = match JobOwner::try_start::(self, span, &key, lookup) { TryGetJob::NotYetStarted(job) => job, TryGetJob::Cycle(result) => return result, #[cfg(parallel_compiler)] @@ -696,7 +698,7 @@ impl<'tcx> TyCtxt<'tcx> { fn force_query_with_job + 'tcx>( self, key: Q::Key, - job: JobOwner<'tcx, Q>, + job: JobOwner<'tcx, Q::Cache>, dep_node: DepNode, ) -> (Q::Value, DepNodeIndex) { // If the following assertion triggers, it can have two reasons: @@ -795,7 +797,7 @@ impl<'tcx> TyCtxt<'tcx> { // Cache hit, do nothing }, |key, lookup| { - let job = match JobOwnerImpl::try_start::(self, span, &key, lookup) { + let job = match JobOwner::try_start::(self, span, &key, lookup) { TryGetJob::NotYetStarted(job) => job, TryGetJob::Cycle(_) => return, #[cfg(parallel_compiler)] diff --git a/src/librustc/ty/query/profiling_support.rs b/src/librustc/ty/query/profiling_support.rs index 290e37f2cf825..256bd86a3de38 100644 --- a/src/librustc/ty/query/profiling_support.rs +++ b/src/librustc/ty/query/profiling_support.rs @@ -157,14 +157,14 @@ where /// Allocate the self-profiling query strings for a single query cache. This /// method is called from `alloc_self_profile_query_strings` which knows all /// the queries via macro magic. -pub(super) fn alloc_self_profile_query_strings_for_query_cache<'tcx, K, V, C>( +pub(super) fn alloc_self_profile_query_strings_for_query_cache<'tcx, C>( tcx: TyCtxt<'tcx>, query_name: &'static str, - query_state: &QueryStateImpl<'tcx, K, V, C>, + query_state: &QueryStateImpl<'tcx, C>, string_cache: &mut QueryKeyStringCache, ) where - K: Debug + Clone, - C: QueryCache, + C: QueryCache, + C::Key: Debug + Clone, { tcx.prof.with_profiler(|profiler| { let event_id_builder = profiler.event_id_builder(); diff --git a/src/librustc/ty/query/stats.rs b/src/librustc/ty/query/stats.rs index c4c81cd5a13cf..20894a2a5d1e1 100644 --- a/src/librustc/ty/query/stats.rs +++ b/src/librustc/ty/query/stats.rs @@ -1,5 +1,5 @@ use crate::ty::query::caches::QueryCache; -use crate::ty::query::config::{QueryAccessors, QueryConfig}; +use crate::ty::query::config::QueryAccessors; use crate::ty::query::plumbing::QueryStateImpl; use crate::ty::query::queries; use crate::ty::TyCtxt; @@ -38,20 +38,17 @@ struct QueryStats { local_def_id_keys: Option, } -fn stats<'tcx, K, V, C: QueryCache>( - name: &'static str, - map: &QueryStateImpl<'tcx, K, V, C>, -) -> QueryStats { +fn stats<'tcx, C: QueryCache>(name: &'static str, map: &QueryStateImpl<'tcx, C>) -> QueryStats { let mut stats = QueryStats { name, #[cfg(debug_assertions)] cache_hits: map.cache_hits.load(Ordering::Relaxed), #[cfg(not(debug_assertions))] cache_hits: 0, - key_size: mem::size_of::(), - key_type: type_name::(), - value_size: mem::size_of::(), - value_type: type_name::(), + key_size: mem::size_of::(), + key_type: type_name::(), + value_size: mem::size_of::(), + value_type: type_name::(), entry_count: map.iter_results(|results| results.count()), local_def_id_keys: None, }; @@ -127,8 +124,6 @@ macro_rules! print_stats { $($( queries.push(stats::< - as QueryConfig<'_>>::Key, - as QueryConfig<'_>>::Value, as QueryAccessors<'_>>::Cache, >( stringify!($name),