Skip to content

Commit a0a5c41

Browse files
committed
Rename collect_active_jobs to several distinct names
1 parent 5ac8ece commit a0a5c41

5 files changed

Lines changed: 52 additions & 21 deletions

File tree

compiler/rustc_interface/src/util.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ pub(crate) fn run_in_thread_pool_with_globals<
247247
let query_map = rustc_span::set_session_globals_then(unsafe { &*(session_globals as *const SessionGlobals) }, || {
248248
// Ensure there was no errors collecting all active jobs.
249249
// We need the complete map to ensure we find a cycle to break.
250-
QueryCtxt::new(tcx).collect_active_jobs(false).expect("failed to collect active queries in deadlock handler")
250+
QueryCtxt::new(tcx).collect_active_jobs_from_all_queries(false).expect("failed to collect active queries in deadlock handler")
251251
});
252252
break_query_cycles(query_map, &registry);
253253
})

compiler/rustc_query_impl/src/plumbing.rs

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ impl<'tcx> QueryCtxt<'tcx> {
5050
}
5151

5252
fn depth_limit_error(self, job: QueryJobId) {
53-
let query_map = self.collect_active_jobs(true).expect("failed to collect active queries");
53+
let query_map = self
54+
.collect_active_jobs_from_all_queries(true)
55+
.expect("failed to collect active queries");
5456
let (info, depth) = job.find_dep_kind_root(query_map);
5557

5658
let suggested_limit = match self.tcx.recursion_limit() {
@@ -98,7 +100,7 @@ impl<'tcx> QueryContext<'tcx> for QueryCtxt<'tcx> {
98100
tls::with_related_context(self.tcx, |icx| icx.query)
99101
}
100102

101-
/// Returns a map of currently active query jobs.
103+
/// Returns a map of currently active query jobs, collected from all queries.
102104
///
103105
/// If `require_complete` is `true`, this function locks all shards of the
104106
/// query results to produce a complete map, which always returns `Ok`.
@@ -108,12 +110,15 @@ impl<'tcx> QueryContext<'tcx> for QueryCtxt<'tcx> {
108110
/// Prefer passing `false` to `require_complete` to avoid potential deadlocks,
109111
/// especially when called from within a deadlock handler, unless a
110112
/// complete map is needed and no deadlock is possible at this call site.
111-
fn collect_active_jobs(self, require_complete: bool) -> Result<QueryMap<'tcx>, QueryMap<'tcx>> {
113+
fn collect_active_jobs_from_all_queries(
114+
self,
115+
require_complete: bool,
116+
) -> Result<QueryMap<'tcx>, QueryMap<'tcx>> {
112117
let mut jobs = QueryMap::default();
113118
let mut complete = true;
114119

115-
for collect in super::COLLECT_ACTIVE_JOBS.iter() {
116-
if collect(self.tcx, &mut jobs, require_complete).is_none() {
120+
for gather_fn in crate::PER_QUERY_GATHER_ACTIVE_JOBS_FNS.iter() {
121+
if gather_fn(self.tcx, &mut jobs, require_complete).is_none() {
117122
complete = false;
118123
}
119124
}
@@ -727,7 +732,10 @@ macro_rules! define_queries {
727732
}
728733
}
729734

730-
pub(crate) fn collect_active_jobs<'tcx>(
735+
/// Internal per-query plumbing for collecting the set of active jobs for this query.
736+
///
737+
/// Should only be called through `PER_QUERY_GATHER_ACTIVE_JOBS_FNS`.
738+
pub(crate) fn gather_active_jobs<'tcx>(
731739
tcx: TyCtxt<'tcx>,
732740
qmap: &mut QueryMap<'tcx>,
733741
require_complete: bool,
@@ -737,12 +745,15 @@ macro_rules! define_queries {
737745
let name = stringify!($name);
738746
$crate::plumbing::create_query_frame(tcx, rustc_middle::query::descs::$name, key, kind, name)
739747
};
740-
let res = tcx.query_system.states.$name.collect_active_jobs(
748+
749+
// Call `gather_active_jobs_inner` to do the actual work.
750+
let res = tcx.query_system.states.$name.gather_active_jobs_inner(
741751
tcx,
742752
make_frame,
743753
qmap,
744754
require_complete,
745755
);
756+
746757
// this can be called during unwinding, and the function has a `try_`-prefix, so
747758
// don't `unwrap()` here, just manually check for `None` and do best-effort error
748759
// reporting.
@@ -812,10 +823,17 @@ macro_rules! define_queries {
812823

813824
// These arrays are used for iteration and can't be indexed by `DepKind`.
814825

815-
const COLLECT_ACTIVE_JOBS: &[
816-
for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap<'tcx>, bool) -> Option<()>
817-
] =
818-
&[$(query_impl::$name::collect_active_jobs),*];
826+
/// Used by `collect_active_jobs_from_all_queries` to iterate over all
827+
/// queries, and gather the active jobs for each query.
828+
///
829+
/// (We arbitrarily use the word "gather" when collecting the jobs for
830+
/// each individual query, so that we have distinct function names to
831+
/// grep for.)
832+
const PER_QUERY_GATHER_ACTIVE_JOBS_FNS: &[
833+
for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap<'tcx>, require_complete: bool) -> Option<()>
834+
] = &[
835+
$(query_impl::$name::gather_active_jobs),*
836+
];
819837

820838
const ALLOC_SELF_PROFILE_QUERY_STRINGS: &[
821839
for<'tcx> fn(TyCtxt<'tcx>, &mut QueryKeyStringCache)

compiler/rustc_query_system/src/query/job.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ impl<'tcx> QueryInfo<QueryStackDeferred<'tcx>> {
3232
}
3333
}
3434

35+
/// Map from query job IDs to job information collected by
36+
/// [`QueryContext::collect_active_jobs_from_all_queries`].
3537
pub type QueryMap<'tcx> = FxHashMap<QueryJobId, QueryJobInfo<'tcx>>;
3638

3739
/// A value uniquely identifying an active query job.
@@ -613,7 +615,7 @@ pub fn print_query_stack<'tcx, Qcx: QueryContext<'tcx>>(
613615
let mut count_total = 0;
614616

615617
// Make use of a partial query map if we fail to take locks collecting active queries.
616-
let query_map = match qcx.collect_active_jobs(false) {
618+
let query_map = match qcx.collect_active_jobs_from_all_queries(false) {
617619
Ok(query_map) => query_map,
618620
Err(query_map) => query_map,
619621
};

compiler/rustc_query_system/src/query/mod.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,10 @@ pub trait QueryContext<'tcx>: HasDepContext {
166166
/// Get the query information from the TLS context.
167167
fn current_query_job(self) -> Option<QueryJobId>;
168168

169-
fn collect_active_jobs(self, require_complete: bool) -> Result<QueryMap<'tcx>, QueryMap<'tcx>>;
169+
fn collect_active_jobs_from_all_queries(
170+
self,
171+
require_complete: bool,
172+
) -> Result<QueryMap<'tcx>, QueryMap<'tcx>>;
170173

171174
/// Load a side effect associated to the node in the previous session.
172175
fn load_side_effect(

compiler/rustc_query_system/src/query/plumbing.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use rustc_data_structures::fingerprint::Fingerprint;
1111
use rustc_data_structures::hash_table::{self, Entry, HashTable};
1212
use rustc_data_structures::sharded::{self, Sharded};
1313
use rustc_data_structures::stack::ensure_sufficient_stack;
14-
use rustc_data_structures::sync::LockGuard;
1514
use rustc_data_structures::{outline, sync};
1615
use rustc_errors::{Diag, FatalError, StashKey};
1716
use rustc_span::{DUMMY_SP, Span};
@@ -79,7 +78,10 @@ where
7978
self.active.lock_shards().all(|shard| shard.is_empty())
8079
}
8180

82-
pub fn collect_active_jobs<Qcx: Copy>(
81+
/// Internal plumbing for collecting the set of active jobs for this query.
82+
///
83+
/// Should only be called from `gather_active_jobs`.
84+
pub fn gather_active_jobs_inner<Qcx: Copy>(
8385
&self,
8486
qcx: Qcx,
8587
make_frame: fn(Qcx, K) -> QueryStackFrame<QueryStackDeferred<'tcx>>,
@@ -88,23 +90,26 @@ where
8890
) -> Option<()> {
8991
let mut active = Vec::new();
9092

91-
let mut collect = |iter: LockGuard<'_, HashTable<(K, ActiveKeyStatus<'tcx>)>>| {
92-
for (k, v) in iter.iter() {
93+
// Helper to gather active jobs from a single shard.
94+
let mut gather_shard_jobs = |shard: &HashTable<(K, ActiveKeyStatus<'tcx>)>| {
95+
for (k, v) in shard.iter() {
9396
if let ActiveKeyStatus::Started(ref job) = *v {
9497
active.push((*k, job.clone()));
9598
}
9699
}
97100
};
98101

102+
// Lock shards and gather jobs from each shard.
99103
if require_complete {
100104
for shard in self.active.lock_shards() {
101-
collect(shard);
105+
gather_shard_jobs(&shard);
102106
}
103107
} else {
104108
// We use try_lock_shards here since we are called from the
105109
// deadlock handler, and this shouldn't be locked.
106110
for shard in self.active.try_lock_shards() {
107-
collect(shard?);
111+
let shard = shard?;
112+
gather_shard_jobs(&shard);
108113
}
109114
}
110115

@@ -294,7 +299,10 @@ where
294299
{
295300
// Ensure there was no errors collecting all active jobs.
296301
// We need the complete map to ensure we find a cycle to break.
297-
let query_map = qcx.collect_active_jobs(false).ok().expect("failed to collect active queries");
302+
let query_map = qcx
303+
.collect_active_jobs_from_all_queries(false)
304+
.ok()
305+
.expect("failed to collect active queries");
298306

299307
let error = try_execute.find_cycle_in_stack(query_map, &qcx.current_query_job(), span);
300308
(mk_cycle(query, qcx, error.lift()), None)

0 commit comments

Comments
 (0)