Remove extra page indirection in Table (#710)

By manually erasing the concrete slot types and dealing with the vtable ourselves

Author: Veykril

src/id.rs

@@ -14,7 +14,7 @@ use crate::Database;
 /// room for niches; currently there is only one niche, so that
 /// `Option<Id>` is the same size as an `Id`.
 ///
-/// As an end-user of `Salsa` you will not use `Id` directly,
+/// As an end-user of `Salsa` you will generally not use `Id` directly,
 /// it is wrapped in new types.
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct Id {
@@ -31,14 +31,22 @@ impl Id {
     /// In general, you should not need to create salsa ids yourself,
     /// but it can be useful if you are using the type as a general
     /// purpose "identifier" internally.
+    ///
+    /// # Safety
+    ///
+    /// The supplied value must be less than [`Self::MAX_U32`].
+    ///
+    /// Additionally, creating an arbitrary `Id` can lead to unsoundness if such an ID ends up being used to index
+    /// the internal allocation tables which end up being out of bounds. Care must be taken that the
+    /// ID is either constructed with a valid value or that it never ends up being used as keys to
+    /// salsa computations.
     #[doc(hidden)]
     #[track_caller]
-    pub const fn from_u32(x: u32) -> Self {
+    pub const unsafe fn from_u32(v: u32) -> Self {
+        debug_assert!(v < Self::MAX_U32);
         Id {
-            value: match NonZeroU32::new(x + 1) {
-                Some(v) => v,
-                None => panic!("given value is too large to be a `salsa::Id`"),
-            },
+            // SAFETY: Caller obligation
+            value: unsafe { NonZeroU32::new_unchecked(v + 1) },
         }
     }
 

src/input.rs

@@ -187,12 +187,7 @@ impl<C: Configuration> IngredientImpl<C> {
         &'db self,
         db: &'db dyn crate::Database,
     ) -> impl Iterator<Item = &'db Value<C>> {
-        db.zalsa()
-            .table()
-            .pages
-            .iter()
-            .filter_map(|(_, page)| page.cast_type::<crate::table::Page<Value<C>>>())
-            .flat_map(|page| page.slots())
+        db.zalsa().table().slots_of::<Value<C>>()
     }
 
     /// Peek at the field values without recording any read dependency.
@@ -318,14 +313,17 @@ impl<C> Slot for Value<C>
 where
     C: Configuration,
 {
+    #[inline]
     unsafe fn memos(&self, _current_revision: Revision) -> &crate::table::memo::MemoTable {
         &self.memos
     }
 
+    #[inline]
     fn memos_mut(&mut self) -> &mut crate::table::memo::MemoTable {
         &mut self.memos
     }
 
+    #[inline]
     unsafe fn syncs(&self, _current_revision: Revision) -> &SyncTable {
         &self.syncs
     }

src/input/singleton.rs

@@ -34,7 +34,8 @@ impl SingletonChoice for Singleton {
     fn index(&self) -> Option<Id> {
         match self.index.load(Ordering::Acquire) {
             0 => None,
-            id => Some(Id::from_u32(id - 1)),
+            // SAFETY: Our u32 is derived from an ID and thus safe to convert back.
+            id => Some(unsafe { Id::from_u32(id - 1) }),
         }
     }
 }

src/interned.rs

@@ -364,12 +364,7 @@ where
         &'db self,
         db: &'db dyn crate::Database,
     ) -> impl Iterator<Item = &'db Value<C>> {
-        db.zalsa()
-            .table()
-            .pages
-            .iter()
-            .filter_map(|(_, page)| page.cast_type::<crate::table::Page<Value<C>>>())
-            .flat_map(|page| page.slots())
+        db.zalsa().table().slots_of::<Value<C>>()
     }
 }
 
@@ -475,14 +470,17 @@ impl<C> Slot for Value<C>
 where
     C: Configuration,
 {
+    #[inline]
     unsafe fn memos(&self, _current_revision: Revision) -> &MemoTable {
         &self.memos
     }
 
+    #[inline]
     fn memos_mut(&mut self) -> &mut MemoTable {
         &mut self.memos
     }
 
+    #[inline]
     unsafe fn syncs(&self, _current_revision: Revision) -> &crate::table::sync::SyncTable {
         &self.syncs
     }