Move memo dropping off to another thread

Author: Veykril

Diff for: components/salsa-macro-rules/src/setup_tracked_fn.rs

@@ -210,6 +210,7 @@ macro_rules! setup_tracked_fn {
                     &self,
                     aux: &dyn $zalsa::JarAux,
                     first_index: $zalsa::IngredientIndex,
+                    memo_drop_sender: $zalsa::MemoDropSender,
                 ) -> Vec<Box<dyn $zalsa::Ingredient>> {
                     let struct_index = $zalsa::macro_if! {
                         if $needs_interner {
@@ -226,6 +227,7 @@ macro_rules! setup_tracked_fn {
                         struct_index,
                         first_index,
                         aux,
+                        memo_drop_sender
                     );
                     $zalsa::macro_if! {
                         if $has_lru {

Diff for: src/accumulator.rs

@@ -12,7 +12,7 @@ use accumulated::AnyAccumulated;
 use crate::{
     cycle::CycleRecoveryStrategy,
     ingredient::{fmt_index, Ingredient, Jar, MaybeChangedAfter},
-    plumbing::JarAux,
+    plumbing::{JarAux, MemoDropSender},
     table::Table,
     zalsa::IngredientIndex,
     zalsa_local::QueryOrigin,
@@ -50,6 +50,7 @@ impl<A: Accumulator> Jar for JarImpl<A> {
         &self,
         _aux: &dyn JarAux,
         first_index: IngredientIndex,
+        _: MemoDropSender,
     ) -> Vec<Box<dyn Ingredient>> {
         vec![Box::new(<IngredientImpl<A>>::new(first_index))]
     }

Diff for: src/exclusive.rs

@@ -0,0 +1,15 @@
+/// `std::sync::Exclusive`
+pub struct Exclusive<T: ?Sized> {
+    inner: T,
+}
+
+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
+    }
+}

Diff for: src/function.rs

@@ -8,19 +8,16 @@ use crate::{
     key::DatabaseKeyIndex,
     plumbing::JarAux,
     salsa_struct::SalsaStructInDb,
-    table::Table,
+    table::{memo::MemoDropSender, Table},
     zalsa::{IngredientIndex, MemoIngredientIndex, Zalsa},
     zalsa_local::QueryOrigin,
     Cycle, Database, Id, Revision,
 };
 
-use self::delete::DeletedEntries;
-
 use super::ingredient::Ingredient;
 
 mod accumulated;
 mod backdate;
-mod delete;
 mod diff_outputs;
 mod execute;
 mod fetch;
@@ -117,7 +114,7 @@ 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,
 }
 
 /// True if `old_value == new_value`. Invoked by the generated
@@ -131,12 +128,17 @@ impl<C> IngredientImpl<C>
 where
     C: Configuration,
 {
-    pub fn new(struct_index: IngredientIndex, index: IngredientIndex, aux: &dyn JarAux) -> Self {
+    pub fn new(
+        struct_index: IngredientIndex,
+        index: IngredientIndex,
+        aux: &dyn JarAux,
+        delete: MemoDropSender,
+    ) -> Self {
         Self {
             index,
             memo_ingredient_index: aux.next_memo_ingredient_index(struct_index, index),
             lru: Default::default(),
-            deleted_entries: Default::default(),
+            delete,
         }
     }
 
@@ -175,13 +177,7 @@ where
         // Unsafety conditions: memo must be in the map (it's not yet, but it will be by the time this
         // value is returned) and anything removed from map is added to deleted entries (ensured elsewhere).
         let db_memo = unsafe { self.extend_memo_lifetime(&memo) };
-        // Safety: We delay the drop of `old_value` until a new revision starts which ensures no
-        // references will exist for the memo contents.
-        if let Some(old_value) = unsafe { self.insert_memo_into_table_for(zalsa, id, memo) } {
-            // 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.
-            self.deleted_entries.push(old_value);
-        }
+        self.insert_memo_into_table_for(zalsa, id, memo);
         db_memo
     }
 }
@@ -238,9 +234,11 @@ where
     }
 
     fn reset_for_new_revision(&mut self, table: &mut Table) {
-        std::mem::take(&mut self.deleted_entries);
-        self.lru
-            .to_be_evicted(|evict| self.evict_value_from_memo_for(table.memos_mut(evict)));
+        self.lru.to_be_evicted(|evict| {
+            if let Some(memo) = self.evict_value_from_memo_for(table.memos_mut(evict)) {
+                self.delete.delay(memo);
+            }
+        });
     }
 
     fn fmt_index(&self, index: Option<crate::Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {

Diff for: src/function/delete.rs

@@ -17,7 +17,7 @@ pub trait LruChoice: sealed::Sealed + Default {
     fn to_be_evicted(&self, cb: impl FnMut(Id));
     /// Sets the capacity of the LRU.
     fn set_capacity(&self, capacity: usize);
-    fn if_enabled(f: impl FnOnce());
+    fn if_enabled<T>(f: impl FnOnce() -> T) -> Option<T>;
 
     fn evicted<T>() -> Self::LruCtor<T>
     where
@@ -53,7 +53,9 @@ impl LruChoice for NoLru {
     fn record_use(&self, _: Id) {}
     fn to_be_evicted(&self, _: impl FnMut(Id)) {}
     fn set_capacity(&self, _: usize) {}
-    fn if_enabled(_: impl FnOnce()) {}
+    fn if_enabled<T>(_: impl FnOnce() -> T) -> Option<T> {
+        None
+    }
 
     fn evicted<T>() -> Self::LruCtor<T>
     where
@@ -124,8 +126,9 @@ impl LruChoice for Lru {
             *self.set.lock() = FxLinkedHashSet::default();
         }
     }
-    fn if_enabled(f: impl FnOnce()) {
-        f();
+
+    fn if_enabled<T>(f: impl FnOnce() -> T) -> Option<T> {
+        Some(f())
     }
 
     fn evicted<T>() -> Self::LruCtor<T>

Diff for: src/function/lru.rs

@@ -37,22 +37,24 @@ impl<C: Configuration> IngredientImpl<C> {
     }
 
     /// Inserts the memo for the given key; (atomically) overwrites and returns any previously existing memo
-    ///
-    /// # Safety
-    ///
-    /// The caller needs to make sure to not drop the returned value until no more references into
-    /// the database exist as there may be outstanding borrows into the `Arc` contents.
-    pub(super) unsafe fn insert_memo_into_table_for<'db>(
+    pub(super) fn insert_memo_into_table_for<'db>(
         &'db self,
         zalsa: &'db Zalsa,
         id: Id,
         memo: ArcMemo<'db, C>,
-    ) -> Option<ArcMemo<'db, C>> {
+    ) {
         let static_memo = unsafe { self.to_static(memo) };
-        let old_static_memo = zalsa
-            .memo_table_for(id)
-            .insert(self.memo_ingredient_index, static_memo)?;
-        unsafe { Some(self.to_self(old_static_memo)) }
+        // SAFETY: We delay the deletion of the old memo until the next revision starts.
+        if let Some(old_static_memo) = unsafe {
+            zalsa
+                .memo_table_for(id)
+                .insert(self.memo_ingredient_index, static_memo)
+        } {
+            // In case there is a reference to the old memo out there, we have to delay its
+            // deletion.
+            // This will get cleared when a new revision starts.
+            self.delete.delay(old_static_memo);
+        }
     }
 
     /// Loads the current memo for `key_index`. This does not hold any sort of
@@ -70,7 +72,10 @@ impl<C: Configuration> IngredientImpl<C> {
     /// Evicts the existing memo for the given key, replacing it
     /// with an equivalent memo that has no value. If the memo is untracked, BaseInput,
     /// or has values assigned as output of another query, this has no effect.
-    pub(super) fn evict_value_from_memo_for(&self, table: &mut MemoTable) {
+    pub(super) fn evict_value_from_memo_for(
+        &self,
+        table: &mut MemoTable,
+    ) -> Option<ArcMemo<'static, C>> {
         C::Lru::if_enabled(|| {
             table.map_memo::<MemoConfigured<'_, C>>(self.memo_ingredient_index, |memo| {
                 match &memo.revisions.origin {
@@ -110,8 +115,9 @@ impl<C: Configuration> IngredientImpl<C> {
                         ))
                     }
                 }
-            });
+            })
         })
