Skip to content

Commit 947b696

Browse files
borsbors
committed
Auto merge of rust-lang#107833 - Zoxc:arena-query-clean, r=cjgillot
Factor query arena allocation out from query caches This moves the logic for arena allocation out from the query caches into conditional code in the query system. The specialized arena caches are removed. A new `QuerySystem` type is added in `rustc_middle` which contains the arenas, providers and query caches. Performance seems to be slightly regressed: <table><tr><td rowspan="2">Benchmark</td><td colspan="1"><b>Before</b></th><td colspan="2"><b>After</b></th></tr><tr><td align="right">Time</td><td align="right">Time</td><td align="right">%</th></tr><tr><td>🟣 <b>clap</b>:check</td><td align="right">1.8053s</td><td align="right">1.8109s</td><td align="right"> 0.31%</td></tr><tr><td>🟣 <b>hyper</b>:check</td><td align="right">0.2600s</td><td align="right">0.2597s</td><td align="right"> -0.10%</td></tr><tr><td>🟣 <b>regex</b>:check</td><td align="right">0.9973s</td><td align="right">1.0006s</td><td align="right"> 0.34%</td></tr><tr><td>🟣 <b>syn</b>:check</td><td align="right">1.6048s</td><td align="right">1.6051s</td><td align="right"> 0.02%</td></tr><tr><td>🟣 <b>syntex_syntax</b>:check</td><td align="right">6.2992s</td><td align="right">6.3159s</td><td align="right"> 0.26%</td></tr><tr><td>Total</td><td align="right">10.9664s</td><td align="right">10.9922s</td><td align="right"> 0.23%</td></tr><tr><td>Summary</td><td align="right">1.0000s</td><td align="right">1.0017s</td><td align="right"> 0.17%</td></tr></table> Incremental performance is a bit worse: <table><tr><td rowspan="2">Benchmark</td><td colspan="1"><b>Before</b></th><td colspan="2"><b>After</b></th></tr><tr><td align="right">Time</td><td align="right">Time</td><td align="right">%</th></tr><tr><td>🟣 <b>clap</b>:check:initial</td><td align="right">2.2103s</td><td align="right">2.2247s</td><td align="right"> 0.65%</td></tr><tr><td>🟣 <b>hyper</b>:check:initial</td><td align="right">0.3335s</td><td align="right">0.3349s</td><td align="right"> 0.41%</td></tr><tr><td>🟣 <b>regex</b>:check:initial</td><td align="right">1.2597s</td><td align="right">1.2650s</td><td align="right"> 0.42%</td></tr><tr><td>🟣 <b>syn</b>:check:initial</td><td align="right">2.0521s</td><td align="right">2.0613s</td><td align="right"> 0.45%</td></tr><tr><td>🟣 <b>syntex_syntax</b>:check:initial</td><td align="right">7.8275s</td><td align="right">7.8583s</td><td align="right"> 0.39%</td></tr><tr><td>Total</td><td align="right">13.6832s</td><td align="right">13.7442s</td><td align="right"> 0.45%</td></tr><tr><td>Summary</td><td align="right">1.0000s</td><td align="right">1.0046s</td><td align="right"> 0.46%</td></tr></table> It does seem like LLVM optimizers struggle a bit with the current state of the query system. Based on top of rust-lang#107782 and rust-lang#107802. r? `@cjgillot`
2 parents 9a7cc6c + caf29b2 commit 947b696

File tree

8 files changed

+147
-239
lines changed

8 files changed

+147
-239
lines changed

‎compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ macro_rules! provide_one {
114114
fn $name<'tcx>(
115115
$tcx: TyCtxt<'tcx>,
116116
def_id_arg: ty::query::query_keys::$name<'tcx>,
117-
) -> ty::query::query_values::$name<'tcx> {
117+
) -> ty::query::query_provided::$name<'tcx> {
118118
let _prof_timer =
119119
$tcx.prof.generic_activity(concat!("metadata_decode_entry_", stringify!($name)));
120120

