From db71a89137c84e4f9755850e0edd83dbc07926fa Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Wed, 23 Oct 2024 16:52:58 -0400 Subject: [PATCH] add plumbing to reuse interned slots --- src/active_query.rs | 2 +- src/function/fetch.rs | 2 +- src/function/specify.rs | 4 ++-- src/interned.rs | 52 ++++++++++++++++++++++++++++++++++------- src/revision.rs | 8 +++++++ src/tracked_struct.rs | 4 ++-- 6 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/active_query.rs b/src/active_query.rs index ef9778d5e..ebdff8a2c 100644 --- a/src/active_query.rs +++ b/src/active_query.rs @@ -142,7 +142,7 @@ impl ActiveQuery { /// Used during cycle recovery, see [`Runtime::unblock_cycle_and_maybe_throw`]. pub(super) fn remove_cycle_participants(&mut self, cycle: &Cycle) { for p in cycle.participant_keys() { - let p: DatabaseKeyIndex = p.into(); + let p: DatabaseKeyIndex = p; self.input_outputs.shift_remove(&(EdgeKind::Input, p)); } } diff --git a/src/function/fetch.rs b/src/function/fetch.rs index 07a08d96c..b2161cbae 100644 --- a/src/function/fetch.rs +++ b/src/function/fetch.rs @@ -21,7 +21,7 @@ where self.evict_value_from_memo_for(zalsa, evicted); } - zalsa_local.report_tracked_read(self.database_key_index(id).into(), durability, changed_at); + zalsa_local.report_tracked_read(self.database_key_index(id), durability, changed_at); value } diff --git a/src/function/specify.rs b/src/function/specify.rs index 9eccad65b..46e70c353 100644 --- a/src/function/specify.rs +++ b/src/function/specify.rs @@ -39,7 +39,7 @@ where // // Now, if We invoke Q3 first, We get one result for Q2, but if We invoke Q4 first, We get a different value. That's no good. let database_key_index = >::database_key_index(db.as_dyn_database(), key); - let dependency_index = database_key_index.into(); + let dependency_index = database_key_index; if !zalsa_local.is_output_of_active_query(dependency_index) { panic!("can only use `specify` on salsa structs created during the current tracked fn"); } @@ -92,7 +92,7 @@ where // Record that the current query *specified* a value for this cell. let database_key_index = self.database_key_index(key); - zalsa_local.add_output(database_key_index.into()); + zalsa_local.add_output(database_key_index); } /// Invoked when the query `executor` has been validated as having green inputs diff --git a/src/interned.rs b/src/interned.rs index e64f9ee50..0477e97f4 100644 --- a/src/interned.rs +++ b/src/interned.rs @@ -2,6 +2,7 @@ use crate::durability::Durability; use crate::id::AsId; use crate::ingredient::fmt_index; use crate::plumbing::{Jar, JarAux}; +use crate::revision::AtomicRevision; use crate::table::memo::MemoTable; use crate::table::sync::SyncTable; use crate::table::Slot; @@ -67,6 +68,10 @@ where data: C::Data<'static>, memos: MemoTable, syncs: SyncTable, + // The revision the value was first interned in. + first_interned_at: Revision, + // The most recent interned revision. + last_interned_at: AtomicRevision, } impl Default for JarImpl { @@ -120,7 +125,8 @@ where db: &'db dyn crate::Database, data: impl Lookup>, ) -> C::Struct<'db> { - let zalsa_local = db.zalsa_local(); + let (zalsa, zalsa_local) = db.zalsas(); + let current_revision = zalsa.current_revision(); // Optimisation to only get read lock on the map if the data has already // been interned. @@ -142,9 +148,13 @@ where // SAFETY: Read lock on map is held during this block let id = unsafe { *bucket.as_ref().1.get() }; + // Sync the value's revision. + let value = zalsa.table().get::>(id); + value.last_interned_at.store(current_revision); + // Record a dependency on this value. let index = self.database_key_index(id); - zalsa_local.report_tracked_read(index, Durability::MAX, Revision::start()); + zalsa_local.report_tracked_read(index, Durability::MAX, value.first_interned_at); return C::struct_from_id(id); } @@ -160,9 +170,13 @@ where let id = *entry.get(); drop(entry); + // Sync the value's revision. + let value = zalsa.table().get::>(id); + value.last_interned_at.store(current_revision); + // Record a dependency on this value. let index = self.database_key_index(id); - zalsa_local.report_tracked_read(index, Durability::MAX, Revision::start()); + zalsa_local.report_tracked_read(index, Durability::MAX, value.first_interned_at); C::struct_from_id(id) } @@ -175,12 +189,14 @@ where data: internal_data, memos: Default::default(), syncs: Default::default(), + first_interned_at: current_revision, + last_interned_at: AtomicRevision::from(current_revision), }); entry.insert(next_id); // Record a dependency on this value. let index = self.database_key_index(next_id); - zalsa_local.report_tracked_read(index, Durability::MAX, Revision::start()); + zalsa_local.report_tracked_read(index, Durability::MAX, current_revision); C::struct_from_id(next_id) } @@ -199,8 +215,12 @@ where /// Rarely used since end-users generally carry a struct with a pointer directly /// to the interned item. pub fn data<'db>(&'db self, db: &'db dyn Database, id: Id) -> &'db C::Data<'db> { - let internal_data = db.zalsa().table().get::>(id); - unsafe { Self::from_internal_data(&internal_data.data) } + let value = db.zalsa().table().get::>(id); + assert!( + value.last_interned_at.load() >= db.zalsa().current_revision(), + "Data was not interned in the current revision." + ); + unsafe { Self::from_internal_data(&value.data) } } /// Lookup the fields from an interned struct. @@ -208,6 +228,12 @@ where pub fn fields<'db>(&'db self, db: &'db dyn Database, s: C::Struct<'db>) -> &'db C::Data<'db> { self.data(db, C::deref_struct(s)) } + + pub fn reset(&mut self, db: &mut dyn Database) { + // Trigger a new revision. + let _zalsa_mut = db.zalsa_mut(); + self.key_map.clear(); + } } impl Ingredient for IngredientImpl @@ -218,8 +244,18 @@ where self.ingredient_index } - fn maybe_changed_after(&self, _db: &dyn Database, _input: Id, _revision: Revision) -> bool { - // Interned data currently never changes. + fn maybe_changed_after(&self, db: &dyn Database, input: Id, revision: Revision) -> bool { + let value = db.zalsa().table().get::>(input); + if value.first_interned_at > revision { + // The slot was reused. + return true; + } + + if value.first_interned_at < revision { + // The slot is valid in this revision but we have to sync the value's revision. + value.last_interned_at.store(db.zalsa().current_revision()); + } + false } diff --git a/src/revision.rs b/src/revision.rs index a74456102..0c62c6216 100644 --- a/src/revision.rs +++ b/src/revision.rs @@ -62,3 +62,11 @@ impl AtomicRevision { self.data.store(r.as_usize(), Ordering::SeqCst); } } + +impl From for AtomicRevision { + fn from(value: Revision) -> Self { + Self { + data: AtomicUsize::from(value.as_usize()), + } + } +} diff --git a/src/tracked_struct.rs b/src/tracked_struct.rs index b0276e0df..6e92a94e8 100644 --- a/src/tracked_struct.rs +++ b/src/tracked_struct.rs @@ -291,7 +291,7 @@ where match zalsa_local.tracked_struct_id(&identity) { Some(id) => { // The struct already exists in the intern map. - zalsa_local.add_output(self.database_key_index(id).into()); + zalsa_local.add_output(self.database_key_index(id)); self.update(zalsa, current_revision, id, ¤t_deps, fields); C::struct_from_id(id) } @@ -300,7 +300,7 @@ where // This is a new tracked struct, so create an entry in the struct map. let id = self.allocate(zalsa, zalsa_local, current_revision, ¤t_deps, fields); let key = self.database_key_index(id); - zalsa_local.add_output(key.into()); + zalsa_local.add_output(key); zalsa_local.store_tracked_struct_id(identity, id); C::struct_from_id(id) }