Skip to content

Commit

Permalink
add plumbing to reuse interned slots
Browse files Browse the repository at this point in the history
  • Loading branch information
ibraheemdev committed Oct 23, 2024
1 parent 99ec0a4 commit db71a89
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/active_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/function/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions src/function/specify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <C::Input<'db>>::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");
}
Expand Down Expand Up @@ -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
Expand Down
52 changes: 44 additions & 8 deletions src/interned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<C: Configuration> Default for JarImpl<C> {
Expand Down Expand Up @@ -120,7 +125,8 @@ where
db: &'db dyn crate::Database,
data: impl Lookup<C::Data<'db>>,
) -> 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.
Expand All @@ -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::<Value<C>>(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);
}
Expand All @@ -160,9 +170,13 @@ where
let id = *entry.get();
drop(entry);

// Sync the value's revision.
let value = zalsa.table().get::<Value<C>>(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)
}
Expand All @@ -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)
}
Expand All @@ -199,15 +215,25 @@ 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::<Value<C>>(id);
unsafe { Self::from_internal_data(&internal_data.data) }
let value = db.zalsa().table().get::<Value<C>>(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.
/// Note that this is not "leaking" since no dependency edge is required.
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<C> Ingredient for IngredientImpl<C>
Expand All @@ -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::<Value<C>>(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
}

Expand Down
8 changes: 8 additions & 0 deletions src/revision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,11 @@ impl AtomicRevision {
self.data.store(r.as_usize(), Ordering::SeqCst);
}
}

impl From<Revision> for AtomicRevision {
fn from(value: Revision) -> Self {
Self {
data: AtomicUsize::from(value.as_usize()),
}
}
}
4 changes: 2 additions & 2 deletions src/tracked_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, &current_deps, fields);
C::struct_from_id(id)
}
Expand All @@ -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, &current_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)
}
Expand Down

0 comments on commit db71a89

Please sign in to comment.