Merge pull request #685 from Veykril/veykril/push-lzwyywmqwmuk

Allow triggering LRU eviction without increasing the current revision

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

@@ -95,8 +95,8 @@ macro_rules! setup_input_struct {
 
                 pub fn ingredient_mut(db: &mut dyn $zalsa::Database) -> (&mut $zalsa_struct::IngredientImpl<Self>, &mut $zalsa::Runtime) {
                     let zalsa_mut = db.zalsa_mut();
+                    let current_revision = zalsa_mut.new_revision();
                     let index = zalsa_mut.add_or_lookup_jar_by_type(&<$zalsa_struct::JarImpl<$Configuration>>::default());
-                    let current_revision = zalsa_mut.current_revision();
                     let (ingredient, runtime) = zalsa_mut.lookup_ingredient_mut(index);
                     let ingredient = ingredient.assert_type_mut::<$zalsa_struct::IngredientImpl<Self>>();
                     (ingredient, runtime)

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

@@ -136,6 +136,13 @@ macro_rules! setup_tracked_fn {
                     })
                 }
 
+                pub fn fn_ingredient_mut(db: &mut dyn $Db) -> &mut $zalsa::function::IngredientImpl<Self> {
+                    let zalsa_mut = db.zalsa_mut();
+                    let index = zalsa_mut.add_or_lookup_jar_by_type(&$Configuration);
+                    let (ingredient, _) = zalsa_mut.lookup_ingredient_mut(index);
+                    ingredient.assert_type_mut::<$zalsa::function::IngredientImpl<Self>>()
+                }
+
                 $zalsa::macro_if! { $needs_interner =>
                     fn intern_ingredient(
                         db: &dyn $Db,
@@ -221,8 +228,8 @@ macro_rules! setup_tracked_fn {
                         struct_index,
                         first_index,
                         aux,
+                        $lru
                     );
-                    fn_ingredient.set_capacity($lru);
                     $zalsa::macro_if! {
                         if $needs_interner {
                             vec![
@@ -278,9 +285,15 @@ macro_rules! setup_tracked_fn {
                 }
 
                 $zalsa::macro_if! { if0 $lru { } else {
+                    /// Sets the lru capacity
+                    ///
+                    /// **WARNING:** Just like an ordinary write, this method triggers
+                    /// cancellation. If you invoke it while a snapshot exists, it
+                    /// will block until that snapshot is dropped -- if that snapshot
+                    /// is owned by the current thread, this could trigger deadlock.
                     #[allow(dead_code)]
-                    fn set_lru_capacity(db: &dyn $Db, value: usize) {
-                        $Configuration::fn_ingredient(db).set_capacity(value);
+                    fn set_lru_capacity(db: &mut dyn $Db, value: usize) {
+                        $Configuration::fn_ingredient_mut(db).set_capacity(value);
                     }
                 } }
             }

src/accumulator.rs

@@ -13,7 +13,6 @@ use crate::{
     cycle::CycleRecoveryStrategy,
     ingredient::{fmt_index, Ingredient, Jar, MaybeChangedAfter},
     plumbing::JarAux,
-    table::Table,
     zalsa::IngredientIndex,
     zalsa_local::QueryOrigin,
     Database, DatabaseKeyIndex, Id, Revision,
@@ -134,14 +133,6 @@ impl<A: Accumulator> Ingredient for IngredientImpl<A> {
     ) {
     }
 
-    fn requires_reset_for_new_revision(&self) -> bool {
-        false
-    }
-
-    fn reset_for_new_revision(&mut self, _: &mut Table) {
-        panic!("unexpected reset on accumulator")
-    }
-
     fn fmt_index(&self, index: Option<crate::Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt_index(A::DEBUG_NAME, index, fmt)
     }

src/database.rs

@@ -18,17 +18,30 @@ pub trait Database: Send + AsDynDatabase + Any + ZalsaDatabase {
     /// * `event`, a fn that, if called, will create the event that occurred
     fn salsa_event(&self, event: &dyn Fn() -> Event);
 
+    /// Enforces current LRU limits, evicting entries if necessary.
+    ///
+    /// **WARNING:** Just like an ordinary write, this method triggers
+    /// cancellation. If you invoke it while a snapshot exists, it
+    /// will block until that snapshot is dropped -- if that snapshot
+    /// is owned by the current thread, this could trigger deadlock.
+    fn trigger_lru_eviction(&mut self) {
+        let zalsa_mut = self.zalsa_mut();
+        zalsa_mut.runtime_mut().reset_cancellation_flag();
+        zalsa_mut.evict_lru();
+    }
+
     /// A "synthetic write" causes the system to act *as though* some
-    /// input of durability `durability` has changed. This is mostly
-    /// useful for profiling scenarios.
+    /// input of durability `durability` has changed, triggering a new revision.
+    /// This is mostly useful for profiling scenarios.
     ///
     /// **WARNING:** Just like an ordinary write, this method triggers
     /// cancellation. If you invoke it while a snapshot exists, it
     /// will block until that snapshot is dropped -- if that snapshot
     /// is owned by the current thread, this could trigger deadlock.
     fn synthetic_write(&mut self, durability: Durability) {
         let zalsa_mut = self.zalsa_mut();
-        zalsa_mut.report_tracked_write(durability);
+        zalsa_mut.new_revision();
+        zalsa_mut.runtime_mut().report_tracked_write(durability);
     }
 
     /// Reports that the query depends on some state unknown to salsa.

src/function.rs

@@ -127,11 +127,16 @@ 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,
+        lru: usize,
+    ) -> Self {
         Self {
             index,
             memo_ingredient_index: aux.next_memo_ingredient_index(struct_index, index),
-            lru: Default::default(),
+            lru: lru::Lru::new(lru),
             deleted_entries: Default::default(),
         }
     }
@@ -143,7 +148,7 @@ where
         }
     }
 
-    pub fn set_capacity(&self, capacity: usize) {
+    pub fn set_capacity(&mut self, capacity: usize) {
         self.lru.set_capacity(capacity);
     }
 
@@ -234,8 +239,13 @@ where
     }
 
     fn reset_for_new_revision(&mut self, table: &mut Table) {
-        self.lru
-            .for_each_evicted(|evict| self.evict_value_from_memo_for(table.memos_mut(evict)));
+        self.lru.for_each_evicted(|evict| {
+            Self::evict_value_from_memo_for(
+                table.memos_mut(evict),
+                &self.deleted_entries,
+                self.memo_ingredient_index,
+            )
+        });
         std::mem::take(&mut self.deleted_entries);
     }
 

src/function/lru.rs

@@ -1,20 +1,24 @@
-use std::sync::atomic::{AtomicUsize, Ordering};
+use std::num::NonZeroUsize;
 
 use crate::{hash::FxLinkedHashSet, Id};
 
 use parking_lot::Mutex;
 
-#[derive(Default)]
 pub(super) struct Lru {
-    capacity: AtomicUsize,
+    capacity: Option<NonZeroUsize>,
     set: Mutex<FxLinkedHashSet<Id>>,
 }
 
 impl Lru {
+    pub fn new(cap: usize) -> Self {
+        Self {
+            capacity: NonZeroUsize::new(cap),
+            set: Mutex::new(FxLinkedHashSet::default()),
+        }
+    }
+
     pub(super) fn record_use(&self, index: Id) {
-        // Relaxed should be fine, we don't need to synchronize on this.
-        let capacity = self.capacity.load(Ordering::Relaxed);
-        if capacity == 0 {
+        if self.capacity.is_none() {
             // LRU is disabled
             return;
         }
@@ -23,22 +27,18 @@ impl Lru {
         set.insert(index);
     }
 
-    pub(super) fn set_capacity(&self, capacity: usize) {
-        // Relaxed should be fine, we don't need to synchronize on this.
-        self.capacity.store(capacity, Ordering::Relaxed);
+    pub(super) fn set_capacity(&mut self, capacity: usize) {
+        self.capacity = NonZeroUsize::new(capacity);
     }
 
-    pub(super) fn for_each_evicted(&self, mut cb: impl FnMut(Id)) {
-        let mut set = self.set.lock();
-        // Relaxed should be fine, we don't need to synchronize on this.
-        let cap = self.capacity.load(Ordering::Relaxed);
-        if set.len() <= cap || cap == 0 {
+    pub(super) fn for_each_evicted(&mut self, mut cb: impl FnMut(Id)) {
+        let Some(cap) = self.capacity else {
             return;
-        }
-        while let Some(id) = set.pop_front() {
-            cb(id);
-            if set.len() <= cap {
-                break;
+        };
+        let set = self.set.get_mut();
+        while set.len() > cap.get() {
+            if let Some(id) = set.pop_front() {
+                cb(id);
             }
         }
     }

src/function/memo.rs

@@ -5,8 +5,10 @@ use std::mem::ManuallyDrop;
 use std::sync::Arc;
 
 use crate::accumulator::accumulated_map::InputAccumulatedValues;
+use crate::function::DeletedEntries;
 use crate::revision::AtomicRevision;
 use crate::table::memo::MemoTable;
+use crate::zalsa::MemoIngredientIndex;
 use crate::zalsa_local::QueryOrigin;
 use crate::{
     key::DatabaseKeyIndex, zalsa::Zalsa, zalsa_local::QueryRevisions, Event, EventKind, Id,
@@ -70,7 +72,11 @@ 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(
+        table: &mut MemoTable,
+        deleted_entries: &DeletedEntries<C>,
+        memo_ingredient_index: MemoIngredientIndex,
+    ) {
         let map = |memo: ArcMemo<'static, C>| -> ArcMemo<'static, C> {
             match &memo.revisions.origin {
                 QueryOrigin::Assigned(_)
@@ -111,11 +117,11 @@ impl<C: Configuration> IngredientImpl<C> {
             }
         };
         // SAFETY: We queue the old value for deletion, delaying its drop until the next revision bump.
-        let old = unsafe { table.map_memo(self.memo_ingredient_index, map) };
+        let old = unsafe { table.map_memo(memo_ingredient_index, map) };
         if let Some(old) = old {
             // 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(ManuallyDrop::into_inner(old));
+            deleted_entries.push(ManuallyDrop::into_inner(old));
         }
     }
 }

src/ingredient.rs

@@ -7,7 +7,7 @@ use crate::{
     accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues},
     cycle::CycleRecoveryStrategy,
     table::Table,
-    zalsa::{IngredientIndex, MemoIngredientIndex},
+    zalsa::{transmute_data_mut_ptr, transmute_data_ptr, IngredientIndex, MemoIngredientIndex},
     zalsa_local::QueryOrigin,
     Database, DatabaseKeyIndex, Id,
 };
@@ -111,7 +111,9 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
 
     /// Returns true if `reset_for_new_revision` should be called when new revisions start.
     /// Invoked once when ingredient is added and not after that.
-    fn requires_reset_for_new_revision(&self) -> bool;
+    fn requires_reset_for_new_revision(&self) -> bool {
+        false
+    }
 
     /// Invoked when a new revision is about to start.
     /// This moment is important because it means that we have an `&mut`-reference to the
@@ -123,7 +125,14 @@ 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);
+    fn reset_for_new_revision(&mut self, table: &mut Table) {
+        _ = table;
+        panic!(
+            "Ingredient `{}` set `Ingredient::requires_reset_for_new_revision` to true but does \
+            not overwrite `Ingredient::reset_for_new_revision`",
+            self.debug_name()
+        );
+    }
 
     fn fmt_index(&self, index: Option<crate::Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result;
 }
@@ -141,9 +150,7 @@ impl dyn Ingredient {
 
         // SAFETY: We know that the underlying data pointer
         // refers to a value of type T because of the `TypeId` check above.
-        let this: *const dyn Ingredient = self;
-        let this = this as *const T; // discards the vtable
-        unsafe { &*this }
+        unsafe { transmute_data_ptr(self) }
     }
 
     /// Equivalent to the `downcast` methods on `any`.
@@ -158,9 +165,7 @@ impl dyn Ingredient {
 
         // SAFETY: We know that the underlying data pointer
         // refers to a value of type T because of the `TypeId` check above.
-        let this: *mut dyn Ingredient = self;
-        let this = this as *mut T; // discards the vtable
-        unsafe { &mut *this }
+        unsafe { transmute_data_mut_ptr(self) }
     }
 }
 

src/input.rs

@@ -256,14 +256,6 @@ impl<C: Configuration> Ingredient for IngredientImpl<C> {
         );
     }
 
-    fn requires_reset_for_new_revision(&self) -> bool {
-        false
-    }
-
-    fn reset_for_new_revision(&mut self, _: &mut Table) {
-        panic!("unexpected call to `reset_for_new_revision`")
-    }
-
     fn fmt_index(&self, index: Option<Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt_index(C::DEBUG_NAME, index, fmt)
     }

src/input/input_field.rs

@@ -1,7 +1,6 @@
 use crate::cycle::CycleRecoveryStrategy;
 use crate::ingredient::{fmt_index, Ingredient, MaybeChangedAfter};
 use crate::input::Configuration;
-use crate::table::Table;
 use crate::zalsa::IngredientIndex;
 use crate::zalsa_local::QueryOrigin;
 use crate::{Database, DatabaseKeyIndex, Id, Revision};
@@ -82,14 +81,6 @@ where
     ) {
     }
 
-    fn requires_reset_for_new_revision(&self) -> bool {
-        false
-    }
-
-    fn reset_for_new_revision(&mut self, _: &mut Table) {
-        panic!("unexpected call: input fields don't register for resets");
-    }
-
     fn fmt_index(&self, index: Option<crate::Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt_index(C::FIELD_DEBUG_NAMES[self.field_index], index, fmt)
     }

src/interned.rs

@@ -7,7 +7,7 @@ use crate::key::InputDependencyIndex;
 use crate::plumbing::{Jar, JarAux};
 use crate::table::memo::MemoTable;
 use crate::table::sync::SyncTable;
-use crate::table::{Slot, Table};
+use crate::table::Slot;
 use crate::zalsa::IngredientIndex;
 use crate::zalsa_local::QueryOrigin;
 use crate::{Database, DatabaseKeyIndex, Id};
@@ -323,17 +323,13 @@ where
         );
     }
 
+    // Interned ingredients do not, normally, get deleted except when they are "reset" en masse.
+    // There ARE methods (e.g., `clear_deleted_entries` and `remove`) for deleting individual
+    // items, but those are only used for tracked struct ingredients.
     fn requires_reset_for_new_revision(&self) -> bool {
         false
     }
 
-    fn reset_for_new_revision(&mut self, _: &mut Table) {
-        // Interned ingredients do not, normally, get deleted except when they are "reset" en masse.
-        // There ARE methods (e.g., `clear_deleted_entries` and `remove`) for deleting individual
-        // items, but those are only used for tracked struct ingredients.
-        panic!("unexpected call to `reset_for_new_revision`")
-    }
-
     fn fmt_index(&self, index: Option<crate::Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt_index(C::DEBUG_NAME, index, fmt)
     }

src/runtime.rs

@@ -132,6 +132,11 @@ impl Runtime {
         self.revision_canceled.store(true, Ordering::Release);
     }
 
+    pub(crate) fn reset_cancellation_flag(&mut self) {
+        *self.revision_canceled.get_mut() = false;
+    }
+
+    /// Returns the [`Table`] used to store the value of salsa structs
     pub(crate) fn table(&self) -> &Table {
         &self.table
     }
@@ -148,7 +153,6 @@ impl Runtime {
         let r_old = self.current_revision();
         let r_new = r_old.next();
         self.revisions[0] = r_new;
-        self.revision_canceled.store(false, Ordering::Release);
         r_new
     }