Skip to content

Commit 5b82484

Browse files
committed
Auto merge of #153685 - Zalathar:for-each-query, r=<try>
Introduce `for_each_query_vtable!` to move more code out of query macros
2 parents d1c7945 + 330259a commit 5b82484

5 files changed

Lines changed: 148 additions & 108 deletions

File tree

compiler/rustc_query_impl/src/execution.rs

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ use rustc_middle::ty::TyCtxt;
1616
use rustc_middle::verify_ich::incremental_verify_ich;
1717
use rustc_span::{DUMMY_SP, Span};
1818

19-
use crate::collect_active_jobs_from_all_queries;
2019
use crate::dep_graph::{DepNode, DepNodeIndex};
20+
use crate::for_each_query_vtable;
2121
use crate::job::{QueryJobInfo, QueryJobMap, find_cycle_in_stack, report_cycle};
2222
use crate::plumbing::{current_query_job, next_job_id, start_query};
2323

@@ -30,20 +30,49 @@ pub(crate) fn all_inactive<'tcx, K>(state: &QueryState<'tcx, K>) -> bool {
3030
state.active.lock_shards().all(|shard| shard.is_empty())
3131
}
3232

33+
/// Returns a map of currently active query jobs, collected from all queries.
34+
///
35+
/// If `require_complete` is `true`, this function locks all shards of the
36+
/// query results to produce a complete map, which always returns `Ok`.
37+
/// Otherwise, it may return an incomplete map as an error if any shard
38+
/// lock cannot be acquired.
39+
///
40+
/// Prefer passing `false` to `require_complete` to avoid potential deadlocks,
41+
/// especially when called from within a deadlock handler, unless a
42+
/// complete map is needed and no deadlock is possible at this call site.
43+
pub fn collect_active_jobs_from_all_queries<'tcx>(
44+
tcx: TyCtxt<'tcx>,
45+
require_complete: bool,
46+
) -> Result<QueryJobMap<'tcx>, QueryJobMap<'tcx>> {
47+
let mut job_map_out = QueryJobMap::default();
48+
let mut complete = true;
49+
50+
for_each_query_vtable!(
51+
gather_active_jobs,
52+
tcx,
53+
ALL_QUERIES,
54+
require_complete,
55+
&mut job_map_out,
56+
&mut complete,
57+
);
58+
59+
if complete { Ok(job_map_out) } else { Err(job_map_out) }
60+
}
61+
3362
/// Internal plumbing for collecting the set of active jobs for this query.
3463
///
3564
/// Should only be called from `collect_active_jobs_from_all_queries`.
3665
///
3766
/// (We arbitrarily use the word "gather" when collecting the jobs for
3867
/// each individual query, so that we have distinct function names to
3968
/// grep for.)
40-
pub(crate) fn gather_active_jobs<'tcx, C>(
41-
query: &'tcx QueryVTable<'tcx, C>,
69+
fn gather_active_jobs<'tcx, C>(
4270
tcx: TyCtxt<'tcx>,
71+
query: &'tcx QueryVTable<'tcx, C>,
4372
require_complete: bool,
4473
job_map_out: &mut QueryJobMap<'tcx>, // Out-param; job info is gathered into this map
45-
) -> Option<()>
46-
where
74+
complete: &mut bool, // Out-param; set to false if `try_lock` fails for any shard
75+
) where
4776
C: QueryCache<Key: QueryKey + DynSend + DynSync>,
4877
QueryVTable<'tcx, C>: DynSync,
4978
{
@@ -76,7 +105,8 @@ where
76105
"Failed to collect active jobs for query with name `{}`!",
77106
query.name
78107
);
79-
return None;
108+
*complete = false;
109+
return;
80110
}
81111
Some(shard) => gather_shard_jobs(&shard),
82112
}
@@ -89,8 +119,6 @@ where
89119
let frame = crate::plumbing::create_deferred_query_stack_frame(tcx, query, key);
90120
job_map_out.insert(job.id, QueryJobInfo { frame, job });
91121
}
92-
93-
Some(())
94122
}
95123

96124
#[cold]

compiler/rustc_query_impl/src/job.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use rustc_middle::ty::TyCtxt;
1414
use rustc_session::Session;
1515
use rustc_span::{DUMMY_SP, Span};
1616

17-
use crate::collect_active_jobs_from_all_queries;
17+
use crate::execution::collect_active_jobs_from_all_queries;
1818

1919
/// Map from query job IDs to job information collected by
2020
/// `collect_active_jobs_from_all_queries`.

compiler/rustc_query_impl/src/lib.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// tidy-alphabetical-start
44
#![allow(internal_features)]
55
#![feature(core_intrinsics)]
6+
#![feature(macro_metavar_expr)]
67
#![feature(min_specialization)]
78
#![feature(rustc_attrs)]
89
#![feature(try_blocks)]
@@ -11,16 +12,16 @@
1112
use rustc_data_structures::sync::AtomicU64;
1213
use rustc_middle::dep_graph;
1314
use rustc_middle::queries::{self, ExternProviders, Providers};
14-
use rustc_middle::query::on_disk_cache::{CacheEncoder, EncodedDepNodeIndex, OnDiskCache};
15+
use rustc_middle::query::on_disk_cache::OnDiskCache;
1516
use rustc_middle::query::plumbing::{QuerySystem, QueryVTable};
1617
use rustc_middle::query::{AsLocalQueryKey, QueryCache, QueryMode};
1718
use rustc_middle::ty::TyCtxt;
1819
use rustc_span::Span;
1920

