Skip to content

Commit 0e1df67

Browse files
authored
Avoid dynamic dispatch to access memo tables (#941)
* avoid dynamic dispatch to access memo tables * update comment
1 parent 962e0b9 commit 0e1df67

File tree

13 files changed

+133
-20
lines changed

13 files changed

+133
-20
lines changed

components/salsa-macro-rules/src/setup_input_struct.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,16 @@ macro_rules! setup_input_struct {
170170
$zalsa::None
171171
}
172172
}
173+
174+
#[inline]
175+
unsafe fn memo_table(
176+
zalsa: &$zalsa::Zalsa,
177+
id: $zalsa::Id,
178+
current_revision: $zalsa::Revision,
179+
) -> $zalsa::MemoTableWithTypes<'_> {
180+
// SAFETY: Guaranteed by caller.
181+
unsafe { zalsa.table().memos::<$zalsa_struct::Value<$Configuration>>(id, current_revision) }
182+
}
173183
}
174184

175185
impl $Struct {

components/salsa-macro-rules/src/setup_interned_struct.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,16 @@ macro_rules! setup_interned_struct {
202202
$zalsa::None
203203
}
204204
}
205+
206+
#[inline]
207+
unsafe fn memo_table(
208+
zalsa: &$zalsa::Zalsa,
209+
id: $zalsa::Id,
210+
current_revision: $zalsa::Revision,
211+
) -> $zalsa::MemoTableWithTypes<'_> {
212+
// SAFETY: Guaranteed by caller.
213+
unsafe { zalsa.table().memos::<$zalsa_struct::Value<$Configuration>>(id, current_revision) }
214+
}
205215
}
206216

207217
unsafe impl< $($db_lt_arg)? > $zalsa::Update for $Struct< $($db_lt_arg)? > {

components/salsa-macro-rules/src/setup_tracked_fn.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,16 @@ macro_rules! setup_tracked_fn {
130130
None
131131
}
132132
}
133+
134+
#[inline]
135+
unsafe fn memo_table(
136+
zalsa: &$zalsa::Zalsa,
137+
id: $zalsa::Id,
138+
current_revision: $zalsa::Revision,
139+
) -> $zalsa::MemoTableWithTypes<'_> {
140+
// SAFETY: Guaranteed by caller.
141+
unsafe { zalsa.table().memos::<$zalsa::interned::Value<$Configuration>>(id, current_revision) }
142+
}
133143
}
134144

135145
impl $zalsa::AsId for $InternedData<'_> {

components/salsa-macro-rules/src/setup_tracked_struct.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,16 @@ macro_rules! setup_tracked_struct {
231231
$zalsa::None
232232
}
233233
}
234+
235+
#[inline]
236+
unsafe fn memo_table(
237+
zalsa: &$zalsa::Zalsa,
238+
id: $zalsa::Id,
239+
current_revision: $zalsa::Revision,
240+
) -> $zalsa::MemoTableWithTypes<'_> {
241+
// SAFETY: Guaranteed by caller.
242+
unsafe { zalsa.table().memos::<$zalsa_struct::Value<$Configuration>>(id, current_revision) }
243+
}
234244
}
235245

236246
impl $zalsa::TrackedStructInDb for $Struct<'_> {

components/salsa-macros/src/supertype.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,19 @@ fn enum_impl(enum_item: syn::ItemEnum) -> syn::Result<TokenStream> {
8989
None
9090
}
9191
}
92+
93+
#[inline]
94+
unsafe fn memo_table(
95+
zalsa: &zalsa::Zalsa,
96+
id: zalsa::Id,
97+
current_revision: zalsa::Revision,
98+
) -> zalsa::MemoTableWithTypes<'_> {
99+
// Note that we need to use `dyn_memos` here, as the `Id` could map to any variant
100+
// of the supertype enum.
101+
//
102+
// SAFETY: Guaranteed by caller.
103+
unsafe { zalsa.table().dyn_memos(id, current_revision) }
104+
}
92105
}
93106
};
94107

