Skip to content

Commit 6efacd9

Browse files
committed
Auto merge of #152835 - nnethercote:fix-query-state-query-cache, r=<try>
Improve how `QueryCache`s and `QueryState`s are stored
2 parents e0cb264 + 156f355 commit 6efacd9

File tree

6 files changed

+79
-109
lines changed

6 files changed

+79
-109
lines changed

compiler/rustc_middle/src/query/inner.rs

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ where
3232
/// Shared implementation of `tcx.$query(..)` and `tcx.at(span).$query(..)`
3333
/// for all queries.
3434
#[inline(always)]
35-
pub(crate) fn query_get_at<'tcx, Cache>(
35+
pub(crate) fn query_get_at<'tcx, C>(
3636
tcx: TyCtxt<'tcx>,
37-
execute_query: fn(TyCtxt<'tcx>, Span, Cache::Key, QueryMode) -> Option<Cache::Value>,
38-
query_cache: &Cache,
37+
execute_query: fn(TyCtxt<'tcx>, Span, C::Key, QueryMode) -> Option<C::Value>,
38+
query_cache: &C,
3939
span: Span,
40-
key: Cache::Key,
41-
) -> Cache::Value
40+
key: C::Key,
41+
) -> C::Value
4242
where
43-
Cache: QueryCache,
43+
C: QueryCache,
4444
{
4545
match try_get_cached(tcx, query_cache, &key) {
4646
Some(value) => value,
@@ -51,14 +51,14 @@ where
5151
/// Shared implementation of `tcx.ensure_ok().$query(..)` for most queries,
5252
/// and `tcx.ensure_done().$query(..)` for all queries.
5353
#[inline]
54-
pub(crate) fn query_ensure<'tcx, Cache>(
54+
pub(crate) fn query_ensure<'tcx, C>(
5555
tcx: TyCtxt<'tcx>,
56-
execute_query: fn(TyCtxt<'tcx>, Span, Cache::Key, QueryMode) -> Option<Cache::Value>,
57-
query_cache: &Cache,
58-
key: Cache::Key,
56+
execute_query: fn(TyCtxt<'tcx>, Span, C::Key, QueryMode) -> Option<C::Value>,
57+
query_cache: &C,
58+
key: C::Key,
5959
check_cache: bool,
6060
) where
61-
Cache: QueryCache,
61+
C: QueryCache,
6262
{
6363
if try_get_cached(tcx, query_cache, &key).is_none() {
6464
execute_query(tcx, DUMMY_SP, key, QueryMode::Ensure { check_cache });
@@ -68,15 +68,15 @@ pub(crate) fn query_ensure<'tcx, Cache>(
6868
/// Shared implementation of `tcx.ensure_ok().$query(..)` for queries that
6969
/// have the `return_result_from_ensure_ok` modifier.
7070
#[inline]
71-
pub(crate) fn query_ensure_error_guaranteed<'tcx, Cache, T>(
71+
pub(crate) fn query_ensure_error_guaranteed<'tcx, C, T>(
7272
tcx: TyCtxt<'tcx>,
73-
execute_query: fn(TyCtxt<'tcx>, Span, Cache::Key, QueryMode) -> Option<Cache::Value>,
74-
query_cache: &Cache,
75-
key: Cache::Key,
73+
execute_query: fn(TyCtxt<'tcx>, Span, C::Key, QueryMode) -> Option<C::Value>,
74+
query_cache: &C,
75+
key: C::Key,
7676
check_cache: bool,
7777
) -> Result<(), ErrorGuaranteed>
7878
where
79-
Cache: QueryCache<Value = Erased<Result<T, ErrorGuaranteed>>>,
79+
C: QueryCache<Value = Erased<Result<T, ErrorGuaranteed>>>,
8080
Result<T, ErrorGuaranteed>: Erasable,
8181
{
8282
if let Some(res) = try_get_cached(tcx, query_cache, &key) {
@@ -96,21 +96,20 @@ where
9696
}
9797

9898
/// Common implementation of query feeding, used by `define_feedable!`.
99-
pub(crate) fn query_feed<'tcx, Cache>(
99+
pub(crate) fn query_feed<'tcx, C>(
100100
tcx: TyCtxt<'tcx>,
101101
dep_kind: DepKind,
102-
query_vtable: &QueryVTable<'tcx, Cache>,
103-
cache: &Cache,
104-
key: Cache::Key,
105-
value: Cache::Value,
102+
query_vtable: &QueryVTable<'tcx, C>,
103+
key: C::Key,
104+
value: C::Value,
106105
) where
107-
Cache: QueryCache,
108-
Cache::Key: DepNodeKey<'tcx>,
106+
C: QueryCache,
107+
C::Key: DepNodeKey<'tcx>,
109108
{
110109
let format_value = query_vtable.format_value;
111110

112111
// Check whether the in-memory cache already has a value for this key.
113-
match try_get_cached(tcx, cache, &key) {
112+
match try_get_cached(tcx, &query_vtable.query_cache, &key) {
114113
Some(old) => {
115114
// The query already has a cached value for this key.
116115
// That's OK if both values are the same, i.e. they have the same hash,
@@ -153,7 +152,7 @@ pub(crate) fn query_feed<'tcx, Cache>(
153152
query_vtable.hash_result,
154153
query_vtable.format_value,
155154
);
156-
cache.complete(key, value, dep_node_index);
155+
query_vtable.query_cache.complete(key, value, dep_node_index);
157156
}
158157
}
159158
}

compiler/rustc_middle/src/query/plumbing.rs

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ pub use sealed::IntoQueryParam;
1414
use crate::dep_graph;
1515
use crate::dep_graph::{DepKind, DepNodeIndex, SerializedDepNodeIndex};
1616
use crate::ich::StableHashingContext;
17-
use crate::queries::{
18-
ExternProviders, PerQueryVTables, Providers, QueryArenas, QueryCaches, QueryEngine, QueryStates,
19-
};
17+
use crate::queries::{ExternProviders, PerQueryVTables, Providers, QueryArenas, QueryEngine};
2018
use crate::query::on_disk_cache::{CacheEncoder, EncodedDepNodeIndex, OnDiskCache};
2119
use crate::query::stack::{QueryStackDeferred, QueryStackFrame, QueryStackFrameExtra};
2220
use crate::query::{QueryCache, QueryInfo, QueryJob};
@@ -108,10 +106,8 @@ pub struct QueryVTable<'tcx, C: QueryCache> {
108106
pub dep_kind: DepKind,
109107
/// How this query deals with query cycle errors.
110108
pub cycle_error_handling: CycleErrorHandling,
111-
// Offset of this query's state field in the QueryStates struct
112-
pub query_state: usize,
113-
// Offset of this query's cache field in the QueryCaches struct
114-
pub query_cache: usize,
109+
pub query_state: QueryState<'tcx, C::Key>,
110+
pub query_cache: C,
115111
pub will_cache_on_disk_for_key_fn: Option<WillCacheOnDiskForKeyFn<'tcx, C::Key>>,
116112

117113
/// Function pointer that calls `tcx.$query(key)` for this query and
@@ -155,9 +151,7 @@ pub struct QuerySystemFns {
155151
}
156152

157153
pub struct QuerySystem<'tcx> {
158-
pub states: QueryStates<'tcx>,
159154
pub arenas: WorkerLocal<QueryArenas<'tcx>>,
160-
pub caches: QueryCaches<'tcx>,
161155
pub query_vtables: PerQueryVTables<'tcx>,
162156

163157
/// This provides access to the incremental compilation on-disk cache for query results.
@@ -445,11 +439,6 @@ macro_rules! define_callbacks {
445439
)*
446440
}
447441

448-
#[derive(Default)]
449-
pub struct QueryCaches<'tcx> {
450-
$($(#[$attr])* pub $name: $name::Storage<'tcx>,)*
451-
}
452-
453442
impl<'tcx> $crate::query::TyCtxtEnsureOk<'tcx> {
454443
$($(#[$attr])*
455444
#[inline(always)]
@@ -461,7 +450,7 @@ macro_rules! define_callbacks {
461450
[$($modifiers)*]
462451
self.tcx,
463452
self.tcx.query_system.fns.engine.$name,
464-
&self.tcx.query_system.caches.$name,
453+
&self.tcx.query_system.query_vtables.$name.query_cache,
465454
$crate::query::IntoQueryParam::into_query_param(key),
466455
false,
467456
)
@@ -475,7 +464,7 @@ macro_rules! define_callbacks {
475464
crate::query::inner::query_ensure(
476465
self.tcx,
477466
self.tcx.query_system.fns.engine.$name,
478-
&self.tcx.query_system.caches.$name,
467+
&self.tcx.query_system.query_vtables.$name.query_cache,
479468
$crate::query::IntoQueryParam::into_query_param(key),
480469
true,
481470
);
@@ -502,7 +491,7 @@ macro_rules! define_callbacks {
502491
erase::restore_val::<$V>(inner::query_get_at(
503492
self.tcx,
504493
self.tcx.query_system.fns.engine.$name,
505-
&self.tcx.query_system.caches.$name,
494+
&self.tcx.query_system.query_vtables.$name.query_cache,
506495
self.span,
507496
$crate::query::IntoQueryParam::into_query_param(key),
508497
))
@@ -518,13 +507,6 @@ macro_rules! define_callbacks {
518507
)*
519508
}
520509

521-
#[derive(Default)]
522-
pub struct QueryStates<'tcx> {
523-
$(
524-
pub $name: $crate::query::QueryState<'tcx, $($K)*>,
525-
)*
526-
}
527-
528510
pub struct Providers {
529511
$(
530512
/// This is the provider for the query. Use `Find references` on this to
@@ -594,7 +576,6 @@ macro_rules! define_feedable {
594576
tcx,
595577
dep_kind,
596578
&tcx.query_system.query_vtables.$name,
597-
&tcx.query_system.caches.$name,
598579
key,
599580
erased_value,
600581
);

compiler/rustc_query_impl/src/dep_kind_vtables.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,18 +110,18 @@ mod non_query {
110110

111111
/// Shared implementation of the [`DepKindVTable`] constructor for queries.
112112
/// Called from macro-generated code for each query.
113-
pub(crate) fn make_dep_kind_vtable_for_query<'tcx, Q, Cache, const FLAGS: QueryFlags>(
113+
pub(crate) fn make_dep_kind_vtable_for_query<'tcx, Q, C, const FLAGS: QueryFlags>(
114114
is_eval_always: bool,
115115
) -> DepKindVTable<'tcx>
116116
where
117-
Q: QueryDispatcherUnerased<'tcx, Cache, FLAGS>,
118-
Cache: QueryCache + 'tcx,
117+
Q: QueryDispatcherUnerased<'tcx, C, FLAGS>,
118+
C: QueryCache + 'tcx,
119119
{
120120
let is_anon = FLAGS.is_anon;
121121
let key_fingerprint_style = if is_anon {
122122
KeyFingerprintStyle::Opaque
123123
} else {
124-
<Cache::Key as DepNodeKey<'tcx>>::key_fingerprint_style()
124+
<C::Key as DepNodeKey<'tcx>>::key_fingerprint_style()
125125
};
126126

127127
if is_anon || !key_fingerprint_style.reconstructible() {

compiler/rustc_query_impl/src/execution.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -244,12 +244,12 @@ fn wait_for_query<'tcx, C: QueryCache, const FLAGS: QueryFlags>(
244244

245245
match result {
246246
Ok(()) => {
247-
let Some((v, index)) = query.query_cache(tcx).lookup(&key) else {
247+
let Some((v, index)) = query.query_cache().lookup(&key) else {
248248
outline(|| {
249249
// We didn't find the query result in the query cache. Check if it was
250250
// poisoned due to a panic instead.
251251
let key_hash = sharded::make_hash(&key);
252-
let shard = query.query_state(tcx).active.lock_shard_by_hash(key_hash);
252+
let shard = query.query_state().active.lock_shard_by_hash(key_hash);
253253
match shard.find(key_hash, equivalent_key(&key)) {
254254
// The query we waited on panicked. Continue unwinding here.
255255
Some((_, ActiveKeyStatus::Poisoned)) => FatalError.raise(),
@@ -278,9 +278,8 @@ fn try_execute_query<'tcx, C: QueryCache, const FLAGS: QueryFlags, const INCR: b
278278
key: C::Key,
279279
dep_node: Option<DepNode>,
280280
) -> (C::Value, Option<DepNodeIndex>) {
281-
let state = query.query_state(tcx);
282281
let key_hash = sharded::make_hash(&key);
283-
let mut state_lock = state.active.lock_shard_by_hash(key_hash);
282+
let mut state_lock = query.query_state().active.lock_shard_by_hash(key_hash);
284283

285284
// For the parallel compiler we need to check both the query cache and query state structures
286285
// while holding the state lock to ensure that 1) the query has not yet completed and 2) the
@@ -289,7 +288,7 @@ fn try_execute_query<'tcx, C: QueryCache, const FLAGS: QueryFlags, const INCR: b
289288
// executing, but another thread may have already completed the query and stores it result
290289
// in the query cache.
291290
if tcx.sess.threads() > 1 {
292-
if let Some((value, index)) = query.query_cache(tcx).lookup(&key) {
291+
if let Some((value, index)) = query.query_cache().lookup(&key) {
293292
tcx.prof.query_cache_hit(index.into());
294293
return (value, Some(index));
295294
}
@@ -308,7 +307,7 @@ fn try_execute_query<'tcx, C: QueryCache, const FLAGS: QueryFlags, const INCR: b
308307
// Drop the lock before we start executing the query
309308
drop(state_lock);
310309

311-
execute_job::<C, FLAGS, INCR>(query, tcx, state, key, key_hash, id, dep_node)
310+
execute_job::<C, FLAGS, INCR>(query, tcx, key, key_hash, id, dep_node)
312311
}
313312
Entry::Occupied(mut entry) => {
314313
match &mut entry.get_mut().1 {
@@ -340,15 +339,14 @@ fn try_execute_query<'tcx, C: QueryCache, const FLAGS: QueryFlags, const INCR: b
340339
fn execute_job<'tcx, C: QueryCache, const FLAGS: QueryFlags, const INCR: bool>(
341340
query: SemiDynamicQueryDispatcher<'tcx, C, FLAGS>,
342341
tcx: TyCtxt<'tcx>,
343-
state: &'tcx QueryState<'tcx, C::Key>,
344342
key: C::Key,
345343
key_hash: u64,
346344
id: QueryJobId,
347345
dep_node: Option<DepNode>,
348346
) -> (C::Value, Option<DepNodeIndex>) {
349347
// Set up a guard object that will automatically poison the query if a
350348
// panic occurs while executing the query (or any intermediate plumbing).
351-
let job_guard = ActiveJobGuard { state, key, key_hash };
349+
let job_guard = ActiveJobGuard { state: query.query_state(), key, key_hash };
352350

353351
debug_assert_eq!(tcx.dep_graph.is_fully_enabled(), INCR);
354352

@@ -359,7 +357,7 @@ fn execute_job<'tcx, C: QueryCache, const FLAGS: QueryFlags, const INCR: bool>(
359357
execute_job_non_incr(query, tcx, key, id)
360358
};
361359

362-
let cache = query.query_cache(tcx);
360+
let cache = query.query_cache();
363361
if query.feedable() {
364362
// We should not compute queries that also got a value via feeding.
365363
// This can't happen, as query feeding adds the very dependencies to the fed query
@@ -680,7 +678,7 @@ pub(crate) fn force_query<'tcx, C: QueryCache, const FLAGS: QueryFlags>(
680678
) {
681679
// We may be concurrently trying both execute and force a query.
682680
// Ensure that only one of them runs the query.
683-
if let Some((_, index)) = query.query_cache(tcx).lookup(&key) {
681+
if let Some((_, index)) = query.query_cache().lookup(&key) {
684682
tcx.prof.query_cache_hit(index.into());
685683
return;
686684
}

compiler/rustc_query_impl/src/lib.rs

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ use std::marker::ConstParamTy;
1414

1515
use rustc_data_structures::sync::AtomicU64;
1616
use rustc_middle::dep_graph::{self, DepKind, DepNode, DepNodeIndex, SerializedDepNodeIndex};
17-
use rustc_middle::queries::{
18-
self, ExternProviders, Providers, QueryCaches, QueryEngine, QueryStates,
19-
};
17+
use rustc_middle::queries::{self, ExternProviders, Providers, QueryEngine};
2018
use rustc_middle::query::on_disk_cache::{CacheEncoder, EncodedDepNodeIndex, OnDiskCache};
2119
use rustc_middle::query::plumbing::{
2220
HashResult, QueryState, QuerySystem, QuerySystemFns, QueryVTable,
@@ -104,26 +102,14 @@ impl<'tcx, C: QueryCache, const FLAGS: QueryFlags> SemiDynamicQueryDispatcher<'t
104102

105103
// Don't use this method to access query results, instead use the methods on TyCtxt.
106104
#[inline(always)]
107-
fn query_state(self, tcx: TyCtxt<'tcx>) -> &'tcx QueryState<'tcx, C::Key> {
108-
// Safety:
109-
// This is just manually doing the subfield referencing through pointer math.
110-
unsafe {
111-
&*(&tcx.query_system.states as *const QueryStates<'tcx>)
112-
.byte_add(self.vtable.query_state)
113-
.cast::<QueryState<'tcx, C::Key>>()
114-
}
105+
fn query_state(self) -> &'tcx QueryState<'tcx, C::Key> {
106+
&self.vtable.query_state
115107
}
116108

117109
// Don't use this method to access query results, instead use the methods on TyCtxt.
118110
#[inline(always)]
119-
fn query_cache(self, tcx: TyCtxt<'tcx>) -> &'tcx C {
120-
// Safety:
121-
// This is just manually doing the subfield referencing through pointer math.
122-
unsafe {
123-
&*(&tcx.query_system.caches as *const QueryCaches<'tcx>)
124-
.byte_add(self.vtable.query_cache)
125-
.cast::<C>()
126-
}
111+
fn query_cache(self) -> &'tcx C {
112+
&self.vtable.query_cache
127113
}
128114

129115
/// Calls `tcx.$query(key)` for this query, and discards the returned value.
@@ -245,9 +231,7 @@ pub fn query_system<'tcx>(
245231
incremental: bool,
246232
) -> QuerySystem<'tcx> {
247233
QuerySystem {
248-
states: Default::default(),
249234
arenas: Default::default(),
250-
caches: Default::default(),
251235
query_vtables: make_query_vtables(),
252236
on_disk_cache,
253237
fns: QuerySystemFns {

0 commit comments

Comments
 (0)