‎compiler/rustc_middle/src/ty/context.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ pub struct GlobalCtxt<'tcx> {
520520
pub on_disk_cache: Option<&'tcx dyn OnDiskCache<'tcx>>,
521521

522522
pub queries: &'tcx dyn query::QueryEngine<'tcx>,
523-
pub query_caches: query::QueryCaches<'tcx>,
523+
pub query_system: query::QuerySystem<'tcx>,
524524
pub(crate) query_kinds: &'tcx [DepKindStruct<'tcx>],
525525

526526
// Internal caches for metadata decoding. No need to track deps on this.
@@ -705,7 +705,7 @@ impl<'tcx> TyCtxt<'tcx> {
705705
untracked,
706706
on_disk_cache,
707707
queries,
708-
query_caches: query::QueryCaches::default(),
708+
query_system: Default::default(),
709709
query_kinds,
710710
ty_rcache: Default::default(),
711711
pred_rcache: Default::default(),

‎compiler/rustc_middle/src/ty/query.rs‎

Lines changed: 80 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#![allow(unused_parens)]
2+
13
use crate::dep_graph;
24
use crate::infer::canonical::{self, Canonical};
35
use crate::lint::LintExpectation;
@@ -34,13 +36,15 @@ use crate::ty::subst::{GenericArg, SubstsRef};
3436
use crate::ty::util::AlwaysRequiresDrop;
3537
use crate::ty::GeneratorDiagnosticData;
3638
use crate::ty::{self, CrateInherentImpls, ParamEnvAnd, Ty, TyCtxt, UnusedGenericParams};
39+
use rustc_arena::TypedArena;
3740
use rustc_ast as ast;
3841
use rustc_ast::expand::allocator::AllocatorKind;
3942
use rustc_attr as attr;
4043
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
4144
use rustc_data_structures::steal::Steal;
4245
use rustc_data_structures::svh::Svh;
4346
use rustc_data_structures::sync::Lrc;
47+
use rustc_data_structures::sync::WorkerLocal;
4448
use rustc_data_structures::unord::UnordSet;
4549
use rustc_errors::ErrorGuaranteed;
4650
use rustc_hir as hir;
@@ -59,13 +63,20 @@ use rustc_span::symbol::Symbol;
5963
use rustc_span::{Span, DUMMY_SP};
6064
use rustc_target::abi;
6165
use rustc_target::spec::PanicStrategy;
66+
use std::mem;
6267
use std::ops::Deref;
6368
use std::path::PathBuf;
6469
use std::sync::Arc;
6570

6671
pub(crate) use rustc_query_system::query::QueryJobId;
6772
use rustc_query_system::query::*;
6873

74+
#[derive(Default)]
75+
pub struct QuerySystem<'tcx> {
76+
pub arenas: QueryArenas<'tcx>,
77+
pub caches: QueryCaches<'tcx>,
78+
}
79+
6980
#[derive(Copy, Clone)]
7081
pub struct TyCtxtAt<'tcx> {
7182
pub tcx: TyCtxt<'tcx>,
@@ -112,10 +123,10 @@ macro_rules! query_helper_param_ty {
112123
}
113124

