diff --git a/crates/salsa/salsa-macros/src/query_group.rs b/crates/salsa/salsa-macros/src/query_group.rs index 7d0eac9f5d8..e535d7ed043 100644 --- a/crates/salsa/salsa-macros/src/query_group.rs +++ b/crates/salsa/salsa-macros/src/query_group.rs @@ -371,6 +371,9 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream QueryStorage::Dependencies => { quote!(salsa::plumbing::DependencyStorage) } + QueryStorage::Input if query.keys.is_empty() => { + quote!(salsa::plumbing::UnitInputStorage) + } QueryStorage::Input => quote!(salsa::plumbing::InputStorage), QueryStorage::Interned => quote!(salsa::plumbing::InternedStorage), QueryStorage::InternedLookup { intern_query_type } => { diff --git a/crates/salsa/src/input.rs b/crates/salsa/src/input.rs index f6188d4a84b..4e8fca6149b 100644 --- a/crates/salsa/src/input.rs +++ b/crates/salsa/src/input.rs @@ -15,6 +15,7 @@ use crate::{DatabaseKeyIndex, QueryDb}; use indexmap::map::Entry; use parking_lot::RwLock; use std::convert::TryFrom; +use std::iter; use tracing::debug; /// Input queries store the result plus a list of the other queries @@ -25,15 +26,12 @@ where Q: Query, { group_index: u16, - slots: RwLock>>, + slots: RwLock>>, } -struct Slot -where - Q: Query, -{ +struct Slot { database_key_index: DatabaseKeyIndex, - stamped_value: RwLock>, + stamped_value: RwLock>, } impl std::panic::RefUnwindSafe for InputStorage @@ -78,7 +76,14 @@ where debug_assert!(revision < db.salsa_runtime().current_revision()); let slots = &self.slots.read(); let slot = slots.get_index(input.key_index as usize).unwrap().1; - slot.maybe_changed_after(db, revision) + + debug!("maybe_changed_after(slot={:?}, revision={:?})", Q::default(), revision,); + + let changed_at = slot.stamped_value.read().changed_at; + + debug!("maybe_changed_after: changed_at = {:?}", changed_at); + + changed_at > revision } fn fetch(&self, db: &>::DynDb, key: &Q::Key) -> Q::Value { @@ -121,21 +126,6 @@ where } } -impl Slot -where - Q: Query, -{ - fn maybe_changed_after(&self, _db: &>::DynDb, revision: Revision) -> bool { - debug!("maybe_changed_after(slot={:?}, revision={:?})", self, revision,); - - let changed_at = self.stamped_value.read().changed_at; - - debug!("maybe_changed_after: changed_at = {:?}", changed_at); - - changed_at > revision - } -} - impl QueryStorageMassOps for InputStorage where Q: Query, @@ -202,6 +192,167 @@ where } } +/// Same as `InputStorage`, but optimized for queries that take no inputs. +pub struct UnitInputStorage +where + Q: Query, +{ + group_index: u16, + slot: UnitSlot, +} + +struct UnitSlot { + database_key_index: DatabaseKeyIndex, + stamped_value: RwLock>>, +} + +impl std::panic::RefUnwindSafe for UnitInputStorage +where + Q: Query, + Q::Key: std::panic::RefUnwindSafe, + Q::Value: std::panic::RefUnwindSafe, +{ +} + +impl QueryStorageOps for UnitInputStorage +where + Q: Query, +{ + const CYCLE_STRATEGY: crate::plumbing::CycleRecoveryStrategy = CycleRecoveryStrategy::Panic; + + fn new(group_index: u16) -> Self { + let database_key_index = + DatabaseKeyIndex { group_index, query_index: Q::QUERY_INDEX, key_index: 0 }; + UnitInputStorage { + group_index, + slot: UnitSlot { database_key_index, stamped_value: RwLock::new(None) }, + } + } + + fn fmt_index( + &self, + _db: &>::DynDb, + index: DatabaseKeyIndex, + fmt: &mut std::fmt::Formatter<'_>, + ) -> std::fmt::Result { + assert_eq!(index.group_index, self.group_index); + assert_eq!(index.query_index, Q::QUERY_INDEX); + write!(fmt, "{}", Q::QUERY_NAME) + } + + fn maybe_changed_after( + &self, + db: &>::DynDb, + input: DatabaseKeyIndex, + revision: Revision, + ) -> bool { + assert_eq!(input.group_index, self.group_index); + assert_eq!(input.query_index, Q::QUERY_INDEX); + debug_assert!(revision < db.salsa_runtime().current_revision()); + + debug!("maybe_changed_after(slot={:?}, revision={:?})", Q::default(), revision,); + + let changed_at = self.slot.stamped_value.read().as_ref().unwrap().changed_at; + + debug!("maybe_changed_after: changed_at = {:?}", changed_at); + + changed_at > revision + } + + fn fetch(&self, db: &>::DynDb, &(): &Q::Key) -> Q::Value { + db.unwind_if_cancelled(); + + let StampedValue { value, durability, changed_at } = self + .slot + .stamped_value + .read() + .clone() + .unwrap_or_else(|| panic!("no value set for {:?}", Q::default())); + + db.salsa_runtime().report_query_read_and_unwind_if_cycle_resulted( + self.slot.database_key_index, + durability, + changed_at, + ); + + value + } + + fn durability(&self, _db: &>::DynDb, &(): &Q::Key) -> Durability { + match &*self.slot.stamped_value.read() { + Some(stamped_value) => stamped_value.durability, + None => panic!("no value set for {:?}", Q::default(),), + } + } + + fn entries(&self, _db: &>::DynDb) -> C + where + C: std::iter::FromIterator>, + { + iter::once(TableEntry::new( + (), + self.slot.stamped_value.read().as_ref().map(|it| it.value.clone()), + )) + .collect() + } +} + +impl QueryStorageMassOps for UnitInputStorage +where + Q: Query, +{ + fn purge(&self) { + *self.slot.stamped_value.write() = Default::default(); + } +} + +impl InputQueryStorageOps for UnitInputStorage +where + Q: Query, +{ + fn set(&self, runtime: &mut Runtime, (): &Q::Key, value: Q::Value, durability: Durability) { + tracing::debug!("{:?} = {:?} ({:?})", Q::default(), value, durability); + + // The value is changing, so we need a new revision (*). We also + // need to update the 'last changed' revision by invoking + // `guard.mark_durability_as_changed`. + // + // CAREFUL: This will block until the global revision lock can + // be acquired. If there are still queries executing, they may + // need to read from this input. Therefore, we wait to acquire + // the lock on `map` until we also hold the global query write + // lock. + // + // (*) Technically, since you can't presently access an input + // for a non-existent key, and you can't enumerate the set of + // keys, we only need a new revision if the key used to + // exist. But we may add such methods in the future and this + // case doesn't generally seem worth optimizing for. + runtime.with_incremented_revision(|next_revision| { + let mut stamped_value_slot = self.slot.stamped_value.write(); + + // Do this *after* we acquire the lock, so that we are not + // racing with somebody else to modify this same cell. + // (Otherwise, someone else might write a *newer* revision + // into the same cell while we block on the lock.) + let stamped_value = StampedValue { value, durability, changed_at: next_revision }; + + match &mut *stamped_value_slot { + Some(slot_stamped_value) => { + let old_durability = slot_stamped_value.durability; + *slot_stamped_value = stamped_value; + Some(old_durability) + } + + stamped_value_slot @ None => { + *stamped_value_slot = Some(stamped_value); + None + } + } + }); + } +} + /// Check that `Slot: Send + Sync` as long as /// `DB::DatabaseData: Send + Sync`, which in turn implies that /// `Q::Key: Send + Sync`, `Q::Value: Send + Sync`. @@ -213,7 +364,8 @@ where Q::Value: Send + Sync, { fn is_send_sync() {} - is_send_sync::>(); + is_send_sync::>(); + is_send_sync::>(); } /// Check that `Slot: 'static` as long as @@ -227,14 +379,6 @@ where Q::Value: 'static, { fn is_static() {} - is_static::>(); -} - -impl std::fmt::Debug for Slot -where - Q: Query, -{ - fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(fmt, "{:?}", Q::default()) - } + is_static::>(); + is_static::>(); } diff --git a/crates/salsa/src/plumbing.rs b/crates/salsa/src/plumbing.rs index 560a9b8315f..71332e39cad 100644 --- a/crates/salsa/src/plumbing.rs +++ b/crates/salsa/src/plumbing.rs @@ -15,7 +15,7 @@ use triomphe::Arc; pub use crate::derived::DependencyStorage; pub use crate::derived::MemoizedStorage; -pub use crate::input::InputStorage; +pub use crate::input::{InputStorage, UnitInputStorage}; pub use crate::interned::InternedStorage; pub use crate::interned::LookupInternedStorage; pub use crate::{revision::Revision, DatabaseKeyIndex, QueryDb, Runtime};