increase ID generations to 32-bits

Author: ibraheemdev

src/id.rs

@@ -6,10 +6,8 @@ use crate::zalsa::Zalsa;
 
 /// The `Id` of a salsa struct in the database [`Table`](`crate::table::Table`).
 ///
-/// The high-order bits of an `Id` store a 16-bit generation counter
-/// as well as an optional 16-bit ingredient index.
-///
-/// The low-order bits pack a [`PageIndex`](`crate::table::PageIndex`) and
+/// The high-order bits of an `Id` store a 32-bit generation counter, while
+/// the low-order bits pack a [`PageIndex`](`crate::table::PageIndex`) and
 /// [`SlotIndex`](`crate::table::SlotIndex`) within the page.
 ///
 /// The low-order bits of `Id` are a `u32` ranging from `0..Id::MAX_U32`.
@@ -28,9 +26,6 @@ impl Id {
     pub const MAX_U32: u32 = u32::MAX - 0xFF;
     pub const MAX_USIZE: usize = Self::MAX_U32 as usize;
 
-    const INGREDIENT_MASK: u64 = 0x0000FFFFFFFFFFFF;
-    const GENERATION_MASK: u64 = 0xFFFF0000FFFFFFFF;
-
     /// Create a `salsa::Id` from a u32 value, without a generation. This
     /// value should be less than [`Self::MAX_U32`].
     ///
@@ -77,10 +72,10 @@ impl Id {
     /// but will differ from other identifiers of the slot based on the
     /// provided generation.
     #[inline]
-    pub fn with_generation(self, generation: u16) -> Id {
+    pub fn with_generation(self, generation: u32) -> Id {
         let mut value = self.value.get();
 
-        value &= Id::GENERATION_MASK;
+        value &= 0xFFFFFFFF;
         value |= (generation as u64) << 32;
 
         Id {
@@ -89,26 +84,6 @@ impl Id {
         }
     }
 
-    /// Mark the `Id` with an ingredient index.
-    #[inline]
-    pub fn with_ingredient_index(self, ingredient: u16) -> Id {
-        let mut value = self.value.get();
-
-        value &= Id::INGREDIENT_MASK;
-        value |= (ingredient as u64) << 48;
-
-        Id {
-            // SAFETY: The niche of `value` is in the lower bits, which we did not touch.
-            value: unsafe { NonZeroU64::new_unchecked(value) },
-        }
-    }
-
-    /// Return the internal `u64` representation of this `Id`.
-    #[inline]
-    pub const fn as_bits(self) -> u64 {
-        self.value.get()
-    }
-
     /// Return the data portion of this `Id`.
     #[inline]
     pub const fn data(self) -> u32 {
@@ -118,14 +93,15 @@ impl Id {
 
     /// Return the generation of this `Id`.
     #[inline]
-    pub const fn generation(self) -> u16 {
-        ((self.value.get() & !Id::GENERATION_MASK) >> 32) as u16
+    pub const fn generation(self) -> u32 {
+        // Shift away the low-order bits.
+        (self.value.get() >> 32) as u32
     }
 
-    /// Return the ingredient index of this `Id`.
+    /// Return the internal `u64` representation of this `Id`.
     #[inline]
-    pub const fn ingredient_index(self) -> u16 {
-        (self.value.get() >> 48) as u16
+    pub const fn as_bits(self) -> u64 {
+        self.value.get()
     }
 }
 

src/key.rs

@@ -12,26 +12,29 @@ use crate::{Database, Id};
 /// only for inserting into maps and the like.
 #[derive(Copy, Clone, PartialEq, Eq, Hash)]
 pub struct DatabaseKeyIndex {
-    packed_key_index: Id,
+    key_index: u32,
+    key_generation: u32,
+    ingredient_index: IngredientIndex,
 }
 // ANCHOR_END: DatabaseKeyIndex
 
 impl DatabaseKeyIndex {
     #[inline]
     pub(crate) fn new(ingredient_index: IngredientIndex, key_index: Id) -> Self {
         Self {
-            // Pack the ingredient index with the key.
-            packed_key_index: key_index.with_ingredient_index(ingredient_index.as_u16()),
+            key_index: key_index.data(),
+            key_generation: key_index.generation(),
+            ingredient_index,
         }
     }
 
     pub fn ingredient_index(self) -> IngredientIndex {
-        IngredientIndex::from(self.packed_key_index.ingredient_index() as usize)
+        self.ingredient_index
     }
 
     pub fn key_index(self) -> Id {
-        // Clear the ingredient index from the key as it not part of the dependency.
-        self.packed_key_index.with_ingredient_index(0)
+        // SAFETY: `self.key_index` was returned by `Id::data`.
+        unsafe { Id::from_data(self.key_index) }.with_generation(self.key_generation)
     }
 
     pub(crate) fn maybe_changed_after(

src/tracked_struct.rs

@@ -434,14 +434,17 @@ where
             memos: Default::default(),
         };
 
-        if let Some(id) = self.free_list.pop() {
+        while let Some(id) = self.free_list.pop() {
             // Increment the ID generation before reusing it, as if we have allocated a new
             // slot in the table.
             //
-            // If the generation will wrap, we instead saturate to the generation `u16::MAX`.
-            // "Leaked" IDs may be reused with the same generation.
-            let id = id.with_generation(id.generation().saturating_add(1));
+            // If the generation will wrap, we are forced to leak the slot. We reserve enough
+            // bits for the generation that this should not be a problem in practice.
+            let Some(generation) = id.generation().checked_add(1) else {
+                continue;
+            };
 
+            let id = id.with_generation(generation);
             return Self::data_raw(zalsa.table(), id).with_mut(|data_raw| {
                 let data_raw = data_raw.cast::<Value<C>>();
 
@@ -727,7 +730,7 @@ where
         db: &'db dyn crate::Database,
         s: C::Struct<'db>,
     ) -> &'db C::Fields<'db> {
-        let (zalsa, zalsa_local) = db.zalsas();
+        let zalsa = db.zalsa();
         let id = AsId::as_id(&s);
         let data = Self::data(zalsa.table(), id);
 
@@ -736,17 +739,6 @@ where
         // Note that we do not need to add a dependency on the tracked struct
         // in general, as IDs that are reused increment their generation,
         // invalidating any dependent queries directly.
-        //
-        // However, IDs that have hit the maximum generation are "leaked", and may
-        // be reused without a new generation, and so a dependency must be created
-        // on any query that reads from them.
-        if id.generation() == u16::MAX {
-            zalsa_local.report_tracked_read_simple(
-                DatabaseKeyIndex::new(self.ingredient_index, id),
-                data.durability,
-                data.created_at,
-            );
-        }
 
         data.fields()
     }

src/zalsa.rs

@@ -76,32 +76,22 @@ crate::loom::maybe_lazy_static! {
 /// The database contains a number of jars, and each jar contains a number of ingredients.
 /// Each ingredient is given a unique index as the database is being created.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
-pub struct IngredientIndex(u16);
+pub struct IngredientIndex(u32);
 
 impl IngredientIndex {
     /// Create an ingredient index from a usize.
     pub(crate) fn from(v: usize) -> Self {
-        assert!(v <= u16::MAX as usize);
-        Self(v as u16)
+        assert!(v <= u32::MAX as usize);
+        Self(v as u32)
     }
 
     /// Convert the ingredient index back into a usize.
     pub(crate) fn as_usize(self) -> usize {
         self.0 as usize
     }
 
-    /// Convert the ingredient index back into a usize.
-    pub(crate) fn as_u16(self) -> u16 {
-        self.0
-    }
-
     pub fn successor(self, index: usize) -> Self {
-        let index = self
-            .0
-            .checked_add((1 + index) as u16)
-            .expect("exceeded maximum number of ingredients");
-
-        IngredientIndex(index)
+        IngredientIndex(self.0 + 1 + index as u32)
     }
 }
 
@@ -508,7 +498,7 @@ where
         let nonce = Nonce::<StorageNonce>::from_u32(unsafe {
             NonZeroU32::new_unchecked((cached_data >> u32::BITS) as u32)
         });
-        let mut index = IngredientIndex(cached_data as u16);
+        let mut index = IngredientIndex(cached_data as u32);
 
         if zalsa.nonce() != nonce {
             index = create_index();