Skip to content

Commit 9d2a978

Browse files
VeykrilVeykril
authored
perf: Store view downcaster in function ingredients directly (#720)
* Store view downcaster in function ingredients directly * Embed typecheck into the downcaster * Force inline `db` proc-macro generated functions * Skip unnecessary type-id check
1 parent ceb9b08 commit 9d2a978

16 files changed

+149
-164
lines changed

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,13 @@ macro_rules! setup_tracked_fn {
142142
impl $Configuration {
143143
fn fn_ingredient(db: &dyn $Db) -> &$zalsa::function::IngredientImpl<$Configuration> {
144144
$FN_CACHE.get_or_create(db.as_dyn_database(), || {
145-
<dyn $Db as $Db>::zalsa_db(db);
145+
<dyn $Db as $Db>::zalsa_register_downcaster(db);
146146
db.zalsa().add_or_lookup_jar_by_type::<$Configuration>()
147147
})
148148
}
149149

150150
pub fn fn_ingredient_mut(db: &mut dyn $Db) -> &mut $zalsa::function::IngredientImpl<Self> {
151+
<dyn $Db as $Db>::zalsa_register_downcaster(db);
151152
let zalsa_mut = db.zalsa_mut();
152153
let index = zalsa_mut.add_or_lookup_jar_by_type::<$Configuration>();
153154
let (ingredient, _) = zalsa_mut.lookup_ingredient_mut(index);
@@ -159,6 +160,7 @@ macro_rules! setup_tracked_fn {
159160
db: &dyn $Db,
160161
) -> &$zalsa::interned::IngredientImpl<$Configuration> {
161162
$INTERN_CACHE.get_or_create(db.as_dyn_database(), || {
163+
<dyn $Db as $Db>::zalsa_register_downcaster(db);
162164
db.zalsa().add_or_lookup_jar_by_type::<$Configuration>().successor(0)
163165
})
164166
}
@@ -249,7 +251,8 @@ macro_rules! setup_tracked_fn {
249251
let fn_ingredient = <$zalsa::function::IngredientImpl<$Configuration>>::new(
250252
first_index,
251253
memo_ingredient_indices,
252-
$lru
254+
$lru,
255+
zalsa.views().downcaster_for::<dyn $Db>()
253256
);
254257
$zalsa::macro_if! {
255258
if $needs_interner {

‎components/salsa-macros/src/db.rs

+27-6
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,12 @@ impl DbMacro {
8989
use salsa::plumbing as #zalsa;
9090

9191
unsafe impl #zalsa::HasStorage for #db {
92+
#[inline(always)]
9293
fn storage(&self) -> &#zalsa::Storage<Self> {
9394
&self.#storage
9495
}
9596

97+
#[inline(always)]
9698
fn storage_mut(&mut self) -> &mut #zalsa::Storage<Self> {
9799
&mut self.#storage
98100
}
@@ -102,16 +104,26 @@ impl DbMacro {
102104
}
103105

104106
fn add_salsa_view_method(&self, input: &mut syn::ItemTrait) -> syn::Result<()> {
107+
let trait_name = &input.ident;
105108
input.items.push(parse_quote! {
106109
#[doc(hidden)]
107-
fn zalsa_db(&self);
110+
fn zalsa_register_downcaster(&self);
111+
});
112+
113+
let comment = format!(" Downcast a [`dyn Database`] to a [`dyn {trait_name}`]");
114+
input.items.push(parse_quote! {
115+
#[doc = #comment]
116+
///
117+
/// # Safety
118+
///
119+
/// The input database must be of type `Self`.
120+
#[doc(hidden)]
121+
unsafe fn downcast(db: &dyn salsa::plumbing::Database) -> &dyn #trait_name where Self: Sized;
108122
});
109123
Ok(())
110124
}
111125

112126
fn add_salsa_view_method_impl(&self, input: &mut syn::ItemImpl) -> syn::Result<()> {
113-
let zalsa = self.hygiene.ident("zalsa");
114-
115127
let Some((_, TraitPath, _)) = &input.trait_ else {
116128
return Err(syn::Error::new_spanned(
117129
&input.self_ty,
@@ -121,9 +133,18 @@ impl DbMacro {
121133

122134
input.items.push(parse_quote! {
123135
#[doc(hidden)]
124-
fn zalsa_db(&self) {
125-
use salsa::plumbing as #zalsa;
126-
#zalsa::views(self).add::<Self, dyn #TraitPath>(|t| t);
136+
#[inline(always)]
137+
fn zalsa_register_downcaster(&self) {
138+
salsa::plumbing::views(self).add(<Self as #TraitPath>::downcast);
139+
}
140+
});
141+
input.items.push(parse_quote! {
142+
#[doc(hidden)]
143+
#[inline(always)]
144+
unsafe fn downcast(db: &dyn salsa::plumbing::Database) -> &dyn #TraitPath where Self: Sized {
145+
debug_assert_eq!(db.type_id(), ::core::any::TypeId::of::<Self>());
146+
// SAFETY: Same as the safety of the `downcast` method.
147+
unsafe { &*salsa::plumbing::transmute_data_ptr::<dyn salsa::plumbing::Database, Self>(db) }
127148
}
128149
});
129150
Ok(())

‎components/salsa-macros/src/lib.rs

-2
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
33
#![recursion_limit = "256"]
44

5-
extern crate proc_macro;
6-
extern crate proc_macro2;
75
#[macro_use]
86
extern crate quote;
97

‎src/accumulator.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ impl<A: Accumulator> Ingredient for IngredientImpl<A> {
100100
self.index
101101
}
102102

103-
fn maybe_changed_after(
103+
unsafe fn maybe_changed_after(
104104
&self,
105105
_db: &dyn Database,
106106
_input: Id,

‎src/database.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ impl dyn Database {
121121
/// If the view has not been added to the database (see [`crate::views::Views`]).
122122
#[track_caller]
123123
pub fn as_view<DbView: ?Sized + Database>(&self) -> &DbView {
124-
self.zalsa().views().try_view_as(self).unwrap()
124+
let views = self.zalsa().views();
125+
views.downcaster_for().downcast(self)
125126
}
126127
}

‎src/function.rs

+15-3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::{
88
plumbing::MemoIngredientMap,
99
salsa_struct::SalsaStructInDb,
1010
table::Table,
11+
views::DatabaseDownCaster,
1112
zalsa::{IngredientIndex, MemoIngredientIndex, Zalsa},
1213
zalsa_local::QueryOrigin,
1314
Cycle, Database, Id, Revision,
@@ -105,6 +106,14 @@ pub struct IngredientImpl<C: Configuration> {
105106
/// Used to find memos to throw out when we have too many memoized values.
106107
lru: lru::Lru,
107108

109+
/// A downcaster from `dyn Database` to `C::DbView`.
110+
///
111+
/// # Safety
112+
///
113+
/// The supplied database must be be the same as the database used to construct the [`Views`]
114+
/// instances that this downcaster was derived from.
115+
view_caster: DatabaseDownCaster<C::DbView>,
116+
108117
/// When `fetch` and friends executes, they return a reference to the
109118
/// value stored in the memo that is extended to live as long as the `&self`
110119
/// reference we start with. This means that whenever we remove something
@@ -135,12 +144,14 @@ where
135144
index: IngredientIndex,
136145
memo_ingredient_indices: <C::SalsaStruct<'static> as SalsaStructInDb>::MemoIngredientMap,
137146
lru: usize,
147+
view_caster: DatabaseDownCaster<C::DbView>,
138148
) -> Self {
139149
Self {
140150
index,
141151
memo_ingredient_indices,
142152
lru: lru::Lru::new(lru),
143153
deleted_entries: Default::default(),
154+
view_caster,
144155
}
145156
}
146157

@@ -213,13 +224,14 @@ where
213224
self.index
214225
}
215226

216-
fn maybe_changed_after(
227+
unsafe fn maybe_changed_after(
217228
&self,
218229
db: &dyn Database,
219230
input: Id,
220231
revision: Revision,
221232
) -> MaybeChangedAfter {
222-
let db = db.as_view::<C::DbView>();
233+
// SAFETY: The `db` belongs to the ingredient as per caller invariant
234+
let db = unsafe { self.view_caster.downcast_unchecked(db) };
223235
self.maybe_changed_after(db, input, revision)
224236
}
225237

@@ -279,7 +291,7 @@ where
279291
db: &'db dyn Database,
280292
key_index: Id,
281293
) -> (Option<&'db AccumulatedMap>, InputAccumulatedValues) {
282-
let db = db.as_view::<C::DbView>();
294+
let db = self.view_caster.downcast(db);
283295
self.accumulated_map(db, key_index)
284296
}
285297
}

‎src/ingredient.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,11 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
5151
fn debug_name(&self) -> &'static str;
5252

5353
/// Has the value for `input` in this ingredient changed after `revision`?
54-
fn maybe_changed_after<'db>(
54+
///
55+
/// # Safety
56+
///
57+
/// The passed in database needs to be the same one that the ingredient was created with.
58+
unsafe fn maybe_changed_after<'db>(
5559
&'db self,
5660
db: &'db dyn Database,
5761
input: Id,

‎src/input.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ impl<C: Configuration> Ingredient for IngredientImpl<C> {
215215
self.ingredient_index
216216
}
217217

218-
fn maybe_changed_after(
218+
unsafe fn maybe_changed_after(
219219
&self,
220220
_db: &dyn Database,
221221
_input: Id,

‎src/input/input_field.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ where
4949
CycleRecoveryStrategy::Panic
5050
}
5151

52-
fn maybe_changed_after(
52+
unsafe fn maybe_changed_after(
5353
&self,
5454
db: &dyn Database,
5555
input: Id,

‎src/interned.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ where
282282
self.ingredient_index
283283
}
284284

285-
fn maybe_changed_after(
285+
unsafe fn maybe_changed_after(
286286
&self,
287287
_db: &dyn Database,
288288
_input: Id,

‎src/key.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,12 @@ impl InputDependencyIndex {
100100
last_verified_at: crate::Revision,
101101
) -> MaybeChangedAfter {
102102
match self.key_index {
103-
Some(key_index) => db
104-
.zalsa()
105-
.lookup_ingredient(self.ingredient_index)
106-
.maybe_changed_after(db, key_index, last_verified_at),
103+
// SAFETY: The `db` belongs to the ingredient
104+
Some(key_index) => unsafe {
105+
db.zalsa()
106+
.lookup_ingredient(self.ingredient_index)
107+
.maybe_changed_after(db, key_index, last_verified_at)
108+
},
107109
// Data in tables themselves remain valid until the table as a whole is reset.
108110
None => MaybeChangedAfter::No(InputAccumulatedValues::Empty),
109111
}

‎src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#![forbid(unsafe_op_in_unsafe_fn)]
22

3+
extern crate self as salsa;
4+
35
mod accumulator;
46
mod active_query;
57
mod array;
@@ -107,6 +109,7 @@ pub mod plumbing {
107109
pub use crate::update::helper::Dispatch as UpdateDispatch;
108110
pub use crate::update::helper::Fallback as UpdateFallback;
109111
pub use crate::update::Update;
112+
pub use crate::zalsa::transmute_data_ptr;
110113
pub use crate::zalsa::views;
111114
pub use crate::zalsa::IngredientCache;
112115
pub use crate::zalsa::IngredientIndex;

‎src/tracked_struct.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,7 @@ where
735735
self.ingredient_index
736736
}
737737

738-
fn maybe_changed_after(
738+
unsafe fn maybe_changed_after(
739739
&self,
740740
db: &dyn Database,
741741
input: Id,

‎src/tracked_struct/tracked_field.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ where
5555
crate::cycle::CycleRecoveryStrategy::Panic
5656
}
5757

58-
fn maybe_changed_after<'db>(
58+
unsafe fn maybe_changed_after<'db>(
5959
&'db self,
6060
db: &'db dyn Database,
6161
input: Id,

0 commit comments

Comments
 (0)