2021
pub use crate::dep_kind_vtables::make_dep_kind_vtables;
22+
pub use crate::execution::collect_active_jobs_from_all_queries;
2123
use crate::from_cycle_error::FromCycleError;
2224
pub use crate::job::{QueryJobMap, break_query_cycles, print_query_stack};
23-
use crate::profiling_support::QueryKeyStringCache;
2425

2526
#[macro_use]
2627
mod plumbing;
@@ -65,7 +66,8 @@ pub fn query_system<'tcx>(
6566
rustc_middle::rustc_with_all_queries! { define_queries! }
6667

6768
pub fn provide(providers: &mut rustc_middle::util::Providers) {
68-
providers.hooks.alloc_self_profile_query_strings = alloc_self_profile_query_strings;
69-
providers.hooks.query_key_hash_verify_all = query_key_hash_verify_all;
70-
providers.hooks.encode_all_query_results = encode_all_query_results;
69+
providers.hooks.alloc_self_profile_query_strings =
70+
profiling_support::alloc_self_profile_query_strings;
71+
providers.hooks.query_key_hash_verify_all = plumbing::query_key_hash_verify_all;
72+
providers.hooks.encode_all_query_results = plumbing::encode_all_query_results;
7173
}

compiler/rustc_query_impl/src/plumbing.rs

Lines changed: 69 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use rustc_span::def_id::LOCAL_CRATE;
3333
use crate::error::{QueryOverflow, QueryOverflowNote};
3434
use crate::execution::{all_inactive, force_query};
3535
use crate::job::find_dep_kind_root;
36-
use crate::{GetQueryVTable, collect_active_jobs_from_all_queries};
36+
use crate::{GetQueryVTable, collect_active_jobs_from_all_queries, for_each_query_vtable};
3737

3838
fn depth_limit_error<'tcx>(tcx: TyCtxt<'tcx>, job: QueryJobId) {
3939
let job_map =
@@ -146,7 +146,21 @@ where
146146
QueryStackFrame::new(info, kind, def_id, def_id_for_ty_in_cycle)
147147
}
148148

149-
pub(crate) fn encode_query_results<'a, 'tcx, C, V>(
149+
pub(crate) fn encode_all_query_results<'tcx>(
150+
tcx: TyCtxt<'tcx>,
151+
encoder: &mut CacheEncoder<'_, 'tcx>,
152+
query_result_index: &mut EncodedDepNodeIndex,
153+
) {
154+
for_each_query_vtable!(
155+
encode_query_results,
156+
tcx,
157+
CACHE_ON_DISK_QUERIES,
158+
encoder,
159+
query_result_index,
160+
);
161+
}
162+
163+
fn encode_query_results<'a, 'tcx, C, V>(
150164
tcx: TyCtxt<'tcx>,
151165
query: &'tcx QueryVTable<'tcx, C>,
152166
encoder: &mut CacheEncoder<'a, 'tcx>,
@@ -172,9 +186,17 @@ pub(crate) fn encode_query_results<'a, 'tcx, C, V>(
172186
});
173187
}
174188

175-
pub(crate) fn query_key_hash_verify<'tcx, C: QueryCache>(
176-
query: &'tcx QueryVTable<'tcx, C>,
189+
pub(crate) fn query_key_hash_verify_all<'tcx>(tcx: TyCtxt<'tcx>) {
190+
if tcx.sess.opts.unstable_opts.incremental_verify_ich || cfg!(debug_assertions) {
191+
tcx.sess.time("query_key_hash_verify_all", || {
192+
for_each_query_vtable!(query_key_hash_verify, tcx, ALL_QUERIES)
193+
});
194+
}
195+
}
196+
197+
fn query_key_hash_verify<'tcx, C: QueryCache>(
177198
tcx: TyCtxt<'tcx>,
199+
query: &'tcx QueryVTable<'tcx, C>,
178200
) {
179201
let _timer = tcx.prof.generic_activity_with_arg("query_key_hash_verify_for", query.name);
180202

@@ -512,95 +534,55 @@ macro_rules! define_queries {
512534
}
513535
}
514536

