Auto merge of #108062 - Zoxc:spec-incr, r=cjgillot

Specialize query execution for incremental and non-incremental

This specializes query execution for incremental and non-incremental by passing in a separate `dyn QueryEngine` types, taking advantage of the virtual dispatch to avoid a branch. This ends up duplicating `try_execute_query`, hopefully the compile time cost of that is relatively low.

This is a performance improvement for the non-incremental path:
<table><tr><td rowspan="2">Benchmark</td><td colspan="1"><b>Before</b></th><td colspan="2"><b>After</b></th></tr><tr><td align="right">Time</td><td align="right">Time</td><td align="right">%</th></tr><tr><td>🟣 <b>clap</b>:check</td><td align="right">1.8420s</td><td align="right">1.8331s</td><td align="right"> -0.48%</td></tr><tr><td>🟣 <b>hyper</b>:check</td><td align="right">0.2652s</td><td align="right">0.2631s</td><td align="right"> -0.78%</td></tr><tr><td>🟣 <b>regex</b>:check</td><td align="right">1.0161s</td><td align="right">1.0062s</td><td align="right"> -0.98%</td></tr><tr><td>🟣 <b>syn</b>:check</td><td align="right">1.6408s</td><td align="right">1.6197s</td><td align="right">💚  -1.28%</td></tr><tr><td>🟣 <b>syntex_syntax</b>:check</td><td align="right">6.3939s</td><td align="right">6.3558s</td><td align="right"> -0.60%</td></tr><tr><td>Total</td><td align="right">11.1580s</td><td align="right">11.0780s</td><td align="right"> -0.72%</td></tr><tr><td>Summary</td><td align="right">1.0000s</td><td align="right">0.9918s</td><td align="right"> -0.82%</td></tr></table>

The incremental path is more neutral:
<table><tr><td rowspan="2">Benchmark</td><td colspan="1"><b>Before</b></th><td colspan="2"><b>After</b></th></tr><tr><td align="right">Time</td><td align="right">Time</td><td align="right">%</th></tr><tr><td>🟣 <b>clap</b>:check:initial</td><td align="right">2.2210s</td><td align="right">2.2227s</td><td align="right"> 0.08%</td></tr><tr><td>🟣 <b>hyper</b>:check:initial</td><td align="right">0.3441s</td><td align="right">0.3443s</td><td align="right"> 0.05%</td></tr><tr><td>🟣 <b>regex</b>:check:initial</td><td align="right">1.2919s</td><td align="right">1.2877s</td><td align="right"> -0.33%</td></tr><tr><td>🟣 <b>syn</b>:check:initial</td><td align="right">2.0749s</td><td align="right">2.0721s</td><td align="right"> -0.14%</td></tr><tr><td>🟣 <b>syntex_syntax</b>:check:initial</td><td align="right">7.9266s</td><td align="right">7.9206s</td><td align="right"> -0.07%</td></tr><tr><td>Total</td><td align="right">13.8585s</td><td align="right">13.8474s</td><td align="right"> -0.08%</td></tr><tr><td>Summary</td><td align="right">1.0000s</td><td align="right">0.9992s</td><td align="right"> -0.08%</td></tr></table>

r? `@cjgillot`

compiler/rustc_interface/src/passes.rs

@@ -691,6 +691,8 @@ pub fn create_global_ctxt<'tcx>(
         callback(sess, &mut local_providers, &mut extern_providers);
     }
 
