refactor: Use CycleHeadSet in maybe_update_after (#953)

Author: MichaReiser

src/accumulator.rs

@@ -7,7 +7,7 @@ use std::panic::UnwindSafe;
 
 use accumulated::{Accumulated, AnyAccumulated};
 
-use crate::cycle::CycleHeads;
+use crate::cycle::CycleHeadKeys;
 use crate::function::VerifyResult;
 use crate::ingredient::{Ingredient, Jar};
 use crate::plumbing::ZalsaLocal;
@@ -106,7 +106,7 @@ impl<A: Accumulator> Ingredient for IngredientImpl<A> {
         _db: crate::database::RawDatabase<'_>,
         _input: Id,
         _revision: Revision,
-        _cycle_heads: &mut CycleHeads,
+        _cycle_heads: &mut CycleHeadKeys,
     ) -> VerifyResult {
         panic!("nothing should ever depend on an accumulator directly")
     }

src/cycle.rs

@@ -178,22 +178,6 @@ impl CycleHeads {
         }
     }
 
-    #[inline]
-    pub(crate) fn push_initial(&mut self, database_key_index: DatabaseKeyIndex) {
-        if let Some(existing) = self
-            .0
-            .iter()
-            .find(|candidate| candidate.database_key_index == database_key_index)
-        {
-            assert_eq!(existing.iteration_count, IterationCount::initial());
-        } else {
-            self.0.push(CycleHead {
-                database_key_index,
-                iteration_count: IterationCount::initial(),
-            });
-        }
-    }
-
     #[inline]
     pub(crate) fn extend(&mut self, other: &Self) {
         self.0.reserve(other.0.len());
@@ -247,6 +231,37 @@ pub(crate) fn empty_cycle_heads() -> &'static CycleHeads {
     EMPTY_CYCLE_HEADS.get_or_init(|| CycleHeads(ThinVec::new()))
 }
 
