Force invalidation on cache eviction

Author: eggyal

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

@@ -60,6 +60,8 @@ macro_rules! setup_tracked_fn {
 
         assert_return_type_is_update: {$($assert_return_type_is_update:tt)*},
 
+        force_invalidation_on_cache_eviction: $force_invalidation_on_cache_eviction:literal,
+
         // Annoyingly macro-rules hygiene does not extend to items defined in the macro.
         // We have the procedural macro generate names for those items that are
         // not used elsewhere in the user's code.
@@ -192,6 +194,7 @@ macro_rules! setup_tracked_fn {
                     line: line!(),
                 };
                 const DEBUG_NAME: &'static str = stringify!($fn_name);
+                const FORCE_INVALIDATION_ON_CACHE_EVICTION: bool = $force_invalidation_on_cache_eviction;
 
                 type DbView = dyn $Db;
 

components/salsa-macros/src/accumulator.rs

@@ -44,6 +44,7 @@ impl AllowedOptions for Accumulator {
     const LRU: bool = false;
     const CONSTRUCTOR_NAME: bool = false;
     const ID: bool = false;
+    const FORCE_INVALIDATION_ON_CACHE_EVICTION: bool = false;
 }
 
 struct StructMacro {

components/salsa-macros/src/input.rs

@@ -62,6 +62,8 @@ impl crate::options::AllowedOptions for InputStruct {
     const CONSTRUCTOR_NAME: bool = true;
 
     const ID: bool = false;
+
+    const FORCE_INVALIDATION_ON_CACHE_EVICTION: bool = false;
 }
 
 impl SalsaStructAllowedOptions for InputStruct {

components/salsa-macros/src/interned.rs

@@ -62,6 +62,8 @@ impl crate::options::AllowedOptions for InternedStruct {
     const CONSTRUCTOR_NAME: bool = true;
 
     const ID: bool = true;
+
+    const FORCE_INVALIDATION_ON_CACHE_EVICTION: bool = false;
 }
 
 impl SalsaStructAllowedOptions for InternedStruct {

components/salsa-macros/src/options.rs

@@ -89,6 +89,8 @@ pub(crate) struct Options<A: AllowedOptions> {
     /// If this is `Some`, the value is the `<ident>`.
     pub id: Option<syn::Path>,
 
+    pub force_invalidation_on_cache_eviction: Option<syn::Ident>,
+
     /// Remember the `A` parameter, which plays no role after parsing.
     phantom: PhantomData<A>,
 }
@@ -112,6 +114,7 @@ impl<A: AllowedOptions> Default for Options<A> {
             lru: Default::default(),
             singleton: Default::default(),
             id: Default::default(),
+            force_invalidation_on_cache_eviction: Default::default(),
         }
     }
 }
@@ -133,6 +136,7 @@ pub(crate) trait AllowedOptions {
     const LRU: bool;
     const CONSTRUCTOR_NAME: bool;
     const ID: bool;
+    const FORCE_INVALIDATION_ON_CACHE_EVICTION: bool;
 }
 
 type Equals = syn::Token![=];
@@ -351,6 +355,20 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
                         "`id` option not allowed here",
                     ));
                 }
