More flexible memo dropping via channels

Author: Veykril

Cargo.toml

@@ -15,6 +15,7 @@ salsa-macros = { version = "0.23.0", path = "components/salsa-macros", optional
 boxcar = "0.2.13"
 crossbeam-queue = "0.3.12"
 crossbeam-utils = "0.8.21"
+crossbeam-channel = "0.5.15"
 hashbrown = "0.15"
 hashlink = "0.10"
 indexmap = "2"
@@ -55,7 +56,6 @@ salsa-macros = { version = "=0.23.0", path = "components/salsa-macros" }
 
 [dev-dependencies]
 # examples
-crossbeam-channel = "0.5.15"
 dashmap = { version = "6", features = ["raw-api"] }
 eyre = "0.6.12"
 notify-debouncer-mini = "0.4.1"

src/database.rs

@@ -42,7 +42,7 @@ pub trait Database: Send + ZalsaDatabase + AsDynDatabase {
     /// is owned by the current thread, this could trigger deadlock.
     fn trigger_lru_eviction(&mut self) {
         let zalsa_mut = self.zalsa_mut();
-        zalsa_mut.evict_lru();
+        zalsa_mut.reset_for_new_revision();
     }
 
     /// A "synthetic write" causes the system to act *as though* some

src/function.rs

@@ -1,24 +1,24 @@
 pub(crate) use maybe_changed_after::VerifyResult;
 use std::any::Any;
-use std::fmt;
+use std::marker::PhantomData;
 use std::ptr::NonNull;
 use std::sync::atomic::Ordering;
 use std::sync::OnceLock;
+use std::{fmt, mem};
 pub(crate) use sync::SyncGuard;
 
 use crate::cycle::{
     empty_cycle_heads, CycleHeadKeys, CycleHeads, CycleRecoveryAction, CycleRecoveryStrategy,
     ProvisionalStatus,
 };
 use crate::database::RawDatabase;
-use crate::function::delete::DeletedEntries;
 use crate::function::sync::{ClaimResult, SyncTable};
 use crate::ingredient::{Ingredient, WaitForResult};
 use crate::key::DatabaseKeyIndex;
 use crate::plumbing::MemoIngredientMap;
 use crate::salsa_struct::SalsaStructInDb;
 use crate::sync::Arc;
-use crate::table::memo::MemoTableTypes;
+use crate::table::memo::{DeletedEntries, MemoTableTypes};
 use crate::table::Table;
 use crate::views::DatabaseDownCaster;
 use crate::zalsa::{IngredientIndex, MemoIngredientIndex, Zalsa};
@@ -28,7 +28,6 @@ use crate::{Id, Revision};
 #[cfg(feature = "accumulator")]
 mod accumulated;
 mod backdate;
-mod delete;
 mod diff_outputs;
 mod execute;
 mod fetch;
@@ -147,7 +146,8 @@ pub struct IngredientImpl<C: Configuration> {
     /// current revision: you would be right, but we are being defensive, because
     /// we don't know that we can trust the database to give us the same runtime
     /// everytime and so forth.
-    deleted_entries: DeletedEntries<C>,
+    delete: DeletedEntries,
+    config: PhantomData<fn(C) -> C>,
 }
 
 impl<C> IngredientImpl<C>
@@ -162,10 +162,11 @@ where
         Self {
             index,
             memo_ingredient_indices,
-            lru: lru::Lru::new(lru),
-            deleted_entries: Default::default(),
             view_caster: OnceLock::new(),
+            lru: lru::Lru::new(lru),
+            delete: DeletedEntries::default(),
             sync_table: SyncTable::new(index),
+            config: PhantomData,
         }
     }
 
@@ -222,16 +223,7 @@ where
         // FIXME: Use `Box::into_non_null` once stable
         let memo = NonNull::from(Box::leak(Box::new(memo)));
 
-        if let Some(old_value) =
-            self.insert_memo_into_table_for(zalsa, id, memo, memo_ingredient_index)
-        {
-            // In case there is a reference to the old memo out there, we have to store it
-            // in the deleted entries. This will get cleared when a new revision starts.
-            //
-            // SAFETY: Once the revision starts, there will be no outstanding borrows to the
-            // memo contents, and so it will be safe to free.
-            unsafe { self.deleted_entries.push(old_value) };
-        }
+        self.insert_memo_into_table_for(zalsa, id, memo, memo_ingredient_index);
         // SAFETY: memo has been inserted into the table
         unsafe { self.extend_memo_lifetime(memo.as_ref()) }
     }
@@ -344,16 +336,19 @@ where
         true
     }
 