+/// Set of cycle head database keys.
+///
+/// Unlike [`CycleHeads`], this type doesn't track the iteration count
+/// of each cycle head.
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct CycleHeadKeys(Vec<DatabaseKeyIndex>);
+
+impl CycleHeadKeys {
+    pub(crate) fn new() -> Self {
+        Self(Vec::new())
+    }
+
+    pub(crate) fn insert(&mut self, database_key_index: DatabaseKeyIndex) {
+        if !self.0.contains(&database_key_index) {
+            self.0.push(database_key_index);
+        }
+    }
+
+    pub(crate) fn remove(&mut self, value: &DatabaseKeyIndex) -> bool {
+        let found = self.0.iter().position(|&head| head == *value);
+        let Some(found) = found else { return false };
+
+        self.0.swap_remove(found);
+        true
+    }
+
+    pub(crate) fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+}
+
 #[derive(Debug, PartialEq, Eq)]
 pub enum ProvisionalStatus {
     Provisional { iteration: IterationCount },

src/function.rs

@@ -7,7 +7,8 @@ use std::sync::OnceLock;
 pub(crate) use sync::SyncGuard;
 
 use crate::cycle::{
-    empty_cycle_heads, CycleHeads, CycleRecoveryAction, CycleRecoveryStrategy, ProvisionalStatus,
+    empty_cycle_heads, CycleHeadKeys, CycleHeads, CycleRecoveryAction, CycleRecoveryStrategy,
+    ProvisionalStatus,
 };
 use crate::database::RawDatabase;
 use crate::function::delete::DeletedEntries;
@@ -265,7 +266,7 @@ where
         db: RawDatabase<'_>,
         input: Id,
         revision: Revision,
-        cycle_heads: &mut CycleHeads,
+        cycle_heads: &mut CycleHeadKeys,
     ) -> VerifyResult {
         // SAFETY: The `db` belongs to the ingredient as per caller invariant
         let db = unsafe { self.view_caster().downcast_unchecked(db) };

src/function/fetch.rs

@@ -1,4 +1,4 @@
-use crate::cycle::{CycleHeads, CycleRecoveryStrategy, IterationCount};
+use crate::cycle::{CycleHeadKeys, CycleHeads, CycleRecoveryStrategy, IterationCount};
 use crate::function::memo::Memo;
 use crate::function::sync::ClaimResult;
 use crate::function::{Configuration, IngredientImpl, VerifyResult};
@@ -222,7 +222,7 @@ where
 
         if let Some(old_memo) = opt_old_memo {
             if old_memo.value.is_some() {
-                let mut cycle_heads = CycleHeads::default();
+                let mut cycle_heads = CycleHeadKeys::new();
                 if let VerifyResult::Unchanged { .. } =
                     self.deep_verify_memo(db, zalsa, old_memo, database_key_index, &mut cycle_heads)
                 {

src/function/maybe_changed_after.rs

@@ -1,6 +1,6 @@
 #[cfg(feature = "accumulator")]
 use crate::accumulator::accumulated_map::InputAccumulatedValues;
-use crate::cycle::{CycleHeads, CycleRecoveryStrategy, IterationCount, ProvisionalStatus};
+use crate::cycle::{CycleHeadKeys, CycleRecoveryStrategy, IterationCount, ProvisionalStatus};
 use crate::function::memo::Memo;
 use crate::function::sync::ClaimResult;
 use crate::function::{Configuration, IngredientImpl};
@@ -51,7 +51,7 @@ where
         db: &'db C::DbView,
         id: Id,
         revision: Revision,
-        cycle_heads: &mut CycleHeads,
+        cycle_heads: &mut CycleHeadKeys,
     ) -> VerifyResult {
         let (zalsa, zalsa_local) = db.zalsas();
         let memo_ingredient_index = self.memo_ingredient_index(zalsa, id);
@@ -108,7 +108,7 @@ where
         key_index: Id,
         revision: Revision,
         memo_ingredient_index: MemoIngredientIndex,
-        cycle_heads: &mut CycleHeads,
+        cycle_heads: &mut CycleHeadKeys,
     ) -> Option<VerifyResult> {
         let database_key_index = self.database_key_index(key_index);
 
@@ -135,7 +135,7 @@ where
                     crate::tracing::debug!(
                         "hit cycle at {database_key_index:?} in `maybe_changed_after`,  returning fixpoint initial value",
                     );
-                    cycle_heads.push_initial(database_key_index);
+                    cycle_heads.insert(database_key_index);
                     return Some(VerifyResult::unchanged());
                 }
             },
@@ -406,7 +406,7 @@ where
         zalsa: &Zalsa,
         old_memo: &Memo<'_, C>,
         database_key_index: DatabaseKeyIndex,
-        cycle_heads: &mut CycleHeads,
+        cycle_heads: &mut CycleHeadKeys,
     ) -> VerifyResult {
         crate::tracing::debug!(
             "{database_key_index:?}: deep_verify_memo(old_memo = {old_memo:#?})",
@@ -447,7 +447,7 @@ where
             // are tracked by the outer query. Nothing should have changed assuming that the
             // fixpoint initial function is deterministic.
             QueryOriginRef::FixpointInitial => {
-                cycle_heads.push_initial(database_key_index);
+                cycle_heads.insert(database_key_index);
                 VerifyResult::unchanged()
             }
             QueryOriginRef::DerivedUntracked(_) => {

src/ingredient.rs

@@ -2,7 +2,8 @@ use std::any::{Any, TypeId};
 use std::fmt;
 
 use crate::cycle::{
-    empty_cycle_heads, CycleHeads, CycleRecoveryStrategy, IterationCount, ProvisionalStatus,
+    empty_cycle_heads, CycleHeadKeys, CycleHeads, CycleRecoveryStrategy, IterationCount,
+    ProvisionalStatus,
 };
 use crate::database::RawDatabase;
 use crate::function::VerifyResult;
@@ -51,7 +52,7 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
         db: crate::database::RawDatabase<'_>,
         input: Id,
         revision: Revision,
-        cycle_heads: &mut CycleHeads,
+        cycle_heads: &mut CycleHeadKeys,
     ) -> VerifyResult;
 
     /// Returns information about the current provisional status of `input`.

src/input.rs

@@ -8,7 +8,7 @@ pub mod singleton;
 
 use input_field::FieldIngredientImpl;
 
-use crate::cycle::CycleHeads;
+use crate::cycle::CycleHeadKeys;
 use crate::function::VerifyResult;
 use crate::id::{AsId, FromId, FromIdWithDb};
 use crate::ingredient::Ingredient;
@@ -226,7 +226,7 @@ impl<C: Configuration> Ingredient for IngredientImpl<C> {
         _db: crate::database::RawDatabase<'_>,
         _input: Id,
         _revision: Revision,
-        _cycle_heads: &mut CycleHeads,
+        _cycle_heads: &mut CycleHeadKeys,
     ) -> VerifyResult {
         // Input ingredients are just a counter, they store no data, they are immortal.
         // Their *fields* are stored in function ingredients elsewhere.

src/input/input_field.rs

@@ -1,7 +1,7 @@
 use std::fmt;
 use std::marker::PhantomData;
 
-use crate::cycle::CycleHeads;
+use crate::cycle::CycleHeadKeys;
 use crate::function::VerifyResult;
 use crate::ingredient::Ingredient;
 use crate::input::{Configuration, IngredientImpl, Value};
@@ -56,7 +56,7 @@ where
         _db: crate::database::RawDatabase<'_>,
         input: Id,
         revision: Revision,
-        _cycle_heads: &mut CycleHeads,
+        _cycle_heads: &mut CycleHeadKeys,
     ) -> VerifyResult {
         let value = <IngredientImpl<C>>::data(zalsa, input);
         VerifyResult::changed_if(value.revisions[self.field_index] > revision)

src/interned.rs

@@ -10,7 +10,7 @@ use crossbeam_utils::CachePadded;
 use intrusive_collections::{intrusive_adapter, LinkedList, LinkedListLink, UnsafeRef};
 use rustc_hash::FxBuildHasher;
 
-use crate::cycle::CycleHeads;
+use crate::cycle::CycleHeadKeys;
 use crate::durability::Durability;
 use crate::function::VerifyResult;
 use crate::id::{AsId, FromId};
@@ -797,7 +797,7 @@ where
         _db: crate::database::RawDatabase<'_>,
         input: Id,
         _revision: Revision,
-        _cycle_heads: &mut CycleHeads,
+        _cycle_heads: &mut CycleHeadKeys,
     ) -> VerifyResult {
         // Record the current revision as active.
         let current_revision = zalsa.current_revision();

src/key.rs

@@ -1,6 +1,6 @@
 use core::fmt;
 
-use crate::cycle::CycleHeads;
+use crate::cycle::CycleHeadKeys;
 use crate::function::VerifyResult;
 use crate::zalsa::{IngredientIndex, Zalsa};
 use crate::Id;
@@ -39,7 +39,7 @@ impl DatabaseKeyIndex {
         db: crate::database::RawDatabase<'_>,
         zalsa: &Zalsa,
         last_verified_at: crate::Revision,
-        cycle_heads: &mut CycleHeads,
+        cycle_heads: &mut CycleHeadKeys,
     ) -> VerifyResult {
         // SAFETY: The `db` belongs to the ingredient
         unsafe {