+            } else if ident == "force_invalidation_on_cache_eviction" {
+                if A::FORCE_INVALIDATION_ON_CACHE_EVICTION {
+                    if let Some(old) = options.force_invalidation_on_cache_eviction.replace(ident) {
+                        return Err(syn::Error::new(
+                            old.span(),
+                            "option `force_invalidation_on_cache_eviction` provided twice",
+                        ));
+                    }
+                } else {
+                    return Err(syn::Error::new(
+                        ident.span(),
+                        "`force_invalidation_on_cache_eviction` option not allowed here",
+                    ));
+                }
             } else {
                 return Err(syn::Error::new(
                     ident.span(),

components/salsa-macros/src/tracked_fn.rs

@@ -55,6 +55,8 @@ impl crate::options::AllowedOptions for TrackedFn {
     const CONSTRUCTOR_NAME: bool = false;
 
     const ID: bool = false;
+
+    const FORCE_INVALIDATION_ON_CACHE_EVICTION: bool = true;
 }
 
 struct Macro {
@@ -147,6 +149,8 @@ impl Macro {
         let lru = Literal::usize_unsuffixed(self.args.lru.unwrap_or(0));
 
         let return_ref: bool = self.args.return_ref.is_some();
+        let force_invalidation_on_cache_eviction: bool =
+            self.args.force_invalidation_on_cache_eviction.is_some();
 
         // The path expression is responsible for emitting the primary span in the diagnostic we
         // want, so by uniformly using `output_ty.span()` we ensure that the diagnostic is emitted
@@ -185,6 +189,7 @@ impl Macro {
                 lru: #lru,
                 return_ref: #return_ref,
                 assert_return_type_is_update: { #assert_return_type_is_update },
+                force_invalidation_on_cache_eviction: #force_invalidation_on_cache_eviction,
                 unused_names: [
                     #zalsa,
                     #Configuration,

components/salsa-macros/src/tracked_struct.rs

@@ -57,6 +57,8 @@ impl crate::options::AllowedOptions for TrackedStruct {
     const CONSTRUCTOR_NAME: bool = true;
 
     const ID: bool = false;
+
+    const FORCE_INVALIDATION_ON_CACHE_EVICTION: bool = false;
 }
 
 impl SalsaStructAllowedOptions for TrackedStruct {

src/function.rs

@@ -38,6 +38,7 @@ pub type Memo<C> = memo::Memo<<C as Configuration>::Output<'static>>;
 pub trait Configuration: Any {
     const DEBUG_NAME: &'static str;
     const LOCATION: crate::ingredient::Location;
+    const FORCE_INVALIDATION_ON_CACHE_EVICTION: bool;
 
     /// The database that this function is associated with.
     type DbView: ?Sized + crate::Database;
@@ -327,6 +328,10 @@ where
         let db = self.view_caster.downcast(db);
         self.accumulated_map(db, key_index)
     }
+
+    fn force_invalidation_on_cache_eviction(&self) -> bool {
+        C::FORCE_INVALIDATION_ON_CACHE_EVICTION
+    }
 }
 
 impl<C> std::fmt::Debug for IngredientImpl<C>

src/function/maybe_changed_after.rs

@@ -5,7 +5,7 @@ use crate::function::sync::ClaimResult;
 use crate::function::{Configuration, IngredientImpl};
 use crate::key::DatabaseKeyIndex;
 use crate::loom::sync::atomic::Ordering;
-use crate::plumbing::ZalsaLocal;
+use crate::plumbing::{Ingredient, ZalsaLocal};
 use crate::zalsa::{MemoIngredientIndex, Zalsa, ZalsaDatabase};
 use crate::zalsa_local::{QueryEdge, QueryOrigin};
 use crate::{AsDynDatabase as _, Id, Revision};
@@ -352,6 +352,10 @@ where
             old_memo = old_memo.tracing_debug()
         );
 
+        if old_memo.value.is_none() && self.force_invalidation_on_cache_eviction() {
+            return VerifyResult::Changed;
+        }
+
         let can_shallow_update = self.shallow_verify_memo(zalsa, database_key_index, old_memo);
         if can_shallow_update.yes()
             && self.validate_may_be_provisional(

src/ingredient.rs

@@ -165,6 +165,10 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
         let _ = (db, key_index);
         (None, InputAccumulatedValues::Empty)
     }
+
+    fn force_invalidation_on_cache_eviction(&self) -> bool {
+        false
+    }
 }
 
 impl dyn Ingredient {

tests/force_invalidation_on_cache_eviction.rs

@@ -0,0 +1,70 @@
+//! Tests that `force_invalidation_on_cache_eviction` causes dependent queries
+//! to be recomputed if our memo is missing, even if our result is unchanged.
+
+mod common;
+use common::LogDatabase;
+use expect_test::expect;
+use salsa::Setter;
+use test_log::test;
+
+#[salsa::input(debug)]
+struct MyInput {
+    field: u32,
+}
+
+#[salsa::tracked]
+struct MyTracked<'db> {
+    field: u32,
+}
+
+#[salsa::tracked(force_invalidation_on_cache_eviction, lru = 1)]
+fn intermediate_result(db: &dyn LogDatabase, input: MyInput) -> MyTracked<'_> {
+    db.push_log(format!("intermediate_result({input:?})"));
+    MyTracked::new(db, input.field(db) / 2)
+}
+
+#[salsa::tracked]
+fn final_result(db: &dyn LogDatabase, input: MyInput) -> u32 {
+    db.push_log(format!("final_result({input:?})"));
+    intermediate_result(db, input).field(db) * 2
+}
+
+#[test]
+fn execute() {
+    let mut db = common::LoggerDatabase::default();
+
+    let input = MyInput::new(&db, 22);
+    assert_eq!(final_result(&db, input), 22);
+    // on first run, both intermediate and final results were computed
+    db.assert_logs(expect![[r#"
+        [
+            "final_result(MyInput { [salsa id]: Id(0), field: 22 })",
+            "intermediate_result(MyInput { [salsa id]: Id(0), field: 22 })",
+        ]"#]]);
+
+    input.set_field(&mut db).to(23);
+    assert_eq!(final_result(&db, input), 22);
+    // intermediate result was the same, so final result was not recomputed
+    db.assert_logs(expect![[r#"
+        [
+            "intermediate_result(MyInput { [salsa id]: Id(0), field: 23 })",
+        ]"#]]);
+
+    assert_eq!(intermediate_result(&db, MyInput::new(&db, 10)).field(&db), 5);
+    input.set_field(&mut db).to(23);
+    // an intermediate result for a different input evicted the original memo
+    // from the cache (because query's lru = 1)
+    db.assert_logs(expect![[r#"
+        [
+            "intermediate_result(MyInput { [salsa id]: Id(1), field: 10 })",
+        ]"#]]);
+
+    assert_eq!(final_result(&db, input), 22);
+    // now, despite unchanged intermediate result, final result was recomputed
+    // (because intermediate query is `force_invalidation_on_cache_eviction`)
+    db.assert_logs(expect![[r#"
+        [
+            "final_result(MyInput { [salsa id]: Id(0), field: 23 })",
+            "intermediate_result(MyInput { [salsa id]: Id(0), field: 23 })",
+        ]"#]]);
+}