From bdd6c96408096a86c33a2ea007a738d0cf0c1cab Mon Sep 17 00:00:00 2001 From: xmakro Date: Mon, 10 Aug 2026 09:04:36 -0700 Subject: [PATCH] Verify disk-cached values with non-local keys in per-kind batches --- .../rustc_middle/src/dep_graph/dep_node.rs | 9 ++++ compiler/rustc_middle/src/dep_graph/graph.rs | 14 +++++++ compiler/rustc_middle/src/dep_graph/mod.rs | 4 +- compiler/rustc_query_impl/src/execution.rs | 42 ++++++++++++++----- compiler/rustc_query_impl/src/plumbing.rs | 2 +- 5 files changed, 59 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index 6abec9a4ff465..d540185b60747 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -296,6 +296,15 @@ macro_rules! define_dep_nodes { _ => Err(()), } } + + /// The label of a `DepKind`, e.g. `"type_of"`. Unlike the `DepKind` + /// discriminant, the label is stable across compiler builds. + pub fn dep_kind_label(kind: DepKind) -> &'static str { + match kind { + $( self::DepKind::$nq_name => stringify!($nq_name), )* + $( self::DepKind::$q_name => stringify!($q_name), )* + } + } }; } diff --git a/compiler/rustc_middle/src/dep_graph/graph.rs b/compiler/rustc_middle/src/dep_graph/graph.rs index cba7c14533484..a1f7d49de5563 100644 --- a/compiler/rustc_middle/src/dep_graph/graph.rs +++ b/compiler/rustc_middle/src/dep_graph/graph.rs @@ -160,6 +160,10 @@ pub struct DepGraphData { /// Pool of read recorders, amortized across tasks. Global rather than per worker so the /// retained memory is bounded by the total number of concurrently recording tasks. read_recorder_pool: Lock>, + + /// Lazily cached `StableCrateId` of the local crate, used when selecting + /// loaded cache values for verification. + local_stable_crate_id: std::sync::OnceLock, } pub fn hash_result(hcx: &mut StableHashState<'_>, result: &R) -> Fingerprint @@ -218,6 +222,7 @@ impl DepGraph { debug_loaded_from_disk: Default::default(), green_edge_buf: WorkerLocal::default(), read_recorder_pool: Lock::new(Vec::new()), + local_stable_crate_id: std::sync::OnceLock::new(), })), virtual_dep_node_index: Arc::new(AtomicU32::new(0)), } @@ -714,6 +719,15 @@ impl DepGraphData { self.previous.session_count() } + /// The `StableCrateId` of the local crate as a `u64`, cached to avoid a + /// query lookup per loaded cache value. + #[inline] + pub fn local_stable_crate_id(&self, tcx: TyCtxt<'_>) -> u64 { + *self + .local_stable_crate_id + .get_or_init(|| tcx.stable_crate_id(rustc_span::def_id::LOCAL_CRATE).as_u64()) + } + #[inline] pub(crate) fn prev_node_of(&self, prev_index: SerializedDepNodeIndex) -> &DepNode { self.previous.index_to_node(prev_index) diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs index 3389c3ec91a5a..be5ed545fa083 100644 --- a/compiler/rustc_middle/src/dep_graph/mod.rs +++ b/compiler/rustc_middle/src/dep_graph/mod.rs @@ -2,7 +2,9 @@ use std::panic; use tracing::instrument; -pub use self::dep_node::{DepKind, DepKindVTable, DepNode, WorkProductId, dep_kind_from_label}; +pub use self::dep_node::{ + DepKind, DepKindVTable, DepNode, WorkProductId, dep_kind_from_label, dep_kind_label, +}; pub use self::dep_node_key::DepNodeKey; pub use self::graph::{ DepGraph, DepGraphData, DepNodeIndex, QuerySideEffect, TaskDepsRef, WorkProduct, diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index c053f961df925..a7623c4688195 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -1,13 +1,16 @@ -use std::hash::Hash; +use std::hash::{Hash, Hasher}; use std::mem::ManuallyDrop; -use rustc_data_structures::fingerprint::{Fingerprint, PackedFingerprint}; +use rustc_data_structures::fingerprint::Fingerprint; +use rustc_data_structures::fx::FxHasher; use rustc_data_structures::hash_table::{Entry, HashTable}; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_data_structures::sync::{DynSend, DynSync}; use rustc_data_structures::{defer, outline, sharded, sync}; use rustc_errors::FatalError; -use rustc_middle::dep_graph::{DepGraphData, DepNodeKey, SerializedDepNodeIndex}; +use rustc_middle::dep_graph::{ + DepGraphData, DepNodeKey, SerializedDepNodeIndex, dep_kind_label, +}; use rustc_middle::query::{ ActiveKeyStatus, Cycle, QueryCache, QueryJob, QueryJobId, QueryKey, QueryLatch, QueryMode, QueryState, QueryVTable, @@ -495,17 +498,36 @@ fn execute_job_incr<'tcx, C: QueryCache>( /// cache every 32 sessions, and is deterministic so that a verification /// failure reproduces on retry. /// +/// Values keyed by a local `DefPathHash` are sampled by their key fingerprint. /// `to_smaller_hash` mixes both fingerprint halves because neither half is -/// evenly distributed on its own (`DefPathHash` keys share the -/// `StableCrateId`, `HirId` keys contain a sequential id). +/// evenly distributed on its own. +/// +/// All other key fingerprints incorporate `StableCrateId`s of other crates +/// (foreign-def keys directly, opaque keys through the types they contain), +/// which embed the rustc version those crates were built with. Sampling by +/// such fingerprints selects different subsets for compilers built from +/// different sources even on identical input, which makes A/B benchmark +/// comparisons noisy. Instead, these values rotate in per-kind batches +/// slotted by the kind's label, which is stable across compiler builds. +/// Unit-keyed values (a single value per kind) and `HirId`-keyed values also +/// take this path, which keeps the classification down to a single +/// comparison. pub(crate) fn should_verify_loaded_value( tcx: TyCtxt<'_>, dep_graph_data: &DepGraphData, - key_fingerprint: PackedFingerprint, + dep_node: &DepNode, ) -> bool { - let hash = Fingerprint::from(key_fingerprint).to_smaller_hash().as_u64(); - hash % 32 == dep_graph_data.session_count() % 32 - || tcx.sess.opts.unstable_opts.incremental_verify_ich + if tcx.sess.opts.unstable_opts.incremental_verify_ich { + return true; + } + let key_fingerprint = Fingerprint::from(dep_node.key_fingerprint); + if key_fingerprint.split().0.as_u64() == dep_graph_data.local_stable_crate_id(tcx) { + let hash = key_fingerprint.to_smaller_hash().as_u64(); + return hash % 32 == dep_graph_data.session_count() % 32; + } + let mut hasher = FxHasher::default(); + hasher.write(dep_kind_label(dep_node.kind).as_bytes()); + hasher.finish() % 32 == dep_graph_data.session_count() % 32 } /// Given that the dep node for this query+key is green, obtain a value for it @@ -541,7 +563,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>( dep_graph_data.mark_debug_loaded_from_disk(*dep_node) } - let verify = should_verify_loaded_value(tcx, dep_graph_data, dep_node.key_fingerprint); + let verify = should_verify_loaded_value(tcx, dep_graph_data, dep_node); (value, verify) } diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 83badcb269af6..0dd977ac0dbe4 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -179,7 +179,7 @@ pub(crate) fn promote_from_disk_inner<'tcx, C: QueryCache>( // Verify the fingerprints of the same subset of loaded values as // `load_from_disk_or_invoke_provider_green` does. - if should_verify_loaded_value(tcx, dep_graph_data, dep_node.key_fingerprint) { + if should_verify_loaded_value(tcx, dep_graph_data, &dep_node) { incremental_verify_ich( tcx, dep_graph_data,