Move memo dropping off to another thread

Author: Veykril

Cargo.toml

@@ -14,6 +14,7 @@ salsa-macros = { version = "0.21.1", 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"] }
 hashbrown = "0.15"
 hashlink = "0.10"

benches/compare.rs

@@ -4,7 +4,7 @@ use std::mem::transmute;
 use codspeed_criterion_compat::{
     criterion_group, criterion_main, BatchSize, BenchmarkId, Criterion,
 };
-use salsa::Setter;
+use salsa::{Database, Setter};
 
 include!("shims/global_alloc_overwrite.rs");
 
@@ -56,13 +56,16 @@ fn mutating_inputs(c: &mut Criterion) {
         group.bench_function(BenchmarkId::new("mutating", n), |b| {
             b.iter_batched_ref(
                 || {
-                    let db = salsa::DatabaseImpl::default();
+                    let mut db = salsa::DatabaseImpl::default();
                     let base_string = "hello, world!".to_owned();
                     let base_len = base_string.len();
 
                     let string = base_string.clone().repeat(*n);
                     let new_len = string.len();
 
+                    // spawn the LRU thread
+                    db.synthetic_write(salsa::Durability::HIGH);
+
                     let input = Input::new(black_box(&db), black_box(base_string.clone()));
                     let actual_len = length(&db, input);
                     assert_eq!(black_box(actual_len), base_len);

benches/incremental.rs

@@ -1,7 +1,7 @@
 use std::hint::black_box;
 
 use codspeed_criterion_compat::{criterion_group, criterion_main, BatchSize, Criterion};
-use salsa::Setter;
+use salsa::{Database, Setter};
 
 include!("shims/global_alloc_overwrite.rs");
 
@@ -32,7 +32,9 @@ fn many_tracked_structs(criterion: &mut Criterion) {
     criterion.bench_function("many_tracked_structs", |b| {
         b.iter_batched_ref(
             || {
-                let db = salsa::DatabaseImpl::new();
+                let mut db = salsa::DatabaseImpl::new();
+                // spawn the LRU thread
+                db.synthetic_write(salsa::Durability::HIGH);
 
                 let input = Input::new(black_box(&db), black_box(1_000));
                 let input2 = Input::new(black_box(&db), black_box(1));

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

@@ -249,6 +249,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 {
@@ -282,12 +283,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![

src/accumulator.rs

@@ -11,7 +11,7 @@ use crate::cycle::CycleHeads;
 use crate::function::VerifyResult;
 use crate::ingredient::{Ingredient, Jar};
 use crate::loom::sync::Arc;
-use crate::plumbing::{IngredientIndices, ZalsaLocal};
+use crate::plumbing::{IngredientIndices, MemoDropSender, ZalsaLocal};
 use crate::table::memo::MemoTableTypes;
 use crate::zalsa::{IngredientIndex, Zalsa};
 use crate::{Database, Id, Revision};
@@ -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/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,17 +1,17 @@
 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::loom::sync::Arc;
-use crate::plumbing::MemoIngredientMap;
+use crate::plumbing::{MemoDropSender, MemoIngredientMap};
 use crate::salsa_struct::SalsaStructInDb;
 use crate::table::memo::MemoTableTypes;
 use crate::table::Table;
@@ -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

@@ -7,6 +7,7 @@ use crate::cycle::{CycleHeadKind, CycleHeads, EMPTY_CYCLE_HEADS};
 use crate::function::{Configuration, IngredientImpl};
 use crate::key::DatabaseKeyIndex;
 use crate::loom::sync::atomic::Ordering;
+use crate::plumbing::MemoDropSender;
 use crate::revision::AtomicRevision;
 use crate::table::memo::MemoTableWithTypesMut;
 use crate::zalsa::{MemoIngredientIndex, Zalsa};
@@ -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

@@ -5,7 +5,7 @@ use crate::accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues
 use crate::cycle::{CycleHeadKind, CycleHeads, CycleRecoveryStrategy};
 use crate::function::VerifyResult;
 use crate::loom::sync::Arc;
-use crate::plumbing::IngredientIndices;
+use crate::plumbing::{IngredientIndices, MemoDropSender};
 use crate::table::memo::MemoTableTypes;
 use crate::table::Table;
 use crate::zalsa::{transmute_data_mut_ptr, transmute_data_ptr, IngredientIndex, Zalsa};
@@ -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;