src/function/memo.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ impl<C: Configuration> IngredientImpl<C> {
3030
let static_memo =
3131
unsafe { transmute::<NonNull<Memo<'db, C>>, NonNull<Memo<'static, C>>>(memo) };
3232
let old_static_memo = zalsa
33-
.memo_table_for(id)
33+
.memo_table_for::<C::SalsaStruct<'_>>(id)
3434
.insert(memo_ingredient_index, static_memo)?;
3535
// SAFETY: The table stores 'static memos (to support `Any`), the memos are in fact valid
3636
// for `'db` though as we delay their dropping to the end of a revision.
@@ -48,7 +48,9 @@ impl<C: Configuration> IngredientImpl<C> {
4848
id: Id,
4949
memo_ingredient_index: MemoIngredientIndex,
5050
) -> Option<&'db Memo<'db, C>> {
51-
let static_memo = zalsa.memo_table_for(id).get(memo_ingredient_index)?;
51+
let static_memo = zalsa
52+
.memo_table_for::<C::SalsaStruct<'_>>(id)
53+
.get(memo_ingredient_index)?;
5254
// SAFETY: The table stores 'static memos (to support `Any`), the memos are in fact valid
5355
// for `'db` though as we delay their dropping to the end of a revision.
5456
Some(unsafe { transmute::<&Memo<'static, C>, &'db Memo<'db, C>>(static_memo.as_ref()) })
@@ -451,8 +453,9 @@ mod _memory_usage {
451453
use crate::cycle::CycleRecoveryStrategy;
452454
use crate::ingredient::Location;
453455
use crate::plumbing::{IngredientIndices, MemoIngredientSingletonIndex, SalsaStructInDb};
456+
use crate::table::memo::MemoTableWithTypes;
454457
use crate::zalsa::Zalsa;
455-
use crate::{CycleRecoveryAction, Database, Id};
458+
use crate::{CycleRecoveryAction, Database, Id, Revision};
456459

457460
use std::any::TypeId;
458461
use std::num::NonZeroUsize;
@@ -473,6 +476,10 @@ mod _memory_usage {
473476
fn cast(_: Id, _: TypeId) -> Option<Self> {
474477
unimplemented!()
475478
}
479+
480+
unsafe fn memo_table(_: &Zalsa, _: Id, _: Revision) -> MemoTableWithTypes<'_> {
481+
unimplemented!()
482+
}
476483
}
477484

478485
struct DummyConfiguration;

src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ pub mod plumbing {
104104
pub use crate::runtime::{stamp, Runtime, Stamp};
105105
pub use crate::salsa_struct::SalsaStructInDb;
106106
pub use crate::storage::{HasStorage, Storage};
107+
pub use crate::table::memo::MemoTableWithTypes;
107108
pub use crate::tracked_struct::TrackedStructInDb;
108109
pub use crate::update::helper::{Dispatch as UpdateDispatch, Fallback as UpdateFallback};
109110
pub use crate::update::{always_update, Update};

src/salsa_struct.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
use std::any::TypeId;
22

33
use crate::memo_ingredient_indices::{IngredientIndices, MemoIngredientMap};
4+
use crate::table::memo::MemoTableWithTypes;
45
use crate::zalsa::Zalsa;
5-
use crate::Id;
6+
use crate::{Id, Revision};
67

78
pub trait SalsaStructInDb: Sized {
89
type MemoIngredientMap: MemoIngredientMap;
@@ -62,4 +63,16 @@ pub trait SalsaStructInDb: Sized {
6263
/// Why `TypeId` and not `IngredientIndex`? Because it's cheaper and easier: the `TypeId` is readily
6364
/// available at compile time, while the `IngredientIndex` requires a runtime lookup.
6465
fn cast(id: Id, type_id: TypeId) -> Option<Self>;
66+
67+
/// Return the memo table associated with `id`.
68+
///
69+
/// # Safety
70+
///
71+
/// The parameter `current_revision` must be the current revision of the owner of database
72+
/// owning this table.
73+
unsafe fn memo_table(
74+
zalsa: &Zalsa,
75+
id: Id,
76+
current_revision: Revision,
77+
) -> MemoTableWithTypes<'_>;
6578
}

src/table.rs

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ pub struct Table {
3434
///
3535
/// Implementors of this trait need to make sure that their type is unique with respect to
3636
/// their owning ingredient as the allocation strategy relies on this.
37-
pub(crate) unsafe trait Slot: Any + Send + Sync {
37+
pub unsafe trait Slot: Any + Send + Sync {
3838
/// Access the [`MemoTable`][] for this slot.
3939
///
4040
/// # Safety condition
@@ -220,17 +220,42 @@ impl Table {
220220
PageIndex::new(self.pages.push(Page::new::<T>(ingredient, memo_types)))
221221
}
222222

223-
/// Get the memo table associated with `id`
223+
/// Get the memo table associated with `id` for the concrete type `T`.
224224
///
225-
/// # Safety condition
225+
/// # Safety
226226
///
227-
/// The parameter `current_revision` MUST be the current revision
228-
/// of the owner of database owning this table.
229-
pub(crate) unsafe fn memos(
227+
/// The parameter `current_revision` must be the current revision of the database
228+
/// owning this table.
229+
///
230+
/// # Panics
231+
///
232+
/// If `page` is out of bounds or the type `T` is incorrect.
233+
pub unsafe fn memos<T: Slot>(
230234
&self,
231235
id: Id,
232236
current_revision: Revision,
233237
) -> MemoTableWithTypes<'_> {
238+
let (page, slot) = split_id(id);
239+
let page = self.pages[page.0].assert_type::<T>();
240+
let slot = &page.data()[slot.0];
241+
242+
// SAFETY: The caller is required to pass the `current_revision`.
243+
let memos = unsafe { slot.memos(current_revision) };
244+
245+
// SAFETY: The `Page` keeps the correct memo types.
246+
unsafe { page.0.memo_types.attach_memos(memos) }
247+
}
248+
249+
/// Get the memo table associated with `id`.
250+
///
251+
/// Unlike `Table::memos`, this does not require a concrete type, and instead uses dynamic
252+
/// dispatch.
253+
///
254+
/// # Safety
255+
///
256+
/// The parameter `current_revision` must be the current revision of the owner of database
257+
/// owning this table.
258+
pub unsafe fn dyn_memos(&self, id: Id, current_revision: Revision) -> MemoTableWithTypes<'_> {
234259
let (page, slot) = split_id(id);
235260
let page = &self.pages[page.0];
236261
// SAFETY: We supply a proper slot pointer and the caller is required to pass the `current_revision`.
@@ -373,6 +398,7 @@ impl Page {
373398
slot.0 < len,
374399
"out of bounds access `{slot:?}` (maximum slot `{len}`)"
375400
);
401+
376402
// SAFETY: We have checked that the resulting pointer will be within bounds.
377403
unsafe {
378404
self.data

src/table/memo.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{zalsa::MemoIngredientIndex, zalsa_local::QueryOriginRef};
99
/// The "memo table" stores the memoized results of tracked function calls.
1010
/// Every tracked function must take a salsa struct as its first argument
1111
/// and memo tables are attached to those salsa structs as auxiliary data.
12-
pub(crate) struct MemoTable {
12+
pub struct MemoTable {
1313
memos: Box<[MemoEntry]>,
1414
}
1515

@@ -168,7 +168,7 @@ impl MemoTableTypes {
168168
}
169169
}
170170

171-
pub(crate) struct MemoTableWithTypes<'a> {
171+
pub struct MemoTableWithTypes<'a> {
172172
types: &'a MemoTableTypes,
173173
memos: &'a MemoTable,
174174
}

0 commit comments

Comments
 (0)