Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(perf): Remove extra page indirection in Table #710

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions src/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) },
}
}

Expand Down
7 changes: 1 addition & 6 deletions src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,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.
Expand Down
3 changes: 2 additions & 1 deletion src/input/singleton.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }),
}
}
}
Expand Down
7 changes: 1 addition & 6 deletions src/interned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,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>>()
}

pub fn reset(&mut self, revision: Revision) {
Expand Down
Loading