515-
/// Returns a map of currently active query jobs, collected from all queries.
537+
/// Given a function name, a `tcx`, and a filter condition
538+
/// (e.g. `ALL_QUERIES` or `CACHE_ON_DISK_QUERIES`),
539+
/// this macro calls that function with each query vtable that satisfies
540+
/// the filter condition.
516541
///
517-
/// If `require_complete` is `true`, this function locks all shards of the
518-
/// query results to produce a complete map, which always returns `Ok`.
519-
/// Otherwise, it may return an incomplete map as an error if any shard
520-
/// lock cannot be acquired.
542+
/// The arguments passed to each function call are:
543+
/// - `tcx`
544+
/// - A query vtable that satisfies the filter condition
545+
/// - All other arguments given after the filter condition
521546
///
522-
/// Prefer passing `false` to `require_complete` to avoid potential deadlocks,
523-
/// especially when called from within a deadlock handler, unless a
524-
/// complete map is needed and no deadlock is possible at this call site.
525-
pub fn collect_active_jobs_from_all_queries<'tcx>(
526-
tcx: TyCtxt<'tcx>,
527-
require_complete: bool,
528-
) -> Result<QueryJobMap<'tcx>, QueryJobMap<'tcx>> {
529-
let mut job_map_out = QueryJobMap::default();
530-
let mut complete = true;
531-
532-
$(
533-
let res = crate::execution::gather_active_jobs(
534-
&tcx.query_system.query_vtables.$name,
535-
tcx,
536-
require_complete,
537-
&mut job_map_out,
538-
);
539-
if res.is_none() {
540-
complete = false;
541-
}
542-
)*
543-
544-
if complete { Ok(job_map_out) } else { Err(job_map_out) }
545-
}
546-
547-
/// All self-profiling events generated by the query engine use
548-
/// virtual `StringId`s for their `event_id`. This method makes all
549-
/// those virtual `StringId`s point to actual strings.
547+
/// This needs to be a macro, because the vtables can have different
548+
/// key/value/cache types for different queries.
550549
///
551-
/// If we are recording only summary data, the ids will point to
552-
/// just the query names. If we are recording query keys too, we
553-
/// allocate the corresponding strings here.
554-
pub fn alloc_self_profile_query_strings(tcx: TyCtxt<'_>) {
555-
if !tcx.prof.enabled() {
556-
return;
557-
}
558-
559-
let _prof_timer = tcx.sess.prof.generic_activity("self_profile_alloc_query_strings");
560-
561-
let mut string_cache = QueryKeyStringCache::new();
562-
563-
$(
564-
$crate::profiling_support::alloc_self_profile_query_strings_for_query_cache(
565-
tcx,
566-
stringify!($name),
567-
&tcx.query_system.query_vtables.$name.cache,
568-
&mut string_cache,
569-
);
570-
)*
571-
572-
tcx.sess.prof.store_query_cache_hits();
573-
}
574-
575-
fn encode_all_query_results<'tcx>(
576-
tcx: TyCtxt<'tcx>,
577-
encoder: &mut CacheEncoder<'_, 'tcx>,
578-
query_result_index: &mut EncodedDepNodeIndex,
579-
) {
580-
$(
581-
#[cfg($cache_on_disk)]
582-
{
583-
$crate::plumbing::encode_query_results(
550+
/// This macro's argument syntax is specifically intended to look like
551+
/// plain Rust code, so that `for_each_query_vtable!(..)` calls will be
552+
/// formatted by rustfmt.
553+
///
554+
/// To avoid too much nested-macro complication, filter conditions are
555+
/// implemented by hand as needed.
556+
macro_rules! for_each_query_vtable {
557+
// Call with all queries.
558+
($inner_fn:expr, $tcx:expr, ALL_QUERIES $$(,$args:expr)* $$(,)?) => {{
559+
let tcx: rustc_middle::ty::TyCtxt<'_> = $tcx;
560+
$(
561+
$inner_fn(
584562
tcx,
585563
&tcx.query_system.query_vtables.$name,
586-
encoder,
587-
query_result_index,
588-
)
589-
}
590-
)*
564+
$$($args),*
565+
);
566+
)*
567+
}};
568+
569+
// Only call with queries that can potentially cache to disk.
570+
//
571+
// This allows the use of trait bounds that only need to be satisfied
572+
// by the subset of queries that actually cache to disk.
573+
($inner_fn:expr, $tcx:expr, CACHE_ON_DISK_QUERIES $$(,$args:expr)* $$(,)?) => {{
574+
let tcx: rustc_middle::ty::TyCtxt<'_> = $tcx;
575+
$(
576+
#[cfg($cache_on_disk)]
577+
$inner_fn(
578+
tcx,
579+
&tcx.query_system.query_vtables.$name,
580+
$$($args),*
581+
);
582+
)*
583+
}};
591584
}
592585

593-
pub fn query_key_hash_verify_all<'tcx>(tcx: TyCtxt<'tcx>) {
594-
if tcx.sess.opts.unstable_opts.incremental_verify_ich || cfg!(debug_assertions) {
595-
tcx.sess.time("query_key_hash_verify_all", || {
596-
$(
597-
$crate::plumbing::query_key_hash_verify(
598-
&tcx.query_system.query_vtables.$name,
599-
tcx
600-
);
601-
)*
602-
})
603-
}
604-
}
586+
pub(crate) use for_each_query_vtable;
605587
}
606588
}

0 commit comments

Comments
 (0)