From 51c5bc45f6567376618458de93a5cd7a6023579b Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sun, 20 Apr 2025 07:52:47 +0300 Subject: [PATCH 1/2] Add a `force_durability` option This is a very dangerous option, but it is needed for rust-analyzer. --- .../salsa-macro-rules/src/setup_tracked_fn.rs | 4 ++++ components/salsa-macros/src/accumulator.rs | 1 + components/salsa-macros/src/input.rs | 2 ++ components/salsa-macros/src/interned.rs | 2 ++ components/salsa-macros/src/options.rs | 20 +++++++++++++++++++ components/salsa-macros/src/tracked_fn.rs | 7 +++++++ components/salsa-macros/src/tracked_struct.rs | 2 ++ src/function.rs | 4 +++- src/function/execute.rs | 12 +++++------ src/function/fetch.rs | 2 +- src/function/memo.rs | 1 + src/zalsa_local.rs | 8 ++++++-- 12 files changed, 55 insertions(+), 10 deletions(-) diff --git a/components/salsa-macro-rules/src/setup_tracked_fn.rs b/components/salsa-macro-rules/src/setup_tracked_fn.rs index e1619095f..a9ac72b2e 100644 --- a/components/salsa-macro-rules/src/setup_tracked_fn.rs +++ b/components/salsa-macro-rules/src/setup_tracked_fn.rs @@ -67,6 +67,8 @@ macro_rules! setup_tracked_fn { // If true, the input and output values implement `serde::{Serialize, Deserialize}`. persist: $persist:tt, + force_durability: $force_durability:expr, + assert_return_type_is_update: {$($assert_return_type_is_update:tt)*}, $(self_ty: $self_ty:ty,)? @@ -286,6 +288,8 @@ macro_rules! setup_tracked_fn { const CYCLE_STRATEGY: $zalsa::CycleRecoveryStrategy = $zalsa::CycleRecoveryStrategy::$cycle_recovery_strategy; + const FORCE_DURABILITY: Option<$zalsa::Durability> = $force_durability; + $($values_equal)+ $( diff --git a/components/salsa-macros/src/accumulator.rs b/components/salsa-macros/src/accumulator.rs index 1c443a688..1036004ed 100644 --- a/components/salsa-macros/src/accumulator.rs +++ b/components/salsa-macros/src/accumulator.rs @@ -49,6 +49,7 @@ impl AllowedOptions for Accumulator { const SELF_TY: bool = false; // TODO: Support serializing accumulators. const PERSIST: AllowedPersistOptions = AllowedPersistOptions::Invalid; + const FORCE_DURABILITY: bool = false; } struct StructMacro { diff --git a/components/salsa-macros/src/input.rs b/components/salsa-macros/src/input.rs index 9b4901419..93a10ce00 100644 --- a/components/salsa-macros/src/input.rs +++ b/components/salsa-macros/src/input.rs @@ -70,6 +70,8 @@ impl AllowedOptions for InputStruct { const SELF_TY: bool = false; const PERSIST: AllowedPersistOptions = AllowedPersistOptions::AllowedValue; + + const FORCE_DURABILITY: bool = false; } impl SalsaStructAllowedOptions for InputStruct { diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs index 8b72b3510..b5f519ddd 100644 --- a/components/salsa-macros/src/interned.rs +++ b/components/salsa-macros/src/interned.rs @@ -70,6 +70,8 @@ impl AllowedOptions for InternedStruct { const SELF_TY: bool = false; const PERSIST: AllowedPersistOptions = AllowedPersistOptions::AllowedValue; + + const FORCE_DURABILITY: bool = false; } impl SalsaStructAllowedOptions for InternedStruct { diff --git a/components/salsa-macros/src/options.rs b/components/salsa-macros/src/options.rs index b7bdc807b..d546da668 100644 --- a/components/salsa-macros/src/options.rs +++ b/components/salsa-macros/src/options.rs @@ -115,6 +115,9 @@ pub(crate) struct Options { /// functions. This is merely used to refine the query name. pub self_ty: Option, + /// Force the durability of the query to always be this value. + pub force_durability: Option, + /// Remember the `A` parameter, which plays no role after parsing. phantom: PhantomData, } @@ -157,6 +160,7 @@ impl Default for Options { heap_size_fn: Default::default(), self_ty: Default::default(), persist: Default::default(), + force_durability: Default::default(), } } } @@ -182,6 +186,7 @@ pub(crate) trait AllowedOptions { const HEAP_SIZE: bool; const SELF_TY: bool; const PERSIST: AllowedPersistOptions; + const FORCE_DURABILITY: bool; } pub(crate) enum AllowedPersistOptions { @@ -536,6 +541,17 @@ impl syn::parse::Parse for Options { "`self_ty` option not allowed here", )); } + } else if ident == "force_durability" { + if A::FORCE_DURABILITY { + let _eq = Equals::parse(input)?; + let path = syn::Expr::parse(input)?; + options.force_durability = Some(path); + } else { + return Err(syn::Error::new( + ident.span(), + "`force_durability` option not allowed here", + )); + } } else { return Err(syn::Error::new( ident.span(), @@ -575,6 +591,7 @@ impl quote::ToTokens for Options { heap_size_fn, self_ty, persist, + force_durability, phantom: _, } = self; if let Some(returns) = returns { @@ -648,5 +665,8 @@ impl quote::ToTokens for Options { tokens.extend(quote::quote! { persist(#args), }); } } + if let Some(force_durability) = force_durability { + tokens.extend(quote::quote! { force_durability = #force_durability, }); + } } } diff --git a/components/salsa-macros/src/tracked_fn.rs b/components/salsa-macros/src/tracked_fn.rs index 12f9170c7..24e13e40a 100644 --- a/components/salsa-macros/src/tracked_fn.rs +++ b/components/salsa-macros/src/tracked_fn.rs @@ -63,6 +63,8 @@ impl AllowedOptions for TrackedFn { const SELF_TY: bool = true; const PERSIST: AllowedPersistOptions = AllowedPersistOptions::AllowedIdent; + + const FORCE_DURABILITY: bool = true; } struct Macro { @@ -209,6 +211,10 @@ impl Macro { Some(ty) => quote! { self_ty: #ty, }, None => quote! {}, }; + let force_durability = match &self.args.force_durability { + Some(durability) => quote!(Some(#durability)), + None => quote!(None), + }; Ok(crate::debug::dump_tokens( fn_name, @@ -234,6 +240,7 @@ impl Macro { lru: #lru, return_mode: #return_mode, persist: #persist, + force_durability: #force_durability, assert_return_type_is_update: { #assert_return_type_is_update }, #self_ty unused_names: [ diff --git a/components/salsa-macros/src/tracked_struct.rs b/components/salsa-macros/src/tracked_struct.rs index d6ee32d47..f82d56384 100644 --- a/components/salsa-macros/src/tracked_struct.rs +++ b/components/salsa-macros/src/tracked_struct.rs @@ -66,6 +66,8 @@ impl AllowedOptions for TrackedStruct { const SELF_TY: bool = false; const PERSIST: AllowedPersistOptions = AllowedPersistOptions::AllowedValue; + + const FORCE_DURABILITY: bool = false; } impl SalsaStructAllowedOptions for TrackedStruct { diff --git a/src/function.rs b/src/function.rs index f7f302727..8d8759af8 100644 --- a/src/function.rs +++ b/src/function.rs @@ -21,7 +21,7 @@ use crate::table::Table; use crate::views::DatabaseDownCaster; use crate::zalsa::{IngredientIndex, JarKind, MemoIngredientIndex, Zalsa}; use crate::zalsa_local::{QueryEdge, QueryOriginRef}; -use crate::{Cycle, Id, Revision}; +use crate::{Cycle, Durability, Id, Revision}; #[cfg(feature = "accumulator")] mod accumulated; @@ -62,6 +62,8 @@ pub trait Configuration: Any { /// (and, if so, how). const CYCLE_STRATEGY: CycleRecoveryStrategy; + const FORCE_DURABILITY: Option; + /// Invokes after a new result `new_value` has been computed for which an older memoized value /// existed `old_value`, or in fixpoint iteration. Returns true if the new value is equal to /// the older one. diff --git a/src/function/execute.rs b/src/function/execute.rs index b0b8b8609..8580bb480 100644 --- a/src/function/execute.rs +++ b/src/function/execute.rs @@ -63,7 +63,7 @@ where zalsa_local.push_query(database_key_index, IterationCount::initial()), opt_old_memo, ); - (new_value, active_query.pop()) + (new_value, active_query.pop(C::FORCE_DURABILITY)) } CycleRecoveryStrategy::FallbackImmediate => { let (mut new_value, active_query) = Self::execute_query( @@ -73,7 +73,7 @@ where opt_old_memo, ); - let mut completed_query = active_query.pop(); + let mut completed_query = active_query.pop(C::FORCE_DURABILITY); if let Some(cycle_heads) = completed_query.revisions.cycle_heads_mut() { // Did the new result we got depend on our own provisional value, in a cycle? @@ -100,7 +100,7 @@ where let active_query = zalsa_local.push_query(database_key_index, IterationCount::initial()); new_value = C::cycle_initial(db, id, C::id_to_input(zalsa, id)); - completed_query = active_query.pop(); + completed_query = active_query.pop(C::FORCE_DURABILITY); // We need to set `cycle_heads` and `verified_final` because it needs to propagate to the callers. // When verifying this, we will see we have fallback and mark ourselves verified. completed_query.revisions.set_cycle_heads(cycle_heads); @@ -232,7 +232,7 @@ where panic!("{database_key_index:?}: execute: too many cycle iterations") }); - let mut completed_query = active_query.pop(); + let mut completed_query = active_query.pop(C::FORCE_DURABILITY); completed_query .revisions .update_iteration_count_mut(database_key_index, iteration_count); @@ -312,7 +312,7 @@ where claim_guard.set_release_mode(ReleaseMode::SelfOnly); } - let mut completed_query = active_query.pop(); + let mut completed_query = active_query.pop(C::FORCE_DURABILITY); *completed_query.revisions.verified_final.get_mut() = false; completed_query.revisions.set_cycle_heads(cycle_heads); @@ -392,7 +392,7 @@ where } } - let mut completed_query = active_query.pop(); + let mut completed_query = active_query.pop(C::FORCE_DURABILITY); let value_converged = C::values_equal(&new_value, last_provisional_value); diff --git a/src/function/fetch.rs b/src/function/fetch.rs index 588b08bb1..1c1ad30ed 100644 --- a/src/function/fetch.rs +++ b/src/function/fetch.rs @@ -265,7 +265,7 @@ where let active_query = zalsa_local.push_query(database_key_index, IterationCount::initial()); let fallback_value = C::cycle_initial(db, id, C::id_to_input(zalsa, id)); - let mut completed_query = active_query.pop(); + let mut completed_query = active_query.pop(C::FORCE_DURABILITY); completed_query .revisions .set_cycle_heads(CycleHeads::initial( diff --git a/src/function/memo.rs b/src/function/memo.rs index 234829cb1..35eb4b3b7 100644 --- a/src/function/memo.rs +++ b/src/function/memo.rs @@ -534,6 +534,7 @@ mod _memory_usage { const LOCATION: Location = Location { file: "", line: 0 }; const PERSIST: bool = false; const CYCLE_STRATEGY: CycleRecoveryStrategy = CycleRecoveryStrategy::Panic; + const FORCE_DURABILITY: Option = None; type DbView = dyn Database; type SalsaStruct<'db> = DummyStruct; diff --git a/src/zalsa_local.rs b/src/zalsa_local.rs index 8f0239e56..e58dfdfe2 100644 --- a/src/zalsa_local.rs +++ b/src/zalsa_local.rs @@ -1249,8 +1249,12 @@ impl ActiveQueryGuard<'_> { /// which summarizes the other queries that were accessed during this /// query's execution. #[inline] - pub(crate) fn pop(self) -> CompletedQuery { - self.complete() + pub(crate) fn pop(self, force_durability: Option) -> CompletedQuery { + let mut result = self.complete(); + if let Some(durability) = force_durability { + result.revisions.durability = durability; + } + result } } From 4c10d325d710bc62282e866ac1de58b08c090b63 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Thu, 12 Jun 2025 22:02:41 +0300 Subject: [PATCH 2/2] Support a new `Durability::NEVER_CHANGE` Inputs with this durability are assumed to never change. This allows us to optimize their storage a lot and not store any data about them except for the value. That makes them very compact. This is done by bit-tagging the memo and using it as one of two types. The overhead of handling the possibility of a second type will be seen in synthetic benchmarks; in practice, if using this durability (for example, for library code), speed will increase, not decrease (in addition to improved memory usage), because we don't need to validate query dependencies if they are assumed to never change. Values participating in cycles cannot have `Durability::NEVER_CHANGE`: while it is possible to support this (and in fact, an early revision of the code did) it introduces a lot of complications to the code, as we need to replace the type of a memo after it has been inserted. --- .../src/setup_input_struct.rs | 2 +- .../salsa-macro-rules/src/setup_tracked_fn.rs | 2 +- src/accumulator/accumulated_map.rs | 4 + src/active_query.rs | 13 +- src/cycle.rs | 2 + src/durability.rs | 21 +- src/function.rs | 72 ++++++- src/function/accumulated.rs | 15 +- src/function/delete.rs | 22 +- src/function/execute.rs | 62 ++++-- src/function/fetch.rs | 77 ++++--- src/function/inputs.rs | 6 +- src/function/maybe_changed_after.rs | 55 +++-- src/function/memo.rs | 183 ++++++++++++++-- src/function/specify.rs | 7 +- src/lib.rs | 2 +- src/table/memo.rs | 198 ++++++++++++++---- src/zalsa_local.rs | 16 +- tests/accumulate-chain.rs | 32 ++- tests/accumulate-execution-order.rs | 36 +++- tests/accumulate-no-duplicates.rs | 31 +-- tests/backtrace.rs | 12 +- tests/cycle_output.rs | 15 +- tests/interned-revisions.rs | 27 ++- 24 files changed, 695 insertions(+), 217 deletions(-) diff --git a/components/salsa-macro-rules/src/setup_input_struct.rs b/components/salsa-macro-rules/src/setup_input_struct.rs index d3c897045..41bbd866c 100644 --- a/components/salsa-macro-rules/src/setup_input_struct.rs +++ b/components/salsa-macro-rules/src/setup_input_struct.rs @@ -392,7 +392,7 @@ macro_rules! setup_input_struct { pub(super) fn new_builder($($field_id: $field_ty),*) -> $Builder { $Builder { fields: ($($field_id,)*), - durabilities: [::salsa::Durability::default(); $N], + durabilities: [::salsa::Durability::MIN; $N], } } diff --git a/components/salsa-macro-rules/src/setup_tracked_fn.rs b/components/salsa-macro-rules/src/setup_tracked_fn.rs index a9ac72b2e..f3c562834 100644 --- a/components/salsa-macro-rules/src/setup_tracked_fn.rs +++ b/components/salsa-macro-rules/src/setup_tracked_fn.rs @@ -391,7 +391,7 @@ macro_rules! setup_tracked_fn { zalsa, struct_index, first_index, - $zalsa::function::MemoEntryType::of::<$zalsa::function::Memo<$Configuration>>(), + $zalsa::function::MemoEntryType::of::<$zalsa::function::AmbiguousMemo<$Configuration>>(), intern_ingredient_memo_types, ) }; diff --git a/src/accumulator/accumulated_map.rs b/src/accumulator/accumulated_map.rs index 19ad63663..4a0e60e93 100644 --- a/src/accumulator/accumulated_map.rs +++ b/src/accumulator/accumulated_map.rs @@ -21,6 +21,10 @@ impl std::fmt::Debug for AccumulatedMap { } impl AccumulatedMap { + pub(crate) const EMPTY: AccumulatedMap = AccumulatedMap { + map: hashbrown::HashMap::with_hasher(FxBuildHasher), + }; + pub fn accumulate(&mut self, index: IngredientIndex, value: A) { self.map .entry(index) diff --git a/src/active_query.rs b/src/active_query.rs index c80cded3b..d0aab0f55 100644 --- a/src/active_query.rs +++ b/src/active_query.rs @@ -106,7 +106,6 @@ impl ActiveQuery { ) { self.durability = self.durability.min(durability); self.changed_at = self.changed_at.max(changed_at); - self.input_outputs.insert(QueryEdge::input(input)); self.cycle_heads.extend(cycle_heads); #[cfg(feature = "accumulator")] { @@ -115,6 +114,10 @@ impl ActiveQuery { false => accumulated_inputs.load(), }); } + if !cycle_heads.is_empty() || durability != Durability::NEVER_CHANGE { + // During cycles we need to record dependencies. + self.input_outputs.insert(QueryEdge::input(input)); + } } pub(super) fn add_read_simple( @@ -125,7 +128,9 @@ impl ActiveQuery { ) { self.durability = self.durability.min(durability); self.changed_at = self.changed_at.max(revision); - self.input_outputs.insert(QueryEdge::input(input)); + if durability != Durability::NEVER_CHANGE { + self.input_outputs.insert(QueryEdge::input(input)); + } } pub(super) fn add_untracked_read(&mut self, changed_at: Revision) { @@ -147,7 +152,9 @@ impl ActiveQuery { /// Adds a key to our list of outputs. pub(super) fn add_output(&mut self, key: DatabaseKeyIndex) { - self.input_outputs.insert(QueryEdge::output(key)); + if self.durability != Durability::NEVER_CHANGE { + self.input_outputs.insert(QueryEdge::output(key)); + } } /// True if the given key was output by this query. diff --git a/src/cycle.rs b/src/cycle.rs index 8ab5dabcd..db79e3c9d 100644 --- a/src/cycle.rs +++ b/src/cycle.rs @@ -217,6 +217,7 @@ impl<'de> serde::Deserialize<'de> for AtomicIterationCount { pub struct CycleHeads(ThinVec); impl CycleHeads { + #[inline] pub(crate) fn is_empty(&self) -> bool { self.0.is_empty() } @@ -505,6 +506,7 @@ pub enum ProvisionalStatus<'db> { iteration: IterationCount, verified_at: Revision, }, + FinalNeverChange, FallbackImmediate, } diff --git a/src/durability.rs b/src/durability.rs index 4691e9372..8bf29e06a 100644 --- a/src/durability.rs +++ b/src/durability.rs @@ -46,6 +46,7 @@ impl std::fmt::Debug for Durability { DurabilityVal::Low => f.write_str("Durability::LOW"), DurabilityVal::Medium => f.write_str("Durability::MEDIUM"), DurabilityVal::High => f.write_str("Durability::HIGH"), + DurabilityVal::NeverChange => f.write_str("Durability::NEVER_CHANGE"), } } else { f.debug_tuple("Durability") @@ -61,14 +62,17 @@ enum DurabilityVal { Low = 0, Medium = 1, High = 2, + NeverChange = 3, } impl From for DurabilityVal { + #[inline] fn from(value: u8) -> Self { match value { 0 => DurabilityVal::Low, 1 => DurabilityVal::Medium, 2 => DurabilityVal::High, + 3 => DurabilityVal::NeverChange, _ => panic!("invalid durability"), } } @@ -87,23 +91,28 @@ impl Durability { /// High durability: things that are not expected to change under /// common usage. - /// - /// Example: the standard library or something from crates.io pub const HIGH: Durability = Durability(DurabilityVal::High); + /// The input is guaranteed to never change. Queries calling it won't have + /// it as a dependency. + /// + /// Example: the standard library or something from crates.io. + pub const NEVER_CHANGE: Durability = Durability(DurabilityVal::NeverChange); + /// The minimum possible durability; equivalent to LOW but /// "conceptually" distinct (i.e., if we add more durability /// levels, this could change). - pub(crate) const MIN: Durability = Self::LOW; + pub const MIN: Durability = Self::LOW; - /// The maximum possible durability; equivalent to HIGH but + /// The maximum possible durability; equivalent to NEVER_CHANGE but /// "conceptually" distinct (i.e., if we add more durability /// levels, this could change). - pub(crate) const MAX: Durability = Self::HIGH; + pub(crate) const MAX: Durability = Self::NEVER_CHANGE; /// Number of durability levels. - pub(crate) const LEN: usize = Self::HIGH.0 as usize + 1; + pub(crate) const LEN: usize = Self::MAX.0 as usize + 1; + #[inline] pub(crate) fn index(self) -> usize { self.0 as usize } diff --git a/src/function.rs b/src/function.rs index 8d8759af8..dcfadc632 100644 --- a/src/function.rs +++ b/src/function.rs @@ -16,13 +16,15 @@ use crate::key::DatabaseKeyIndex; use crate::plumbing::{self, MemoIngredientMap}; use crate::salsa_struct::SalsaStructInDb; use crate::sync::Arc; -use crate::table::memo::MemoTableTypes; +use crate::table::memo::{Either, MemoTableTypes}; use crate::table::Table; use crate::views::DatabaseDownCaster; use crate::zalsa::{IngredientIndex, JarKind, MemoIngredientIndex, Zalsa}; use crate::zalsa_local::{QueryEdge, QueryOriginRef}; use crate::{Cycle, Durability, Id, Revision}; +pub use crate::function::memo::AmbiguousMemo; + #[cfg(feature = "accumulator")] mod accumulated; mod backdate; @@ -37,7 +39,9 @@ mod memo; mod specify; mod sync; -pub type Memo = memo::Memo<'static, C>; +type EitherMemoNonNull<'db, C> = + Either>, NonNull>>; +type EitherMemoRef<'a, 'db, C> = Either<&'a memo::Memo<'db, C>, &'a memo::NeverChangeMemo<'db, C>>; pub trait Configuration: Any { const DEBUG_NAME: &'static str; @@ -246,6 +250,7 @@ where /// when this function is called and (b) ensuring that any entries /// removed from the memo-map are added to `deleted_entries`, which is /// only cleared with `&mut self`. + #[inline] unsafe fn extend_memo_lifetime<'this>( &'this self, memo: &memo::Memo<'this, C>, @@ -254,6 +259,15 @@ where unsafe { std::mem::transmute(memo) } } + #[inline] + unsafe fn extend_either_memo_lifetime<'this>( + &'this self, + memo: EitherMemoRef<'_, 'this, C>, + ) -> EitherMemoRef<'this, 'this, C> { + // SAFETY: the caller must guarantee that the memo will not be released before `&self` + unsafe { std::mem::transmute(memo) } + } + fn insert_memo<'db>( &'db self, zalsa: &'db Zalsa, @@ -284,6 +298,39 @@ where unsafe { self.extend_memo_lifetime(memo.as_ref()) } } + fn insert_never_change_memo<'db>( + &'db self, + zalsa: &'db Zalsa, + id: Id, + memo: memo::NeverChangeMemo<'db, C>, + memo_ingredient_index: MemoIngredientIndex, + ) -> EitherMemoRef<'db, 'db, C> { + // We convert to a `NonNull` here as soon as possible because we are going to alias + // into the `Box`, which is a `noalias` type. + // SAFETY: memo is not null + let memo = unsafe { NonNull::new_unchecked(Box::into_raw(Box::new(memo))) }; + + // SAFETY: memo must be in the map (it's not yet, but it will be by the time this + // value is returned) and anything removed from map is added to deleted entries (ensured elsewhere). + let db_memo = unsafe { self.extend_either_memo_lifetime(Either::Right(memo.as_ref())) }; + + if let Some(old_value) = + // SAFETY: We delay the drop of `old_value` until a new revision starts which ensures no + // references will exist for the memo contents. + unsafe { + self.insert_never_change_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) }; + } + db_memo + } + #[inline] fn memo_ingredient_index(&self, zalsa: &Zalsa, id: Id) -> MemoIngredientIndex { self.memo_ingredient_indices.get_zalsa_id(zalsa, id) @@ -335,6 +382,7 @@ where else { return; }; + let Either::Left(memo) = memo else { todo!() }; let origin = memo.revisions.origin.as_ref(); @@ -371,8 +419,11 @@ where zalsa: &'db Zalsa, input: Id, ) -> Option> { - let memo = - self.get_memo_from_table_for(zalsa, input, self.memo_ingredient_index(zalsa, input))?; + let Either::Left(memo) = + self.get_memo_from_table_for(zalsa, input, self.memo_ingredient_index(zalsa, input))? + else { + return Some(ProvisionalStatus::FinalNeverChange); + }; let iteration = memo.revisions.iteration(); let verified_final = memo.revisions.verified_final.load(Ordering::Relaxed); @@ -396,7 +447,7 @@ where } fn set_cycle_iteration_count(&self, zalsa: &Zalsa, input: Id, iteration_count: IterationCount) { - let Some(memo) = + let Some(Either::Left(memo)) = self.get_memo_from_table_for(zalsa, input, self.memo_ingredient_index(zalsa, input)) else { return; @@ -407,7 +458,7 @@ where } fn finalize_cycle_head(&self, zalsa: &Zalsa, input: Id) { - let Some(memo) = + let Some(Either::Left(memo)) = self.get_memo_from_table_for(zalsa, input, self.memo_ingredient_index(zalsa, input)) else { return; @@ -417,7 +468,7 @@ where } fn cycle_converged(&self, zalsa: &Zalsa, input: Id) -> bool { - let Some(memo) = + let Some(Either::Left(memo)) = self.get_memo_from_table_for(zalsa, input, self.memo_ingredient_index(zalsa, input)) else { return true; @@ -534,7 +585,12 @@ where let memo = self.get_memo_from_table_for(zalsa, entry.key_index(), memo_ingredient_index); - if memo.is_some_and(|memo| memo.should_serialize()) { + let should_serialize = match memo { + Some(Either::Left(memo)) => memo.should_serialize(), + Some(Either::Right(memo)) => memo.should_serialize(), + None => false, + }; + if should_serialize { return true; } } diff --git a/src/function/accumulated.rs b/src/function/accumulated.rs index a65804e64..108e25871 100644 --- a/src/function/accumulated.rs +++ b/src/function/accumulated.rs @@ -2,6 +2,7 @@ use crate::accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues use crate::accumulator::{self}; use crate::function::{Configuration, IngredientImpl}; use crate::hash::FxHashSet; +use crate::table::memo::Either; use crate::zalsa::ZalsaDatabase; use crate::zalsa_local::QueryOriginRef; use crate::{DatabaseKeyIndex, Id}; @@ -100,9 +101,15 @@ where let (zalsa, zalsa_local) = db.zalsas(); // NEXT STEP: stash and refactor `fetch` to return an `&Memo` so we can make this work let memo = self.refresh_memo(db, zalsa, zalsa_local, key); - ( - memo.revisions.accumulated(), - memo.revisions.accumulated_inputs.load(), - ) + match memo { + Either::Left(memo) => ( + memo.revisions.accumulated(), + memo.revisions.accumulated_inputs.load(), + ), + Either::Right(_) => ( + Some(const { &AccumulatedMap::EMPTY }), + InputAccumulatedValues::Empty, + ), + } } } diff --git a/src/function/delete.rs b/src/function/delete.rs index d061917b0..0f14db61f 100644 --- a/src/function/delete.rs +++ b/src/function/delete.rs @@ -1,13 +1,14 @@ use std::ptr::NonNull; -use crate::function::memo::Memo; -use crate::function::Configuration; +use crate::function::memo::{Memo, NeverChangeMemo}; +use crate::function::{Configuration, EitherMemoNonNull}; +use crate::table::memo::Either; /// Stores the list of memos that have been deleted so they can be freed /// once the next revision starts. See the comment on the field /// `deleted_entries` of [`FunctionIngredient`][] for more details. pub(super) struct DeletedEntries { - memos: boxcar::Vec>>, + memos: boxcar::Vec>, SharedBox>>>, } #[allow(clippy::undocumented_unsafe_blocks)] // TODO(#697) document safety @@ -27,12 +28,17 @@ impl DeletedEntries { /// # Safety /// /// The memo must be valid and safe to free when the `DeletedEntries` list is cleared or dropped. - pub(super) unsafe fn push(&self, memo: NonNull>) { + pub(super) unsafe fn push(&self, memo: EitherMemoNonNull<'_, C>) { // Safety: The memo must be valid and safe to free when the `DeletedEntries` list is cleared or dropped. - let memo = - unsafe { std::mem::transmute::>, NonNull>>(memo) }; - - self.memos.push(SharedBox(memo)); + let memo = unsafe { + std::mem::transmute::, EitherMemoNonNull<'static, C>>(memo) + }; + + let memo = match memo { + Either::Left(it) => Either::Left(SharedBox(it)), + Either::Right(it) => Either::Right(SharedBox(it)), + }; + self.memos.push(memo); } /// Free all deleted memos, keeping the list available for reuse. diff --git a/src/function/execute.rs b/src/function/execute.rs index 8580bb480..3ca6834b0 100644 --- a/src/function/execute.rs +++ b/src/function/execute.rs @@ -2,17 +2,18 @@ use smallvec::SmallVec; use crate::active_query::CompletedQuery; use crate::cycle::{CycleHeads, CycleRecoveryStrategy, IterationCount}; -use crate::function::memo::Memo; +use crate::function::memo::{Memo, NeverChangeMemo}; use crate::function::sync::ReleaseMode; -use crate::function::{ClaimGuard, Configuration, IngredientImpl}; +use crate::function::{ClaimGuard, Configuration, EitherMemoRef, IngredientImpl}; use crate::ingredient::WaitForResult; use crate::plumbing::ZalsaLocal; use crate::sync::atomic::{AtomicBool, Ordering}; use crate::sync::thread; +use crate::table::memo::Either; use crate::tracked_struct::Identity; use crate::zalsa::{MemoIngredientIndex, Zalsa}; use crate::zalsa_local::{ActiveQueryGuard, QueryRevisions}; -use crate::{tracing, Cancelled, Cycle}; +use crate::{tracing, Cancelled, Cycle, Durability}; use crate::{DatabaseKeyIndex, Event, EventKind, Id}; impl IngredientImpl @@ -39,12 +40,13 @@ where db: &'db C::DbView, mut claim_guard: ClaimGuard<'db>, zalsa_local: &'db ZalsaLocal, - opt_old_memo: Option<&Memo<'db, C>>, - ) -> Option<&'db Memo<'db, C>> { + opt_old_memo: Option<&'db Memo<'db, C>>, + ) -> Option> { let database_key_index = claim_guard.database_key_index(); let zalsa = claim_guard.zalsa(); let id = database_key_index.key_index(); + let memo_ingredient_index = self.memo_ingredient_index(zalsa, id); crate::tracing::info!("{:?}: executing query", database_key_index); @@ -79,18 +81,21 @@ where // Did the new result we got depend on our own provisional value, in a cycle? if cycle_heads.contains(&database_key_index) { // Ignore the computed value, leave the fallback value there. - let memo = self + let Either::Left(memo) = self .get_memo_from_table_for(zalsa, id, memo_ingredient_index) .unwrap_or_else(|| { unreachable!( "{database_key_index:#?} is a `FallbackImmediate` cycle head, \ but no memo found" ) - }); + }) + else { + unreachable!("cycle participants cannot be `NeverChangeMemo`s") + }; // We need to mark the memo as finalized so other cycle participants that have fallbacks // will be verified (participants that don't have fallbacks will not be verified). memo.revisions.verified_final.store(true, Ordering::Release); - return Some(memo); + return Some(Either::Left(memo)); } // If we're in the middle of a cycle and we have a fallback, use it instead. @@ -135,16 +140,31 @@ where self.diff_outputs(zalsa, database_key_index, old_memo, &completed_query); } - let memo = self.insert_memo( - zalsa, - id, - Memo::new( - Some(new_value), - zalsa.current_revision(), - completed_query.revisions, - ), - memo_ingredient_index, - ); + let memo = if completed_query.revisions.durability == Durability::NEVER_CHANGE + && completed_query.revisions.cycle_heads().is_empty() + { + // Only insert a `NeverChangeMemo` if we are not inside a cycle, or the cycle completed successfully. + // Cycles need dependency tracking. + self.insert_never_change_memo( + zalsa, + id, + NeverChangeMemo { + value: Some(new_value), + }, + memo_ingredient_index, + ) + } else { + Either::Left(self.insert_memo( + zalsa, + id, + Memo::new( + Some(new_value), + zalsa.current_revision(), + completed_query.revisions, + ), + memo_ingredient_index, + )) + }; if claim_guard.drop() { None @@ -341,6 +361,12 @@ where but no provisional memo found" ) }); + let Either::Left(memo) = memo else { + panic!( + "during a cycle, no `NeverChangeMemo` is \ + inserted as fallback" + ); + }; debug_assert!(memo.may_be_provisional()); memo diff --git a/src/function/fetch.rs b/src/function/fetch.rs index 1c1ad30ed..b0e4ca471 100644 --- a/src/function/fetch.rs +++ b/src/function/fetch.rs @@ -4,7 +4,8 @@ use crate::cycle::{CycleHeads, CycleRecoveryStrategy, IterationCount}; use crate::function::maybe_changed_after::VerifyCycleHeads; use crate::function::memo::Memo; use crate::function::sync::ClaimResult; -use crate::function::{Configuration, IngredientImpl, Reentrancy}; +use crate::function::{Configuration, EitherMemoRef, IngredientImpl, Reentrancy}; +use crate::table::memo::Either; use crate::zalsa::{MemoIngredientIndex, Zalsa}; use crate::zalsa_local::{QueryRevisions, ZalsaLocal}; use crate::{DatabaseKeyIndex, Id}; @@ -30,33 +31,41 @@ where let memo = self.refresh_memo(db, zalsa, zalsa_local, id); - // SAFETY: We just refreshed the memo so it is guaranteed to contain a value now. - let memo_value = unsafe { memo.value.as_ref().unwrap_unchecked() }; - self.lru.record_use(id); - zalsa_local.report_tracked_read( - database_key_index, - memo.revisions.durability, - memo.revisions.changed_at, - memo.cycle_heads(), - #[cfg(feature = "accumulator")] - memo.revisions.accumulated().is_some(), - #[cfg(feature = "accumulator")] - &memo.revisions.accumulated_inputs, - ); + let memo_value = match memo { + Either::Left(memo) => { + zalsa_local.report_tracked_read( + database_key_index, + memo.revisions.durability, + memo.revisions.changed_at, + memo.cycle_heads(), + #[cfg(feature = "accumulator")] + memo.revisions.accumulated().is_some(), + #[cfg(feature = "accumulator")] + &memo.revisions.accumulated_inputs, + ); + + // SAFETY: We just refreshed the memo so it is guaranteed to contain a value now. + unsafe { memo.value.as_ref().unwrap_unchecked() } + } + Either::Right(memo) => { + // SAFETY: We just refreshed the memo so it is guaranteed to contain a value now. + unsafe { memo.value.as_ref().unwrap_unchecked() } + } + }; memo_value } - #[inline(always)] + #[inline] pub(super) fn refresh_memo<'db>( &'db self, db: &'db C::DbView, zalsa: &'db Zalsa, zalsa_local: &'db ZalsaLocal, id: Id, - ) -> &'db Memo<'db, C> { + ) -> EitherMemoRef<'db, 'db, C> { let memo_ingredient_index = self.memo_ingredient_index(zalsa, id); loop { @@ -75,10 +84,11 @@ where zalsa: &'db Zalsa, id: Id, memo_ingredient_index: MemoIngredientIndex, - ) -> Option<&'db Memo<'db, C>> { - let memo = self.get_memo_from_table_for(zalsa, id, memo_ingredient_index)?; - - memo.value.as_ref()?; + ) -> Option> { + let memo = match self.get_memo_from_table_for(zalsa, id, memo_ingredient_index)? { + Either::Left(memo) => memo, + memo @ Either::Right(_) => return Some(memo), + }; let database_key_index = self.database_key_index(id); @@ -89,12 +99,13 @@ where // SAFETY: memo is present in memo_map and we have verified that it is // still valid for the current revision. - unsafe { Some(self.extend_memo_lifetime(memo)) } + unsafe { Some(Either::Left(self.extend_memo_lifetime(memo))) } } else { None } } + #[inline] fn fetch_cold<'db>( &'db self, zalsa: &'db Zalsa, @@ -102,7 +113,7 @@ where db: &'db C::DbView, id: Id, memo_ingredient_index: MemoIngredientIndex, - ) -> Option<&'db Memo<'db, C>> { + ) -> Option> { let database_key_index = self.database_key_index(id); // Try to claim this query: if someone else has claimed it already, go back and start again. let claim_guard = match self.sync_table.try_claim(zalsa, id, Reentrancy::Allow) { @@ -113,7 +124,7 @@ where if C::CYCLE_STRATEGY == CycleRecoveryStrategy::FallbackImmediate { let memo = self.get_memo_from_table_for(zalsa, id, memo_ingredient_index); - if let Some(memo) = memo { + if let Some(Either::Left(memo)) = memo { if memo.value.is_some() { memo.block_on_heads(zalsa); } @@ -123,19 +134,23 @@ where return None; } ClaimResult::Cycle { .. } => { - return Some(self.fetch_cold_cycle( + return Some(Either::Left(self.fetch_cold_cycle( zalsa, zalsa_local, db, id, database_key_index, memo_ingredient_index, - )); + ))); } }; // Now that we've claimed the item, check again to see if there's a "hot" value. - let opt_old_memo = self.get_memo_from_table_for(zalsa, id, memo_ingredient_index); + let opt_old_memo = match self.get_memo_from_table_for(zalsa, id, memo_ingredient_index) { + Some(Either::Left(it)) => Some(it), + memo @ Some(Either::Right(_)) => return memo, + None => None, + }; if let Some(old_memo) = opt_old_memo { if old_memo.value.is_some() { @@ -153,7 +168,7 @@ where // SAFETY: memo is present in memo_map and we have verified that it is // still valid for the current revision. - return unsafe { Some(self.extend_memo_lifetime(old_memo)) }; + return unsafe { Some(Either::Left(self.extend_memo_lifetime(old_memo))) }; } let mut cycle_heads = Vec::new(); @@ -171,7 +186,7 @@ where if verify_result.is_unchanged() && cycle_heads.is_empty() { // SAFETY: memo is present in memo_map and we have verified that it is // still valid for the current revision. - return unsafe { Some(self.extend_memo_lifetime(old_memo)) }; + return unsafe { Some(Either::Left(self.extend_memo_lifetime(old_memo))) }; } } } @@ -193,7 +208,11 @@ where // check if there's a provisional value for this query // Note we don't `validate_may_be_provisional` the memo here as we want to reuse an // existing provisional memo if it exists - let memo_guard = self.get_memo_from_table_for(zalsa, id, memo_ingredient_index); + let memo_guard = match self.get_memo_from_table_for(zalsa, id, memo_ingredient_index) { + Some(Either::Left(it)) => Some(it), + Some(Either::Right(_)) => panic!("a never-change memo cannot participate in cycles"), + None => None, + }; if let Some(memo) = memo_guard { // Ideally, we'd use the last provisional memo even if it wasn't a cycle head in the last iteration // but that would require inserting itself as a cycle head, which either requires clone diff --git a/src/function/inputs.rs b/src/function/inputs.rs index 78b1adb36..2a2072f16 100644 --- a/src/function/inputs.rs +++ b/src/function/inputs.rs @@ -1,4 +1,5 @@ use crate::function::{Configuration, IngredientImpl}; +use crate::table::memo::Either; use crate::zalsa::Zalsa; use crate::zalsa_local::QueryOriginRef; use crate::Id; @@ -10,6 +11,9 @@ where pub(super) fn origin<'db>(&self, zalsa: &'db Zalsa, key: Id) -> Option> { let memo_ingredient_index = self.memo_ingredient_index(zalsa, key); self.get_memo_from_table_for(zalsa, key, memo_ingredient_index) - .map(|m| m.revisions.origin.as_ref()) + .map(|m| match m { + Either::Left(m) => m.revisions.origin.as_ref(), + Either::Right(_) => QueryOriginRef::Derived(&[]), + }) } } diff --git a/src/function/maybe_changed_after.rs b/src/function/maybe_changed_after.rs index 165a3fb02..4b13d87ba 100644 --- a/src/function/maybe_changed_after.rs +++ b/src/function/maybe_changed_after.rs @@ -9,6 +9,7 @@ use crate::function::{Configuration, IngredientImpl, Reentrancy}; use crate::key::DatabaseKeyIndex; use crate::sync::atomic::Ordering; +use crate::table::memo::Either; use crate::zalsa::{MemoIngredientIndex, Zalsa, ZalsaDatabase}; use crate::zalsa_local::{QueryEdgeKind, QueryOriginRef, ZalsaLocal}; use crate::{Id, Revision}; @@ -94,6 +95,13 @@ where // No memo? Assume has changed. return VerifyResult::changed(); }; + let memo = match memo { + Either::Left(memo) => memo, + Either::Right(_) => { + // A `NEVER_CHANGE` memo. Consider it verified. + return VerifyResult::unchanged(); + } + }; let can_shallow_update = self.shallow_verify_memo(zalsa, database_key_index, memo); if can_shallow_update.yes() && !memo.may_be_provisional() { @@ -163,6 +171,10 @@ where else { return Some(VerifyResult::changed()); }; + let Either::Left(old_memo) = old_memo else { + // A never-change memo? Consider it verified. + return Some(VerifyResult::unchanged()); + }; crate::tracing::debug!( "{database_key_index:?}: maybe_changed_after_cold, successful claim, \ @@ -229,23 +241,33 @@ where // if **this query** encountered a cycle (which means there's some provisional value somewhere floating around). if old_memo.value.is_some() && !cycle_heads.has_any() { let memo = self.execute(db, claim_guard, zalsa_local, Some(old_memo))?; - let changed_at = memo.revisions.changed_at; + match memo { + Either::Left(memo) => { + let changed_at = memo.revisions.changed_at; - // Always assume that a provisional value has changed. - // - // We don't know if a provisional value has actually changed. To determine whether a provisional - // value has changed, we need to iterate the outer cycle, which cannot be done here. - return Some(if changed_at > revision || memo.may_be_provisional() { - VerifyResult::changed() - } else { - VerifyResult::unchanged_with_accumulated( - #[cfg(feature = "accumulator")] - match memo.revisions.accumulated() { - Some(_) => InputAccumulatedValues::Any, - None => memo.revisions.accumulated_inputs.load(), - }, - ) - }); + // Always assume that a provisional value has changed. + // + // We don't know if a provisional value has actually changed. To determine whether a provisional + // value has changed, we need to iterate the outer cycle, which cannot be done here. + return Some(if changed_at > revision || memo.may_be_provisional() { + VerifyResult::changed() + } else { + VerifyResult::unchanged_with_accumulated( + #[cfg(feature = "accumulator")] + match memo.revisions.accumulated() { + Some(_) => InputAccumulatedValues::Any, + None => memo.revisions.accumulated_inputs.load(), + }, + ) + }); + } + Either::Right(_) => { + // Don't backdate never-change memos. We have no way to backdate them (they don't store `changed_at`) + // and changing a memo from non-never-change to never-change is likely rare enough (or even impossible) + // that this is not a problem. + return Some(VerifyResult::Changed); + } + } } // Otherwise, nothing for it: have to consider the value to have changed. @@ -442,6 +464,7 @@ where panic!("cannot mix `cycle_fn` and `cycle_result` in cycles") } } + ProvisionalStatus::FinalNeverChange => {} ProvisionalStatus::FallbackImmediate => match C::CYCLE_STRATEGY { CycleRecoveryStrategy::Panic => { // Queries without fallback are not considered when inside a cycle. diff --git a/src/function/memo.rs b/src/function/memo.rs index 35eb4b3b7..d1d485f90 100644 --- a/src/function/memo.rs +++ b/src/function/memo.rs @@ -1,23 +1,57 @@ -use std::any::Any; +use std::any::{Any, TypeId}; use std::fmt::{Debug, Formatter}; -use std::mem::transmute; +use std::marker::PhantomData; +use std::mem; use std::ptr::NonNull; use crate::cycle::{ empty_cycle_heads, CycleHeads, CycleHeadsIterator, IterationCount, ProvisionalStatus, }; -use crate::function::{Configuration, IngredientImpl}; +use crate::function::{Configuration, EitherMemoNonNull, EitherMemoRef, IngredientImpl}; use crate::ingredient::WaitForResult; use crate::key::DatabaseKeyIndex; use crate::revision::AtomicRevision; use crate::runtime::Running; use crate::sync::atomic::Ordering; -use crate::table::memo::MemoTableWithTypesMut; +use crate::table::memo::{Either, MemoTableWithTypesMut}; use crate::zalsa::{MemoIngredientIndex, Zalsa}; use crate::zalsa_local::{QueryOriginRef, QueryRevisions}; use crate::{Event, EventKind, Id, Revision}; impl IngredientImpl { + /// Memos have to be stored internally using `'static` as the database lifetime. + /// This (unsafe) function call converts from something tied to self to static. + /// Values transmuted this way have to be transmuted back to being tied to self + /// when they are returned to the user. + unsafe fn to_static<'db>(&self, memo: NonNull>) -> NonNull> { + memo.cast() + } + pub(super) unsafe fn to_static_never_change<'db>( + &self, + memo: NonNull>, + ) -> NonNull> { + memo.cast() + } + + /// Convert from an internal memo (which uses `'static`) to one tied to self + /// so it can be publicly released. + unsafe fn to_self<'db>( + &self, + memo: EitherMemoNonNull<'static, C>, + ) -> EitherMemoNonNull<'db, C> { + // SAFETY: We're only transmuting lifetimes, so layout didn't change. + unsafe { mem::transmute(memo) } + } + + /// Convert from an internal memo (which uses `'static`) to one tied to self + /// so it can be publicly released. + unsafe fn to_self_ref<'db>( + &self, + memo: EitherMemoRef<'db, 'static, C>, + ) -> EitherMemoRef<'db, 'db, C> { + unsafe { std::mem::transmute(memo) } + } + /// 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, @@ -25,19 +59,36 @@ impl IngredientImpl { id: Id, memo: NonNull>, memo_ingredient_index: MemoIngredientIndex, - ) -> Option>> { - // 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) }; - let old_static_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>>(old_static_memo) - }) + ) -> Option> { + let static_memo = unsafe { self.to_static(memo) }; + let old_static_memo = unsafe { + zalsa + .memo_table_for::>(id) + .insert_false::>(memo_ingredient_index, static_memo) + }?; + Some(unsafe { self.to_self(old_static_memo) }) + } + + /// Inserts the memo for the given key; (atomically) overwrites and returns any previously existing memo + /// + /// # Safety + /// + /// The caller needs to make sure to not drop the returned value until no more references into + /// the database exist as there may be outstanding borrows into the memo contents. + pub(super) unsafe fn insert_never_change_memo_into_table_for<'db>( + &'db self, + zalsa: &'db Zalsa, + id: Id, + memo: NonNull>, + memo_ingredient_index: MemoIngredientIndex, + ) -> Option> { + let static_memo = unsafe { self.to_static_never_change(memo) }; + let old_static_memo = unsafe { + zalsa + .memo_table_for::>(id) + .insert_true::>(memo_ingredient_index, static_memo) + }?; + Some(unsafe { self.to_self(old_static_memo) }) } /// Loads the current memo for `key_index`. This does not hold any sort of @@ -48,13 +99,12 @@ impl IngredientImpl { zalsa: &'db Zalsa, id: Id, memo_ingredient_index: MemoIngredientIndex, - ) -> Option<&'db Memo<'db, C>> { + ) -> Option> { let static_memo = zalsa .memo_table_for::>(id) - .get(memo_ingredient_index)?; - // 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::<&Memo<'static, C>, &'db Memo<'db, C>>(static_memo.as_ref()) }) + .get::>(memo_ingredient_index)?; + + unsafe { Some(self.to_self_ref(static_memo)) } } /// Evicts the existing memo for the given key, replacing it @@ -81,11 +131,65 @@ impl IngredientImpl { } }; - table.map_memo(memo_ingredient_index, map) + table.map_memo::>(memo_ingredient_index, |memo| match memo { + Either::Left(memo) => map(memo), + Either::Right(memo) => memo.value = None, + }) + } +} + +pub struct AmbiguousMemo(PhantomData); + +// SAFETY: The `value_type_id()` and the disambiguator are enough to uniquely identify a memo. +unsafe impl crate::table::memo::AmbiguousMemo for AmbiguousMemo { + fn value_type_id() -> std::any::TypeId + where + Self: Sized, + { + TypeId::of::>() + } + type MFalse = Memo<'static, C>; + type MTrue = NeverChangeMemo<'static, C>; +} + +#[derive(Debug)] +#[repr(align(2))] // Needed for table/memo.rs +pub struct NeverChangeMemo<'db, C: Configuration> { + /// The result of the query, if we decide to memoize it. + pub(super) value: Option>, + // What we *don't* store for never-changing memos: + // - verified_at, changed_at - they're always valid. + // - durability - we know it, it's `NEVER_CHANGE`. + // - origin - dependencies aren't important. + // - tracked_struct_ids - we never diff tracked structs, + // because we won't execute it the second time. + // - accumulated, accumulated_inputs - accumulations are + // mainly for errors, and `NEVER_CHANGE` things are mostly + // for libraries, and we may assume libraries have no + // errors. This is a finicky assumption, I know, I + // mostly do that because I dislike accumulators and + // rust-analyzer does not use them. + // verified_final, cycle_heads - cycle handling for + // never-changing memos is a bit complicated. We depend + // on the query dependencies to track cycles, so we + // cannot use a `NEVER_CHANGE` memo for that. Instead, + // during the cycle we only use normal memos for participants, + // and when exiting the cycle we replace the cycle head + // with a `NeverChangeMemo`. When participants get validated, + // they also get replaced with `NeverChangeMemo`s. +} + +impl<'db, C: Configuration> NeverChangeMemo<'db, C> { + /// Returns `true` if this memo should be serialized. + pub(super) fn should_serialize(&self) -> bool { + // TODO: Serialization is a good opportunity to prune old query results based on + // the `verified_at` revision. + self.value.is_some() } } #[derive(Debug)] +#[repr(align(2))] // Needed for table/memo.rs pub struct Memo<'db, C: Configuration> { /// The result of the query, if we decide to memoize it. pub(super) value: Option>, @@ -287,6 +391,34 @@ where } } +impl crate::table::memo::Memo for NeverChangeMemo<'static, C> +where + C::Output<'static>: Send + Sync + Any, +{ + fn remove_outputs(&self, _zalsa: &Zalsa, _executor: DatabaseKeyIndex) {} + + #[cfg(feature = "salsa_unstable")] + fn memory_usage(&self) -> crate::database::MemoInfo { + let size_of = std::mem::size_of::>(); + let heap_size = if let Some(value) = self.value.as_ref() { + C::heap_size(value) + } else { + Some(0) + }; + + crate::database::MemoInfo { + debug_name: C::DEBUG_NAME, + output: crate::database::SlotInfo { + size_of_metadata: size_of - std::mem::size_of::>(), + debug_name: std::any::type_name::>(), + size_of_fields: std::mem::size_of::>(), + heap_size_of_fields: heap_size, + memos: Vec::new(), + }, + } + } +} + #[cfg(feature = "persistence")] mod persistence { use crate::function::memo::Memo; @@ -471,6 +603,9 @@ impl<'me> Iterator for TryClaimCycleHeadsIter<'me> { ProvisionalStatus::FallbackImmediate => { (IterationCount::initial(), self.zalsa.current_revision()) } + ProvisionalStatus::FinalNeverChange => { + panic!("Never-changing memos cannot participate in cycles") + } }; Some(TryClaimHeadsResult::Cycle { @@ -504,6 +639,8 @@ mod _memory_usage { // Memo's are stored a lot, make sure their size is doesn't randomly increase. const _: [(); std::mem::size_of::>()] = [(); std::mem::size_of::<[usize; 6]>()]; + const _: [(); std::mem::size_of::>()] = + [(); std::mem::size_of::<[usize; 1]>()]; struct DummyStruct; diff --git a/src/function/specify.rs b/src/function/specify.rs index 99539d375..f148dd14c 100644 --- a/src/function/specify.rs +++ b/src/function/specify.rs @@ -5,6 +5,7 @@ use crate::function::memo::Memo; use crate::function::{Configuration, IngredientImpl}; use crate::revision::AtomicRevision; use crate::sync::atomic::AtomicBool; +use crate::table::memo::Either; use crate::tracked_struct::TrackedStructInDb; use crate::zalsa::{Zalsa, ZalsaDatabase}; use crate::zalsa_local::{QueryOrigin, QueryOriginRef, QueryRevisions, QueryRevisionsExtra}; @@ -79,6 +80,9 @@ where let memo_ingredient_index = self.memo_ingredient_index(zalsa, key); if let Some(old_memo) = self.get_memo_from_table_for(zalsa, key, memo_ingredient_index) { + let Either::Left(old_memo) = old_memo else { + panic!("`NEVER_CHANGE` queries cannot be specified") + }; self.backdate_if_appropriate( old_memo, database_key_index, @@ -119,7 +123,8 @@ where let memo_ingredient_index = self.memo_ingredient_index(zalsa, key); let memo = match self.get_memo_from_table_for(zalsa, key, memo_ingredient_index) { - Some(m) => m, + Some(Either::Left(m)) => m, + Some(Either::Right(_)) => unreachable!("`NEVER_CHANGE` queries cannot be specified"), None => return, }; diff --git a/src/lib.rs b/src/lib.rs index f90fce338..b0f0cc45e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -157,9 +157,9 @@ pub mod plumbing { } pub mod function { + pub use crate::function::AmbiguousMemo; pub use crate::function::Configuration; pub use crate::function::IngredientImpl; - pub use crate::function::Memo; pub use crate::table::memo::MemoEntryType; } diff --git a/src/table/memo.rs b/src/table/memo.rs index dd0d71e58..f1996a22f 100644 --- a/src/table/memo.rs +++ b/src/table/memo.rs @@ -40,6 +40,32 @@ impl MemoTable { } } +#[derive(Debug, Clone, Copy)] +pub(crate) enum Either { + Left(A), + Right(B), +} + +/// Represents a `Memo` that has one of two possible types. +/// +/// # Safety +/// +/// If the `value_type_id()` and the disambiguator match, the value must have the type of +/// the corresponding associated type. +pub unsafe trait AmbiguousMemo { + /// The `TypeId` of the contained value. + /// + /// This can be shared to at most two `Memo` types, distinguished by `MFalse` and `MTrue`. + /// The important property is that *both* can be stored in a slot. That is, the `MemoEntryType` + /// only holds the `value_type_id()`, and the disambiguator (true or false) is stored + /// in the `MemoEntry`. + fn value_type_id() -> TypeId + where + Self: Sized; + type MFalse: Memo; + type MTrue: Memo; +} + pub trait Memo: Any + Send + Sync { /// Removes the outputs that were created when this query ran. This includes /// tracked structs and specified queries. @@ -73,25 +99,62 @@ struct MemoEntry { atomic_memo: AtomicPtr, } +const DISAMBIGUATOR_MASK: usize = 0b1; + +/// # Safety +/// +/// `ptr` must stay non-null after removing the 0th bit. +#[inline] +unsafe fn unpack_memo_ptr(ptr: NonNull) -> (NonNull, bool) { + let ptr = ptr.as_ptr(); + // SAFETY: Our precondition. + let new_ptr = + unsafe { NonNull::new_unchecked(ptr.map_addr(|addr| addr & !DISAMBIGUATOR_MASK)) }; + (new_ptr, ptr.addr() & DISAMBIGUATOR_MASK != 0) +} + +#[inline] +fn pack_memo_ptr(ptr: NonNull, disambiguator: bool) -> NonNull { + // SAFETY: We're ORing bits, it cannot make it null. + unsafe { + NonNull::new_unchecked( + ptr.as_ptr() + .map_addr(|addr| addr | usize::from(disambiguator)), + ) + } +} + +/// # Safety +/// +/// `ptr` must stay non-null after removing the 0th bit. `value_type_id()` must be correct. +#[inline] +unsafe fn unpack_memo_ptr_typed( + ptr: NonNull, +) -> Either, NonNull> { + // SAFETY: Our precondition. + let (new_ptr, disambiguator) = unsafe { unpack_memo_ptr(ptr) }; + match disambiguator { + false => Either::Left(new_ptr.cast::()), + true => Either::Right(new_ptr.cast::()), + } +} + #[derive(Clone, Copy, Debug)] pub struct MemoEntryType { /// The `type_id` of the erased memo type `M` type_id: TypeId, - /// A type-coercion function for the erased memo type `M` - to_dyn_fn: fn(NonNull) -> NonNull, + /// A type-coercion function for the erased memo type `M`, indexed by `type_id_disambiguator()`. + to_dyn_fns: [fn(NonNull) -> NonNull; 2], } impl MemoEntryType { - fn to_dummy(memo: NonNull) -> NonNull { - memo.cast() - } - - unsafe fn from_dummy(memo: NonNull) -> NonNull { - memo.cast() + #[inline] + fn to_dyn_fn(&self, disambiguator: bool) -> fn(NonNull) -> NonNull { + self.to_dyn_fns[usize::from(disambiguator)] } - const fn to_dyn_fn() -> fn(NonNull) -> NonNull { + const fn create_to_dyn_fn() -> fn(NonNull) -> NonNull { let f: fn(NonNull) -> NonNull = |x| x; // SAFETY: `M: Sized` and `DummyMemo: Sized`, as such they are ABI compatible behind a @@ -105,10 +168,23 @@ impl MemoEntryType { } #[inline] - pub fn of() -> Self { + pub fn of() -> Self { + const { + assert!( + align_of::() >= 2, + "need enough space to encode the disambiguator" + ); + assert!( + align_of::() >= 2, + "need enough space to encode the disambiguator" + ); + }; Self { - type_id: TypeId::of::(), - to_dyn_fn: Self::to_dyn_fn::(), + type_id: M::value_type_id(), + to_dyn_fns: [ + Self::create_to_dyn_fn::(), + Self::create_to_dyn_fn::(), + ], } } } @@ -182,12 +258,46 @@ pub struct MemoTableWithTypes<'a> { memos: &'a MemoTable, } -impl MemoTableWithTypes<'_> { - pub(crate) fn insert( +impl<'a> MemoTableWithTypes<'a> { + /// # Safety + /// + /// The caller needs to make sure to not drop the returned value until no more references into + /// the database exist as there may be outstanding borrows into the memo contents. + #[inline] + pub(crate) unsafe fn insert_false( + self, + memo_ingredient_index: MemoIngredientIndex, + memo: NonNull, + ) -> Option, NonNull>> { + let memo = pack_memo_ptr(memo.cast::(), false); + // SAFETY: Our preconditions. + unsafe { self.insert_impl::(memo_ingredient_index, memo) } + } + /// # Safety + /// + /// The caller needs to make sure to not drop the returned value until no more references into + /// the database exist as there may be outstanding borrows into the memo contents. + #[inline] + pub(crate) unsafe fn insert_true( + self, + memo_ingredient_index: MemoIngredientIndex, + memo: NonNull, + ) -> Option, NonNull>> { + let memo = pack_memo_ptr(memo.cast::(), true); + // SAFETY: Our preconditions. + unsafe { self.insert_impl::(memo_ingredient_index, memo) } + } + + /// # Safety + /// + /// The caller needs to make sure to not drop the returned value until no more references into + /// the database exist as there may be outstanding borrows into the memo contents. + #[inline] + unsafe fn insert_impl( self, memo_ingredient_index: MemoIngredientIndex, - memo: NonNull, - ) -> Option> { + memo: NonNull, + ) -> Option, NonNull>> { let MemoEntry { atomic_memo } = self.memos.memos.get(memo_ingredient_index.as_usize())?; // SAFETY: Any indices that are in-bounds for the `MemoTable` are also in-bounds for its @@ -199,22 +309,25 @@ impl MemoTableWithTypes<'_> { }; // Verify that the we are casting to the correct type. - if type_.type_id != TypeId::of::() { + if type_.type_id != M::value_type_id() { type_assert_failed(memo_ingredient_index); } - let old_memo = atomic_memo.swap(MemoEntryType::to_dummy(memo).as_ptr(), Ordering::AcqRel); + let old_memo = atomic_memo.swap(memo.as_ptr(), Ordering::AcqRel); + + let old_memo = NonNull::new(old_memo); - // SAFETY: We asserted that the type is correct above. - NonNull::new(old_memo).map(|old_memo| unsafe { MemoEntryType::from_dummy(old_memo) }) + // SAFETY: `value_type_id()` check asserted above. The pointer points to a valid memo (otherwise + // it'd be null) so not null. + old_memo.map(|old_memo| unsafe { unpack_memo_ptr_typed::(old_memo) }) } /// Returns a pointer to the memo at the given index, if one has been inserted. #[inline] - pub(crate) fn get( + pub(crate) fn get( self, memo_ingredient_index: MemoIngredientIndex, - ) -> Option> { + ) -> Option> { let MemoEntry { atomic_memo } = self.memos.memos.get(memo_ingredient_index.as_usize())?; // SAFETY: Any indices that are in-bounds for the `MemoTable` are also in-bounds for its @@ -226,13 +339,19 @@ impl MemoTableWithTypes<'_> { }; // Verify that the we are casting to the correct type. - if type_.type_id != TypeId::of::() { + if type_.type_id != M::value_type_id() { type_assert_failed(memo_ingredient_index); } - NonNull::new(atomic_memo.load(Ordering::Acquire)) - // SAFETY: We asserted that the type is correct above. - .map(|memo| unsafe { MemoEntryType::from_dummy(memo) }) + let memo = NonNull::new(atomic_memo.load(Ordering::Acquire)); + // SAFETY: `value_type_id()` check asserted above. The pointer points to a valid memo (otherwise + // it'd be null) so not null. + memo.map(|old_memo| unsafe { + match unpack_memo_ptr_typed::(old_memo) { + Either::Left(it) => Either::Left(it.as_ref()), + Either::Right(it) => Either::Right(it.as_ref()), + } + }) } #[cfg(feature = "salsa_unstable")] @@ -242,13 +361,15 @@ impl MemoTableWithTypes<'_> { let Some(memo) = NonNull::new(memo.atomic_memo.load(Ordering::Acquire)) else { continue; }; + // SAFETY: There exists a memo so it's not null. + let (memo, disambiguator) = unsafe { unpack_memo_ptr(memo) }; let Some(type_) = self.types.types.get(index) else { continue; }; // SAFETY: The `TypeId` is asserted in `insert()`. - let dyn_memo: &dyn Memo = unsafe { (type_.to_dyn_fn)(memo).as_ref() }; + let dyn_memo: &dyn Memo = unsafe { type_.to_dyn_fn(disambiguator)(memo).as_ref() }; memory_usage.push(dyn_memo.memory_usage()); } @@ -265,10 +386,10 @@ impl MemoTableWithTypesMut<'_> { /// Calls `f` on the memo at `memo_ingredient_index`. /// /// If the memo is not present, `f` is not called. - pub(crate) fn map_memo( + pub(crate) fn map_memo( self, memo_ingredient_index: MemoIngredientIndex, - f: impl FnOnce(&mut M), + f: impl FnOnce(Either<&mut M::MFalse, &mut M::MTrue>), ) { let Some(MemoEntry { atomic_memo }) = self.memos.memos.get_mut(memo_ingredient_index.as_usize()) @@ -285,7 +406,7 @@ impl MemoTableWithTypesMut<'_> { }; // Verify that the we are casting to the correct type. - if type_.type_id != TypeId::of::() { + if type_.type_id != M::value_type_id() { type_assert_failed(memo_ingredient_index); } @@ -293,8 +414,14 @@ impl MemoTableWithTypesMut<'_> { return; }; - // SAFETY: We asserted that the type is correct above. - f(unsafe { MemoEntryType::from_dummy(memo).as_mut() }); + // SAFETY: `value_type_id()` check asserted above. The pointer points to a valid memo (otherwise + // it'd be null) so not null. + f(unsafe { + match unpack_memo_ptr_typed::(memo) { + Either::Left(mut it) => Either::Left(it.as_mut()), + Either::Right(mut it) => Either::Right(it.as_mut()), + } + }); } /// To drop an entry, we need its type, so we don't implement `Drop`, and instead have this method. @@ -349,10 +476,11 @@ impl MemoEntry { /// The type must match. #[inline] unsafe fn take(&mut self, type_: &MemoEntryType) -> Option> { - let memo = mem::replace(self.atomic_memo.get_mut(), ptr::null_mut()); - let memo = NonNull::new(memo)?; + let memo = NonNull::new(mem::replace(self.atomic_memo.get_mut(), ptr::null_mut()))?; + // SAFETY: We store an actual memo (otherwise `self.atomic_memo` would be null) of this type (our precondition). + let (memo, disambiguator) = unsafe { unpack_memo_ptr(memo) }; // SAFETY: Our preconditions. - Some(unsafe { Box::from_raw((type_.to_dyn_fn)(memo).as_ptr()) }) + Some(unsafe { Box::from_raw(type_.to_dyn_fn(disambiguator)(memo).as_ptr()) }) } } diff --git a/src/zalsa_local.rs b/src/zalsa_local.rs index e58dfdfe2..6c6c409e7 100644 --- a/src/zalsa_local.rs +++ b/src/zalsa_local.rs @@ -309,13 +309,15 @@ impl ZalsaLocal { changed_at ); - // SAFETY: We do not access the query stack reentrantly. - unsafe { - self.with_query_stack_unchecked_mut(|stack| { - if let Some(top_query) = stack.last_mut() { - top_query.add_read_simple(input, durability, changed_at); - } - }) + if durability < Durability::NEVER_CHANGE { + // SAFETY: We do not access the query stack reentrantly. + unsafe { + self.with_query_stack_unchecked_mut(|stack| { + if let Some(top_query) = stack.last_mut() { + top_query.add_read_simple(input, durability, changed_at); + } + }) + } } } diff --git a/tests/accumulate-chain.rs b/tests/accumulate-chain.rs index 4a26cec76..7767767aa 100644 --- a/tests/accumulate-chain.rs +++ b/tests/accumulate-chain.rs @@ -13,38 +13,50 @@ use test_log::test; #[derive(Debug)] struct Log(#[allow(dead_code)] String); +// We need a dummy input, otherwise our query will have a `NEVER_CHANGE` durability, +// and queries with this durability don't accumulate. +#[salsa::input] +struct Input { + dummy: (), +} + #[salsa::tracked] -fn push_logs(db: &dyn Database) { - push_a_logs(db); +fn push_logs(db: &dyn Database, input: Input) { + _ = input.dummy(db); + push_a_logs(db, input); } #[salsa::tracked] -fn push_a_logs(db: &dyn Database) { +fn push_a_logs(db: &dyn Database, input: Input) { + _ = input.dummy(db); Log("log a".to_string()).accumulate(db); - push_b_logs(db); + push_b_logs(db, input); } #[salsa::tracked] -fn push_b_logs(db: &dyn Database) { +fn push_b_logs(db: &dyn Database, input: Input) { + _ = input.dummy(db); // No logs - push_c_logs(db); + push_c_logs(db, input); } #[salsa::tracked] -fn push_c_logs(db: &dyn Database) { +fn push_c_logs(db: &dyn Database, input: Input) { + _ = input.dummy(db); // No logs - push_d_logs(db); + push_d_logs(db, input); } #[salsa::tracked] -fn push_d_logs(db: &dyn Database) { +fn push_d_logs(db: &dyn Database, input: Input) { + _ = input.dummy(db); Log("log d".to_string()).accumulate(db); } #[test] fn accumulate_chain() { DatabaseImpl::new().attach(|db| { - let logs = push_logs::accumulated::(db); + let logs = push_logs::accumulated::(db, Input::new(db, ())); // Check that we get all the logs. expect![[r#" [ diff --git a/tests/accumulate-execution-order.rs b/tests/accumulate-execution-order.rs index c71732a55..71709b1f5 100644 --- a/tests/accumulate-execution-order.rs +++ b/tests/accumulate-execution-order.rs @@ -13,39 +13,53 @@ use test_log::test; #[derive(Debug)] struct Log(#[allow(dead_code)] String); +// We need a dummy input, otherwise our query will have a `NEVER_CHANGE` durability, +// and queries with this durability don't accumulate. +#[salsa::input] +struct Input { + dummy: (), +} + #[salsa::tracked] -fn push_logs(db: &dyn Database) { - push_a_logs(db); +fn push_logs(db: &dyn Database, input: Input) { + // We need a dummy input, otherwise our query will have a `NEVER_CHANGE` durability, + // and queries with this durability don't accumulate. + _ = input.dummy(db); + push_a_logs(db, input); } #[salsa::tracked] -fn push_a_logs(db: &dyn Database) { +fn push_a_logs(db: &dyn Database, input: Input) { + _ = input.dummy(db); Log("log a".to_string()).accumulate(db); - push_b_logs(db); - push_c_logs(db); - push_d_logs(db); + push_b_logs(db, input); + push_c_logs(db, input); + push_d_logs(db, input); } #[salsa::tracked] -fn push_b_logs(db: &dyn Database) { +fn push_b_logs(db: &dyn Database, input: Input) { + _ = input.dummy(db); Log("log b".to_string()).accumulate(db); - push_d_logs(db); + push_d_logs(db, input); } #[salsa::tracked] -fn push_c_logs(db: &dyn Database) { +fn push_c_logs(db: &dyn Database, input: Input) { + _ = input.dummy(db); Log("log c".to_string()).accumulate(db); } #[salsa::tracked] -fn push_d_logs(db: &dyn Database) { +fn push_d_logs(db: &dyn Database, input: Input) { + _ = input.dummy(db); Log("log d".to_string()).accumulate(db); } #[test] fn accumulate_execution_order() { salsa::DatabaseImpl::new().attach(|db| { - let logs = push_logs::accumulated::(db); + let logs = push_logs::accumulated::(db, Input::new(db, ())); // Check that we get logs in execution order expect![[r#" [ diff --git a/tests/accumulate-no-duplicates.rs b/tests/accumulate-no-duplicates.rs index 96bae0629..d7117c957 100644 --- a/tests/accumulate-no-duplicates.rs +++ b/tests/accumulate-no-duplicates.rs @@ -41,36 +41,40 @@ fn push_logs(db: &dyn Database) { fn push_a_logs(db: &dyn Database, input: MyInput) { Log("log a".to_string()).accumulate(db); if input.n(db) == 1 { - push_b_logs(db); - push_b_logs(db); - push_c_logs(db); - push_b_logs(db); + push_b_logs(db, input); + push_b_logs(db, input); + push_c_logs(db, input); + push_b_logs(db, input); } else { - push_b_logs(db); + push_b_logs(db, input); } } #[salsa::tracked] -fn push_b_logs(db: &dyn Database) { +fn push_b_logs(db: &dyn Database, input: MyInput) { + _ = input.n(db); Log("log b".to_string()).accumulate(db); } #[salsa::tracked] -fn push_c_logs(db: &dyn Database) { +fn push_c_logs(db: &dyn Database, input: MyInput) { + _ = input.n(db); Log("log c".to_string()).accumulate(db); - push_d_logs(db); - push_e_logs(db); + push_d_logs(db, input); + push_e_logs(db, input); } // Note this isn't tracked -fn push_d_logs(db: &dyn Database) { +fn push_d_logs(db: &dyn Database, input: MyInput) { + _ = input.n(db); Log("log d".to_string()).accumulate(db); push_a_logs(db, MyInput::new(db, 2)); - push_b_logs(db); + push_b_logs(db, input); } #[salsa::tracked] -fn push_e_logs(db: &dyn Database) { +fn push_e_logs(db: &dyn Database, input: MyInput) { + _ = input.n(db); Log("log e".to_string()).accumulate(db); } @@ -98,6 +102,9 @@ fn accumulate_no_duplicates() { Log( "log a", ), + Log( + "log b", + ), Log( "log e", ), diff --git a/tests/backtrace.rs b/tests/backtrace.rs index 3cc5bbad0..d49baede8 100644 --- a/tests/backtrace.rs +++ b/tests/backtrace.rs @@ -81,13 +81,13 @@ fn backtrace_works() { query stacktrace: 0: query_e(Id(1)) -> (R1, Durability::LOW) at tests/backtrace.rs:32 - 1: query_d(Id(1)) -> (R1, Durability::HIGH) + 1: query_d(Id(1)) -> (R1, Durability::NEVER_CHANGE) at tests/backtrace.rs:27 - 2: query_c(Id(1)) -> (R1, Durability::HIGH) + 2: query_c(Id(1)) -> (R1, Durability::NEVER_CHANGE) at tests/backtrace.rs:22 - 3: query_b(Id(1)) -> (R1, Durability::HIGH) + 3: query_b(Id(1)) -> (R1, Durability::NEVER_CHANGE) at tests/backtrace.rs:17 - 4: query_a(Id(1)) -> (R1, Durability::HIGH) + 4: query_a(Id(1)) -> (R1, Durability::NEVER_CHANGE) at tests/backtrace.rs:12 "#]] .assert_eq(&backtrace); @@ -110,10 +110,10 @@ fn backtrace_works() { query stacktrace: 0: query_e(Id(3)) -> (R1, Durability::LOW) at tests/backtrace.rs:32 - 1: query_cycle(Id(3)) -> (R1, Durability::HIGH, iteration = 0) + 1: query_cycle(Id(3)) -> (R1, Durability::NEVER_CHANGE, iteration = 0) at tests/backtrace.rs:45 cycle heads: query_cycle(Id(3)) -> iteration = 0 - 2: query_f(Id(3)) -> (R1, Durability::HIGH) + 2: query_f(Id(3)) -> (R1, Durability::NEVER_CHANGE) at tests/backtrace.rs:40 "#]] .assert_eq(&backtrace); diff --git a/tests/cycle_output.rs b/tests/cycle_output.rs index c4a9384e0..6de592d71 100644 --- a/tests/cycle_output.rs +++ b/tests/cycle_output.rs @@ -14,6 +14,7 @@ struct Output<'db> { #[salsa::input] struct InputValue { value: u32, + dummy: (), } #[salsa::tracked] @@ -23,6 +24,8 @@ fn read_value<'db>(db: &'db dyn Db, output: Output<'db>) -> u32 { #[salsa::tracked] fn query_a(db: &dyn Db, input: InputValue) -> u32 { + // Dummy read so that the durability is not `NEVER_CHANGE`. + _ = input.dummy(db); let val = query_b(db, input); let output = Output::new(db, val); let read = read_value(db, output); @@ -37,6 +40,8 @@ fn query_a(db: &dyn Db, input: InputValue) -> u32 { #[salsa::tracked(cycle_initial=cycle_initial)] fn query_b(db: &dyn Db, input: InputValue) -> u32 { + // Dummy read so that the durability is not `NEVER_CHANGE`. + _ = input.dummy(db); query_a(db, input) } @@ -118,7 +123,7 @@ impl Db for Database {} #[test_log::test] fn single_revision() { let db = Database::default(); - let input = InputValue::new(&db, 1); + let input = InputValue::new(&db, 1, ()); assert_eq!(query_b(&db, input), 3); } @@ -127,8 +132,8 @@ fn single_revision() { fn revalidate_no_changes() { let mut db = Database::default(); - let ab_input = InputValue::new(&db, 1); - let c_input = InputValue::new(&db, 10); + let ab_input = InputValue::new(&db, 1, ()); + let c_input = InputValue::new(&db, 10, ()); assert_eq!(query_c(&db, c_input), 10); assert_eq!(query_b(&db, ab_input), 3); @@ -156,8 +161,8 @@ fn revalidate_no_changes() { fn revalidate_with_change_after_output_read() { let mut db = Database::default(); - let ab_input = InputValue::new(&db, 1); - let d_input = InputValue::new(&db, 10); + let ab_input = InputValue::new(&db, 1, ()); + let d_input = InputValue::new(&db, 10, ()); db.set_input(d_input); assert_eq!(query_b(&db, ab_input), 3); diff --git a/tests/interned-revisions.rs b/tests/interned-revisions.rs index bef1db61c..f904c71bc 100644 --- a/tests/interned-revisions.rs +++ b/tests/interned-revisions.rs @@ -12,6 +12,7 @@ use test_log::test; #[salsa::input] struct Input { field1: usize, + dummy: (), } #[salsa::interned(revisions = 3)] @@ -44,7 +45,7 @@ fn test_intern_new() { } let mut db = common::EventLoggerDatabase::default(); - let input = Input::new(&db, 0); + let input = Input::new(&db, 0, ()); let result_in_rev_1 = function(&db, input); assert_eq!(result_in_rev_1.field1(&db).0, 0); @@ -77,7 +78,7 @@ fn test_reintern() { let mut db = common::EventLoggerDatabase::default(); - let input = Input::new(&db, 0); + let input = Input::new(&db, 0, ()); let result_in_rev_1 = function(&db, input); db.assert_logs(expect![[r#" [ @@ -107,11 +108,14 @@ fn test_reintern() { fn test_durability() { #[salsa::tracked] fn function<'db>(db: &'db dyn Database, _input: Input) -> Interned<'db> { + // Dummy read so the durability won't be `NEVER_CHANGE`. We can't read `field` because we + // want to not depend on it, as we change it later. + _ = _input.dummy(db); Interned::new(db, BadHash(0)) } let mut db = common::EventLoggerDatabase::default(); - let input = Input::new(&db, 0); + let input = Input::new(&db, 0, ()); let result_in_rev_1 = function(&db, input); assert_eq!(result_in_rev_1.field1(&db).0, 0); @@ -130,6 +134,7 @@ fn test_durability() { "DidInternValue { key: Interned(Id(400)), revision: R1 }", "DidSetCancellationFlag", "WillCheckCancellation", + "DidValidateInternedValue { key: Interned(Id(400)), revision: R2 }", "DidValidateMemoizedValue { database_key: function(Id(0)) }", ]"#]]); } @@ -148,7 +153,7 @@ fn test_immortal() { } let mut db = common::EventLoggerDatabase::default(); - let input = Input::new(&db, 0); + let input = Input::new(&db, 0, ()); let result = function(&db, input); assert_eq!(result.field1(&db).0, 0); @@ -172,7 +177,7 @@ fn test_reuse() { } let mut db = common::EventLoggerDatabase::default(); - let input = Input::new(&db, 0); + let input = Input::new(&db, 0, ()); let result = function(&db, input); assert_eq!(result.field1(&db).0, 0); @@ -190,7 +195,7 @@ fn test_reuse() { // Values that have been reused should be re-interned. for i in 1..10 { - let result = function(&db, Input::new(&db, i)); + let result = function(&db, Input::new(&db, i, ())); assert_eq!(result.field1(&db).0, i); } @@ -278,7 +283,7 @@ fn test_reuse_indirect() { } let mut db = common::EventLoggerDatabase::default(); - let input = Input::builder(0).durability(Durability::LOW).new(&db); + let input = Input::builder(0, ()).durability(Durability::LOW).new(&db); // Intern `i0`. let i0 = intern(&db, input, 0); @@ -323,7 +328,7 @@ fn test_reuse_interned_input() { } let mut db = common::EventLoggerDatabase::default(); - let input = Input::new(&db, 0); + let input = Input::new(&db, 0, ()); // Create and use I0 in R0. let interned = create_interned(&db, input); @@ -383,7 +388,7 @@ fn test_reuse_multiple_interned_input() { } let mut db = common::EventLoggerDatabase::default(); - let input = Input::new(&db, 0); + let input = Input::new(&db, 0, ()); // Create and use NI0, which wraps I0, in R0. let interned = create_interned(&db, input); @@ -429,8 +434,8 @@ fn test_durability_increase() { let mut db = common::EventLoggerDatabase::default(); - let high_durability = Input::builder(0).durability(Durability::HIGH).new(&db); - let low_durability = Input::builder(1).durability(Durability::LOW).new(&db); + let high_durability = Input::builder(0, ()).durability(Durability::HIGH).new(&db); + let low_durability = Input::builder(1, ()).durability(Durability::LOW).new(&db); // Intern `i0`. let _i0 = intern(&db, low_durability, 0);