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;

src/function/maybe_changed_after.rs

@@ -70,6 +70,10 @@ where
                 return VerifyResult::Changed;
             };
 
+            if C::FORCE_INVALIDATION_ON_CACHE_EVICTION && memo.value.is_none() {
+                return VerifyResult::Changed;
+            }
+
             let can_shallow_update = self.shallow_verify_memo(zalsa, database_key_index, memo);
             if can_shallow_update.yes() && !memo.may_be_provisional() {
                 self.update_shallow(zalsa, database_key_index, memo, can_shallow_update);

tests/force_invalidation_on_cache_eviction.rs

@@ -0,0 +1,73 @@
+//! 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 })",
+        ]"#]]);
+}