+    let incremental = dep_graph.is_fully_enabled();
+
     sess.time("setup_global_ctxt", || {
         gcx_cell.get_or_init(move || {
             TyCtxt::create_global_ctxt(
@@ -705,6 +707,7 @@ pub fn create_global_ctxt<'tcx>(
                     local_providers,
                     extern_providers,
                     query_result_on_disk_cache,
+                    incremental,
                 ),
             )
         })

compiler/rustc_query_impl/src/lib.rs

@@ -34,7 +34,8 @@ use rustc_middle::ty::TyCtxt;
 use rustc_query_system::dep_graph::SerializedDepNodeIndex;
 use rustc_query_system::ich::StableHashingContext;
 use rustc_query_system::query::{
-    get_query, HashResult, QueryCache, QueryConfig, QueryInfo, QueryMap, QueryMode, QueryState,
+    get_query_incr, get_query_non_incr, HashResult, QueryCache, QueryConfig, QueryInfo, QueryMap,
+    QueryMode, QueryState,
 };
 use rustc_query_system::HandleCycleError;
 use rustc_query_system::Value;
@@ -203,6 +204,7 @@ pub fn query_system<'tcx>(
     local_providers: Providers,
     extern_providers: ExternProviders,
     on_disk_cache: Option<OnDiskCache<'tcx>>,
+    incremental: bool,
 ) -> QuerySystem<'tcx> {
     QuerySystem {
         states: Default::default(),
@@ -211,7 +213,7 @@ pub fn query_system<'tcx>(
         dynamic_queries: dynamic_queries(),
         on_disk_cache,
         fns: QuerySystemFns {
-            engine: engine(),
+            engine: engine(incremental),
             local_providers,
             extern_providers,
             query_structs: make_dep_kind_array!(query_structs).to_vec(),

compiler/rustc_query_impl/src/plumbing.rs

@@ -494,7 +494,7 @@ macro_rules! define_queries {
     (
      $($(#[$attr:meta])*
         [$($modifiers:tt)*] fn $name:ident($($K:tt)*) -> $V:ty,)*) => {
-        mod get_query {
+        mod get_query_incr {
             use super::*;
 
             $(
@@ -506,7 +506,7 @@ macro_rules! define_queries {
                     key: query_keys::$name<'tcx>,
                     mode: QueryMode,
                 ) -> Option<Erase<query_values::$name<'tcx>>> {
-                    get_query(
+                    get_query_incr(
                         queries::$name::config(tcx),
                         QueryCtxt::new(tcx),
                         span,
@@ -517,9 +517,37 @@ macro_rules! define_queries {
             )*
         }
 
-        pub(crate) fn engine() -> QueryEngine {
-            QueryEngine {
-                $($name: get_query::$name,)*
+        mod get_query_non_incr {
+            use super::*;
+
+            $(
+                #[inline(always)]
+                #[tracing::instrument(level = "trace", skip(tcx))]
+                pub(super) fn $name<'tcx>(
+                    tcx: TyCtxt<'tcx>,
+                    span: Span,
+                    key: query_keys::$name<'tcx>,
+                    __mode: QueryMode,
+                ) -> Option<Erase<query_values::$name<'tcx>>> {
+                    Some(get_query_non_incr(
+                        queries::$name::config(tcx),
+                        QueryCtxt::new(tcx),
+                        span,
+                        key,
+                    ))
+                }
+            )*
+        }
+
+        pub(crate) fn engine(incremental: bool) -> QueryEngine {
+            if incremental {
+                QueryEngine {
+                    $($name: get_query_incr::$name,)*
+                }
+            } else {
+                QueryEngine {
+                    $($name: get_query_non_incr::$name,)*
+                }
             }
         }
 

compiler/rustc_query_system/src/query/plumbing.rs

@@ -312,7 +312,7 @@ where
 }
 
 #[inline(never)]
-fn try_execute_query<Q, Qcx>(
+fn try_execute_query<Q, Qcx, const INCR: bool>(
     query: Q,
     qcx: Qcx,
     span: Span,
@@ -355,7 +355,7 @@ where
             // Drop the lock before we start executing the query
             drop(state_lock);
 
-            execute_job(query, qcx, state, key, id, dep_node)
+            execute_job::<_, _, INCR>(query, qcx, state, key, id, dep_node)
         }
         Entry::Occupied(mut entry) => {
             match entry.get_mut() {
@@ -383,7 +383,7 @@ where
 }
 
 #[inline(always)]
-fn execute_job<Q, Qcx>(
+fn execute_job<Q, Qcx, const INCR: bool>(
     query: Q,
     qcx: Qcx,
     state: &QueryState<Q::Key, Qcx::DepKind>,
@@ -398,9 +398,19 @@ where
     // Use `JobOwner` so the query will be poisoned if executing it panics.
     let job_owner = JobOwner { state, key };
 
-    let (result, dep_node_index) = match qcx.dep_context().dep_graph().data() {
-        None => execute_job_non_incr(query, qcx, key, id),
-        Some(data) => execute_job_incr(query, qcx, data, key, dep_node, id),
+    debug_assert_eq!(qcx.dep_context().dep_graph().is_fully_enabled(), INCR);
+
+    let (result, dep_node_index) = if INCR {
+        execute_job_incr(
+            query,
+            qcx,
+            qcx.dep_context().dep_graph().data().unwrap(),
+            key,
+            dep_node,
+            id,
+        )
+    } else {
+        execute_job_non_incr(query, qcx, key, id)
     };
 
     let cache = query.query_cache(qcx);
@@ -784,7 +794,18 @@ pub enum QueryMode {
 }
 
 #[inline(always)]
-pub fn get_query<Q, Qcx>(
+pub fn get_query_non_incr<Q, Qcx>(query: Q, qcx: Qcx, span: Span, key: Q::Key) -> Q::Value
+where
+    Q: QueryConfig<Qcx>,
+    Qcx: QueryContext,
+{
+    debug_assert!(!qcx.dep_context().dep_graph().is_fully_enabled());
+
+    ensure_sufficient_stack(|| try_execute_query::<Q, Qcx, false>(query, qcx, span, key, None).0)
+}
+
+#[inline(always)]
+pub fn get_query_incr<Q, Qcx>(
     query: Q,
     qcx: Qcx,
     span: Span,
@@ -795,6 +816,8 @@ where
     Q: QueryConfig<Qcx>,
     Qcx: QueryContext,
 {
+    debug_assert!(qcx.dep_context().dep_graph().is_fully_enabled());
+
     let dep_node = if let QueryMode::Ensure { check_cache } = mode {
         let (must_run, dep_node) = ensure_must_run(query, qcx, &key, check_cache);
         if !must_run {
@@ -805,8 +828,9 @@ where
         None
     };
 
-    let (result, dep_node_index) =
-        ensure_sufficient_stack(|| try_execute_query(query, qcx, span, key, dep_node));
+    let (result, dep_node_index) = ensure_sufficient_stack(|| {
+        try_execute_query::<_, _, true>(query, qcx, span, key, dep_node)
+    });
     if let Some(dep_node_index) = dep_node_index {
         qcx.dep_context().dep_graph().read_index(dep_node_index)
     }
@@ -831,5 +855,7 @@ pub fn force_query<Q, Qcx>(
 
     debug_assert!(!query.anon());
 
-    ensure_sufficient_stack(|| try_execute_query(query, qcx, DUMMY_SP, key, Some(dep_node)));
+    ensure_sufficient_stack(|| {
+        try_execute_query::<_, _, true>(query, qcx, DUMMY_SP, key, Some(dep_node))
+    });
 }