Rollup merge of #154283 - Zoxc:rem-nodes_in_current_session, r=nnethercote

Remove `nodes_in_current_session` field and related assertions

This removes the `nodes_in_current_session` field and related assertions. These are enabled if `-Z incremental-verify-ich` is passed or `debug_assertions` is on. Historically these have been useful to catch query keys with improper `HashStable` impls which lead to collisions.

We currently also check for duplicate nodes when loading the dep graph. This check is more complete as it covers the entire dep graph and is enabled by default. It doesn't provide a query key for a collision however. This check is also delayed to the next incremental session.

We also have the `verify_query_key_hashes` which is also enabled if `-Z incremental-verify-ich` is passed or `debug_assertions` is on. This checks for dep node conflicts in each query cache and provides 2 conflicting keys if present.

I think these remaining checks are sufficient and so we can remove `nodes_in_current_session`.
This commit is contained in:
Jonathan Brouwer
2026-04-22 19:18:28 +02:00
committed by GitHub
+6 -76
View File
@@ -5,12 +5,11 @@
use std::sync::atomic::{AtomicU32, Ordering};
use rustc_data_structures::fingerprint::{Fingerprint, PackedFingerprint};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::outline;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::profiling::QueryInvocationId;
use rustc_data_structures::sharded::{self, ShardedHashMap};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::sync::{AtomicU64, Lock, is_dyn_thread_safe};
use rustc_data_structures::sync::{AtomicU64, Lock};
use rustc_data_structures::unord::UnordMap;
use rustc_errors::DiagInner;
use rustc_index::IndexVec;
@@ -635,11 +634,6 @@ fn assert_dep_node_not_yet_allocated_in_current_session<S: std::fmt::Display>(
if !ok {
panic!("{}", msg())
}
} else if let Some(nodes_in_current_session) = &self.current.nodes_in_current_session {
outline(|| {
let seen = nodes_in_current_session.lock().contains_key(dep_node);
assert!(!seen, "{}", msg());
});
}
}
@@ -775,7 +769,8 @@ fn alloc_and_color_node(
is_green,
);
self.current.record_node(dep_node_index, key, value_fingerprint);
#[cfg(debug_assertions)]
self.current.record_edge(dep_node_index, key, value_fingerprint);
dep_node_index
} else {
@@ -787,8 +782,6 @@ fn promote_node_and_deps_to_current(
&self,
prev_index: SerializedDepNodeIndex,
) -> Option<DepNodeIndex> {
self.current.debug_assert_not_in_new_nodes(&self.previous, prev_index);
let dep_node_index = self.current.encoder.send_promoted(prev_index, &self.colors);
#[cfg(debug_assertions)]
@@ -1113,13 +1106,6 @@ pub(super) struct CurrentDepGraph {
#[cfg(debug_assertions)]
forbidden_edge: Option<EdgeFilter>,
/// Used to verify the absence of hash collisions among DepNodes.
/// This field is only `Some` if the `-Z incremental_verify_ich` option is present
/// or if `debug_assertions` are enabled.
///
/// The map contains all DepNodes that have been allocated in the current session so far.
nodes_in_current_session: Option<Lock<FxHashMap<DepNode, DepNodeIndex>>>,
/// Anonymous `DepNode`s are nodes whose IDs we compute from the list of
/// their edges. This has the beneficial side-effect that multiple anonymous
/// nodes can be coalesced into one without changing the semantics of the
@@ -1160,9 +1146,6 @@ fn new(
let new_node_count_estimate = 102 * prev_graph_node_count / 100 + 200;
let new_node_dbg =
session.opts.unstable_opts.incremental_verify_ich || cfg!(debug_assertions);
CurrentDepGraph {
encoder: GraphEncoder::new(session, encoder, prev_graph_node_count, previous),
anon_node_to_index: ShardedHashMap::with_capacity(
@@ -1174,12 +1157,6 @@ fn new(
forbidden_edge,
#[cfg(debug_assertions)]
value_fingerprints: Lock::new(IndexVec::from_elem_n(None, new_node_count_estimate)),
nodes_in_current_session: new_node_dbg.then(|| {
Lock::new(FxHashMap::with_capacity_and_hasher(
new_node_count_estimate,
Default::default(),
))
}),
total_read_count: AtomicU64::new(0),
total_duplicate_read_count: AtomicU64::new(0),
}
@@ -1202,25 +1179,6 @@ fn record_edge(
assert_eq!(prior_value_fingerprint, value_fingerprint, "Unstable fingerprints for {key:?}");
}
#[inline(always)]
fn record_node(
&self,
dep_node_index: DepNodeIndex,
key: DepNode,
_value_fingerprint: Fingerprint,
) {
#[cfg(debug_assertions)]
self.record_edge(dep_node_index, key, _value_fingerprint);
if let Some(ref nodes_in_current_session) = self.nodes_in_current_session {
outline(|| {
if nodes_in_current_session.lock().insert(key, dep_node_index).is_some() {
panic!("Found duplicate dep-node {key:?}");
}
});
}
}
/// Writes the node to the current dep-graph and allocates a `DepNodeIndex` for it.
/// Assumes that this is a node that has no equivalent in the previous dep-graph.
#[inline(always)]
@@ -1232,28 +1190,11 @@ fn alloc_new_node(
) -> DepNodeIndex {
let dep_node_index = self.encoder.send_new(key, value_fingerprint, edges);
self.record_node(dep_node_index, key, value_fingerprint);
#[cfg(debug_assertions)]
self.record_edge(dep_node_index, key, value_fingerprint);
dep_node_index
}
#[inline]
fn debug_assert_not_in_new_nodes(
&self,
prev_graph: &SerializedDepGraph,
prev_index: SerializedDepNodeIndex,
) {
if !is_dyn_thread_safe()
&& let Some(ref nodes_in_current_session) = self.nodes_in_current_session
{
debug_assert!(
!nodes_in_current_session
.lock()
.contains_key(&prev_graph.index_to_node(prev_index)),
"node from previous graph present in new node collection"
);
}
}
}
#[derive(Debug, Clone, Copy)]
@@ -1430,17 +1371,6 @@ fn panic_on_forbidden_read(data: &DepGraphData, dep_node_index: DepNodeIndex) ->
}
}
if dep_node.is_none()
&& let Some(nodes) = &data.current.nodes_in_current_session
{
// Try to find it among the nodes allocated so far in this session
// This is OK, there's only ever one node result possible so this is deterministic.
#[allow(rustc::potential_query_instability)]
if let Some((node, _)) = nodes.lock().iter().find(|&(_, index)| *index == dep_node_index) {
dep_node = Some(*node);
}
}
let dep_node = dep_node.map_or_else(
|| format!("with index {:?}", dep_node_index),
|dep_node| format!("`{:?}`", dep_node),