Enable Garbage Collection for Interned Values (#602)

* remove table-wide dependencies

* add plumbing to reuse interned slots

* record durabilities on interned values

* appease clippy

* remove immortal interned value logic

* pass correct revision when tracking interned reads

* force new revision when resetting interned values

* avoid unnecessary calls to `Database::zalsa`

* add log events for value internment

* Only log event kind because thread id can differ between runs/computers

* cargo fmt

---------

Co-authored-by: Micha Reiser <[email protected]>

Author: ibraheemdev

book/src/plumbing/jars_and_ingredients.md

@@ -146,9 +146,6 @@ It combines an [`IngredientIndex`] with a `key_index`, which is a `salsa::Id`:
 {{#include ../../../src/key.rs:DatabaseKeyIndex}}
 ```
 
-A `DependencyIndex` is similar, but the `key_index` is optional.
-This is used when we sometimes wish to refer to the ingredient as a whole, and not any specific value within the ingredient.
-
 These kinds of indices are used to store connetions between ingredients.
 For example, each memoized value has to track its inputs.
 Those inputs are stored as dependency indices.

src/accumulator.rs

@@ -138,7 +138,7 @@ impl<A: Accumulator> Ingredient for IngredientImpl<A> {
     ) {
     }
 
-    fn fmt_index(&self, index: Option<crate::Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+    fn fmt_index(&self, index: crate::Id, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt_index(A::DEBUG_NAME, index, fmt)
     }
 

src/active_query.rs

@@ -2,15 +2,14 @@ use std::ops::Not;
 
 use super::zalsa_local::{QueryEdges, QueryOrigin, QueryRevisions};
 use crate::accumulator::accumulated_map::AtomicInputAccumulatedValues;
-use crate::key::OutputDependencyIndex;
 use crate::tracked_struct::{DisambiguatorMap, IdentityHash, IdentityMap};
 use crate::zalsa_local::QueryEdge;
 use crate::{
     accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues},
     cycle::CycleHeads,
     durability::Durability,
     hash::FxIndexSet,
-    key::{DatabaseKeyIndex, InputDependencyIndex},
+    key::DatabaseKeyIndex,
     tracked_struct::Disambiguator,
     Revision,
 };
@@ -81,7 +80,7 @@ impl ActiveQuery {
 
     pub(super) fn add_read(
         &mut self,
-        input: InputDependencyIndex,
+        input: DatabaseKeyIndex,
         durability: Durability,
         revision: Revision,
         accumulated: InputAccumulatedValues,
@@ -96,7 +95,7 @@ impl ActiveQuery {
 
     pub(super) fn add_read_simple(
         &mut self,
-        input: InputDependencyIndex,
+        input: DatabaseKeyIndex,
         durability: Durability,
         revision: Revision,
     ) {
@@ -118,12 +117,12 @@ impl ActiveQuery {
     }
 
     /// Adds a key to our list of outputs.
-    pub(super) fn add_output(&mut self, key: OutputDependencyIndex) {
+    pub(super) fn add_output(&mut self, key: DatabaseKeyIndex) {
         self.input_outputs.insert(QueryEdge::Output(key));
     }
 
     /// True if the given key was output by this query.
-    pub(super) fn is_output(&self, key: OutputDependencyIndex) -> bool {
+    pub(super) fn is_output(&self, key: DatabaseKeyIndex) -> bool {
         self.input_outputs.contains(&QueryEdge::Output(key))
     }
 

src/database.rs

@@ -56,7 +56,7 @@ pub trait Database: Send + AsDynDatabase + Any + ZalsaDatabase {
     /// which are the fine-grained components we use to track data. This is intended
     /// for debugging and the contents of the returned string are not semver-guaranteed.
     ///
-    /// Ingredient indices can be extracted from [`DependencyIndex`](`crate::DependencyIndex`) values.
+    /// Ingredient indices can be extracted from [`DatabaseKeyIndex`](`crate::DatabaseKeyIndex`) values.
     fn ingredient_debug_name(&self, ingredient_index: IngredientIndex) -> Cow<'_, str> {
         Cow::Borrowed(
             self.zalsa()

src/durability.rs

@@ -35,6 +35,17 @@ enum DurabilityVal {
     High = 2,
 }
 
+impl From<u8> for DurabilityVal {
+    fn from(value: u8) -> Self {
+        match value {
+            0 => DurabilityVal::Low,
+            1 => DurabilityVal::Medium,
+            2 => DurabilityVal::High,
+            _ => panic!("invalid durability"),
+        }
+    }
+}
+
 impl Durability {
     /// Low durability: things that change frequently.
     ///
@@ -68,6 +79,14 @@ impl Durability {
     pub(crate) fn index(self) -> usize {
         self.0 as usize
     }
+
+    pub(crate) fn as_u8(self) -> u8 {
+        self.0 as u8
+    }
+
+    pub(crate) fn from_u8(value: u8) -> Self {
+        Self(DurabilityVal::from(value))
+    }
 }
 
 impl Default for Durability {

src/event.rs

@@ -1,9 +1,6 @@
 use std::thread::ThreadId;
 
-use crate::{
-    key::DatabaseKeyIndex,
-    key::{InputDependencyIndex, OutputDependencyIndex},
-};
+use crate::{key::DatabaseKeyIndex, Id, Revision};
 
 /// The `Event` struct identifies various notable things that can
 /// occur during salsa execution. Instances of this struct are given
@@ -76,7 +73,7 @@ pub enum EventKind {
         execute_key: DatabaseKeyIndex,
 
         /// Key for the query that is no longer output
-        output_key: OutputDependencyIndex,
+        output_key: DatabaseKeyIndex,
     },
 
     /// Tracked structs or memoized data were discarded (freed).
@@ -91,6 +88,24 @@ pub enum EventKind {
         executor_key: DatabaseKeyIndex,
 
         /// Accumulator that was accumulated into
-        accumulator: InputDependencyIndex,
+        accumulator: DatabaseKeyIndex,
+    },
+
+    /// Indicates that a value was newly interned.
+    DidInternValue {
+        // The ID of the interned value.
+        id: Id,
+
+        // The revision the value was interned in.
+        revision: Revision,
+    },
+
+    /// Indicates that a previously interned value was read in a new revision.
+    DidReinternValue {
+        // The ID of the interned value.
+        id: Id,
+
+        // The revision the value was interned in.
+        revision: Revision,
     },
 }

src/function.rs

@@ -160,11 +160,8 @@ where
         }
     }
 
-    pub fn database_key_index(&self, k: Id) -> DatabaseKeyIndex {
-        DatabaseKeyIndex {
-            ingredient_index: self.index,
-            key_index: k,
-        }
+    pub fn database_key_index(&self, key: Id) -> DatabaseKeyIndex {
+        DatabaseKeyIndex::new(self.index, key)
     }
 
     pub fn set_capacity(&mut self, capacity: usize) {
@@ -302,7 +299,7 @@ where
         std::mem::take(&mut self.deleted_entries);
     }
 
-    fn fmt_index(&self, index: Option<crate::Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+    fn fmt_index(&self, index: crate::Id, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt_index(C::DEBUG_NAME, index, fmt)
     }
 

src/function/accumulated.rs

@@ -54,9 +54,9 @@ where
                 continue;
             }
 
-            let ingredient = zalsa.lookup_ingredient(k.ingredient_index);
+            let ingredient = zalsa.lookup_ingredient(k.ingredient_index());
             // Extend `output` with any values accumulated by `k`.
-            let (accumulated_map, input) = ingredient.accumulated(db, k.key_index);
+            let (accumulated_map, input) = ingredient.accumulated(db, k.key_index());
             if let Some(accumulated_map) = accumulated_map {
                 accumulated_map.extend_with_accumulated(accumulator.index(), &mut output);
             }
@@ -71,7 +71,7 @@ where
             // output vector, we want to push in execution order, so reverse order to
             // ensure the first child that was executed will be the first child popped
             // from the stack.
-            let Some(origin) = ingredient.origin(db, k.key_index) else {
+            let Some(origin) = ingredient.origin(db, k.key_index()) else {
                 continue;
             };
 

src/function/diff_outputs.rs

@@ -1,7 +1,7 @@
 use super::{memo::Memo, Configuration, IngredientImpl};
 use crate::{
-    hash::FxHashSet, key::OutputDependencyIndex, zalsa::Zalsa, zalsa_local::QueryRevisions,
-    AsDynDatabase as _, Database, DatabaseKeyIndex, Event, EventKind,
+    hash::FxHashSet, zalsa::Zalsa, zalsa_local::QueryRevisions, AsDynDatabase as _, Database,
+    DatabaseKeyIndex, Event, EventKind,
 };
 
 impl<C> IngredientImpl<C>
@@ -38,7 +38,7 @@ where
             // Remove the outputs that are no longer present in the current revision
             // to prevent that the next revision is seeded with a id mapping that no longer exists.
             revisions.tracked_struct_ids.retain(|&k, &mut value| {
-                !old_outputs.contains(&OutputDependencyIndex::new(k.ingredient_index(), value))
+                !old_outputs.contains(&DatabaseKeyIndex::new(k.ingredient_index(), value))
             });
         }
 
@@ -51,7 +51,7 @@ where
         zalsa: &Zalsa,
         db: &C::DbView,
         key: DatabaseKeyIndex,
-        output: OutputDependencyIndex,
+        output: DatabaseKeyIndex,
         provisional: bool,
     ) {
         db.salsa_event(&|| {

src/function/execute.rs

@@ -29,7 +29,7 @@ where
         let (zalsa, zalsa_local) = db.zalsas();
         let revision_now = zalsa.current_revision();
         let database_key_index = active_query.database_key_index;
-        let id = database_key_index.key_index;
+        let id = database_key_index.key_index();
 
         tracing::info!("{:?}: executing query", database_key_index);
 

src/function/fetch.rs

@@ -30,7 +30,7 @@ where
         self.lru.record_use(id);
 
         zalsa_local.report_tracked_read(
-            self.database_key_index(id).into(),
+            self.database_key_index(id),
             durability,
             changed_at,
             match &memo.revisions.accumulated {
@@ -130,7 +130,7 @@ where
                 }
                 // no provisional value; create/insert/return initial provisional value
                 return self
-                    .initial_value(db, database_key_index.key_index)
+                    .initial_value(db, database_key_index.key_index())
                     .map(|initial_value| {
                         tracing::debug!(
                             "hit cycle at {database_key_index:#?}, \

src/function/maybe_changed_after.rs

@@ -250,8 +250,8 @@ where
         );
         if (&memo.revisions.cycle_heads).into_iter().any(|cycle_head| {
             zalsa
-                .lookup_ingredient(cycle_head.ingredient_index)
-                .is_provisional_cycle_head(db.as_dyn_database(), cycle_head.key_index)
+                .lookup_ingredient(cycle_head.ingredient_index())
+                .is_provisional_cycle_head(db.as_dyn_database(), cycle_head.key_index())
         }) {
             return false;
         }

src/function/memo.rs

@@ -183,13 +183,13 @@ impl<V> Memo<V> {
             .into_iter()
             .filter(|&head| head != database_key_index)
             .any(|head| {
-                let ingredient = zalsa.lookup_ingredient(head.ingredient_index);
-                if !ingredient.is_provisional_cycle_head(db, head.key_index) {
+                let ingredient = zalsa.lookup_ingredient(head.ingredient_index());
+                if !ingredient.is_provisional_cycle_head(db, head.key_index()) {
                     // This cycle is already finalized, so we don't need to wait on it;
                     // keep looping through cycle heads.
                     retry = true;
                     false
-                } else if ingredient.wait_for(db, head.key_index) {
+                } else if ingredient.wait_for(db, head.key_index()) {
                     // There's a new memo available for the cycle head; fetch our own
                     // updated memo and see if it's still provisional or if the cycle
                     // has resolved.

src/function/specify.rs

@@ -41,8 +41,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();
-        if !zalsa_local.is_output_of_active_query(dependency_index) {
+        if !zalsa_local.is_output_of_active_query(database_key_index) {
             panic!("can only use `specify` on salsa structs created during the current tracked fn");
         }
 
@@ -105,7 +104,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

src/ingredient.rs

@@ -141,7 +141,7 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
         );
     }
 
-    fn fmt_index(&self, index: Option<crate::Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result;
+    fn fmt_index(&self, index: crate::Id, fmt: &mut fmt::Formatter<'_>) -> fmt::Result;
 }
 
 impl dyn Ingredient {
@@ -177,14 +177,6 @@ impl dyn Ingredient {
 }
 
 /// A helper function to show human readable fmt.
-pub(crate) fn fmt_index(
-    debug_name: &str,
-    id: Option<Id>,
-    fmt: &mut fmt::Formatter<'_>,
-) -> fmt::Result {
-    if let Some(i) = id {
-        write!(fmt, "{debug_name}({i:?})")
-    } else {
-        write!(fmt, "{debug_name}()")
-    }
+pub(crate) fn fmt_index(debug_name: &str, id: Id, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+    write!(fmt, "{debug_name}({id:?})")
 }

src/input.rs

@@ -15,7 +15,7 @@ use crate::{
     id::{AsId, FromIdWithDb},
     ingredient::{fmt_index, Ingredient},
     input::singleton::{Singleton, SingletonChoice},
-    key::{DatabaseKeyIndex, InputDependencyIndex},
+    key::DatabaseKeyIndex,
     plumbing::{Jar, Stamp},
     table::{memo::MemoTable, sync::SyncTable, Slot, Table},
     zalsa::{IngredientIndex, Zalsa},
@@ -96,10 +96,7 @@ impl<C: Configuration> IngredientImpl<C> {
     }
 
     pub fn database_key_index(&self, id: C::Struct) -> DatabaseKeyIndex {
-        DatabaseKeyIndex {
-            ingredient_index: self.ingredient_index,
-            key_index: id.as_id(),
-        }
+        DatabaseKeyIndex::new(self.ingredient_index, id.as_id())
     }
 
     pub fn new_input(&self, db: &dyn Database, fields: C::Fields, stamps: C::Stamps) -> C::Struct {
@@ -177,7 +174,7 @@ impl<C: Configuration> IngredientImpl<C> {
         let value = Self::data(zalsa, id);
         let stamp = &value.stamps[field_index];
         zalsa_local.report_tracked_read_simple(
-            InputDependencyIndex::new(field_ingredient_index, id),
+            DatabaseKeyIndex::new(field_ingredient_index, id),
             stamp.durability,
             stamp.changed_at,
         );
@@ -261,7 +258,7 @@ impl<C: Configuration> Ingredient for IngredientImpl<C> {
         );
     }
 
-    fn fmt_index(&self, index: Option<Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+    fn fmt_index(&self, index: Id, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt_index(C::DEBUG_NAME, index, fmt)
     }
 

src/input/input_field.rs

@@ -85,7 +85,7 @@ where
     ) {
     }
 
-    fn fmt_index(&self, index: Option<crate::Id>, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+    fn fmt_index(&self, index: crate::Id, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt_index(C::FIELD_DEBUG_NAMES[self.field_index], index, fmt)
     }