Move memo dropping off to another thread

Author: Veykril

Cargo.toml

@@ -14,6 +14,7 @@ salsa-macros = { version = "0.22.0", path = "components/salsa-macros", optional
 
 boxcar = { version = "0.2.12" }
 crossbeam-queue = "0.3.11"
+crossbeam-utils = "0.8.21"
 dashmap = { version = "6", features = ["raw-api"] }
 # the version of hashbrown used by dashmap
 hashbrown_14 = { version = "0.14", package = "hashbrown" }

components/salsa-macro-rules/src/setup_tracked_fn.rs

@@ -243,6 +243,7 @@ macro_rules! setup_tracked_fn {
                     zalsa: &$zalsa::Zalsa,
                     first_index: $zalsa::IngredientIndex,
                     struct_index: $zalsa::IngredientIndices,
+                    memo_drop_sender: $zalsa::MemoDropSender,
                 ) -> Vec<Box<dyn $zalsa::Ingredient>> {
                     let struct_index: $zalsa::IngredientIndices = $zalsa::macro_if! {
                         if $needs_interner {
@@ -276,12 +277,16 @@ macro_rules! setup_tracked_fn {
                         )
                     };
 
-                    let fn_ingredient = <$zalsa::function::IngredientImpl<$Configuration>>::new(
-                        first_index,
-                        memo_ingredient_indices,
-                        $lru,
-                        zalsa.views().downcaster_for::<dyn $Db>(),
-                    );
+                    // SAFETY: We pass the MemoEntryType for this Configuration, and we lookup the memo types table correctly.
+                    let fn_ingredient = unsafe {
+                        <$zalsa::function::IngredientImpl<$Configuration>>::new(
+                            first_index,
+                            memo_ingredient_indices,
+                            $lru,
+                            zalsa.views().downcaster_for::<dyn $Db>(),
+                            memo_drop_sender
+                        )
+                    };
                     $zalsa::macro_if! {
                         if $needs_interner {
                             vec![

examples/lazy-input/main.rs

@@ -89,15 +89,18 @@ impl LazyInputDatabase {
     fn new(tx: Sender<DebounceEventResult>) -> Self {
         let logs: Arc<Mutex<Vec<String>>> = Default::default();
         Self {
-            storage: Storage::new(Some(Box::new({
-                let logs = logs.clone();
-                move |event| {
-                    // don't log boring events
-                    if let salsa::EventKind::WillExecute { .. } = event.kind {
-                        logs.lock().unwrap().push(format!("{event:?}"));
+            storage: Storage::new(
+                false,
+                Some(Box::new({
+                    let logs = logs.clone();
+                    move |event| {
+                        // don't log boring events
+                        if let salsa::EventKind::WillExecute { .. } = event.kind {
+                            logs.lock().unwrap().push(format!("{event:?}"));
+                        }
                     }
-                }
-            }))),
+                })),
+            ),
             logs,
             files: DashMap::new(),
             file_watcher: Arc::new(Mutex::new(

src/accumulator.rs

@@ -10,7 +10,7 @@ use accumulated::{Accumulated, AnyAccumulated};
 use crate::cycle::CycleHeads;
 use crate::function::VerifyResult;
 use crate::ingredient::{Ingredient, Jar};
-use crate::plumbing::{IngredientIndices, ZalsaLocal};
+use crate::plumbing::{IngredientIndices, MemoDropSender, ZalsaLocal};
 use crate::sync::Arc;
 use crate::table::memo::MemoTableTypes;
 use crate::zalsa::{IngredientIndex, Zalsa};
@@ -47,6 +47,7 @@ impl<A: Accumulator> Jar for JarImpl<A> {
         _zalsa: &Zalsa,
         first_index: IngredientIndex,
         _dependencies: IngredientIndices,
+        _: MemoDropSender,
     ) -> Vec<Box<dyn Ingredient>> {
         vec![Box::new(<IngredientImpl<A>>::new(first_index))]
     }

src/database_impl.rs

@@ -14,13 +14,16 @@ impl Default for DatabaseImpl {
     fn default() -> Self {
         Self {
             // Default behavior: tracing debug log the event.
-            storage: Storage::new(if tracing::enabled!(Level::DEBUG) {
-                Some(Box::new(|event| {
-                    tracing::debug!("salsa_event({:?})", event)
-                }))
-            } else {
-                None
-            }),
+            storage: Storage::new(
+                false,
+                if tracing::enabled!(Level::DEBUG) {
+                    Some(Box::new(|event| {
+                        tracing::debug!("salsa_event({:?})", event)
+                    }))
+                } else {
+                    None
+                },
+            ),
         }
     }
 }

src/exclusive.rs

@@ -0,0 +1,20 @@
+//! Bare-bones polyfill for the unstable [`std::sync::Exclusive`] type.
+
+pub struct Exclusive<T: ?Sized> {
+    inner: T,
+}
+
+// SAFETY: We only hand out mutable access to the inner value through a mutable reference to the
+// wrapper.
+// Therefore we cannot alias the inner value making it trivially sync.
+unsafe impl<T> Sync for Exclusive<T> {}
+
+impl<T> Exclusive<T> {
+    pub fn new(inner: T) -> Self {
+        Self { inner }
+    }
+
+    pub fn get_mut(&mut self) -> &mut T {
+        &mut self.inner
+    }
+}

src/function.rs

@@ -1,16 +1,16 @@
 use std::any::Any;
 use std::fmt;
+use std::marker::PhantomData;
 use std::ptr::NonNull;
 
 pub(crate) use maybe_changed_after::VerifyResult;
 
 use crate::accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues};
 use crate::cycle::{CycleHeadKind, CycleHeads, CycleRecoveryAction, CycleRecoveryStrategy};
-use crate::function::delete::DeletedEntries;
 use crate::function::sync::{ClaimResult, SyncTable};
 use crate::ingredient::Ingredient;
 use crate::key::DatabaseKeyIndex;
-use crate::plumbing::MemoIngredientMap;
+use crate::plumbing::{MemoDropSender, MemoIngredientMap};
 use crate::salsa_struct::SalsaStructInDb;
 use crate::sync::Arc;
 use crate::table::memo::MemoTableTypes;
@@ -22,7 +22,6 @@ use crate::{Database, Id, Revision};
 
 mod accumulated;
 mod backdate;
-mod delete;
 mod diff_outputs;
 mod execute;
 mod fetch;
@@ -137,7 +136,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: MemoDropSender,
+    config: PhantomData<fn(C) -> C>,
 }
 
 impl<C> IngredientImpl<C>
@@ -149,14 +149,16 @@ where
         memo_ingredient_indices: <C::SalsaStruct<'static> as SalsaStructInDb>::MemoIngredientMap,
         lru: usize,
         view_caster: DatabaseDownCaster<C::DbView>,
+        delete: MemoDropSender,
     ) -> Self {
         Self {
             index,
             memo_ingredient_indices,
             lru: lru::Lru::new(lru),
-            deleted_entries: Default::default(),
             view_caster,
             sync_table: SyncTable::new(index),
+            delete,
+            config: PhantomData,
         }
     }
 
@@ -196,16 +198,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()) }
     }
@@ -301,10 +294,9 @@ where
             Self::evict_value_from_memo_for(
                 table.memos_mut(evict),
                 self.memo_ingredient_indices.get(ingredient_index),
+                &self.delete,
             )
         });
-
-        self.deleted_entries.clear();
     }
 
     fn debug_name(&self) -> &'static str {

src/function/delete.rs

@@ -6,6 +6,7 @@ use std::ptr::NonNull;
 use crate::cycle::{empty_cycle_heads, CycleHeadKind, CycleHeads};
 use crate::function::{Configuration, IngredientImpl};
 use crate::key::DatabaseKeyIndex;
+use crate::plumbing::MemoDropSender;
 use crate::revision::AtomicRevision;
 use crate::sync::atomic::Ordering;
 use crate::table::memo::MemoTableWithTypesMut;
@@ -16,27 +17,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<C::Output<'db>>>,
         memo_ingredient_index: MemoIngredientIndex,
-    ) -> Option<NonNull<Memo<C::Output<'db>>>> {
+    ) {
         // 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<C::Output<'db>>>, NonNull<Memo<C::Output<'static>>>>(memo)
         };
-        let old_static_memo = zalsa
+        let old_memo = zalsa
             .memo_table_for(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<C::Output<'static>>>, NonNull<Memo<C::Output<'db>>>>(
-                old_static_memo,
-            )
-        })
+            .insert(memo_ingredient_index, static_memo);
+        if let Some(old_memo) = old_memo {
+            self.delete.delay(old_memo);
+        }
     }
 
     /// Loads the current memo for `key_index`. This does not hold any sort of
