record durabilities on interned values

Author: ibraheemdev

src/durability.rs

@@ -47,6 +47,16 @@ impl Durability {
     pub(crate) fn index(self) -> usize {
         self.0 as usize
     }
+
+    pub(crate) fn as_u8(self) -> u8 {
+        self.0
+    }
+
+    pub(crate) fn from_u8(value: u8) -> Self {
+        assert!((value as usize) < Self::LEN, "invalid durability");
+
+        Self(value)
+    }
 }
 
 impl Default for Durability {

src/interned.rs

@@ -13,6 +13,7 @@ use std::fmt;
 use std::hash::{BuildHasher, Hash, Hasher};
 use std::marker::PhantomData;
 use std::path::{Path, PathBuf};
+use std::sync::atomic::{AtomicU8, Ordering};
 
 use super::hash::FxDashMap;
 use super::ingredient::Ingredient;
@@ -68,10 +69,27 @@ 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,
+
+    /// The minimum durability of all inputs consumed by the creator
+    /// query prior to creating this tracked struct. If any of those
+    /// inputs changes, then the creator query may create this struct
+    /// with different values.
+    durability: AtomicU8,
+}
+
+impl<C> Value<C>
+where
+    C: Configuration,
+{
+    fn durability(&self) -> Durability {
+        Durability::from_u8(self.durability.load(Ordering::Acquire))
+    }
 }
 
 impl<C: Configuration> Default for JarImpl<C> {
@@ -148,13 +166,25 @@ 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);
+
+                // Sync the value's revision.
                 value.last_interned_at.store(current_revision);
 
+                let durability = if let Some((_, stamp)) = zalsa_local.active_query() {
+                    // Record the maximum durability across all queries that intern this value.
+                    let previous_durability = value
+                        .durability
+                        .fetch_max(stamp.durability.as_u8(), Ordering::AcqRel);
+
+                    Durability::from_u8(previous_durability).max(stamp.durability)
+                } else {
+                    value.durability()
+                };
+
                 // Record a dependency on this value.
                 let index = self.database_key_index(id);
-                zalsa_local.report_tracked_read(index, Durability::MAX, current_revision);
+                zalsa_local.report_tracked_read(index, durability, current_revision);
 
                 return C::struct_from_id(id);
             }
@@ -170,13 +200,25 @@ where
                 let id = *entry.get();
                 drop(entry);
 
-                // Sync the value's revision.
                 let value = zalsa.table().get::<Value<C>>(id);
+
+                // Sync the value's revision.
                 value.last_interned_at.store(current_revision);
 
+                let durability = if let Some((_, stamp)) = zalsa_local.active_query() {
+                    // Record the maximum durability across all queries that intern this value.
+                    let previous_durability = value
+                        .durability
+                        .fetch_max(stamp.durability.as_u8(), Ordering::AcqRel);
+
+                    Durability::from_u8(previous_durability).max(stamp.durability)
+                } else {
+                    value.durability()
+                };
+
                 // Record a dependency on this value.
                 let index = self.database_key_index(id);
-                zalsa_local.report_tracked_read(index, Durability::MAX, current_revision);
+                zalsa_local.report_tracked_read(index, durability, current_revision);
 
                 C::struct_from_id(id)
             }
@@ -186,18 +228,30 @@ where
                 let zalsa = db.zalsa();
                 let table = zalsa.table();
 
+                let (durability, last_interned_at) = match zalsa_local.active_query() {
+                    // Record the durability of the current query, along with the revision
+                    // we are interning in.
+                    Some((_, stamp)) => (stamp.durability, current_revision),
+
+                    // An interned value created outside of a query is considered immortal.
+                    // The durability in this case doesn't really matter.
+                    None => (Durability::MAX, Revision::from(usize::MAX)),
+                };
+
                 let next_id = zalsa_local.allocate(table, self.ingredient_index, || Value::<C> {
                     data: internal_data,
                     memos: Default::default(),
                     syncs: Default::default(),
                     first_interned_at: current_revision,
-                    last_interned_at: AtomicRevision::from(current_revision),
+                    durability: AtomicU8::new(durability.as_u8()),
+                    last_interned_at: AtomicRevision::from(last_interned_at),
                 });
+
                 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, current_revision);
+                zalsa_local.report_tracked_read(index, durability, current_revision);
 
                 C::struct_from_id(next_id)
             }
@@ -217,9 +271,10 @@ where
     /// to the interned item.
     pub fn data<'db>(&'db self, db: &'db dyn Database, id: Id) -> &'db C::Data<'db> {
         let value = db.zalsa().table().get::<Value<C>>(id);
