diff --git a/src/active_query.rs b/src/active_query.rs index bb5987fcd..69555c3b3 100644 --- a/src/active_query.rs +++ b/src/active_query.rs @@ -203,13 +203,6 @@ impl ActiveQuery { accumulated_inputs, } = self; - let origin = if untracked_read { - QueryOrigin::derived_untracked(input_outputs.drain(..).collect()) - } else { - QueryOrigin::derived(input_outputs.drain(..).collect()) - }; - disambiguator_map.clear(); - #[cfg(feature = "accumulator")] let accumulated_inputs = AtomicInputAccumulatedValues::new(accumulated_inputs); let verified_final = cycle_heads.is_empty(); @@ -222,6 +215,23 @@ impl ActiveQuery { mem::take(cycle_heads), iteration_count, ); + #[cfg(not(feature = "accumulator"))] + let has_accumulated_inputs = || false; + #[cfg(feature = "accumulator")] + let has_accumulated_inputs = || accumulated_inputs.load().is_any(); + let origin = + if durability == Durability::IMMUTABLE && extra.is_empty() && !has_accumulated_inputs() + { + // We only depend on immutable inputs, we can discard our dependencies + // as we will never be invalidated again + input_outputs.clear(); + QueryOrigin::derived_immutable() + } else if untracked_read { + QueryOrigin::derived_untracked(input_outputs.drain(..).collect()) + } else { + QueryOrigin::derived(input_outputs.drain(..).collect()) + }; + disambiguator_map.clear(); let revisions = QueryRevisions { changed_at, diff --git a/src/database.rs b/src/database.rs index 0df83b03b..046c40856 100644 --- a/src/database.rs +++ b/src/database.rs @@ -53,6 +53,10 @@ pub trait Database: Send + ZalsaDatabase + AsDynDatabase { /// cancellation. If you invoke it while a snapshot exists, it /// will block until that snapshot is dropped -- if that snapshot /// is owned by the current thread, this could trigger deadlock. + /// + /// # Panics + /// + /// Panics if `durability` is `Durability::IMMUTABLE`. fn synthetic_write(&mut self, durability: Durability) { let zalsa_mut = self.zalsa_mut(); zalsa_mut.new_revision(); diff --git a/src/durability.rs b/src/durability.rs index 4691e9372..852bf406d 100644 --- a/src/durability.rs +++ b/src/durability.rs @@ -1,3 +1,5 @@ +use std::fmt; + /// Describes how likely a value is to change—how "durable" it is. /// /// By default, inputs have `Durability::LOW` and interned values have @@ -42,11 +44,7 @@ impl<'de> serde::Deserialize<'de> for Durability { impl std::fmt::Debug for Durability { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if f.alternate() { - match self.0 { - DurabilityVal::Low => f.write_str("Durability::LOW"), - DurabilityVal::Medium => f.write_str("Durability::MEDIUM"), - DurabilityVal::High => f.write_str("Durability::HIGH"), - } + fmt::Display::fmt(self, f) } else { f.debug_tuple("Durability") .field(&(self.0 as usize)) @@ -55,12 +53,26 @@ impl std::fmt::Debug for Durability { } } +impl fmt::Display for Durability { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Durability::")?; + match self.0 { + DurabilityVal::Low => write!(f, "Low"), + DurabilityVal::Medium => write!(f, "Medium"), + DurabilityVal::High => write!(f, "High"), + DurabilityVal::Immutable => write!(f, "Immutable"), + } + } +} + // We use an enum here instead of a u8 for niches. -#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +// Note that the order is important here +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug)] enum DurabilityVal { Low = 0, Medium = 1, High = 2, + Immutable = 3, } impl From for DurabilityVal { @@ -69,7 +81,8 @@ impl From for DurabilityVal { 0 => DurabilityVal::Low, 1 => DurabilityVal::Medium, 2 => DurabilityVal::High, - _ => panic!("invalid durability"), + 3 => DurabilityVal::Immutable, + _ => unreachable!("invalid durability"), } } } @@ -91,26 +104,33 @@ impl Durability { /// Example: the standard library or something from crates.io pub const HIGH: Durability = Durability(DurabilityVal::High); + /// Immutable durability: things that are not expected to change. + /// + /// Example: the standard library or something from crates.io + pub const IMMUTABLE: Durability = Durability(DurabilityVal::Immutable); +} + +impl Default for Durability { + fn default() -> Self { + Durability::LOW + } +} + +impl Durability { /// 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; - /// The maximum possible durability; equivalent to HIGH but + /// The maximum possible durability; equivalent to IMMUTABLE 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::IMMUTABLE; /// Number of durability levels. - pub(crate) const LEN: usize = Self::HIGH.0 as usize + 1; + pub(crate) const LEN: usize = Self::IMMUTABLE.0 as usize + 1; pub(crate) fn index(self) -> usize { self.0 as usize } } - -impl Default for Durability { - fn default() -> Self { - Durability::LOW - } -} diff --git a/src/function.rs b/src/function.rs index 045825e19..1e322bcee 100644 --- a/src/function.rs +++ b/src/function.rs @@ -188,6 +188,7 @@ pub struct IngredientImpl { /// we don't know that we can trust the database to give us the same runtime /// everytime and so forth. deleted_entries: DeletedEntries, + immutable_memos: crossbeam_queue::SegQueue, } impl IngredientImpl @@ -206,6 +207,7 @@ where deleted_entries: Default::default(), view_caster: OnceLock::new(), sync_table: SyncTable::new(index), + immutable_memos: crossbeam_queue::SegQueue::new(), } } @@ -659,6 +661,7 @@ mod persistence { QueryOrigin::derived_untracked(flattened_edges.drain(..).collect()) } + QueryOriginRef::DerivedImmutable => QueryOrigin::derived_immutable(), QueryOriginRef::Assigned(key) => { let dependency = zalsa.lookup_ingredient(key.ingredient_index()); assert!( diff --git a/src/function/execute.rs b/src/function/execute.rs index 9d6758730..4f901b8f7 100644 --- a/src/function/execute.rs +++ b/src/function/execute.rs @@ -129,7 +129,7 @@ where // outputs and update the tracked struct IDs for seeding the next revision. self.diff_outputs(zalsa, database_key_index, old_memo, &completed_query); } - + let immutable = completed_query.revisions.origin.is_immutable(); let memo = self.insert_memo( zalsa, id, @@ -144,6 +144,9 @@ where if claim_guard.drop() { None } else { + if immutable { + self.immutable_memos.push(id); + } Some(memo) } } @@ -470,6 +473,7 @@ where .revisions .update_iteration_count_mut(database_key_index, iteration_count); + let immutable = completed_query.revisions.origin.is_immutable(); let new_memo = self.insert_memo( zalsa, id, @@ -480,6 +484,9 @@ where ), memo_ingredient_index, ); + if immutable { + self.immutable_memos.push(id); + } last_provisional_memo = Some(new_memo); diff --git a/src/function/fetch.rs b/src/function/fetch.rs index 588b08bb1..de540ddf3 100644 --- a/src/function/fetch.rs +++ b/src/function/fetch.rs @@ -274,7 +274,8 @@ where )); // We need this for `cycle_heads()` to work. We will unset this in the outer `execute()`. *completed_query.revisions.verified_final.get_mut() = false; - self.insert_memo( + let immutable = completed_query.revisions.origin.is_immutable(); + let memo = self.insert_memo( zalsa, id, Memo::new( @@ -283,7 +284,11 @@ where completed_query.revisions, ), memo_ingredient_index, - ) + ); + if immutable { + self.immutable_memos.push(id); + } + memo } } } diff --git a/src/function/maybe_changed_after.rs b/src/function/maybe_changed_after.rs index 20440883e..66921b6c9 100644 --- a/src/function/maybe_changed_after.rs +++ b/src/function/maybe_changed_after.rs @@ -30,8 +30,8 @@ pub enum VerifyResult { } impl VerifyResult { - pub(crate) const fn changed_if(changed: bool) -> Self { - if changed { + pub(crate) fn changed_after(revision: Revision, after: Revision) -> Self { + if revision > after { Self::changed() } else { Self::unchanged() @@ -557,6 +557,8 @@ where debug_assert!(!cycle_heads.contains_head(database_key_index)); match old_memo.revisions.origin.as_ref() { + // Shouldn't end up here, shallow verify ought to always pass + QueryOriginRef::DerivedImmutable => VerifyResult::unchanged(), QueryOriginRef::Derived(edges) => { #[cfg(feature = "accumulator")] let mut inputs = InputAccumulatedValues::Empty; diff --git a/src/function/memo.rs b/src/function/memo.rs index d8faf3e0b..6c984a12d 100644 --- a/src/function/memo.rs +++ b/src/function/memo.rs @@ -68,7 +68,8 @@ impl IngredientImpl { match memo.revisions.origin.as_ref() { QueryOriginRef::Assigned(_) | QueryOriginRef::DerivedUntracked(_) - | QueryOriginRef::FixpointInitial => { + | QueryOriginRef::FixpointInitial + | QueryOriginRef::DerivedImmutable => { // Careful: Cannot evict memos whose values were // assigned as output of another query // or those with untracked inputs diff --git a/src/input.rs b/src/input.rs index 0ce0f5c87..a41bdabf7 100644 --- a/src/input.rs +++ b/src/input.rs @@ -176,6 +176,10 @@ impl IngredientImpl { /// * `field_index`, index of the field that will be changed /// * `durability`, durability of the new value. If omitted, uses the durability of the previous value. /// * `setter`, function that modifies the fields tuple; should only modify the element for `field_index` + /// + /// # Panics + /// + /// Panics if `durability` is [`Durability::IMMUTABLE`]. pub fn set_field( &mut self, runtime: &mut Runtime, @@ -195,9 +199,7 @@ impl IngredientImpl { data.revisions[field_index] = runtime.current_revision(); let field_durability = &mut data.durabilities[field_index]; - if *field_durability != Durability::MIN { - runtime.report_tracked_write(*field_durability); - } + runtime.report_tracked_write(*field_durability); *field_durability = durability.unwrap_or(*field_durability); setter(&mut data.fields) diff --git a/src/input/input_field.rs b/src/input/input_field.rs index f874142e8..08425905a 100644 --- a/src/input/input_field.rs +++ b/src/input/input_field.rs @@ -60,7 +60,7 @@ where _cycle_heads: &mut VerifyCycleHeads, ) -> VerifyResult { let value = >::data(zalsa, input); - VerifyResult::changed_if(value.revisions[self.field_index] > revision) + VerifyResult::changed_after(value.revisions[self.field_index], revision) } fn collect_minimum_serialized_edges( diff --git a/src/input/setter.rs b/src/input/setter.rs index 4f157565c..e09f98913 100644 --- a/src/input/setter.rs +++ b/src/input/setter.rs @@ -6,7 +6,13 @@ use crate::{Durability, Runtime}; /// Setter for a field of an input. pub trait Setter: Sized { type FieldTy; + /// Sets a new durability for the field. fn with_durability(self, durability: Durability) -> Self; + /// Sets the value of the field. + /// + /// # Panics + /// + /// Panics if `with_durability` was called with [`Durability::IMMUTABLE`]. fn to(self, value: Self::FieldTy) -> Self::FieldTy; } diff --git a/src/interned.rs b/src/interned.rs index 544a8d0ee..14263d114 100644 --- a/src/interned.rs +++ b/src/interned.rs @@ -475,7 +475,7 @@ where .map(|(_, stamp)| (stamp.durability, current_revision)) // If there is no active query this durability does not actually matter. // `last_interned_at` needs to be `Revision::MAX`, see the `intern_access_in_different_revision` test. - .unwrap_or((Durability::MAX, Revision::max())); + .unwrap_or((Durability::IMMUTABLE, Revision::max())); let old_id = value_shared.id; @@ -605,7 +605,7 @@ where .map(|(_, stamp)| (stamp.durability, current_revision)) // If there is no active query this durability does not actually matter. // `last_interned_at` needs to be `Revision::MAX`, see the `intern_access_in_different_revision` test. - .unwrap_or((Durability::MAX, Revision::max())); + .unwrap_or((Durability::IMMUTABLE, Revision::max())); // Allocate the value slot. let (id, value) = zalsa_local.allocate(zalsa, self.ingredient_index, |id| Value:: { diff --git a/src/runtime.rs b/src/runtime.rs index 48caf53ec..d49fa4371 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -19,9 +19,9 @@ pub struct Runtime { #[cfg_attr(feature = "persistence", serde(skip))] revision_canceled: AtomicBool, - /// Stores the "last change" revision for values of each duration. - /// This vector is always of length at least 1 (for Durability 0) - /// but its total length depends on the number of durations. The + /// Stores the "last change" revision for values of each [`Durability`]. + /// This vector is always of length at least 1 (for [`Durability::MIN`]) + /// but its total length depends on the number of durabilities. The /// element at index 0 is special as it represents the "current /// revision". In general, we have the invariant that revisions /// in here are *declining* -- that is, `revisions[i] >= @@ -209,7 +209,16 @@ impl Runtime { /// Reports that an input with durability `durability` changed. /// This will update the 'last changed at' values for every durability /// less than or equal to `durability` to the current revision. + /// + /// # Panics + /// + /// Panics if `durability` is `Durability::IMMUTABLE`. pub(crate) fn report_tracked_write(&mut self, durability: Durability) { + assert_ne!( + durability, + Durability::IMMUTABLE, + "can't write revision of immutable durability" + ); let new_revision = self.current_revision(); self.revisions[1..=durability.index()].fill(new_revision); } diff --git a/src/tracked_struct/tracked_field.rs b/src/tracked_struct/tracked_field.rs index abe435bea..b68eb880c 100644 --- a/src/tracked_struct/tracked_field.rs +++ b/src/tracked_struct/tracked_field.rs @@ -66,7 +66,7 @@ where ) -> VerifyResult { let data = >::data(zalsa.table(), input); let field_changed_at = data.revisions[self.field_index]; - VerifyResult::changed_if(field_changed_at > revision) + VerifyResult::changed_after(field_changed_at, revision) } fn collect_minimum_serialized_edges( diff --git a/src/zalsa_local.rs b/src/zalsa_local.rs index f43eb78eb..26cc9a35f 100644 --- a/src/zalsa_local.rs +++ b/src/zalsa_local.rs @@ -522,6 +522,10 @@ impl QueryRevisionsExtra { Self(inner) } + + pub fn is_empty(&self) -> bool { + self.0.is_none() + } } #[cfg_attr(feature = "persistence", derive(serde::Serialize, serde::Deserialize))] @@ -805,6 +809,10 @@ pub enum QueryOriginRef<'a> { /// (but we know there were more). DerivedUntracked(&'a [QueryEdge]) = QueryOriginKind::DerivedUntracked as u8, + /// The value was derived by executing a function + /// but that function only read from immutable inputs. + DerivedImmutable, + /// The value is an initial provisional value for a query that supports fixpoint iteration. FixpointInitial = QueryOriginKind::FixpointInitial as u8, } @@ -816,7 +824,9 @@ impl<'a> QueryOriginRef<'a> { pub(crate) fn inputs(self) -> impl DoubleEndedIterator + use<'a> { let opt_edges = match self { QueryOriginRef::Derived(edges) | QueryOriginRef::DerivedUntracked(edges) => Some(edges), - QueryOriginRef::Assigned(_) | QueryOriginRef::FixpointInitial => None, + QueryOriginRef::Assigned(_) + | QueryOriginRef::FixpointInitial + | QueryOriginRef::DerivedImmutable => None, }; opt_edges.into_iter().flat_map(input_edges) } @@ -825,7 +835,9 @@ impl<'a> QueryOriginRef<'a> { pub(crate) fn outputs(self) -> impl DoubleEndedIterator + use<'a> { let opt_edges = match self { QueryOriginRef::Derived(edges) | QueryOriginRef::DerivedUntracked(edges) => Some(edges), - QueryOriginRef::Assigned(_) | QueryOriginRef::FixpointInitial => None, + QueryOriginRef::Assigned(_) + | QueryOriginRef::FixpointInitial + | QueryOriginRef::DerivedImmutable => None, }; opt_edges.into_iter().flat_map(output_edges) } @@ -834,7 +846,9 @@ impl<'a> QueryOriginRef<'a> { pub(crate) fn edges(self) -> &'a [QueryEdge] { let opt_edges = match self { QueryOriginRef::Derived(edges) | QueryOriginRef::DerivedUntracked(edges) => Some(edges), - QueryOriginRef::Assigned(_) | QueryOriginRef::FixpointInitial => None, + QueryOriginRef::Assigned(_) + | QueryOriginRef::FixpointInitial + | QueryOriginRef::DerivedImmutable => None, }; opt_edges.unwrap_or_default() @@ -859,11 +873,15 @@ enum QueryOriginKind { /// The value was derived by executing a function /// _and_ Salsa was able to track all of said function's inputs. - Derived = 0b11, + Derived = 0b10, /// The value was derived by executing a function /// but that function also reported that it read untracked inputs. - DerivedUntracked = 0b10, + DerivedImmutable = 0b11, + + /// The value was derived by executing a function + /// but that function also reported that it read untracked inputs. + DerivedUntracked = 0b100, } /// Tracks how a memoized value for a given query was created. @@ -958,6 +976,14 @@ impl QueryOrigin { origin } + pub(crate) fn derived_immutable() -> QueryOrigin { + QueryOrigin { + kind: QueryOriginKind::DerivedImmutable, + metadata: 0, + data: QueryOriginData { empty: () }, + } + } + /// Create a query origin of type `QueryOriginKind::Assigned`, with the given key. pub fn assigned(key: DatabaseKeyIndex) -> QueryOrigin { QueryOrigin { @@ -969,6 +995,10 @@ impl QueryOrigin { } } + pub fn is_immutable(&self) -> bool { + matches!(self.kind, QueryOriginKind::DerivedImmutable) + } + /// Return a read-only reference to this query origin. pub fn as_ref(&self) -> QueryOriginRef<'_> { match self.kind { @@ -1010,6 +1040,7 @@ impl QueryOrigin { } QueryOriginKind::FixpointInitial => QueryOriginRef::FixpointInitial, + QueryOriginKind::DerivedImmutable => QueryOriginRef::DerivedImmutable, } } } @@ -1039,6 +1070,7 @@ impl<'de> serde::Deserialize<'de> for QueryOrigin { Derived(Box<[QueryEdge]>) = QueryOriginKind::Derived as u8, DerivedUntracked(Box<[QueryEdge]>) = QueryOriginKind::DerivedUntracked as u8, FixpointInitial = QueryOriginKind::FixpointInitial as u8, + DerivedImmutable = QueryOriginKind::DerivedImmutable as u8, } Ok(match QueryOriginOwned::deserialize(deserializer)? { @@ -1046,6 +1078,7 @@ impl<'de> serde::Deserialize<'de> for QueryOrigin { QueryOriginOwned::Derived(edges) => QueryOrigin::derived(edges), QueryOriginOwned::DerivedUntracked(edges) => QueryOrigin::derived_untracked(edges), QueryOriginOwned::FixpointInitial => QueryOrigin::fixpoint_initial(), + QueryOriginOwned::DerivedImmutable => QueryOrigin::derived_immutable(), }) } } @@ -1071,7 +1104,9 @@ impl Drop for QueryOrigin { } // The data stored for this variants is `Copy`. - QueryOriginKind::FixpointInitial | QueryOriginKind::Assigned => {} + QueryOriginKind::FixpointInitial + | QueryOriginKind::Assigned + | QueryOriginKind::DerivedImmutable => {} } } } diff --git a/tests/backtrace.rs b/tests/backtrace.rs index 3cc5bbad0..749c8846f 100644 --- a/tests/backtrace.rs +++ b/tests/backtrace.rs @@ -79,15 +79,15 @@ fn backtrace_works() { let backtrace = query_a(&db, Thing::new(&db, true)).replace("\\", "/"); expect![[r#" query stacktrace: - 0: query_e(Id(1)) -> (R1, Durability::LOW) + 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::Immutable) at tests/backtrace.rs:27 - 2: query_c(Id(1)) -> (R1, Durability::HIGH) + 2: query_c(Id(1)) -> (R1, Durability::Immutable) at tests/backtrace.rs:22 - 3: query_b(Id(1)) -> (R1, Durability::HIGH) + 3: query_b(Id(1)) -> (R1, Durability::Immutable) at tests/backtrace.rs:17 - 4: query_a(Id(1)) -> (R1, Durability::HIGH) + 4: query_a(Id(1)) -> (R1, Durability::Immutable) at tests/backtrace.rs:12 "#]] .assert_eq(&backtrace); @@ -108,12 +108,12 @@ fn backtrace_works() { let backtrace = query_f(&db, Thing::new(&db, true)).replace("\\", "/"); expect![[r#" query stacktrace: - 0: query_e(Id(3)) -> (R1, Durability::LOW) + 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::Immutable, 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::Immutable) at tests/backtrace.rs:40 "#]] .assert_eq(&backtrace);