Auto merge of #109371 - Zoxc:verify-hash-opt, r=cjgillot

Optimize `incremental_verify_ich`

This optimizes `incremental_verify_ich` by operating on `SerializedDepNodeIndex`, saving 2 hashmap lookups. The panic paths are also changed to get a `TyCtxt` reference using TLS.

1 files changed, 37 insertions, 63 deletions

diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs
index ba2f859ff0f..186417e862a 100644
--- a/compiler/rustc_query_system/src/query/plumbing.rs
+++ b/compiler/rustc_query_system/src/query/plumbing.rs
@@ -7,6 +7,7 @@ use crate::dep_graph::{DepGraphData, HasDepContext};
 use crate::ich::StableHashingContext;
 use crate::query::caches::QueryCache;
 use crate::query::job::{report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobInfo};
+use crate::query::SerializedDepNodeIndex;
 use crate::query::{QueryContext, QueryMap, QuerySideEffects, QueryStackFrame};
 use crate::values::Value;
 use crate::HandleCycleError;
@@ -19,7 +20,6 @@ use rustc_data_structures::sharded::Sharded;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_data_structures::sync::{Lock, LockGuard};
 use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed, FatalError};
-use rustc_session::Session;
 use rustc_span::{Span, DUMMY_SP};
 use std::cell::Cell;
 use std::collections::hash_map::Entry;
@@ -537,7 +537,7 @@ where
 
     let (prev_dep_node_index, dep_node_index) = dep_graph_data.try_mark_green(qcx, &dep_node)?;
 
-    debug_assert!(dep_graph_data.is_green(dep_node));
+    debug_assert!(dep_graph_data.is_index_green(prev_dep_node_index));
 
     // First we try to load the result from the on-disk cache.
     // Some things are never cached on disk.
@@ -561,8 +561,7 @@ where
                 dep_graph_data.mark_debug_loaded_from_disk(*dep_node)
             }
 
-            let prev_fingerprint =
-                dep_graph_data.prev_fingerprint_of(dep_node).unwrap_or(Fingerprint::ZERO);
+            let prev_fingerprint = dep_graph_data.prev_fingerprint_of(prev_dep_node_index);
             // If `-Zincremental-verify-ich` is specified, re-hash results from
             // the cache and make sure that they have the expected fingerprint.
             //
@@ -578,7 +577,7 @@ where
                     *qcx.dep_context(),
                     dep_graph_data,
                     &result,
-                    dep_node,
+                    prev_dep_node_index,
                     query.hash_result(),
                 );
             }
@@ -623,7 +622,7 @@ where
         *qcx.dep_context(),
         dep_graph_data,
         &result,
-        dep_node,
+        prev_dep_node_index,
         query.hash_result(),
     );
 
