Auto merge of #158059 - xmakro:perf/lexer-ascii-symbolgallery, r=<try>

Skip query machinery when promoting disk-cached values

Author: bors

compiler/rustc_middle/src/dep_graph/dep_node.rs

@@ -57,7 +57,7 @@ use rustc_hir::definitions::DefPathHash;
 use rustc_macros::{Decodable, Encodable, StableHash};
 use rustc_span::Symbol;
 
-use super::{KeyFingerprintStyle, SerializedDepNodeIndex};
+use super::{DepNodeIndex, KeyFingerprintStyle, SerializedDepNodeIndex};
 use crate::dep_graph::DepNodeKey;
 use crate::mono::MonoItem;
 use crate::ty::{TyCtxt, tls};
@@ -208,8 +208,17 @@ pub struct DepKindVTable<'tcx> {
         fn(tcx: TyCtxt<'tcx>, dep_node: DepNode, prev_index: SerializedDepNodeIndex) -> bool,
     >,
 
-    /// Invoke a query to put the on-disk cached value in memory.
-    pub promote_from_disk_fn: Option<fn(TyCtxt<'tcx>, DepNode)>,
+    /// Load the on-disk cached value of a query into memory. The node is known
+    /// to be green, with `prev_index` its index in the previous session's dep
+    /// graph and `dep_node_index` its index in the current session's dep graph.
+    pub promote_from_disk_fn: Option<
+        fn(
+            tcx: TyCtxt<'tcx>,
+            dep_node: DepNode,
+            prev_index: SerializedDepNodeIndex,
+            dep_node_index: DepNodeIndex,
+        ),
+    >,
 }
 
 /// A "work product" corresponds to a `.o` (or other) file that we

compiler/rustc_middle/src/dep_graph/graph.rs

@@ -1063,12 +1063,12 @@ impl DepGraph {
         let data = self.data.as_ref().unwrap();
         for prev_index in data.colors.values.indices() {
             match data.colors.get(prev_index) {
-                DepNodeColor::Green(_) => {
+                DepNodeColor::Green(dep_node_index) => {
                     let dep_node = data.previous.index_to_node(prev_index);
                     if let Some(promote_fn) =
                         tcx.dep_kind_vtable(dep_node.kind).promote_from_disk_fn
                     {
-                        promote_fn(tcx, *dep_node)
+                        promote_fn(tcx, *dep_node, prev_index, dep_node_index)
                     };
                 }
                 DepNodeColor::Unknown | DepNodeColor::Red => {

compiler/rustc_query_impl/src/dep_kind_vtables.rs

@@ -117,10 +117,12 @@ where
                 crate::execution::force_query_dep_node(tcx, query, dep_node)
             },
         ),
-        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)
-        }),
+        promote_from_disk_fn: (can_recover && is_cache_on_disk).then_some(
+            |tcx, dep_node, prev_index, dep_node_index| {
+                let query = Q::query_vtable(tcx);
+                promote_from_disk_inner(tcx, query, dep_node, prev_index, dep_node_index)
+            },
+        ),
     }
 }
 

compiler/rustc_query_impl/src/execution.rs

@@ -1,6 +1,7 @@
 use std::hash::Hash;
 use std::mem::ManuallyDrop;
 
+use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::hash_table::{Entry, HashTable};
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_data_structures::sync::{DynSend, DynSync};
@@ -484,6 +485,20 @@ fn execute_job_incr<'tcx, C: QueryCache>(
     (result, dep_node_index)
 }
 
+/// Whether a value loaded from the on-disk cache should have its fingerprint
+/// verified with `incremental_verify_ich`. If `-Zincremental-verify-ich` is
+/// specified, re-hash results from the cache and make sure that they have the
+/// expected fingerprint.
+///
+/// If not, we still seek to verify a subset of fingerprints loaded from disk.
+/// Re-hashing results is fairly expensive, so we can't currently afford to
+/// verify every hash. This subset should still give us some coverage of
+/// potential bugs.
+pub(crate) fn should_verify_loaded_value(tcx: TyCtxt<'_>, prev_fingerprint: Fingerprint) -> bool {
+    prev_fingerprint.split().1.as_u64().is_multiple_of(32)
+        || tcx.sess.opts.unstable_opts.incremental_verify_ich
+}
+
 /// Given that the dep node for this query+key is green, obtain a value for it
 /// by loading one from disk if possible, or by invoking its query provider if
 /// necessary.