+        .flatten()
     }
 }
 

Diff for: src/function/memo.rs

@@ -6,6 +6,7 @@ use std::{
 use crate::{
     accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues},
     cycle::CycleRecoveryStrategy,
+    plumbing::MemoDropSender,
     table::Table,
     zalsa::{IngredientIndex, MemoIngredientIndex},
     zalsa_local::QueryOrigin,
@@ -23,6 +24,7 @@ pub trait Jar: Any {
         &self,
         aux: &dyn JarAux,
         first_index: IngredientIndex,
+        memo_drop_sender: MemoDropSender,
     ) -> Vec<Box<dyn Ingredient>>;
 
     /// If this jar's first ingredient is a salsa struct, return its `TypeId`

Diff for: src/ingredient.rs

@@ -17,7 +17,7 @@ use crate::{
     ingredient::{fmt_index, Ingredient, MaybeChangedAfter},
     input::singleton::{Singleton, SingletonChoice},
     key::{DatabaseKeyIndex, InputDependencyIndex},
-    plumbing::{Jar, JarAux, Stamp},
+    plumbing::{Jar, JarAux, MemoDropSender, Stamp},
     table::{memo::MemoTable, sync::SyncTable, Slot, Table},
     zalsa::{IngredientIndex, Zalsa},
     zalsa_local::QueryOrigin,
@@ -58,6 +58,7 @@ impl<C: Configuration> Jar for JarImpl<C> {
         &self,
         _aux: &dyn JarAux,
         struct_index: crate::zalsa::IngredientIndex,
+        _: MemoDropSender,
     ) -> Vec<Box<dyn Ingredient>> {
         let struct_ingredient: IngredientImpl<C> = IngredientImpl::new(struct_index);
 

Diff for: src/input.rs

@@ -4,7 +4,7 @@ use crate::accumulator::accumulated_map::InputAccumulatedValues;
 use crate::durability::Durability;
 use crate::ingredient::{fmt_index, MaybeChangedAfter};
 use crate::key::InputDependencyIndex;
-use crate::plumbing::{Jar, JarAux};
+use crate::plumbing::{Jar, JarAux, MemoDropSender};
 use crate::table::memo::MemoTable;
 use crate::table::sync::SyncTable;
 use crate::table::{Slot, Table};
@@ -92,6 +92,7 @@ impl<C: Configuration> Jar for JarImpl<C> {
         &self,
         _aux: &dyn JarAux,
         first_index: IngredientIndex,
+        _: MemoDropSender,
     ) -> Vec<Box<dyn Ingredient>> {
         vec![Box::new(IngredientImpl::<C>::new(first_index)) as _]
     }

Diff for: src/interned.rs

@@ -8,6 +8,7 @@ mod database;
 mod database_impl;
 mod durability;
 mod event;
+mod exclusive;
 mod function;
 mod hash;
 mod id;
@@ -91,6 +92,7 @@ pub mod plumbing {
     pub use crate::salsa_struct::SalsaStructInDb;
     pub use crate::storage::HasStorage;
     pub use crate::storage::Storage;
+    pub use crate::table::memo::MemoDropSender;
     pub use crate::tracked_struct::TrackedStructInDb;
     pub use crate::update::always_update;
     pub use crate::update::helper::Dispatch as UpdateDispatch;