From ff7589498710bc7ac971d1a19cc128a08cb17b9c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 23 Mar 2026 15:20:09 +1100 Subject: [PATCH 1/4] Remove an unhelpful assertion from `force_from_dep_node_inner` This assertion and its comment are much visually heavier than the part of the function that actually performs important work. This assertion is also useless, because the `codegen_unit` query does not have a `force_from_dep_node_fn` in its DepKindVTable, so the assertion would never be reached even in the situation it is trying to guard against. This assertion is also redundant, because we have plenty of incremental tests that fail if the corresponding calls to `tcx.ensure_ok().codegen_unit(..)` are removed. --- compiler/rustc_query_impl/src/plumbing.rs | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index b8aa125aaf96..f4a80a62cc68 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -6,7 +6,7 @@ use rustc_middle::bug; #[expect(unused_imports, reason = "used by doc comments")] use rustc_middle::dep_graph::DepKindVTable; -use rustc_middle::dep_graph::{DepKind, DepNode, DepNodeIndex, DepNodeKey, SerializedDepNodeIndex}; +use rustc_middle::dep_graph::{DepNode, DepNodeIndex, DepNodeKey, SerializedDepNodeIndex}; use rustc_middle::query::erase::{Erasable, Erased}; use rustc_middle::query::on_disk_cache::{ AbsoluteBytePos, CacheDecoder, CacheEncoder, EncodedDepNodeIndex, @@ -230,24 +230,6 @@ pub(crate) fn force_from_dep_node_inner<'tcx, Q: GetQueryVTable<'tcx>>( ) -> bool { let query = Q::query_vtable(tcx); - // We must avoid ever having to call `force_from_dep_node()` for a - // `DepNode::codegen_unit`: - // Since we cannot reconstruct the query key of a `DepNode::codegen_unit`, we - // would always end up having to evaluate the first caller of the - // `codegen_unit` query that *is* reconstructible. This might very well be - // the `compile_codegen_unit` query, thus re-codegenning the whole CGU just - // to re-trigger calling the `codegen_unit` query with the right key. At - // that point we would already have re-done all the work we are trying to - // avoid doing in the first place. - // The solution is simple: Just explicitly call the `codegen_unit` query for - // each CGU, right after partitioning. This way `try_mark_green` will always - // hit the cache instead of having to go through `force_from_dep_node`. - // This assertion makes sure, we actually keep applying the solution above. - debug_assert!( - dep_node.kind != DepKind::codegen_unit, - "calling force_from_dep_node() on dep_kinds::codegen_unit" - ); - if let Some(key) = ::Key::try_recover_key(tcx, &dep_node) { force_query(query, tcx, key, dep_node); true From b29d6df04f6c7e7eaadaa9c858f6877aab999f57 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 23 Mar 2026 15:20:09 +1100 Subject: [PATCH 2/4] Remove a redundant query cache lookup while forcing In the serial compiler, we should never be trying to force a query node that has a cached value, because we would have already marked the node red or green when putting its value in cache. In the parallel compiler, we immediately re-check the cache while holding the state shard lock anyway, and trying to avoid that lock doesn't seem worthwhile. --- compiler/rustc_query_impl/src/execution.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index bbd55e9ead11..1d998cabc909 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -686,13 +686,6 @@ pub(crate) fn force_query<'tcx, C: QueryCache>( key: C::Key, dep_node: DepNode, ) { - // We may be concurrently trying both execute and force a query. - // Ensure that only one of them runs the query. - if let Some((_, index)) = query.cache.lookup(&key) { - tcx.prof.query_cache_hit(index.into()); - return; - } - ensure_sufficient_stack(|| { try_execute_query::(query, tcx, DUMMY_SP, key, Some(dep_node)) }); From 4b52951832d9921bca16713db0c66683a1d0e750 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 23 Mar 2026 15:20:09 +1100 Subject: [PATCH 3/4] Combine `force_from_dep_node_inner` and `force_query` The different parts of this function used to be split across different crates, but nowadays they are both in `rustc_query_impl`, so they can be combined. This commit also hoists the vtable lookup to a closure in `make_dep_kind_vtable_for_query`, to reduce the amount of code that deals with `GetQueryVTable`, and to make the inner function more consistent with other functions in `execution`. --- .../rustc_query_impl/src/dep_kind_vtables.rs | 10 ++++++--- compiler/rustc_query_impl/src/execution.rs | 21 +++++++++++++++---- compiler/rustc_query_impl/src/plumbing.rs | 19 +---------------- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_query_impl/src/dep_kind_vtables.rs b/compiler/rustc_query_impl/src/dep_kind_vtables.rs index 44b92dc727ab..4346e29f4754 100644 --- a/compiler/rustc_query_impl/src/dep_kind_vtables.rs +++ b/compiler/rustc_query_impl/src/dep_kind_vtables.rs @@ -4,7 +4,7 @@ use rustc_middle::query::QueryCache; use crate::GetQueryVTable; -use crate::plumbing::{force_from_dep_node_inner, promote_from_disk_inner}; +use crate::plumbing::promote_from_disk_inner; /// [`DepKindVTable`] constructors for special dep kinds that aren't queries. #[expect(non_snake_case, reason = "use non-snake case to avoid collision with query names")] @@ -111,8 +111,12 @@ pub(crate) fn make_dep_kind_vtable_for_query<'tcx, Q>( DepKindVTable { is_eval_always, key_fingerprint_style, - force_from_dep_node_fn: (can_recover && !is_no_force) - .then_some(force_from_dep_node_inner::), + force_from_dep_node_fn: (can_recover && !is_no_force).then_some( + |tcx, dep_node, _prev_index| { + let query = Q::query_vtable(tcx); + crate::execution::force_query_dep_node(tcx, query, dep_node) + }, + ), promote_from_disk_fn: (can_recover && is_cache_on_disk) .then_some(promote_from_disk_inner::), } diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 1d998cabc909..b503a914618e 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -680,13 +680,26 @@ pub(super) fn execute_query_incr_inner<'tcx, C: QueryCache>( Some(result) } -pub(crate) fn force_query<'tcx, C: QueryCache>( - query: &'tcx QueryVTable<'tcx, C>, +/// Inner implementation of [`DepKindVTable::force_from_dep_node_fn`][force_fn] +/// for query nodes. +/// +/// [force_fn]: rustc_middle::dep_graph::DepKindVTable::force_from_dep_node_fn +pub(crate) fn force_query_dep_node<'tcx, C: QueryCache>( tcx: TyCtxt<'tcx>, - key: C::Key, + query: &'tcx QueryVTable<'tcx, C>, dep_node: DepNode, -) { +) -> bool { + let Some(key) = C::Key::try_recover_key(tcx, &dep_node) else { + // We couldn't recover a key from the node's key fingerprint. + // Tell the caller that we couldn't force the node. + return false; + }; + ensure_sufficient_stack(|| { try_execute_query::(query, tcx, DUMMY_SP, key, Some(dep_node)) }); + + // We did manage to recover a key and force the node, though it's up to + // the caller to check whether the node ended up marked red or green. + true } diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index f4a80a62cc68..12dda5029429 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -20,7 +20,7 @@ use rustc_span::def_id::LOCAL_CRATE; use crate::error::{QueryOverflow, QueryOverflowNote}; -use crate::execution::{all_inactive, force_query}; +use crate::execution::all_inactive; use crate::job::find_dep_kind_root; use crate::query_impl::for_each_query_vtable; use crate::{CollectActiveJobsKind, GetQueryVTable, collect_active_query_jobs}; @@ -220,20 +220,3 @@ pub(crate) fn try_load_from_disk<'tcx, V>( value } - -/// Implementation of [`DepKindVTable::force_from_dep_node_fn`] for queries. -pub(crate) fn force_from_dep_node_inner<'tcx, Q: GetQueryVTable<'tcx>>( - tcx: TyCtxt<'tcx>, - dep_node: DepNode, - // Needed by the vtable function signature, but not used when forcing queries. - _prev_index: SerializedDepNodeIndex, -) -> bool { - let query = Q::query_vtable(tcx); - - if let Some(key) = ::Key::try_recover_key(tcx, &dep_node) { - force_query(query, tcx, key, dep_node); - true - } else { - false - } -} From 97b1c314892ef4497c0ce5656daa3a54c4e052d3 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 23 Mar 2026 17:50:39 +1100 Subject: [PATCH 4/4] Also restore the wrapper closure for `promote_from_disk_fn` This effectively reverses . At that time, I thought GetQueryVTable might be useful for other kinds of static lookup in the future, but after various other simplifications and cleanups that now seems less likely, and this style is more consistent with other vtable-related functions. --- compiler/rustc_query_impl/src/dep_kind_vtables.rs | 6 ++++-- compiler/rustc_query_impl/src/plumbing.rs | 10 +++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_query_impl/src/dep_kind_vtables.rs b/compiler/rustc_query_impl/src/dep_kind_vtables.rs index 4346e29f4754..b70fe3008cb1 100644 --- a/compiler/rustc_query_impl/src/dep_kind_vtables.rs +++ b/compiler/rustc_query_impl/src/dep_kind_vtables.rs @@ -117,8 +117,10 @@ pub(crate) fn make_dep_kind_vtable_for_query<'tcx, Q>( crate::execution::force_query_dep_node(tcx, query, dep_node) }, ), - promote_from_disk_fn: (can_recover && is_cache_on_disk) - .then_some(promote_from_disk_inner::), + promote_from_disk_fn: (can_recover && is_cache_on_disk).then_some(|tcx, dep_node| { + let query = Q::query_vtable(tcx); + promote_from_disk_inner(tcx, query, dep_node) + }), } } diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 12dda5029429..0ea272a2c192 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -23,7 +23,7 @@ use crate::execution::all_inactive; use crate::job::find_dep_kind_root; use crate::query_impl::for_each_query_vtable; -use crate::{CollectActiveJobsKind, GetQueryVTable, collect_active_query_jobs}; +use crate::{CollectActiveJobsKind, collect_active_query_jobs}; fn depth_limit_error<'tcx>(tcx: TyCtxt<'tcx>, job: QueryJobId) { let job_map = collect_active_query_jobs(tcx, CollectActiveJobsKind::Full); @@ -151,15 +151,15 @@ fn verify_query_key_hashes_inner<'tcx, C: QueryCache>( }); } -/// Implementation of [`DepKindVTable::promote_from_disk_fn`] for queries. -pub(crate) fn promote_from_disk_inner<'tcx, Q: GetQueryVTable<'tcx>>( +/// Inner implementation of [`DepKindVTable::promote_from_disk_fn`] for queries. +pub(crate) fn promote_from_disk_inner<'tcx, C: QueryCache>( tcx: TyCtxt<'tcx>, + query: &'tcx QueryVTable<'tcx, C>, dep_node: DepNode, ) { - let query = Q::query_vtable(tcx); debug_assert!(tcx.dep_graph.is_green(&dep_node)); - let key = ::Key::try_recover_key(tcx, &dep_node).unwrap_or_else(|| { + let key = C::Key::try_recover_key(tcx, &dep_node).unwrap_or_else(|| { panic!( "Failed to recover key for {dep_node:?} with key fingerprint {}", dep_node.key_fingerprint