@@ -518,15 +533,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>(
             }
 
             let prev_fingerprint = dep_graph_data.prev_value_fingerprint_of(prev_index);
-            // If `-Zincremental-verify-ich` is specified, re-hash results from
-            // the cache and make sure that they have the expected fingerprint.
-            //
-            // If not, we still seek to verify a subset of fingerprints loaded
-            // from disk. Re-hashing results is fairly expensive, so we can't
-            // currently afford to verify every hash. This subset should still
-            // give us some coverage of potential bugs.
-            let verify = prev_fingerprint.split().1.as_u64().is_multiple_of(32)
-                || tcx.sess.opts.unstable_opts.incremental_verify_ich;
+            let verify = should_verify_loaded_value(tcx, prev_fingerprint);
 
             (value, verify)
         }

compiler/rustc_query_impl/src/plumbing.rs

@@ -1,22 +1,24 @@
 use std::num::NonZero;
 
+use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_data_structures::unord::UnordMap;
 use rustc_hir::limit::Limit;
 use rustc_middle::bug;
 #[expect(unused_imports, reason = "used by doc comments")]
 use rustc_middle::dep_graph::DepKindVTable;
-use rustc_middle::dep_graph::{DepNode, 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::{CacheDecoder, CacheEncoder};
 use rustc_middle::query::{QueryCache, QueryJobId, QueryMode, QueryVTable, erase};
 use rustc_middle::ty::TyCtxt;
 use rustc_middle::ty::tls::{self, ImplicitCtxt};
+use rustc_middle::verify_ich::incremental_verify_ich;
 use rustc_serialize::{Decodable, Encodable};
 use rustc_span::DUMMY_SP;
 use rustc_span::def_id::LOCAL_CRATE;
 
 use crate::error::{QueryOverflow, QueryOverflowNote};
-use crate::execution::all_inactive;
+use crate::execution::{all_inactive, should_verify_loaded_value};
 use crate::job::find_dep_kind_root;
 use crate::query_impl::for_each_query_vtable;
 use crate::{CollectActiveJobsKind, collect_active_query_jobs};
@@ -140,6 +142,8 @@ pub(crate) fn promote_from_disk_inner<'tcx, C: QueryCache>(
     tcx: TyCtxt<'tcx>,
     query: &'tcx QueryVTable<'tcx, C>,
     dep_node: DepNode,
+    prev_index: SerializedDepNodeIndex,
+    dep_node_index: DepNodeIndex,
 ) {
     debug_assert!(tcx.dep_graph.is_green(&dep_node));
 
@@ -156,17 +160,41 @@ pub(crate) fn promote_from_disk_inner<'tcx, C: QueryCache>(
         return;
     }
 
-    match query.cache.lookup(&key) {
-        // If the value is already in memory, then promotion isn't needed.
-        Some(_) => {}
-
-        // "Execute" the query to load its disk-cached value into memory.
-        //
-        // We know that the key is cache-on-disk and its node is green,
-        // so there _must_ be a value on disk to load.
-        //
-        // FIXME(Zalathar): Is there a reasonable way to skip more of the
-        // query bookkeeping when doing this?
+    // If the value is already in memory, then promotion isn't needed.
+    if query.cache.lookup(&key).is_some() {
+        return;
+    }
+
+    // Load the disk-cached value into memory.
+    let dep_graph_data =
+        tcx.dep_graph.data().expect("should always be present in incremental mode");
+
+    let prof_timer = tcx.prof.incr_cache_loading();
+    let value = ensure_sufficient_stack(|| (query.try_load_from_disk_fn)(tcx, prev_index));
+    prof_timer.finish_with_query_invocation_id(dep_node_index.into());
+
+    match value {
+        Some(value) => {
+            // Verify the fingerprints of the same subset of loaded values as
+            // `load_from_disk_or_invoke_provider_green` does.
+            let prev_fingerprint = dep_graph_data.prev_value_fingerprint_of(prev_index);
+            if should_verify_loaded_value(tcx, prev_fingerprint) {
+                incremental_verify_ich(
+                    tcx,
+                    dep_graph_data,
+                    &value,
+                    prev_index,
+                    query.hash_value_fn,
+                    query.format_value,
+                );
+            }
+
+            query.cache.complete(key, value, dep_node_index);
+        }
+
+        // There was no value on disk, despite the key being cache-on-disk and
+        // its node being green. "Execute" the query through the normal
+        // machinery, which will recompute the value and put it in memory.
         None => {
             (query.execute_query_fn)(tcx, DUMMY_SP, key, QueryMode::Get);
         }