@@ -636,77 +635,50 @@ pub(crate) fn incremental_verify_ich<Tcx, V: Debug>(
     tcx: Tcx,
     dep_graph_data: &DepGraphData<Tcx::DepKind>,
     result: &V,
-    dep_node: &DepNode<Tcx::DepKind>,
+    prev_index: SerializedDepNodeIndex,
     hash_result: Option<fn(&mut StableHashingContext<'_>, &V) -> Fingerprint>,
-) -> Fingerprint
-where
+) where
     Tcx: DepContext,
 {
-    assert!(
-        dep_graph_data.is_green(dep_node),
-        "fingerprint for green query instance not loaded from cache: {dep_node:?}",
-    );
+    if !dep_graph_data.is_index_green(prev_index) {
+        incremental_verify_ich_not_green(tcx, prev_index)
+    }
 
     let new_hash = hash_result.map_or(Fingerprint::ZERO, |f| {
         tcx.with_stable_hashing_context(|mut hcx| f(&mut hcx, result))
     });
 
-    let old_hash = dep_graph_data.prev_fingerprint_of(dep_node);
+    let old_hash = dep_graph_data.prev_fingerprint_of(prev_index);
 
-    if Some(new_hash) != old_hash {
-        incremental_verify_ich_failed(
-            tcx.sess(),
-            DebugArg::from(&dep_node),
-            DebugArg::from(&result),
-        );
+    if new_hash != old_hash {
+        incremental_verify_ich_failed(tcx, prev_index, result);
     }
-
-    new_hash
 }
 
-// This DebugArg business is largely a mirror of std::fmt::ArgumentV1, which is
-// currently not exposed publicly.
-//
-// The PR which added this attempted to use `&dyn Debug` instead, but that
-// showed statistically significant worse compiler performance. It's not
-// actually clear what the cause there was -- the code should be cold. If this
-// can be replaced with `&dyn Debug` with on perf impact, then it probably
-// should be.
-extern "C" {
-    type Opaque;
-}
-
-struct DebugArg<'a> {
-    value: &'a Opaque,
-    fmt: fn(&Opaque, &mut std::fmt::Formatter<'_>) -> std::fmt::Result,
-}
-
-impl<'a, T> From<&'a T> for DebugArg<'a>
+#[cold]
+#[inline(never)]
+fn incremental_verify_ich_not_green<Tcx>(tcx: Tcx, prev_index: SerializedDepNodeIndex)
 where
-    T: std::fmt::Debug,
+    Tcx: DepContext,
 {
-    fn from(value: &'a T) -> DebugArg<'a> {
-        DebugArg {
-            value: unsafe { std::mem::transmute(value) },
-            fmt: unsafe {
-                std::mem::transmute(<T as std::fmt::Debug>::fmt as fn(_, _) -> std::fmt::Result)
-            },
-        }
-    }
-}
-
-impl std::fmt::Debug for DebugArg<'_> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        (self.fmt)(self.value, f)
-    }
+    panic!(
+        "fingerprint for green query instance not loaded from cache: {:?}",
+        tcx.dep_graph().data().unwrap().prev_node_of(prev_index)
+    )
 }
 
-// Note that this is marked #[cold] and intentionally takes the equivalent of
-// `dyn Debug` for its arguments, as we want to avoid generating a bunch of
-// different implementations for LLVM to chew on (and filling up the final
-// binary, too).
+// Note that this is marked #[cold] and intentionally takes `dyn Debug` for `result`,
+// as we want to avoid generating a bunch of different implementations for LLVM to
+// chew on (and filling up the final binary, too).
 #[cold]
-fn incremental_verify_ich_failed(sess: &Session, dep_node: DebugArg<'_>, result: DebugArg<'_>) {
+#[inline(never)]
+fn incremental_verify_ich_failed<Tcx>(
+    tcx: Tcx,
+    prev_index: SerializedDepNodeIndex,
+    result: &dyn Debug,
+) where
+    Tcx: DepContext,
+{
     // When we emit an error message and panic, we try to debug-print the `DepNode`
     // and query result. Unfortunately, this can cause us to run additional queries,
     // which may result in another fingerprint mismatch while we're in the middle
@@ -720,15 +692,17 @@ fn incremental_verify_ich_failed(sess: &Session, dep_node: DebugArg<'_>, result:
     let old_in_panic = INSIDE_VERIFY_PANIC.with(|in_panic| in_panic.replace(true));
 
     if old_in_panic {
-        sess.emit_err(crate::error::Reentrant);
+        tcx.sess().emit_err(crate::error::Reentrant);
     } else {
-        let run_cmd = if let Some(crate_name) = &sess.opts.crate_name {
+        let run_cmd = if let Some(crate_name) = &tcx.sess().opts.crate_name {
             format!("`cargo clean -p {crate_name}` or `cargo clean`")
         } else {
             "`cargo clean`".to_string()
         };
 
-        sess.emit_err(crate::error::IncrementCompilation {
+        let dep_node = tcx.dep_graph().data().unwrap().prev_node_of(prev_index);
+
+        let dep_node = tcx.sess().emit_err(crate::error::IncrementCompilation {
             run_cmd,
             dep_node: format!("{dep_node:?}"),
         });