@@ -62,9 +59,11 @@ impl<C: Configuration> IngredientImpl<C> {
     pub(super) fn evict_value_from_memo_for(
         table: MemoTableWithTypesMut<'_>,
         memo_ingredient_index: MemoIngredientIndex,
+        delayed: &MemoDropSender,
     ) {
-        let map = |memo: &mut Memo<C::Output<'static>>| {
-            match &memo.revisions.origin {
+        if let Some(memo) = table.fetch_raw::<Memo<C::Output<'static>>>(memo_ingredient_index) {
+            // SAFETY: The memo is live
+            match unsafe { &memo.as_ref().revisions.origin } {
                 QueryOrigin::Assigned(_)
                 | QueryOrigin::DerivedUntracked(_)
                 | QueryOrigin::FixpointInitial => {
@@ -73,14 +72,9 @@ impl<C: Configuration> IngredientImpl<C> {
                     // or those with untracked inputs
                     // as their values cannot be reconstructed.
                 }
-                QueryOrigin::Derived(_) => {
-                    // Set the memo value to `None`.
-                    memo.value = None;
-                }
+                QueryOrigin::Derived(_) => delayed.clear_value(memo),
             }
-        };
-
-        table.map_memo(memo_ingredient_index, map)
+        }
     }
 }
 
@@ -255,4 +249,7 @@ impl<V: Send + Sync + Any> crate::table::memo::Memo for Memo<V> {
     fn origin(&self) -> &QueryOrigin {
         &self.revisions.origin
     }
+    fn clear_value(&mut self) {
+        self.value = None;
+    }
 }

src/function/memo.rs

@@ -4,7 +4,7 @@ use std::fmt;
 use crate::accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues};
 use crate::cycle::{CycleHeadKind, CycleHeads, CycleRecoveryStrategy};
 use crate::function::VerifyResult;
-use crate::plumbing::IngredientIndices;
+use crate::plumbing::{IngredientIndices, MemoDropSender};
 use crate::sync::Arc;
 use crate::table::memo::MemoTableTypes;
 use crate::table::Table;
@@ -33,6 +33,7 @@ pub trait Jar: Any {
         zalsa: &Zalsa,
         first_index: IngredientIndex,
         dependencies: IngredientIndices,
+        memo_drop_sender: MemoDropSender,
     ) -> Vec<Box<dyn Ingredient>>
     where
         Self: Sized;