refactor: Push active query in execute (#996)

* refactor: Push active query in execute

* Remove inline from `execute_maybe_iterate`

* Pass Zalsa and ZalsaLocal

* Remove inline from `push_query`

* Remove `id` from `execute_query`

* Discard changes to src/zalsa_local.rs

.github/workflows/test.yml

@@ -152,7 +152,8 @@ jobs:
             target/
           key: ${{ runner.os }}-cargo-${{ steps.rust-toolchain.outputs.cachekey }}-${{ hashFiles('**/Cargo.toml') }}
           restore-keys: |
-            ${{ runner.os }}-cargo-${{ steps.rust-toolchain.outputs.cachekey }}-
+            ${{ runner.os }}-cargo-${{ steps.rust-toolchain.outputs.cachekey }}-benchmark
+            ${{ runner.os }}-cargo-${{ steps.rust-toolchain.outputs.cachekey }}
             ${{ runner.os }}-cargo-
 
       - name: "Build benchmarks"

src/function/execute.rs

@@ -2,11 +2,12 @@ use crate::active_query::CompletedQuery;
 use crate::cycle::{CycleRecoveryStrategy, IterationCount};
 use crate::function::memo::Memo;
 use crate::function::{Configuration, IngredientImpl};
+use crate::plumbing::ZalsaLocal;
 use crate::sync::atomic::{AtomicBool, Ordering};
 use crate::tracked_struct::Identity;
-use crate::zalsa::{MemoIngredientIndex, Zalsa, ZalsaDatabase};
+use crate::zalsa::{MemoIngredientIndex, Zalsa};
 use crate::zalsa_local::{ActiveQueryGuard, QueryRevisions};
-use crate::{Event, EventKind, Id};
+use crate::{DatabaseKeyIndex, Event, EventKind, Id};
 
 impl<C> IngredientImpl<C>
 where
@@ -25,14 +26,14 @@ where
     pub(super) fn execute<'db>(
         &'db self,
         db: &'db C::DbView,
-        active_query: ActiveQueryGuard<'db>,
+        zalsa: &'db Zalsa,
+        zalsa_local: &'db ZalsaLocal,
+        database_key_index: DatabaseKeyIndex,
         opt_old_memo: Option<&Memo<'db, C>>,
     ) -> &'db Memo<'db, C> {
-        let database_key_index = active_query.database_key_index;
         let id = database_key_index.key_index();
 
         crate::tracing::info!("{:?}: executing query", database_key_index);
-        let zalsa = db.zalsa();
 
         zalsa.event(&|| {
             Event::new(EventKind::WillExecute {
@@ -42,12 +43,19 @@ where
         let memo_ingredient_index = self.memo_ingredient_index(zalsa, id);
 
         let (new_value, mut completed_query) = match C::CYCLE_STRATEGY {
-            CycleRecoveryStrategy::Panic => {
+            CycleRecoveryStrategy::Panic => Self::execute_query(
-                Self::execute_query(db, zalsa, active_query, opt_old_memo, id)
+                db,
-            }
+                zalsa,
+                zalsa_local.push_query(database_key_index, IterationCount::initial()),
+                opt_old_memo,
+            ),
             CycleRecoveryStrategy::FallbackImmediate => {
-                let (mut new_value, mut completed_query) =
+                let (mut new_value, mut completed_query) = Self::execute_query(
-                    Self::execute_query(db, zalsa, active_query, opt_old_memo, id);
+                    db,
+                    zalsa,
+                    zalsa_local.push_query(database_key_index, IterationCount::initial()),
+                    opt_old_memo,
+                );
 
                 if let Some(cycle_heads) = completed_query.revisions.cycle_heads_mut() {
                     // Did the new result we got depend on our own provisional value, in a cycle?
@@ -71,9 +79,8 @@ where
                     // Cycle participants that don't have a fallback will be discarded in
                     // `validate_provisional()`.
                     let cycle_heads = std::mem::take(cycle_heads);
-                    let active_query = db
+                    let active_query =
-                        .zalsa_local()
+                        zalsa_local.push_query(database_key_index, IterationCount::initial());
-                        .push_query(database_key_index, IterationCount::initial());
                     new_value = C::cycle_initial(db, C::id_to_input(zalsa, id));
                     completed_query = active_query.pop();
                     // We need to set `cycle_heads` and `verified_final` because it needs to propagate to the callers.
@@ -86,10 +93,10 @@ where
             }
             CycleRecoveryStrategy::Fixpoint => self.execute_maybe_iterate(
                 db,
-                active_query,
                 opt_old_memo,
                 zalsa,
-                id,
+                zalsa_local,
+                database_key_index,
                 memo_ingredient_index,
             ),
         };
@@ -122,19 +129,18 @@ where
         )
     }
 
-    #[inline]
     fn execute_maybe_iterate<'db>(
         &'db self,
         db: &'db C::DbView,
-        mut active_query: ActiveQueryGuard<'db>,
         opt_old_memo: Option<&Memo<'db, C>>,
         zalsa: &'db Zalsa,
-        id: Id,
+        zalsa_local: &'db ZalsaLocal,
+        database_key_index: DatabaseKeyIndex,
         memo_ingredient_index: MemoIngredientIndex,
     ) -> (C::Output<'db>, CompletedQuery) {
-        let database_key_index = active_query.database_key_index;
+        let id = database_key_index.key_index();
         let mut iteration_count = IterationCount::initial();
-        let zalsa_local = db.zalsa_local();
+        let mut active_query = zalsa_local.push_query(database_key_index, iteration_count);
 
         // Our provisional value from the previous iteration, when doing fixpoint iteration.
         // Initially it's set to None, because the initial provisional value is created lazily,
@@ -155,7 +161,7 @@ where
             active_query.seed_tracked_struct_ids(&last_stale_tracked_ids);
 
             let (mut new_value, mut completed_query) =
-                Self::execute_query(db, zalsa, active_query, previous_memo, id);
+                Self::execute_query(db, zalsa, active_query, previous_memo);
 
             // Did the new result we got depend on our own provisional value, in a cycle?
             if let Some(cycle_heads) = completed_query
@@ -272,7 +278,6 @@ where
         zalsa: &'db Zalsa,
         active_query: ActiveQueryGuard<'db>,
         opt_old_memo: Option<&Memo<'db, C>>,
-        id: Id,
     ) -> (C::Output<'db>, CompletedQuery) {
         if let Some(old_memo) = opt_old_memo {
             // If we already executed this query once, then use the tracked-struct ids from the
@@ -293,7 +298,10 @@ where
 
         // Query was not previously executed, or value is potentially
         // stale, or value is absent. Let's execute!
-        let new_value = C::execute(db, C::id_to_input(zalsa, id));
+        let new_value = C::execute(
+            db,
+            C::id_to_input(zalsa, active_query.database_key_index.key_index()),
+        );
 
         (new_value, active_query.pop())
     }

src/function/fetch.rs

@@ -232,11 +232,7 @@ where
             }
         }
 
-        let memo = self.execute(
+        let memo = self.execute(db, zalsa, zalsa_local, database_key_index, opt_old_memo);
-            db,
-            zalsa_local.push_query(database_key_index, IterationCount::initial()),
-            opt_old_memo,
-        );
 
         Some(memo)
     }

src/function/maybe_changed_after.rs

@@ -2,7 +2,7 @@ use rustc_hash::FxHashMap;
 
 #[cfg(feature = "accumulator")]
 use crate::accumulator::accumulated_map::InputAccumulatedValues;
-use crate::cycle::{CycleRecoveryStrategy, IterationCount, ProvisionalStatus};
+use crate::cycle::{CycleRecoveryStrategy, ProvisionalStatus};
 use crate::function::memo::Memo;
 use crate::function::sync::ClaimResult;
 use crate::function::{Configuration, IngredientImpl};
@@ -227,9 +227,7 @@ where
         // `in_cycle` tracks if the enclosing query is in a cycle. `deep_verify.cycle_heads` tracks
         // if **this query** encountered a cycle (which means there's some provisional value somewhere floating around).
         if old_memo.value.is_some() && !cycle_heads.has_any() {
-            let active_query =
+            let memo = self.execute(db, zalsa, zalsa_local, database_key_index, Some(old_memo));
-                zalsa_local.push_query(database_key_index, IterationCount::initial());
-            let memo = self.execute(db, active_query, Some(old_memo));
             let changed_at = memo.revisions.changed_at;
 
             // Always assume that a provisional value has changed.