+        let last_changed_revision = db.zalsa().last_changed_revision(value.durability());
         assert!(
-            value.last_interned_at.load() >= db.zalsa().current_revision(),
-            "Data was not interned in the current revision."
+            value.last_interned_at.load() >= last_changed_revision,
+            "Data was not interned in the latest revision for its durability."
         );
         unsafe { Self::from_internal_data(&value.data) }
     }

src/tracked_struct.rs

@@ -182,10 +182,10 @@ pub struct Value<C>
 where
     C: Configuration,
 {
-    /// The durability minimum durability of all inputs consumed
-    /// by the creator query prior to creating this tracked struct.
-    /// If any of those inputs changes, then the creator query may
-    /// create this struct with different values.
+    /// The minimum durability of all inputs consumed by the creator
+    /// query prior to creating this tracked struct. If any of those
+    /// inputs changes, then the creator query may create this struct
+    /// with different values.
     durability: Durability,
 
     /// The revision when this tracked struct was last updated.
@@ -283,7 +283,6 @@ where
 
         let identity = Identity {
             identity_hash,
-
             disambiguator,
         };
 

tests/interned-revisions.rs

@@ -2,6 +2,8 @@
 //! compiles and executes successfully.
 
 mod common;
+use common::LogDatabase;
+use expect_test::expect;
 use salsa::{Database, Setter};
 use test_log::test;
 
@@ -16,7 +18,7 @@ struct Interned<'db> {
 }
 
 #[test]
-fn test_shallow_memo() {
+fn test_durability() {
     #[salsa::tracked]
     fn function<'db>(db: &'db dyn Database, _input: Input) -> Interned<'db> {
         Interned::new(db, 0)
@@ -34,39 +36,40 @@ fn test_shallow_memo() {
     assert_eq!(result_in_rev_2.field1(&db), 0);
 }
 
-// #[test]
-// fn test_no_reintern_input() {
-//     #[salsa::tracked]
-//     fn function<'db>(db: &'db dyn Database, input: Input) -> Interned<'db> {
-//         function2(db, input.field1(db))
-//     }
-//
-//     fn function2<'db>(db: &'db dyn Database, value: usize) -> Interned<'db> {
-//         Interned::new(db, value)
-//     }
-//
-//     let mut db = common::EventLoggerDatabase::default();
-//
-//     let input = Input::new(&db, 0);
-//     let result_in_rev_1 = function(&db, input);
-//     db.assert_logs(expect![[r#"
-//         [
-//             "Event { thread_id: ThreadId(3), kind: WillCheckCancellation }",
-//             "Event { thread_id: ThreadId(3), kind: WillExecute { database_key: function(Id(0)) } }",
-//         ]"#]]);
-//
-//     assert_eq!(result_in_rev_1.field1(&db), 0);
-//
-//     // Modify the input to force the value to be re-interned.
-//     input.set_field1(&mut db).to(1);
-//
-//     let result_in_rev_2 = function(&db, input);
-//     db.assert_logs(expect![[r#"
-//         [
-//             "Event { thread_id: ThreadId(3), kind: DidSetCancellationFlag }",
-//             "Event { thread_id: ThreadId(3), kind: WillCheckCancellation }",
-//             "Event { thread_id: ThreadId(3), kind: WillExecute { database_key: function(Id(0)) } }",
-//         ]"#]]);
-//
-//     assert_eq!(result_in_rev_2.field1(&db), 1);
-// }
+#[test]
+fn test_durability2() {
+    #[salsa::tracked]
+    fn function<'db>(db: &'db dyn Database, input: Input) -> Interned<'db> {
+        let _ = input.field1(db);
+        function2(db)
+    }
+
+    fn function2<'db>(db: &'db dyn Database) -> Interned<'db> {
+        Interned::new(db, 0)
+    }
+
+    let mut db = common::EventLoggerDatabase::default();
+
+    let input = Input::new(&db, 0);
+    let result_in_rev_1 = function(&db, input);
+    db.assert_logs(expect![[r#"
+        [
+            "Event { thread_id: ThreadId(3), kind: WillCheckCancellation }",
+            "Event { thread_id: ThreadId(3), kind: WillExecute { database_key: function(Id(0)) } }",
+        ]"#]]);
+
+    assert_eq!(result_in_rev_1.field1(&db), 0);
+
+    // Modify the input to force the value to be re-interned.
+    input.set_field1(&mut db).to(1);
+
+    let result_in_rev_2 = function(&db, input);
+    db.assert_logs(expect![[r#"
+        [
+            "Event { thread_id: ThreadId(3), kind: DidSetCancellationFlag }",
+            "Event { thread_id: ThreadId(3), kind: WillCheckCancellation }",
+            "Event { thread_id: ThreadId(3), kind: WillExecute { database_key: function(Id(0)) } }",
+        ]"#]]);
+
+    assert_eq!(result_in_rev_2.field1(&db), 0);
+}