-    fn reset_for_new_revision(&mut self, table: &mut Table) {
+    fn reset_for_new_revision(
+        &mut self,
+        table: &mut Table,
+        new_buffer: DeletedEntries,
+    ) -> DeletedEntries {
         self.lru.for_each_evicted(|evict| {
             let ingredient_index = table.ingredient_index(evict);
             Self::evict_value_from_memo_for(
                 table.memos_mut(evict),
                 self.memo_ingredient_indices.get(ingredient_index),
             )
         });
-
-        self.deleted_entries.clear();
+        mem::replace(&mut self.delete, new_buffer)
     }
 
     fn debug_name(&self) -> &'static str {

src/function/delete.rs

@@ -19,24 +19,23 @@ use crate::{Event, EventKind, Id, Revision};
 impl<C: Configuration> IngredientImpl<C> {
     /// Inserts the memo for the given key; (atomically) overwrites and returns any previously existing memo
     pub(super) fn insert_memo_into_table_for<'db>(
-        &self,
+        &'db self,
         zalsa: &'db Zalsa,
         id: Id,
         memo: NonNull<Memo<'db, C>>,
         memo_ingredient_index: MemoIngredientIndex,
-    ) -> Option<NonNull<Memo<'db, C>>> {
+    ) {
         // SAFETY: The table stores 'static memos (to support `Any`), the memos are in fact valid
         // for `'db` though as we delay their dropping to the end of a revision.
         let static_memo =
             unsafe { transmute::<NonNull<Memo<'db, C>>, NonNull<Memo<'static, C>>>(memo) };
         let old_static_memo = zalsa
             .memo_table_for::<C::SalsaStruct<'_>>(id)
-            .insert(memo_ingredient_index, static_memo)?;
-        // SAFETY: The table stores 'static memos (to support `Any`), the memos are in fact valid
-        // for `'db` though as we delay their dropping to the end of a revision.
-        Some(unsafe {
-            transmute::<NonNull<Memo<'static, C>>, NonNull<Memo<'db, C>>>(old_static_memo)
-        })
+            .insert(memo_ingredient_index, static_memo);
+        if let Some(old_memo) = old_static_memo {
+            // SAFETY: We delay clearing properly
+            unsafe { self.delete.push(old_memo) };
+        }
     }
 
     /// Loads the current memo for `key_index`. This does not hold any sort of
@@ -62,9 +61,10 @@ impl<C: Configuration> IngredientImpl<C> {
     pub(super) fn evict_value_from_memo_for(
         table: MemoTableWithTypesMut<'_>,
         memo_ingredient_index: MemoIngredientIndex,
+        //FIXME should provide a page to move the value into so we can delay the drop
     ) {
-        let map = |memo: &mut Memo<'static, C>| {
-            match memo.revisions.origin.as_ref() {
+        if let Some(memo) = table.fetch::<Memo<'static, C>>(memo_ingredient_index) {
+            match &memo.revisions.origin.as_ref() {
                 QueryOriginRef::Assigned(_)
                 | QueryOriginRef::DerivedUntracked(_)
                 | QueryOriginRef::FixpointInitial => {
@@ -73,14 +73,9 @@ impl<C: Configuration> IngredientImpl<C> {
                     // or those with untracked inputs
                     // as their values cannot be reconstructed.
                 }
-                QueryOriginRef::Derived(_) => {
-                    // Set the memo value to `None`.
-                    memo.value = None;
-                }
+                QueryOriginRef::Derived(_) => _ = memo.value.take(),
             }
-        };
-
-        table.map_memo(memo_ingredient_index, map)
+        }
     }
 }
 
@@ -333,6 +328,10 @@ where
             },
         }
     }
+
+    fn clear_value(&mut self) {
+        self.value = None;
+    }
 }
 
 pub(super) enum TryClaimHeadsResult<'me> {

src/function/memo.rs

@@ -9,7 +9,7 @@ use crate::database::RawDatabase;
 use crate::function::VerifyResult;
 use crate::runtime::Running;
 use crate::sync::Arc;
-use crate::table::memo::MemoTableTypes;
+use crate::table::memo::{DeletedEntries, MemoTableTypes};
 use crate::table::Table;
 use crate::zalsa::{transmute_data_mut_ptr, transmute_data_ptr, IngredientIndex, Zalsa};
 use crate::zalsa_local::QueryOriginRef;
@@ -128,8 +128,12 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
     ///
     /// **Important:** to actually receive resets, the ingredient must set
     /// [`IngredientRequiresReset::RESET_ON_NEW_REVISION`] to true.
-    fn reset_for_new_revision(&mut self, table: &mut Table) {
-        _ = table;
+    fn reset_for_new_revision(
+        &mut self,
+        table: &mut Table,
+        new_buffer: DeletedEntries,
+    ) -> DeletedEntries {
+        _ = (table, new_buffer);
         panic!(
             "Ingredient `{}` set `Ingredient::requires_reset_for_new_revision` to true but does \
             not overwrite `Ingredient::reset_for_new_revision`",

src/ingredient.rs

@@ -66,7 +66,7 @@ pub use self::revision::Revision;
 pub use self::runtime::Runtime;
 pub use self::storage::{Storage, StorageHandle};
 pub use self::update::Update;
-pub use self::zalsa::IngredientIndex;
+pub use self::zalsa::{DeletedEntriesDropper, DropChannelReceiver, IngredientIndex};
 pub use crate::attach::{attach, with_attached_database};
 
 pub mod prelude {

src/lib.rs

@@ -2,9 +2,11 @@ use self::dependency_graph::DependencyGraph;
 use crate::durability::Durability;
 use crate::function::SyncGuard;
 use crate::key::DatabaseKeyIndex;
+use crate::plumbing::Ingredient;
 use crate::sync::atomic::{AtomicBool, Ordering};
 use crate::sync::thread::{self, ThreadId};
 use crate::sync::Mutex;
+use crate::table::memo::DeletedEntries;
 use crate::table::Table;
 use crate::zalsa::Zalsa;
 use crate::{Cancelled, Event, EventKind, Revision};
@@ -194,10 +196,6 @@ impl Runtime {
         &self.table
     }
 
-    pub(crate) fn table_mut(&mut self) -> &mut Table {
-        &mut self.table
-    }
-
     /// Increments the "current revision" counter and clears
     /// the cancellation flag.
     ///
@@ -263,4 +261,12 @@ impl Runtime {
             .lock()
             .unblock_runtimes_blocked_on(database_key, wait_result);
     }
+
+    pub(crate) fn reset_ingredient_for_new_revision(
+        &mut self,
+        ingredient: &mut (dyn Ingredient + 'static),
+        new_buffer: DeletedEntries,
+    ) -> DeletedEntries {
+        ingredient.reset_for_new_revision(&mut self.table, new_buffer)
+    }
 }