diff --git a/components/salsa-macro-rules/src/setup_tracked_fn.rs b/components/salsa-macro-rules/src/setup_tracked_fn.rs index 6b2139963..4a96037ab 100644 --- a/components/salsa-macro-rules/src/setup_tracked_fn.rs +++ b/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; diff --git a/components/salsa-macros/src/accumulator.rs b/components/salsa-macros/src/accumulator.rs index 220d0e941..e569e044d 100644 --- a/components/salsa-macros/src/accumulator.rs +++ b/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 { diff --git a/components/salsa-macros/src/input.rs b/components/salsa-macros/src/input.rs index 38241a959..0bb33f4a4 100644 --- a/components/salsa-macros/src/input.rs +++ b/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 { diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs index 646470fce..586db300a 100644 --- a/components/salsa-macros/src/interned.rs +++ b/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 { diff --git a/components/salsa-macros/src/options.rs b/components/salsa-macros/src/options.rs index 4852ee2be..06143d5c8 100644 --- a/components/salsa-macros/src/options.rs +++ b/components/salsa-macros/src/options.rs @@ -89,6 +89,8 @@ pub(crate) struct Options { /// If this is `Some`, the value is the ``. pub id: Option, + pub force_invalidation_on_cache_eviction: Option, + /// Remember the `A` parameter, which plays no role after parsing. phantom: PhantomData, } @@ -112,6 +114,7 @@ impl Default for Options { 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 syn::parse::Parse for Options { "`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(), diff --git a/components/salsa-macros/src/tracked_fn.rs b/components/salsa-macros/src/tracked_fn.rs index e8f4b71ff..527f223a7 100644 --- a/components/salsa-macros/src/tracked_fn.rs +++ b/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, diff --git a/components/salsa-macros/src/tracked_struct.rs b/components/salsa-macros/src/tracked_struct.rs index ba4b33ad3..2dbcbb42b 100644 --- a/components/salsa-macros/src/tracked_struct.rs +++ b/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 { diff --git a/src/function.rs b/src/function.rs index f7e659915..f496fe978 100644 --- a/src/function.rs +++ b/src/function.rs @@ -38,6 +38,7 @@ pub type Memo = memo::Memo<::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; diff --git a/src/function/execute.rs b/src/function/execute.rs index bcb43a8ce..c11d85dd0 100644 --- a/src/function/execute.rs +++ b/src/function/execute.rs @@ -4,7 +4,7 @@ use crate::function::{Configuration, IngredientImpl}; use crate::loom::sync::atomic::{AtomicBool, Ordering}; use crate::zalsa::{MemoIngredientIndex, Zalsa, ZalsaDatabase}; use crate::zalsa_local::{ActiveQueryGuard, QueryRevisions}; -use crate::{Event, EventKind, Id, Revision}; +use crate::{Durability, Event, EventKind, Id, Revision}; impl IngredientImpl where @@ -269,7 +269,12 @@ where // Query was not previously executed, or value is potentially // stale, or value is absent. Let's execute! let new_value = C::execute(db, C::id_to_input(db, id)); + let mut revisions = active_query.pop(); - (new_value, active_query.pop()) + if C::FORCE_INVALIDATION_ON_CACHE_EVICTION { + revisions.durability = Durability::MIN; + } + + (new_value, revisions) } } diff --git a/src/function/maybe_changed_after.rs b/src/function/maybe_changed_after.rs index 47921f033..27cfc8cf9 100644 --- a/src/function/maybe_changed_after.rs +++ b/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); diff --git a/tests/force_invalidation_on_cache_eviction.rs b/tests/force_invalidation_on_cache_eviction.rs new file mode 100644 index 000000000..0c58be7b8 --- /dev/null +++ b/tests/force_invalidation_on_cache_eviction.rs @@ -0,0 +1,61 @@ +//! 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::{Durability, Setter}; +use test_log::test; + +#[salsa::input] +struct MyInput { + field: (), +} + +#[salsa::tracked(force_invalidation_on_cache_eviction, lru = 1)] +fn intermediate_result(db: &dyn LogDatabase, input: MyInput) { + db.push_log(format!("intermediate_result({})", input.0.as_u32())); + input.field(db); +} + +#[salsa::tracked] +fn final_result(db: &dyn LogDatabase, input: MyInput) { + db.push_log("final_result".to_string()); + intermediate_result(db, input); +} + +#[test] +fn execute() { + let mut db = common::LoggerDatabase::default(); + + let high = MyInput::builder(()).durability(Durability::HIGH).new(&db); + let low = MyInput::new(&db, ()); + + final_result(&db, high); + // on first run, both intermediate and final results were computed + db.assert_logs(expect![[r#" + [ + "final_result", + "intermediate_result(0)", + ]"#]]); + + // an intermediate result for a different input will evict the original memo + // from the cache (because query's lru = 1) when revision is bumped + intermediate_result(&db, low); + db.assert_logs(expect![[r#" + [ + "intermediate_result(1)", + ]"#]]); + + // bump the revision, causing cache eviction + low.set_field(&mut db).to(()); + + final_result(&db, high); + // 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", + "intermediate_result(0)", + ]"#]]); +}