Skip to content

Commit

Permalink
Auto merge of #81855 - cjgillot:ensure-cache, r=oli-obk
Browse files Browse the repository at this point in the history
Check the result cache before the DepGraph when ensuring queries

Split out of #70951

Calling `ensure` on already forced queries is a common operation.
Looking at the results cache first is faster than checking the DepGraph for a green node.
  • Loading branch information
bors committed Feb 15, 2021
2 parents 9503ea1 + 3fc8ed6 commit d1206f9
Show file tree
Hide file tree
Showing 9 changed files with 298 additions and 237 deletions.
30 changes: 15 additions & 15 deletions compiler/rustc_data_structures/src/sharded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,23 +63,9 @@ impl<T> Sharded<T> {
if SHARDS == 1 { &self.shards[0].0 } else { self.get_shard_by_hash(make_hash(val)) }
}

/// Get a shard with a pre-computed hash value. If `get_shard_by_value` is
/// ever used in combination with `get_shard_by_hash` on a single `Sharded`
/// instance, then `hash` must be computed with `FxHasher`. Otherwise,
/// `hash` can be computed with any hasher, so long as that hasher is used
/// consistently for each `Sharded` instance.
#[inline]
pub fn get_shard_index_by_hash(&self, hash: u64) -> usize {
let hash_len = mem::size_of::<usize>();
// Ignore the top 7 bits as hashbrown uses these and get the next SHARD_BITS highest bits.
// hashbrown also uses the lowest bits, so we can't use those
let bits = (hash >> (hash_len * 8 - 7 - SHARD_BITS)) as usize;
bits % SHARDS
}

#[inline]
pub fn get_shard_by_hash(&self, hash: u64) -> &Lock<T> {
&self.shards[self.get_shard_index_by_hash(hash)].0
&self.shards[get_shard_index_by_hash(hash)].0
}

#[inline]
Expand Down Expand Up @@ -166,3 +152,17 @@ fn make_hash<K: Hash + ?Sized>(val: &K) -> u64 {
val.hash(&mut state);
state.finish()
}

/// Get a shard with a pre-computed hash value. If `get_shard_by_value` is
/// ever used in combination with `get_shard_by_hash` on a single `Sharded`
/// instance, then `hash` must be computed with `FxHasher`. Otherwise,
/// `hash` can be computed with any hasher, so long as that hasher is used
/// consistently for each `Sharded` instance.
#[inline]
pub fn get_shard_index_by_hash(hash: u64) -> usize {
let hash_len = mem::size_of::<usize>();
// Ignore the top 7 bits as hashbrown uses these and get the next SHARD_BITS highest bits.
// hashbrown also uses the lowest bits, so we can't use those
let bits = (hash >> (hash_len * 8 - 7 - SHARD_BITS)) as usize;
bits % SHARDS
}
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,7 @@ pub struct GlobalCtxt<'tcx> {
pub(crate) definitions: &'tcx Definitions,

pub queries: query::Queries<'tcx>,
pub query_caches: query::QueryCaches<'tcx>,

