Skip to content

Commit bbebda9

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

File tree

6 files changed

+216
-206
lines changed

6 files changed

+216
-206
lines changed

src/id.rs

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

src/input.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -190,12 +190,7 @@ impl<C: Configuration> IngredientImpl<C> {
190190
&'db self,
191191
db: &'db dyn crate::Database,
192192
) -> impl Iterator<Item = &'db Value<C>> {
193-
db.zalsa()
194-
.table()
195-
.pages
196-
.iter()
197-
.filter_map(|(_, page)| page.cast_type::<crate::table::Page<Value<C>>>())
198-
.flat_map(|page| page.slots())
193+
db.zalsa().table().slots_of::<Value<C>>()
199194
}
200195

201196
/// 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)