114125
macro_rules! query_if_arena {
115-
([] $arena:ty, $no_arena:ty) => {
126+
([] $arena:tt $no_arena:tt) => {
116127
$no_arena
117128
};
118-
([(arena_cache) $($rest:tt)*] $arena:ty, $no_arena:ty) => {
129+
([(arena_cache) $($rest:tt)*] $arena:tt $no_arena:tt) => {
119130
$arena
120131
};
121132
([$other:tt $($modifiers:tt)*]$($args:tt)*) => {
@@ -131,7 +142,7 @@ macro_rules! separate_provide_extern_decl {
131142
for<'tcx> fn(
132143
TyCtxt<'tcx>,
133144
query_keys::$name<'tcx>,
134-
) -> query_values::$name<'tcx>
145+
) -> query_provided::$name<'tcx>
135146
};
136147
([$other:tt $($modifiers:tt)*][$($args:tt)*]) => {
137148
separate_provide_extern_decl!([$($modifiers)*][$($args)*])
@@ -183,30 +194,77 @@ macro_rules! define_callbacks {
183194

184195
$(pub type $name<'tcx> = $($K)*;)*
185196
}
186-
#[allow(nonstandard_style, unused_lifetimes, unused_parens)]
197+
#[allow(nonstandard_style, unused_lifetimes)]
187198
pub mod query_values {
188199
use super::*;
189200

190-
$(pub type $name<'tcx> = query_if_arena!([$($modifiers)*] <$V as Deref>::Target, $V);)*
201+
$(pub type $name<'tcx> = $V;)*
191202
}
192-
#[allow(nonstandard_style, unused_lifetimes, unused_parens)]
193-
pub mod query_storage {
203+
204+
/// This module specifies the type returned from query providers and the type used for
205+
/// decoding. For regular queries this is the declared returned type `V`, but
206+
/// `arena_cache` will use `<V as Deref>::Target` instead.
207+
#[allow(nonstandard_style, unused_lifetimes)]
208+
pub mod query_provided {
194209
use super::*;
195210

196211
$(
197-
pub type $name<'tcx> = query_if_arena!([$($modifiers)*]
198-
<<$($K)* as Key>::CacheSelector
199-
as CacheSelector<'tcx, <$V as Deref>::Target>>::ArenaCache,
200-
<<$($K)* as Key>::CacheSelector as CacheSelector<'tcx, $V>>::Cache
201-
);
212+
pub type $name<'tcx> = query_if_arena!([$($modifiers)*] (<$V as Deref>::Target) ($V));
202213
)*
203214
}
204215

216+
/// This module has a function per query which takes a `query_provided` value and coverts
217+
/// it to a regular `V` value by allocating it on an arena if the query has the
218+
/// `arena_cache` modifier. This will happen when computing the query using a provider or
219+
/// decoding a stored result.
205220
#[allow(nonstandard_style, unused_lifetimes)]
206-
pub mod query_stored {
221+
pub mod query_provided_to_value {
207222
use super::*;
208223

209-
$(pub type $name<'tcx> = $V;)*
224+
$(
225+
#[inline(always)]
226+
pub fn $name<'tcx>(
227+
_tcx: TyCtxt<'tcx>,
228+
value: query_provided::$name<'tcx>,
229+
) -> query_values::$name<'tcx> {
230+
query_if_arena!([$($modifiers)*]
231+
{
232+
if mem::needs_drop::<query_provided::$name<'tcx>>() {
233+
&*_tcx.query_system.arenas.$name.alloc(value)
234+
} else {
235+
&*_tcx.arena.dropless.alloc(value)
236+
}
237+
}
238+
(value)
239+
)
240+
}
241+
)*
242+
}
243+
#[allow(nonstandard_style, unused_lifetimes)]
244+
pub mod query_storage {
245+
use super::*;
246+
247+
$(
248+
pub type $name<'tcx> = <<$($K)* as Key>::CacheSelector as CacheSelector<'tcx, $V>>::Cache;
249+
)*
250+
}
251+
252+
pub struct QueryArenas<'tcx> {
253+
$($(#[$attr])* pub $name: query_if_arena!([$($modifiers)*]
254+
(WorkerLocal<TypedArena<<$V as Deref>::Target>>)
255+
()
256+
),)*
257+
}
258+
259+
impl Default for QueryArenas<'_> {
260+
fn default() -> Self {
261+
Self {
262+
$($name: query_if_arena!([$($modifiers)*]
263+
(WorkerLocal::new(|_| Default::default()))
264+
()
265+
),)*
266+
}
267+
}
210268
}
211269

212270
#[derive(Default)]
@@ -221,7 +279,7 @@ macro_rules! define_callbacks {
221279
let key = key.into_query_param();
222280
opt_remap_env_constness!([$($modifiers)*][key]);
223281

224-
match try_get_cached(self.tcx, &self.tcx.query_caches.$name, &key) {
282+
match try_get_cached(self.tcx, &self.tcx.query_system.caches.$name, &key) {
225283
Some(_) => return,
226284
None => self.tcx.queries.$name(self.tcx, DUMMY_SP, key, QueryMode::Ensure),
227285
};
@@ -246,7 +304,7 @@ macro_rules! define_callbacks {
246304
let key = key.into_query_param();
247305
opt_remap_env_constness!([$($modifiers)*][key]);
248306

249-
match try_get_cached(self.tcx, &self.tcx.query_caches.$name, &key) {
307+
match try_get_cached(self.tcx, &self.tcx.query_system.caches.$name, &key) {
250308
Some(value) => value,
251309
None => self.tcx.queries.$name(self.tcx, self.span, key, QueryMode::Get).unwrap(),
252310
}
@@ -257,7 +315,7 @@ macro_rules! define_callbacks {
257315
$(pub $name: for<'tcx> fn(
258316
TyCtxt<'tcx>,
259317
query_keys::$name<'tcx>,
260-
) -> query_values::$name<'tcx>,)*
318+
) -> query_provided::$name<'tcx>,)*
261319
}
262320

263321
pub struct ExternProviders {
@@ -334,12 +392,13 @@ macro_rules! define_feedable {
334392
$(impl<'tcx, K: IntoQueryParam<$($K)*> + Copy> TyCtxtFeed<'tcx, K> {
335393
$(#[$attr])*
336394
#[inline(always)]
337-
pub fn $name(self, value: query_values::$name<'tcx>) -> $V {
395+
pub fn $name(self, value: query_provided::$name<'tcx>) -> $V {
338396
let key = self.key().into_query_param();
339397
opt_remap_env_constness!([$($modifiers)*][key]);
340398

341399
let tcx = self.tcx;
342-
let cache = &tcx.query_caches.$name;
400+
let value = query_provided_to_value::$name(tcx, value);
401+
let cache = &tcx.query_system.caches.$name;
343402

344403
match try_get_cached(tcx, cache, &key) {
345404
Some(old) => {
@@ -357,7 +416,8 @@ macro_rules! define_feedable {
357416
&value,
358417
hash_result!([$($modifiers)*]),
359418
);
360-
cache.complete(key, value, dep_node_index)
419+
cache.complete(key, value, dep_node_index);
420+
value
361421
}
362422
}
363423
}

‎compiler/rustc_query_impl/src/lib.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ use rustc_data_structures::sync::AtomicU64;
2121
use rustc_middle::arena::Arena;
2222
use rustc_middle::dep_graph::{self, DepKindStruct};
2323
use rustc_middle::query::Key;
24-
use rustc_middle::ty::query::{query_keys, query_storage, query_stored, query_values};
24+
use rustc_middle::ty::query::{
25+
query_keys, query_provided, query_provided_to_value, query_storage, query_values,
26+
};
2527
use rustc_middle::ty::query::{ExternProviders, Providers, QueryEngine};
2628
use rustc_middle::ty::TyCtxt;
2729
use rustc_span::Span;

‎compiler/rustc_query_impl/src/plumbing.rs‎

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -293,14 +293,14 @@ macro_rules! get_provider {
293293
}
294294

295295
macro_rules! should_ever_cache_on_disk {
296-
([]) => {{
297-
None
296+
([]$yes:tt $no:tt) => {{
297+
$no
298298
}};
299-
([(cache) $($rest:tt)*]) => {{
300-
Some($crate::plumbing::try_load_from_disk::<Self::Value>)
299+
([(cache) $($rest:tt)*]$yes:tt $no:tt) => {{
300+
$yes
301301
}};
302-
([$other:tt $($modifiers:tt)*]) => {
303-
should_ever_cache_on_disk!([$($modifiers)*])
302+
([$other:tt $($modifiers:tt)*]$yes:tt $no:tt) => {
303+
should_ever_cache_on_disk!([$($modifiers)*]$yes $no)
304304
};
305305
}
306306

@@ -472,7 +472,6 @@ macro_rules! define_queries {
472472
$(impl<'tcx> QueryConfig<QueryCtxt<'tcx>> for queries::$name<'tcx> {
473473
type Key = query_keys::$name<'tcx>;
474474
type Value = query_values::$name<'tcx>;
475-
type Stored = query_stored::$name<'tcx>;
476475
const NAME: &'static str = stringify!($name);
477476

478477
#[inline]
@@ -493,24 +492,39 @@ macro_rules! define_queries {
493492
fn query_cache<'a>(tcx: QueryCtxt<'tcx>) -> &'a Self::Cache
494493
where 'tcx:'a
495494
{
496-
&tcx.query_caches.$name
495+
&tcx.query_system.caches.$name
497496
}
498497

499-
fn execute_query(tcx: TyCtxt<'tcx>, key: Self::Key) -> Self::Stored {
498+
fn execute_query(tcx: TyCtxt<'tcx>, key: Self::Key) -> Self::Value {
500499
tcx.$name(key)
501500
}
502501

503502
#[inline]
504-
// key is only sometimes used
505503
#[allow(unused_variables)]
506-
fn compute(qcx: QueryCtxt<'tcx>, key: &Self::Key) -> fn(TyCtxt<'tcx>, Self::Key) -> Self::Value {
507-
get_provider!([$($modifiers)*][qcx, $name, key])
504+
fn compute(qcx: QueryCtxt<'tcx>, key: Self::Key) -> Self::Value {
505+
query_provided_to_value::$name(
506+
qcx.tcx,
507+
get_provider!([$($modifiers)*][qcx, $name, key])(qcx.tcx, key)
508+
)
508509
}
509510

510511
#[inline]
511-
fn try_load_from_disk(qcx: QueryCtxt<'tcx>, key: &Self::Key) -> rustc_query_system::query::TryLoadFromDisk<QueryCtxt<'tcx>, Self> {
512-
let cache_on_disk = Self::cache_on_disk(qcx.tcx, key);
513-
if cache_on_disk { should_ever_cache_on_disk!([$($modifiers)*]) } else { None }
512+
fn try_load_from_disk(_qcx: QueryCtxt<'tcx>, _key: &Self::Key) -> rustc_query_system::query::TryLoadFromDisk<QueryCtxt<'tcx>, Self> {
513+
should_ever_cache_on_disk!([$($modifiers)*] {
514+
if Self::cache_on_disk(_qcx.tcx, _key) {
515+
Some(|qcx: QueryCtxt<'tcx>, dep_node| {
516+
let value = $crate::plumbing::try_load_from_disk::<query_provided::$name<'tcx>>(
517+
qcx,
518+
dep_node
519+
);
520+
value.map(|value| query_provided_to_value::$name(qcx.tcx, value))
521+
})
522+
} else {
523+
None
524+
}
525+
} {
526+
None
527+
})
514528
}
515529

516530
const ANON: bool = is_anon!([$($modifiers)*]);
@@ -633,7 +647,7 @@ macro_rules! define_queries {
633647
$crate::profiling_support::alloc_self_profile_query_strings_for_query_cache(
634648
tcx,
635649
stringify!($name),
636-
&tcx.query_caches.$name,
650+
&tcx.query_system.caches.$name,
637651
string_cache,
638652
)
639653
},
@@ -725,7 +739,7 @@ macro_rules! define_queries_struct {
725739
span: Span,
726740
key: <queries::$name<'tcx> as QueryConfig<QueryCtxt<'tcx>>>::Key,
727741
mode: QueryMode,
728-
) -> Option<query_stored::$name<'tcx>> {
742+
) -> Option<query_values::$name<'tcx>> {
729743
let qcx = QueryCtxt { tcx, queries: self };
730744
get_query::<queries::$name<'tcx>, _, rustc_middle::dep_graph::DepKind>(qcx, span, key, mode)
731745
})*

0 commit comments

Comments
 (0)