Skip to content

Commit 2ae5f93

Browse files
committed
Remove extra page indirection in Table
By manually erasing the concrete slot types and dealing with the vtable ourselves
1 parent 5c37082 commit 2ae5f93

File tree

6 files changed

+217
-213
lines changed

6 files changed

+217
-213
lines changed

src/id.rs

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

src/input.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -192,12 +192,7 @@ impl<C: Configuration> IngredientImpl<C> {
192192
&'db self,
193193
db: &'db dyn crate::Database,
194194
) -> impl Iterator<Item = &'db Value<C>> {
195-
db.zalsa()
196-
.table()
197-
.pages
198-
.iter()
199-
.filter_map(|(_, page)| page.cast_type::<crate::table::Page<Value<C>>>())
200-
.flat_map(|page| page.slots())
195+
db.zalsa().table().slots_of::<Value<C>>()
201196
}
202197

203198
/// Peek at the field values without recording any read dependency.

src/input/singleton.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ impl SingletonChoice for Singleton {
3434
fn index(&self) -> Option<Id> {
3535
match self.index.load(Ordering::Acquire) {
3636
0 => None,
37-
id => Some(Id::from_u32(id - 1)),
37+
// SAFETY: Our u32 is derived from an ID and thus safe to convert back.
38+
id => Some(unsafe { Id::from_u32(id - 1) }),
3839
}
3940
}
4041
}

src/interned.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -259,12 +259,7 @@ where
259259
&'db self,
260260
db: &'db dyn crate::Database,
261261
) -> impl Iterator<Item = &'db Value<C>> {
262-
db.zalsa()
263-
.table()
264-
.pages
265-
.iter()
266-
.filter_map(|(_, page)| page.cast_type::<crate::table::Page<Value<C>>>())
267-
.flat_map(|page| page.slots())
262+
db.zalsa().table().slots_of::<Value<C>>()
268263
}
269264

270265
pub fn reset(&mut self, revision: Revision) {

0 commit comments

Comments
 (0)