maybe_unused_trait_imports: FxHashSet<LocalDefId>,
maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,
Expand Down Expand Up @@ -1154,6 +1155,7 @@ impl<'tcx> TyCtxt<'tcx> {
untracked_crate: krate,
definitions,
queries: query::Queries::new(providers, extern_providers, on_disk_query_result_cache),
query_caches: query::QueryCaches::default(),
ty_rcache: Default::default(),
pred_rcache: Default::default(),
selection_cache: Default::default(),
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_middle/src/ty/query/on_disk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1244,10 +1244,9 @@ where
.prof
.extra_verbose_generic_activity("encode_query_results_for", std::any::type_name::<Q>());

let state = Q::query_state(tcx);
assert!(state.all_inactive());

state.iter_results(|results| {
assert!(Q::query_state(tcx).all_inactive());
let cache = Q::query_cache(tcx);
cache.iter_results(|results| {
for (key, value, dep_node) in results {
if Q::cache_on_disk(tcx, &key, Some(value)) {
let dep_node = SerializedDepNodeIndex::new(dep_node.index());
Expand Down
69 changes: 53 additions & 16 deletions compiler/rustc_middle/src/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,14 +342,28 @@ macro_rules! define_queries {

$(pub type $name<$tcx> = $V;)*
}
#[allow(nonstandard_style, unused_lifetimes)]
pub mod query_storage {
use super::*;

$(pub type $name<$tcx> = query_storage!([$($modifiers)*][$($K)*, $V]);)*
}
#[allow(nonstandard_style, unused_lifetimes)]
pub mod query_stored {
use super::*;

$(pub type $name<$tcx> = <query_storage::$name<$tcx> as QueryStorage>::Stored;)*
}

#[derive(Default)]
pub struct QueryCaches<$tcx> {
$($(#[$attr])* $name: QueryCacheStore<query_storage::$name<$tcx>>,)*
}

$(impl<$tcx> QueryConfig for queries::$name<$tcx> {
type Key = $($K)*;
type Value = $V;
type Stored = <
query_storage!([$($modifiers)*][$($K)*, $V])
as QueryStorage
>::Stored;
type Stored = query_stored::$name<$tcx>;
const NAME: &'static str = stringify!($name);
}

Expand All @@ -358,13 +372,20 @@ macro_rules! define_queries {
const EVAL_ALWAYS: bool = is_eval_always!([$($modifiers)*]);
const DEP_KIND: dep_graph::DepKind = dep_graph::DepKind::$name;

type Cache = query_storage!([$($modifiers)*][$($K)*, $V]);
type Cache = query_storage::$name<$tcx>;

#[inline(always)]
fn query_state<'a>(tcx: TyCtxt<$tcx>) -> &'a QueryState<crate::dep_graph::DepKind, <TyCtxt<$tcx> as QueryContext>::Query, Self::Cache> {
fn query_state<'a>(tcx: TyCtxt<$tcx>) -> &'a QueryState<crate::dep_graph::DepKind, Query<$tcx>, Self::Key> {
&tcx.queries.$name
}

#[inline(always)]
fn query_cache<'a>(tcx: TyCtxt<$tcx>) -> &'a QueryCacheStore<Self::Cache>
where 'tcx:'a
{
&tcx.query_caches.$name
}

#[inline]
fn compute(tcx: TyCtxt<'tcx>, key: Self::Key) -> Self::Value {
let provider = tcx.queries.providers.get(key.query_crate())
Expand Down Expand Up @@ -401,7 +422,15 @@ macro_rules! define_queries {
$($(#[$attr])*
#[inline(always)]
pub fn $name(self, key: query_helper_param_ty!($($K)*)) {
ensure_query::<queries::$name<'_>, _>(self.tcx, key.into_query_param())
let key = key.into_query_param();
let cached = try_get_cached(self.tcx, &self.tcx.query_caches.$name, &key, |_| {});

let lookup = match cached {
Ok(()) => return,
Err(lookup) => lookup,
};

get_query::<queries::$name<'_>, _>(self.tcx, DUMMY_SP, key, lookup, QueryMode::Ensure);
})*
}

Expand Down Expand Up @@ -442,10 +471,9 @@ macro_rules! define_queries {
$($(#[$attr])*
#[inline(always)]
#[must_use]
pub fn $name(self, key: query_helper_param_ty!($($K)*))
-> <queries::$name<$tcx> as QueryConfig>::Stored
pub fn $name(self, key: query_helper_param_ty!($($K)*)) -> query_stored::$name<$tcx>
{
self.at(DUMMY_SP).$name(key.into_query_param())
self.at(DUMMY_SP).$name(key)
})*

/// All self-profiling events generated by the query engine use
Expand All @@ -471,7 +499,7 @@ macro_rules! define_queries {
alloc_self_profile_query_strings_for_query_cache(
self,
stringify!($name),
&self.queries.$name,
&self.query_caches.$name,
&mut string_cache,
);
})*
Expand All @@ -481,10 +509,19 @@ macro_rules! define_queries {
impl TyCtxtAt<$tcx> {
$($(#[$attr])*
#[inline(always)]
pub fn $name(self, key: query_helper_param_ty!($($K)*))
-> <queries::$name<$tcx> as QueryConfig>::Stored
pub fn $name(self, key: query_helper_param_ty!($($K)*)) -> query_stored::$name<$tcx>
{
get_query::<queries::$name<'_>, _>(self.tcx, self.span, key.into_query_param())
let key = key.into_query_param();
let cached = try_get_cached(self.tcx, &self.tcx.query_caches.$name, &key, |value| {
value.clone()
});

let lookup = match cached {
Ok(value) => return value,
Err(lookup) => lookup,
};

get_query::<queries::$name<'_>, _>(self.tcx, self.span, key, lookup, QueryMode::Get).unwrap()
})*
}

Expand Down Expand Up @@ -518,8 +555,8 @@ macro_rules! define_queries_struct {

$($(#[$attr])* $name: QueryState<
crate::dep_graph::DepKind,
<TyCtxt<$tcx> as QueryContext>::Query,
<queries::$name<$tcx> as QueryAccessors<TyCtxt<'tcx>>>::Cache,
Query<$tcx>,
query_keys::$name<$tcx>,
>,)*
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_middle/src/ty/query/profiling_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::profiling::SelfProfiler;
use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::definitions::DefPathData;
use rustc_query_system::query::{QueryCache, QueryContext, QueryState};
use rustc_query_system::query::{QueryCache, QueryCacheStore};
use std::fmt::Debug;
use std::io::Write;

Expand Down Expand Up @@ -230,7 +230,7 @@ where
pub(super) fn alloc_self_profile_query_strings_for_query_cache<'tcx, C>(
tcx: TyCtxt<'tcx>,
query_name: &'static str,
query_state: &QueryState<crate::dep_graph::DepKind, <TyCtxt<'tcx> as QueryContext>::Query, C>,
query_cache: &QueryCacheStore<C>,
string_cache: &mut QueryKeyStringCache,
) where
C: QueryCache,
Expand All @@ -251,7 +251,7 @@ pub(super) fn alloc_self_profile_query_strings_for_query_cache<'tcx, C>(
// need to invoke queries itself, we cannot keep the query caches
// locked while doing so. Instead we copy out the
// `(query_key, dep_node_index)` pairs and release the lock again.
let query_keys_and_indices: Vec<_> = query_state
let query_keys_and_indices: Vec<_> = query_cache
.iter_results(|results| results.map(|(k, _, i)| (k.clone(), i)).collect());

// Now actually allocate the strings. If allocating the strings
Expand All @@ -276,7 +276,7 @@ pub(super) fn alloc_self_profile_query_strings_for_query_cache<'tcx, C>(
let query_name = profiler.get_or_alloc_cached_string(query_name);
let event_id = event_id_builder.from_label(query_name).to_string_id();

query_state.iter_results(|results| {
query_cache.iter_results(|results| {
let query_invocation_ids: Vec<_> = results.map(|v| v.2.into()).collect();

profiler.bulk_map_query_invocation_id_to_single_string(
Expand Down
11 changes: 3 additions & 8 deletions compiler/rustc_middle/src/ty/query/stats.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use crate::ty::query::queries;
use crate::ty::TyCtxt;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_query_system::query::{QueryAccessors, QueryCache, QueryContext, QueryState};
use rustc_query_system::query::{QueryAccessors, QueryCache, QueryCacheStore};

use std::any::type_name;
use std::hash::Hash;
use std::mem;
#[cfg(debug_assertions)]
use std::sync::atomic::Ordering;
Expand Down Expand Up @@ -37,10 +36,8 @@ struct QueryStats {
local_def_id_keys: Option<usize>,
}

fn stats<D, Q, C>(name: &'static str, map: &QueryState<D, Q, C>) -> QueryStats
fn stats<C>(name: &'static str, map: &QueryCacheStore<C>) -> QueryStats
where
D: Copy + Clone + Eq + Hash,
Q: Clone,
C: QueryCache,
{
let mut stats = QueryStats {
Expand Down Expand Up @@ -128,12 +125,10 @@ macro_rules! print_stats {

$(
queries.push(stats::<
crate::dep_graph::DepKind,
<TyCtxt<'_> as QueryContext>::Query,
<queries::$name<'_> as QueryAccessors<TyCtxt<'_>>>::Cache,
>(
stringify!($name),
&tcx.queries.$name,
&tcx.query_caches.$name,
));
)*

Expand Down
58 changes: 27 additions & 31 deletions compiler/rustc_query_system/src/query/caches.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::dep_graph::DepNodeIndex;
use crate::query::plumbing::{QueryLookup, QueryState};
use crate::query::plumbing::{QueryCacheStore, QueryLookup};

use rustc_arena::TypedArena;
use rustc_data_structures::fx::FxHashMap;
Expand Down Expand Up @@ -31,17 +31,15 @@ pub trait QueryCache: QueryStorage {
/// It returns the shard index and a lock guard to the shard,
/// which will be used if the query is not in the cache and we need
/// to compute it.
fn lookup<D, Q, R, OnHit, OnMiss>(
fn lookup<'s, R, OnHit>(
&self,
state: &QueryState<D, Q, Self>,
key: Self::Key,
state: &'s QueryCacheStore<Self>,
key: &Self::Key,
// `on_hit` can be called while holding a lock to the query state shard.
on_hit: OnHit,
on_miss: OnMiss,
) -> R
) -> Result<R, QueryLookup>
where
OnHit: FnOnce(&Self::Stored, DepNodeIndex) -> R,
OnMiss: FnOnce(Self::Key, QueryLookup<'_, D, Q, Self::Key, Self::Sharded>) -> R;
OnHit: FnOnce(&Self::Stored, DepNodeIndex) -> R;

fn complete(
&self,
Expand Down Expand Up @@ -95,23 +93,24 @@ where
type Sharded = FxHashMap<K, (V, DepNodeIndex)>;

#[inline(always)]
fn lookup<D, Q, R, OnHit, OnMiss>(
fn lookup<'s, R, OnHit>(
&self,
state: &QueryState<D, Q, Self>,
key: K,
state: &'s QueryCacheStore<Self>,
key: &K,
on_hit: OnHit,
on_miss: OnMiss,
) -> R
) -> Result<R, QueryLookup>
where
OnHit: FnOnce(&V, DepNodeIndex) -> R,
OnMiss: FnOnce(K, QueryLookup<'_, D, Q, K, Self::Sharded>) -> R,
{
let mut lookup = state.get_lookup(&key);
let lock = &mut *lookup.lock;
let (lookup, lock) = state.get_lookup(key);
let result = lock.raw_entry().from_key_hashed_nocheck(lookup.key_hash, key);

let result = lock.cache.raw_entry().from_key_hashed_nocheck(lookup.key_hash, &key);

if let Some((_, value)) = result { on_hit(&value.0, value.1) } else { on_miss(key, lookup) }
if let Some((_, value)) = result {
let hit_result = on_hit(&value.0, value.1);
Ok(hit_result)
} else {
Err(lookup)
}
}

#[inline]
Expand Down Expand Up @@ -177,26 +176,23 @@ where
type Sharded = FxHashMap<K, &'tcx (V, DepNodeIndex)>;

#[inline(always)]
fn lookup<D, Q, R, OnHit, OnMiss>(
fn lookup<'s, R, OnHit>(
&self,
state: &QueryState<D, Q, Self>,
key: K,
state: &'s QueryCacheStore<Self>,
key: &K,
on_hit: OnHit,
on_miss: OnMiss,
) -> R
) -> Result<R, QueryLookup>
where
OnHit: FnOnce(&&'tcx V, DepNodeIndex) -> R,
OnMiss: FnOnce(K, QueryLookup<'_, D, Q, K, Self::Sharded>) -> R,
{
let mut lookup = state.get_lookup(&key);
let lock = &mut *lookup.lock;

let result = lock.cache.raw_entry().from_key_hashed_nocheck(lookup.key_hash, &key);
let (lookup, lock) = state.get_lookup(key);
let result = lock.raw_entry().from_key_hashed_nocheck(lookup.key_hash, key);

if let Some((_, value)) = result {
on_hit(&&value.0, value.1)
let hit_result = on_hit(&&value.0, value.1);
Ok(hit_result)
} else {
on_miss(key, lookup)
Err(lookup)
}
}

Expand Down
Loading

0 comments on commit d1206f9

Please sign in to comment.