diff --git a/components/salsa-macro-rules/src/setup_input_struct.rs b/components/salsa-macro-rules/src/setup_input_struct.rs index e8fb1924b..e53dc00f4 100644 --- a/components/salsa-macro-rules/src/setup_input_struct.rs +++ b/components/salsa-macro-rules/src/setup_input_struct.rs @@ -152,6 +152,16 @@ macro_rules! setup_input_struct { Self::ingredient_(db.zalsa()) } + #[inline] + fn ingredient_in_db<'db, $Db>(db: &'db $Db) -> ($zalsa::IngredientInDb<'db, $Db, $zalsa_struct::IngredientImpl>, &'db $zalsa::ZalsaLocal) + where + $Db: ?Sized + $zalsa::Database, + { + // SAFETY: `ingredient_` looks up the input ingredient in the `Zalsa` + // supplied by `IngredientInDb`. + unsafe { $zalsa::IngredientInDb::new_unchecked_with_zalsa_local(db, Self::ingredient_) } + } + fn ingredient_(zalsa: &$zalsa::Zalsa) -> &$zalsa_struct::IngredientImpl { static CACHE: $zalsa::IngredientCache<$zalsa_struct::IngredientImpl<$Configuration>> = $zalsa::IngredientCache::new(); @@ -280,13 +290,8 @@ macro_rules! setup_input_struct { // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` $Db: ?Sized + $zalsa::Database, { - let (zalsa, zalsa_local) = db.zalsas(); - let fields = $Configuration::ingredient_(zalsa).field( - zalsa, - zalsa_local, - self, - $field_index, - ); + let (ingredient, zalsa_local) = $Configuration::ingredient_in_db(db); + let fields = ingredient.field(zalsa_local, self, $field_index); $zalsa::return_mode_expression!( $field_option, $field_ty, diff --git a/components/salsa-macro-rules/src/setup_interned_struct.rs b/components/salsa-macro-rules/src/setup_interned_struct.rs index ae3dc77d8..1bfc2dac7 100644 --- a/components/salsa-macro-rules/src/setup_interned_struct.rs +++ b/components/salsa-macro-rules/src/setup_interned_struct.rs @@ -207,6 +207,16 @@ macro_rules! setup_interned_struct { CACHE.get_or_create::<$zalsa_struct::JarImpl<$Configuration>, 0>(zalsa) } } + + #[inline] + fn ingredient_in_db<'db, $Db>(db: &'db $Db) -> $zalsa::IngredientInDb<'db, $Db, $zalsa_struct::IngredientImpl> + where + $Db: ?Sized + $zalsa::Database, + { + // SAFETY: `ingredient` looks up the interned ingredient in the `Zalsa` + // supplied by `IngredientInDb`. + unsafe { $zalsa::IngredientInDb::new_unchecked(db, Self::ingredient) } + } } impl< $($db_lt_arg)? > $zalsa::AsId for $Struct< $($db_lt_arg)? > { @@ -324,8 +334,7 @@ macro_rules! setup_interned_struct { // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` $Db: ?Sized + $zalsa::Database, { - let zalsa = db.zalsa(); - let fields = $Configuration::ingredient(zalsa).fields(zalsa, self); + let fields = $Configuration::ingredient_in_db(db).fields(self); $zalsa::return_mode_expression!( $field_option, $field_ty, diff --git a/components/salsa-macro-rules/src/setup_tracked_fn.rs b/components/salsa-macro-rules/src/setup_tracked_fn.rs index 5fb3142bb..4d4497f26 100644 --- a/components/salsa-macro-rules/src/setup_tracked_fn.rs +++ b/components/salsa-macro-rules/src/setup_tracked_fn.rs @@ -96,16 +96,17 @@ macro_rules! setup_tracked_fn { use ::salsa::plumbing as $zalsa; $zalsa::attach($db, || { - let (zalsa, zalsa_local) = $db.zalsas(); + let (fn_ingredient, zalsa_local) = $fn_name::fn_ingredient_($db); let result = $zalsa::macro_if! { if $needs_interner { { + let zalsa = fn_ingredient.zalsa(); let key = $fn_name::intern_ingredient_(zalsa).intern_id(zalsa, zalsa_local, ($($input_id),*), |_, data| data); - $fn_name::fn_ingredient_($db, zalsa).fetch($db, zalsa, zalsa_local, key) + fn_ingredient.fetch(zalsa_local, key) } } else { { - $fn_name::fn_ingredient_($db, zalsa).fetch($db, zalsa, zalsa_local, $zalsa::AsId::as_id(&($($input_id),*))) + fn_ingredient.fetch(zalsa_local, $zalsa::AsId::as_id(&($($input_id),*))) } } }; @@ -255,19 +256,18 @@ macro_rules! setup_tracked_fn { } impl $Configuration { - fn fn_ingredient(db: &dyn $Db) -> &$zalsa::function::IngredientImpl<$Configuration> { - let zalsa = db.zalsa(); - Self::fn_ingredient_(db, zalsa) - } - #[inline] - fn fn_ingredient_<'z>(db: &dyn $Db, zalsa: &'z $zalsa::Zalsa) -> &'z $zalsa::function::IngredientImpl<$Configuration> { + fn fn_ingredient<'db>(db: &'db dyn $Db) -> ($zalsa::IngredientInDb<'db, dyn $Db, $zalsa::function::IngredientImpl<$Configuration>>, &'db $zalsa::ZalsaLocal) { // SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the first - // ingredient created by our jar is the function ingredient. + // ingredient created by our jar is the function ingredient in the provided + // `zalsa`. unsafe { - $FN_CACHE.get_or_create::<$fn_name, 0>(zalsa) + $zalsa::IngredientInDb::new_unchecked_with_zalsa_local(db, |zalsa| { + $FN_CACHE + .get_or_create::<$fn_name, 0>(zalsa) + .get_or_init(|| *::zalsa_register_downcaster(db)) + }) } - .get_or_init(|| *::zalsa_register_downcaster(db)) } pub fn fn_ingredient_mut(db: &mut dyn $Db) -> &mut $zalsa::function::IngredientImpl { @@ -456,16 +456,17 @@ macro_rules! setup_tracked_fn { $($input_id: $interned_input_ty,)* ) -> Vec<&$db_lt A> { use ::salsa::plumbing as $zalsa; + let (fn_ingredient, zalsa_local) = $Configuration::fn_ingredient($db); let key = $zalsa::macro_if! { if $needs_interner {{ - let (zalsa, zalsa_local) = $db.zalsas(); + let zalsa = fn_ingredient.zalsa(); $Configuration::intern_ingredient(zalsa).intern_id(zalsa, zalsa_local, ($($input_id),*), |_, data| data) }} else { $zalsa::AsId::as_id(&($($input_id),*)) } }; - $Configuration::fn_ingredient($db).accumulated_by::($db, key) + fn_ingredient.accumulated_by::(zalsa_local, key) } } @@ -476,8 +477,9 @@ macro_rules! setup_tracked_fn { value: $output_ty, ) { let key = $zalsa::AsId::as_id(&($($input_id),*)); - $Configuration::fn_ingredient($db).specify_and_record( - $db, + let (fn_ingredient, zalsa_local) = $Configuration::fn_ingredient($db); + fn_ingredient.specify_and_record( + zalsa_local, key, value, ) @@ -504,8 +506,8 @@ macro_rules! setup_tracked_fn { } #[inline] - fn fn_ingredient_<'z>(db: &dyn $Db, zalsa: &'z $zalsa::Zalsa) -> &'z $zalsa::function::IngredientImpl<$Configuration> { - $Configuration::fn_ingredient_(db, zalsa) + fn fn_ingredient_<'db>(db: &'db dyn $Db) -> ($zalsa::IngredientInDb<'db, dyn $Db, $zalsa::function::IngredientImpl<$Configuration>>, &'db $zalsa::ZalsaLocal) { + $Configuration::fn_ingredient(db) } } }; diff --git a/components/salsa-macro-rules/src/setup_tracked_struct.rs b/components/salsa-macro-rules/src/setup_tracked_struct.rs index fa8ae20fb..8f3068206 100644 --- a/components/salsa-macro-rules/src/setup_tracked_struct.rs +++ b/components/salsa-macro-rules/src/setup_tracked_struct.rs @@ -224,6 +224,26 @@ macro_rules! setup_tracked_struct { Self::ingredient_(db.zalsa()) } + #[inline] + fn ingredient_in_db<'db, $Db>(db: &'db $Db) -> $zalsa::IngredientInDb<'db, $Db, $zalsa_struct::IngredientImpl> + where + $Db: ?Sized + $zalsa::Database, + { + // SAFETY: `ingredient_` looks up the tracked-struct ingredient in the `Zalsa` + // supplied by `IngredientInDb`. + unsafe { $zalsa::IngredientInDb::new_unchecked(db, Self::ingredient_) } + } + + #[inline] + fn ingredient_in_db_with_zalsa_local<'db, $Db>(db: &'db $Db) -> ($zalsa::IngredientInDb<'db, $Db, $zalsa_struct::IngredientImpl>, &'db $zalsa::ZalsaLocal) + where + $Db: ?Sized + $zalsa::Database, + { + // SAFETY: `ingredient_` looks up the tracked-struct ingredient in the `Zalsa` + // supplied by `IngredientInDb`. + unsafe { $zalsa::IngredientInDb::new_unchecked_with_zalsa_local(db, Self::ingredient_) } + } + fn ingredient_(zalsa: &$zalsa::Zalsa) -> &$zalsa_struct::IngredientImpl { static CACHE: $zalsa::IngredientCache<$zalsa_struct::IngredientImpl<$Configuration>> = $zalsa::IngredientCache::new(); @@ -356,8 +376,8 @@ macro_rules! setup_tracked_struct { // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` $Db: ?Sized + $zalsa::Database, { - let (zalsa, zalsa_local) = db.zalsas(); - let fields = $Configuration::ingredient_(zalsa).tracked_field(zalsa, zalsa_local, self, $relative_tracked_index); + let (ingredient, zalsa_local) = $Configuration::ingredient_in_db_with_zalsa_local(db); + let fields = ingredient.tracked_field(zalsa_local, self, $relative_tracked_index); $crate::return_mode_expression!( $tracked_option, $tracked_ty, @@ -373,8 +393,7 @@ macro_rules! setup_tracked_struct { // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` $Db: ?Sized + $zalsa::Database, { - let zalsa = db.zalsa(); - let fields = $Configuration::ingredient_(zalsa).untracked_field(zalsa, self); + let fields = $Configuration::ingredient_in_db(db).untracked_field(self); $crate::return_mode_expression!( $untracked_option, $untracked_ty, diff --git a/src/function.rs b/src/function.rs index d66cf2f96..0c7c41473 100644 --- a/src/function.rs +++ b/src/function.rs @@ -503,7 +503,11 @@ where ) { // SAFETY: The `db` belongs to the ingredient as per caller invariant let db = unsafe { self.view_caster().downcast_unchecked(db) }; - self.accumulated_map(db, key_index) + // SAFETY: The caller guarantees that `self` is registered in the `Zalsa` owned by `db`. + let (ingredient, zalsa_local) = unsafe { + crate::ingredient::IngredientInDb::new_unchecked_with_zalsa_local(db, |_| self) + }; + ingredient.accumulated_map(zalsa_local, key_index) } fn is_persistable(&self) -> bool { diff --git a/src/function/accumulated.rs b/src/function/accumulated.rs index 72f4ccae5..bc4184010 100644 --- a/src/function/accumulated.rs +++ b/src/function/accumulated.rs @@ -2,21 +2,22 @@ use crate::accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues use crate::accumulator::{self}; use crate::function::{Configuration, IngredientImpl}; use crate::hash::FxHashSet; -use crate::zalsa::ZalsaDatabase; -use crate::zalsa_local::QueryOriginRef; +use crate::ingredient::IngredientInDb; +use crate::zalsa_local::{QueryOriginRef, ZalsaLocal}; use crate::{DatabaseKeyIndex, Id}; -impl IngredientImpl +impl<'db, C> IngredientInDb<'db, C::DbView, IngredientImpl> where C: Configuration, { - /// Helper used by `accumulate` functions. Computes the results accumulated by `database_key_index` - /// and its inputs. - pub fn accumulated_by<'db, A>(&self, db: &'db C::DbView, key: Id) -> Vec<&'db A> + /// Helper used by `accumulate` functions. Computes the results accumulated by + /// `database_key_index` and its inputs. + pub fn accumulated_by(&self, zalsa_local: &'db ZalsaLocal, key: Id) -> Vec<&'db A> where A: accumulator::Accumulator, { - let (zalsa, zalsa_local) = db.zalsas(); + let zalsa = self.zalsa(); + let db = self.db(); // NOTE: We don't have a precise way to track accumulated values at present, // so we report any read of them as an untracked read. @@ -37,9 +38,9 @@ where let mut output = vec![]; // First ensure the result is up to date - self.fetch(db, zalsa, zalsa_local, key); + self.fetch(zalsa_local, key); - let db_key = self.database_key_index(key); + let db_key = self.ingredient().database_key_index(key); let mut visited: FxHashSet = FxHashSet::default(); let mut stack: Vec = vec![db_key]; @@ -53,7 +54,7 @@ where let ingredient = zalsa.lookup_ingredient(k.ingredient_index()); // Extend `output` with any values accumulated by `k`. - // SAFETY: `db` owns the `ingredient` + // SAFETY: `ingredient` and `k` belong to the `Zalsa` obtained from `db`. let (accumulated_map, input) = unsafe { ingredient.accumulated(db.into(), k.key_index()) }; if let Some(accumulated_map) = accumulated_map { @@ -87,14 +88,13 @@ where output } - pub(super) fn accumulated_map<'db>( - &'db self, - db: &'db C::DbView, + pub(super) fn accumulated_map( + &self, + zalsa_local: &'db ZalsaLocal, key: Id, ) -> (Option<&'db AccumulatedMap>, InputAccumulatedValues) { - let (zalsa, zalsa_local) = db.zalsas(); // NEXT STEP: stash and refactor `fetch` to return an `&Memo` so we can make this work - let memo = self.refresh_memo(db, zalsa, zalsa_local, key); + let memo = self.refresh_memo(zalsa_local, key); ( memo.header.revisions.accumulated(), memo.header.revisions.accumulated_inputs.load(), diff --git a/src/function/fetch.rs b/src/function/fetch.rs index e155cc3d4..3a8a786a3 100644 --- a/src/function/fetch.rs +++ b/src/function/fetch.rs @@ -3,35 +3,32 @@ use crate::function::eviction::EvictionPolicy; use crate::function::memo::Memo; use crate::function::sync::ClaimResult; use crate::function::{Configuration, IngredientImpl, Reentrancy}; +use crate::ingredient::IngredientInDb; use crate::zalsa::{MemoIngredientIndex, Zalsa}; use crate::zalsa_local::{QueryRevisions, ZalsaLocal}; use crate::{DatabaseKeyIndex, Id}; -impl IngredientImpl +impl<'db, C> IngredientInDb<'db, C::DbView, IngredientImpl> where C: Configuration, { #[inline] - pub fn fetch<'db>( - &'db self, - db: &'db C::DbView, - zalsa: &'db Zalsa, - zalsa_local: &'db ZalsaLocal, - id: Id, - ) -> &'db C::Output<'db> { + pub fn fetch(&self, zalsa_local: &'db ZalsaLocal, id: Id) -> &'db C::Output<'db> { + let zalsa = self.zalsa(); + zalsa.unwind_if_revision_cancelled(zalsa_local); - let database_key_index = self.database_key_index(id); + let database_key_index = self.ingredient().database_key_index(id); #[cfg(feature = "detailed-trace")] let _span = crate::tracing::debug_span!("fetch", query = ?database_key_index).entered(); - let memo = self.refresh_memo(db, zalsa, zalsa_local, id); + let memo = self.refresh_memo(zalsa_local, id); // SAFETY: We just refreshed the memo so it is guaranteed to contain a value now. let memo_value = unsafe { memo.value.as_ref().unwrap_unchecked() }; - self.eviction.record_use(id); + self.ingredient().eviction.record_use(id); let revisions = &memo.header.revisions; zalsa_local.report_tracked_read( @@ -49,41 +46,46 @@ where } #[inline(always)] - pub(super) fn refresh_memo<'db>( - &'db self, - db: &'db C::DbView, - zalsa: &'db Zalsa, - zalsa_local: &'db ZalsaLocal, - id: Id, - ) -> &'db Memo<'db, C> { - let memo_ingredient_index = self.memo_ingredient_index(zalsa, id); + pub(super) fn refresh_memo(&self, zalsa_local: &'db ZalsaLocal, id: Id) -> &'db Memo<'db, C> { + let ingredient = self.ingredient(); + let db = self.db(); + let zalsa = self.zalsa(); + let memo_ingredient_index = ingredient.memo_ingredient_index(zalsa, id); loop { // Keep the hot and cold probes in distinct control-flow blocks. Using `or_else` // here can outline both into one function, making hot hits pay for the cold path's // stack frame. - if let Some(memo) = self.fetch_hot(zalsa, id, memo_ingredient_index) { + if let Some(memo) = self.fetch_hot(id, memo_ingredient_index) { return memo; } - if let Some(memo) = self.fetch_cold(zalsa, zalsa_local, db, id, memo_ingredient_index) { + if let Some(memo) = + ingredient.fetch_cold(zalsa, zalsa_local, db, id, memo_ingredient_index) + { return memo; } } } #[inline(always)] - fn fetch_hot<'db>( - &'db self, - zalsa: &'db Zalsa, + fn fetch_hot( + &self, id: Id, memo_ingredient_index: MemoIngredientIndex, ) -> Option<&'db Memo<'db, C>> { - let memo = self.get_memo_from_table_for(zalsa, id, memo_ingredient_index)?; + let ingredient = self.ingredient(); + let zalsa = self.zalsa(); + + // SAFETY: `IngredientInDb` guarantees that the ingredient is registered in `zalsa`, and + // `memo_ingredient_index` was read from that ingredient's memo map. + let memo = unsafe { + ingredient.get_memo_from_table_for_unchecked(zalsa, id, memo_ingredient_index)? + }; memo.value.as_ref()?; - let database_key_index = self.database_key_index(id); + let database_key_index = ingredient.database_key_index(id); let can_shallow_update = memo .header @@ -95,12 +97,17 @@ where // SAFETY: memo is present in memo_map and we have verified that it is // still valid for the current revision. - unsafe { Some(self.extend_memo_lifetime(memo)) } + unsafe { Some(ingredient.extend_memo_lifetime(memo)) } } else { None } } +} +impl IngredientImpl +where + C: Configuration, +{ fn fetch_cold<'db>( &'db self, zalsa: &'db Zalsa, diff --git a/src/function/memo.rs b/src/function/memo.rs index 1d7aac538..e798beaed 100644 --- a/src/function/memo.rs +++ b/src/function/memo.rs @@ -56,6 +56,30 @@ impl IngredientImpl { Some(unsafe { transmute::<&Memo<'static, C>, &'db Memo<'db, C>>(static_memo.as_ref()) }) } + /// Loads the current memo without checking its registered type. + /// + /// # Safety + /// + /// `self` must be registered in `zalsa`, and `memo_ingredient_index` must come from this + /// ingredient's memo ingredient map. + pub(super) unsafe fn get_memo_from_table_for_unchecked<'db>( + &self, + zalsa: &'db Zalsa, + id: Id, + memo_ingredient_index: MemoIngredientIndex, + ) -> Option<&'db Memo<'db, C>> { + // SAFETY: The caller guarantees that this ingredient's memo map registered `Memo` at + // `memo_ingredient_index` in `zalsa`. + let static_memo = unsafe { + zalsa + .memo_table_for::>(id) + .get_unchecked(memo_ingredient_index)? + }; + // SAFETY: The table stores 'static memos (to support `Any`), the memos are in fact valid + // for `'db` though as we delay their dropping to the end of a revision. + Some(unsafe { transmute::<&Memo<'static, C>, &'db Memo<'db, C>>(static_memo.as_ref()) }) + } + /// Evicts the existing memo for the given key, replacing it /// with an equivalent memo that has no value. If the memo is untracked /// or has values assigned as output of another query, this has no effect. diff --git a/src/function/specify.rs b/src/function/specify.rs index 9503680b8..9ef8fa16e 100644 --- a/src/function/specify.rs +++ b/src/function/specify.rs @@ -4,24 +4,26 @@ use crate::active_query::CompletedQuery; use crate::function::memo::{Memo, MemoHeader}; use crate::function::sync::{ClaimResult, Reentrancy}; use crate::function::{Configuration, IngredientImpl}; +use crate::ingredient::IngredientInDb; use crate::revision::AtomicRevision; use crate::sync::atomic::AtomicBool; use crate::tracked_struct::TrackedStructInDb; -use crate::zalsa::{Zalsa, ZalsaDatabase}; -use crate::zalsa_local::{OriginAndExtra, QueryOriginRef, QueryRevisions}; +use crate::zalsa::Zalsa; +use crate::zalsa_local::{OriginAndExtra, QueryOriginRef, QueryRevisions, ZalsaLocal}; use crate::{DatabaseKeyIndex, Id}; -impl IngredientImpl +impl<'db, C> IngredientInDb<'db, C::DbView, IngredientImpl> where C: Configuration, { /// Specify the value for `key` *and* record that we did so. /// Used for explicit calls to `specify`, but not needed for pre-declared tracked struct fields. - pub fn specify_and_record<'db>(&'db self, db: &'db C::DbView, key: Id, value: C::Output<'db>) + pub fn specify_and_record(&self, zalsa_local: &'db ZalsaLocal, key: Id, value: C::Output<'db>) where C::Input<'db>: TrackedStructInDb, { - let (zalsa, zalsa_local) = db.zalsas(); + let ingredient = self.ingredient(); + let zalsa = self.zalsa(); let (active_query_key, current_deps, cycle_heads) = match zalsa_local.active_query_with_cycle_heads() { @@ -66,12 +68,12 @@ where // - a result that is NOT verified and has untracked inputs, which will re-execute (and likely panic) let revision = zalsa.current_revision(); - let database_key_index = self.database_key_index(key); - let memo_ingredient_index = self.memo_ingredient_index(zalsa, key); + let database_key_index = ingredient.database_key_index(key); + let memo_ingredient_index = ingredient.memo_ingredient_index(zalsa, key); zalsa.unwind_if_revision_cancelled(zalsa_local); let _claim_guard = - match self + match ingredient .sync_table .try_claim(zalsa, zalsa_local, key, Reentrancy::Deny) { @@ -84,7 +86,7 @@ where // Re-read the memo after claiming the query so that no concurrent execution or // specification can replace it while we decide whether to keep or overwrite it. - let old_memo = self.get_memo_from_table_for(zalsa, key, memo_ingredient_index); + let old_memo = ingredient.get_memo_from_table_for(zalsa, key, memo_ingredient_index); if let Some(old_memo) = old_memo { if old_memo.header.verified_at.load() == revision && old_memo.value.is_some() { @@ -129,7 +131,7 @@ where completed_query .stale_tracked_structs .extend_from_slice(old_memo.header.revisions.tracked_struct_ids()); - self.backdate_if_appropriate( + ingredient.backdate_if_appropriate( old_memo, database_key_index, &mut completed_query.revisions, @@ -153,12 +155,17 @@ where memo.tracing_debug(), key ); - self.insert_memo(zalsa, key, memo, memo_ingredient_index); + ingredient.insert_memo(zalsa, key, memo, memo_ingredient_index); // Record that the current query *specified* a value for this cell. zalsa_local.add_output(database_key_index); } +} +impl IngredientImpl +where + C: Configuration, +{ /// Invoked when the query `executor` has been validated as having green inputs /// and `key` is a value that was specified by `executor`. /// Marks `key` as valid in the current revision since if `executor` had re-executed, diff --git a/src/ingredient.rs b/src/ingredient.rs index 23cb11478..73087e331 100644 --- a/src/ingredient.rs +++ b/src/ingredient.rs @@ -7,10 +7,12 @@ use crate::function::VerifyResult; use crate::hash::{FxHashSet, FxIndexSet}; use crate::runtime::Running; use crate::sync::Arc; -use crate::table::Table; use crate::table::memo::MemoTableTypes; -use crate::zalsa::{IngredientIndex, JarKind, Zalsa, transmute_data_mut_ptr, transmute_data_ptr}; -use crate::zalsa_local::{QueryEdge, QueryOriginRef}; +use crate::table::{Slot, Table}; +use crate::zalsa::{ + IngredientIndex, JarKind, Zalsa, ZalsaDatabase, transmute_data_mut_ptr, transmute_data_ptr, +}; +use crate::zalsa_local::{QueryEdge, QueryOriginRef, ZalsaLocal}; use crate::{DatabaseKeyIndex, Id, Revision}; /// A "jar" is a group of ingredients that are added atomically. @@ -35,6 +37,112 @@ pub struct Location { pub line: u32, } +/// An ingredient bound to the database that registered it. +#[doc(hidden)] +pub struct IngredientInDb<'db, Db: ?Sized, I> { + ingredient: &'db I, + db: &'db Db, + zalsa: &'db Zalsa, +} + +impl<'db, Db, I> IngredientInDb<'db, Db, I> +where + Db: ?Sized + ZalsaDatabase, + I: Ingredient, +{ + /// Creates an ingredient bound to a database. + /// + /// # Safety + /// + /// `get_ingredient` must return an ingredient registered in the `Zalsa` it receives. + #[inline] + pub unsafe fn new_unchecked( + db: &'db Db, + get_ingredient: impl FnOnce(&'db Zalsa) -> &'db I, + ) -> Self { + Self::new_unchecked_with_zalsa(db, db.zalsa(), get_ingredient) + } + + /// Creates an ingredient bound to a database and returns its thread-local state. + /// + /// # Safety + /// + /// `get_ingredient` must return an ingredient registered in the `Zalsa` it receives. + #[inline] + pub unsafe fn new_unchecked_with_zalsa_local( + db: &'db Db, + get_ingredient: impl FnOnce(&'db Zalsa) -> &'db I, + ) -> (Self, &'db ZalsaLocal) { + let (zalsa, zalsa_local) = db.zalsas(); + ( + Self::new_unchecked_with_zalsa(db, zalsa, get_ingredient), + zalsa_local, + ) + } + + fn new_unchecked_with_zalsa( + db: &'db Db, + zalsa: &'db Zalsa, + get_ingredient: impl FnOnce(&'db Zalsa) -> &'db I, + ) -> Self { + let ingredient = get_ingredient(zalsa); + + debug_assert!(std::ptr::eq( + ingredient, + zalsa + .lookup_ingredient(ingredient.ingredient_index()) + .assert_type::() + )); + + Self { + ingredient, + db, + zalsa, + } + } +} + +impl<'db, Db: ?Sized, I> IngredientInDb<'db, Db, I> { + #[inline(always)] + pub(crate) fn ingredient(&self) -> &'db I { + self.ingredient + } + + #[inline(always)] + pub(crate) fn db(&self) -> &'db Db { + self.db + } + + #[inline(always)] + pub fn zalsa(&self) -> &'db Zalsa { + self.zalsa + } +} + +impl<'db, Db: ?Sized, I: TableIngredient> IngredientInDb<'db, Db, I> { + #[inline(always)] + pub(crate) fn slot(&self, id: Id) -> &'db I::Slot { + // SAFETY: `IngredientInDb` guarantees that `ingredient` is registered in `zalsa`. + // `TableIngredient` guarantees that pages owned by the ingredient store `I::Slot`. + unsafe { + self.zalsa + .table() + .get_for_ingredient(id, self.ingredient.ingredient_index()) + } + } + + #[inline(always)] + pub(crate) fn slot_raw(&self, id: Id) -> *mut I::Slot { + // SAFETY: `IngredientInDb` guarantees that `ingredient` is registered in `zalsa`. + // `TableIngredient` guarantees that pages owned by the ingredient store `I::Slot`. + unsafe { + self.zalsa + .table() + .get_raw_for_ingredient(id, self.ingredient.ingredient_index()) + } + } +} + pub trait Ingredient: Any + fmt::Debug + Send + Sync { fn debug_name(&self) -> &'static str; fn location(&self) -> &'static Location; @@ -284,6 +392,16 @@ pub trait Ingredient: Any + fmt::Debug + Send + Sync { } } +/// Associates a table-owning ingredient with the type stored in its pages. +/// +/// # Safety +/// +/// Every table page owned by an implementation's ingredient index must store `Self::Slot`. +#[doc(hidden)] +pub unsafe trait TableIngredient: Ingredient { + type Slot: Slot; +} + impl dyn Ingredient { /// Equivalent to the `downcast` method on `Any`. /// diff --git a/src/ingredient_cache.rs b/src/ingredient_cache.rs index bc375956c..8147a4641 100644 --- a/src/ingredient_cache.rs +++ b/src/ingredient_cache.rs @@ -56,7 +56,8 @@ mod imp { &self, zalsa: &'db Zalsa, ) -> &'db I { - let mut ingredient_index = self.ingredient_index.load(Ordering::Acquire); + // The cached number does not publish any other state. + let mut ingredient_index = self.ingredient_index.load(Ordering::Relaxed); if ingredient_index == Self::UNINITIALIZED { ingredient_index = get_or_create_index_slow( &self.ingredient_index, @@ -101,7 +102,7 @@ mod imp { // It doesn't matter if we overwrite any stores, as the jar lookup should // always return the same index when the `inventory` feature is enabled. - cached_index.store(ingredient_index.as_u32(), Ordering::Release); + cached_index.store(ingredient_index.as_u32(), Ordering::Relaxed); ingredient_index } @@ -194,7 +195,8 @@ mod imp { mem::size_of::<(Nonce, IngredientIndex)>() == mem::size_of::() ); - let cached_data = self.cached_data.load(Ordering::Acquire); + // The cached number does not publish any other state. + let cached_data = self.cached_data.load(Ordering::Relaxed); if cached_data == UNINITIALIZED { return get_or_create_index_slow( &self.cached_data, @@ -243,7 +245,7 @@ mod imp { debug_assert_ne!(packed, UNINITIALIZED); // Discard the result, whether we won over the cache or not doesn't matter. - _ = cache.compare_exchange(UNINITIALIZED, packed, Ordering::Release, Ordering::Relaxed); + _ = cache.compare_exchange(UNINITIALIZED, packed, Ordering::Relaxed, Ordering::Relaxed); // Use our locally computed index regardless of which one was cached. index diff --git a/src/input.rs b/src/input.rs index ee02fece5..43b422513 100644 --- a/src/input.rs +++ b/src/input.rs @@ -121,6 +121,32 @@ pub struct IngredientImpl { _phantom: std::marker::PhantomData, } +impl<'db, Db: ?Sized, C: Configuration> + crate::ingredient::IngredientInDb<'db, Db, IngredientImpl> +{ + /// Access a field of an input. + #[inline] + pub fn field( + &self, + zalsa_local: &ZalsaLocal, + id: C::Struct, + field_index: usize, + ) -> &'db C::Fields { + let ingredient = self.ingredient(); + let field_ingredient_index = ingredient.ingredient_index.successor(field_index); + let id = id.as_id(); + let value = self.slot(id); + let durability = value.durabilities[field_index]; + let revision = value.revisions[field_index]; + zalsa_local.report_tracked_read_simple( + DatabaseKeyIndex::new(field_ingredient_index, id), + durability, + revision, + ); + &value.fields + } +} + impl IngredientImpl { pub fn new(index: IngredientIndex) -> Self { Self { @@ -224,29 +250,6 @@ impl IngredientImpl { .map(|id| FromIdWithDb::from_id(id, zalsa)) } - /// Access field of an input. - /// Note that this function returns the entire tuple of value fields. - /// The caller is responsible for selecting the appropriate element. - pub fn field<'db>( - &'db self, - zalsa: &'db Zalsa, - zalsa_local: &'db ZalsaLocal, - id: C::Struct, - field_index: usize, - ) -> &'db C::Fields { - let field_ingredient_index = self.ingredient_index.successor(field_index); - let id = id.as_id(); - let value = Self::data(zalsa, id); - let durability = value.durabilities[field_index]; - let revision = value.revisions[field_index]; - zalsa_local.report_tracked_read_simple( - DatabaseKeyIndex::new(field_ingredient_index, id), - durability, - revision, - ); - &value.fields - } - /// Returns all data corresponding to the input struct. pub fn entries<'db>(&'db self, zalsa: &'db Zalsa) -> impl Iterator> { zalsa @@ -468,6 +471,11 @@ pub trait HasBuilder { type Builder; } +// SAFETY: `IngredientImpl` only allocates pages containing `Value`. +unsafe impl crate::ingredient::TableIngredient for IngredientImpl { + type Slot = Value; +} + // SAFETY: `Value` is our private type branded over the unique configuration `C`. unsafe impl Slot for Value where diff --git a/src/interned.rs b/src/interned.rs index 1e750d0ef..7e01ff20b 100644 --- a/src/interned.rs +++ b/src/interned.rs @@ -105,6 +105,22 @@ pub struct IngredientImpl { _marker: PhantomData C>, } +impl<'db, Db: ?Sized, C: Configuration> + crate::ingredient::IngredientInDb<'db, Db, IngredientImpl> +{ + /// Lookup the fields from an interned struct. + #[inline] + pub fn fields(&self, s: C::Struct<'db>) -> &'db C::Fields<'db> { + self.data(AsId::as_id(&s)) + } + + fn data(&self, id: Id) -> &'db C::Fields<'db> { + let ingredient = self.ingredient(); + let value = self.slot(id); + ingredient.data_from_value(self.zalsa(), id, value) + } +} + struct IngredientShard { /// Maps from data to the existing interned ID for that data. /// @@ -814,6 +830,15 @@ where pub fn data<'db>(&'db self, zalsa: &'db Zalsa, id: Id) -> &'db C::Fields<'db> { let value = zalsa.table().get::>(id); + self.data_from_value(zalsa, id, value) + } + + fn data_from_value<'db>( + &'db self, + zalsa: &'db Zalsa, + id: Id, + value: &'db Value, + ) -> &'db C::Fields<'db> { debug_assert!( { let _shard = self.shards[value.shard as usize].lock(); @@ -1091,6 +1116,11 @@ where } } +// SAFETY: `IngredientImpl` only allocates pages containing `Value`. +unsafe impl crate::ingredient::TableIngredient for IngredientImpl { + type Slot = Value; +} + // SAFETY: `Value` is our private type branded over the unique configuration `C`. unsafe impl Slot for Value where diff --git a/src/lib.rs b/src/lib.rs index a3994a48c..a49dfd68a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -339,7 +339,7 @@ pub mod plumbing { pub use crate::database::{Database, current_revision}; pub use crate::durability::Durability; pub use crate::id::{AsId, FromId, FromIdWithDb, Id}; - pub use crate::ingredient::{Ingredient, Jar, Location}; + pub use crate::ingredient::{Ingredient, IngredientInDb, Jar, Location}; pub use crate::ingredient_cache::IngredientCache; pub use crate::interned::{HashEqLike, Lookup}; pub use crate::key::DatabaseKeyIndex; diff --git a/src/table.rs b/src/table.rs index 09cea09f4..39c90c7d4 100644 --- a/src/table.rs +++ b/src/table.rs @@ -197,6 +197,60 @@ impl Table { page_ref.page_data()[slot.0].get().cast::() } + /// Get a reference to the data for `id`, checking its page owner instead of its type. + /// + /// # Safety + /// + /// Pages owned by `ingredient` must store values of type `T`. + #[inline(always)] + pub(crate) unsafe fn get_for_ingredient( + &self, + id: Id, + ingredient: IngredientIndex, + ) -> &T { + let (page, slot) = split_id(id); + // SAFETY: Guaranteed by the caller. + let page_ref = unsafe { self.page_for_ingredient::(page, ingredient) }; + &page_ref.data()[slot.0] + } + + /// Get a raw pointer to the data for `id`, checking its page owner instead of its type. + /// + /// # Safety + /// + /// Pages owned by `ingredient` must store values of type `T`. See [`Page::get_raw`][]. + #[inline(always)] + pub(crate) unsafe fn get_raw_for_ingredient( + &self, + id: Id, + ingredient: IngredientIndex, + ) -> *mut T { + let (page, slot) = split_id(id); + // SAFETY: Guaranteed by the caller. + let page_ref = unsafe { self.page_for_ingredient::(page, ingredient) }; + page_ref.page_data()[slot.0].get().cast::() + } + + /// Gets a typed view of a page owned by `ingredient`. + /// + /// # Safety + /// + /// Pages owned by `ingredient` must store values of type `T`. + #[inline(always)] + unsafe fn page_for_ingredient( + &self, + page: PageIndex, + ingredient: IngredientIndex, + ) -> PageView<'_, T> { + let page = &self.pages[page.0]; + if page.ingredient != ingredient { + ingredient_assert_failed(page, ingredient); + } + + debug_assert_eq!(page.slot_type_id, TypeId::of::()); + PageView(page, PhantomData) + } + /// Returns the number of pages that have been allocated. pub fn page_count(&self) -> usize { self.pages.count() @@ -573,6 +627,16 @@ fn type_assert_failed(page: &Page) -> ! { ) } +/// This function is explicitly outlined to keep debug machinery out of the hot path. +#[cold] +#[inline(never)] +fn ingredient_assert_failed(page: &Page, expected: IngredientIndex) -> ! { + panic!( + "page belongs to ingredient `{:?}` but ingredient `{expected:?}` was expected", + page.ingredient, + ) +} + impl Drop for Page { fn drop(&mut self) { let len = *self.allocated.get_mut(); diff --git a/src/table/memo.rs b/src/table/memo.rs index 309b48e51..ec7658a8f 100644 --- a/src/table/memo.rs +++ b/src/table/memo.rs @@ -352,6 +352,31 @@ impl MemoTableWithTypes<'_> { .map(|memo| unsafe { MemoEntryType::from_dummy(memo) }) } + /// Returns a pointer to the memo at the given index, if one has been inserted, without + /// checking its registered type. + /// + /// # Safety + /// + /// `M` must be the type registered for `memo_ingredient_index`. + #[inline] + pub(crate) unsafe fn get_unchecked( + self, + memo_ingredient_index: MemoIngredientIndex, + ) -> Option> { + let MemoEntry { atomic_memo } = self.memos.memos.get(memo_ingredient_index.as_usize())?; + + debug_assert_eq!( + self.types.types[memo_ingredient_index.as_usize()].type_id, + TypeId::of::(), + "inconsistent type-id for `{memo_ingredient_index:?}`" + ); + + NonNull::new(atomic_memo.load(Ordering::Acquire)) + // SAFETY: The caller guarantees that the registered type is `M`, and `insert` checks + // the registered type before storing the pointer. + .map(|memo| unsafe { MemoEntryType::from_dummy(memo) }) + } + #[cfg(feature = "salsa_unstable")] pub(crate) fn memory_usage(&self) -> Vec { let mut memory_usage = Vec::new(); diff --git a/src/tracked_struct.rs b/src/tracked_struct.rs index dbe3b0d6c..acc784ac1 100644 --- a/src/tracked_struct.rs +++ b/src/tracked_struct.rs @@ -202,6 +202,53 @@ where memo_table_types: Arc, } +impl<'db, Db: ?Sized, C: Configuration> + crate::ingredient::IngredientInDb<'db, Db, IngredientImpl> +{ + /// Access a tracked field. + #[inline] + pub fn tracked_field( + &self, + zalsa_local: &ZalsaLocal, + s: C::Struct<'db>, + relative_tracked_index: usize, + ) -> &'db C::Fields<'db> { + let ingredient = self.ingredient(); + let id = AsId::as_id(&s); + let field_ingredient_index = ingredient + .ingredient_index + .successor(relative_tracked_index); + let data = self.slot_raw(id); + + // SAFETY: `data` belongs to the bound ingredient and database. + let fields = unsafe { ingredient.lock_fields(data, self.zalsa().current_revision()) }; + + // SAFETY: We acquired the read lock above, so `revisions` and `durability` are not aliased. + let field_changed_at = unsafe { (&(*data).revisions)[relative_tracked_index].load() }; + let durability = unsafe { (*data).durability }; + zalsa_local.report_tracked_read_simple( + DatabaseKeyIndex::new(field_ingredient_index, id), + durability, + field_changed_at, + ); + + fields + } + + /// Access an untracked field. + #[inline] + pub fn untracked_field(&self, s: C::Struct<'db>) -> &'db C::Fields<'db> { + let data = self.slot_raw(AsId::as_id(&s)); + + // IDs that are reused increment their generation, invalidating dependent queries directly. + // SAFETY: `data` belongs to the bound ingredient and database. + unsafe { + self.ingredient() + .lock_fields(data, self.zalsa().current_revision()) + } + } +} + /// Defines the identity of a tracked struct. /// This is the key to a hashmap that is (initially) /// stored in the [`ActiveQuery`](`crate::active_query::ActiveQuery`) @@ -871,56 +918,6 @@ where unsafe { self.lock_fields(data, zalsa.current_revision()) } } - /// Access to this tracked field. - /// - /// Note that this function returns the entire tuple of value fields. - /// The caller is responsible for selecting the appropriate element. - pub fn tracked_field<'db>( - &'db self, - zalsa: &'db Zalsa, - zalsa_local: &'db ZalsaLocal, - s: C::Struct<'db>, - relative_tracked_index: usize, - ) -> &'db C::Fields<'db> { - let id = AsId::as_id(&s); - let field_ingredient_index = self.ingredient_index.successor(relative_tracked_index); - let data = Self::data_raw(zalsa.table(), id); - - // SAFETY: `data` is a valid pointer acquired from the table. - let fields = unsafe { self.lock_fields(data, zalsa.current_revision()) }; - - // SAFETY: We just acquired the read lock (in `Self::fields`), so accessing `revisions` will not be able to alias - let field_changed_at = unsafe { (&(*data).revisions)[relative_tracked_index].load() }; - - // SAFETY: We just acquired the read lock (in `Self::fields`), so accessing `durability` will not be able to alias - zalsa_local.report_tracked_read_simple( - DatabaseKeyIndex::new(field_ingredient_index, id), - unsafe { (*data).durability }, - field_changed_at, - ); - - fields - } - - /// Access to this untracked field. - /// - /// Note that this function returns the entire tuple of value fields. - /// The caller is responsible for selecting the appropriate element. - pub fn untracked_field<'db>( - &'db self, - zalsa: &'db Zalsa, - s: C::Struct<'db>, - ) -> &'db C::Fields<'db> { - let id = AsId::as_id(&s); - let data = Self::data_raw(zalsa.table(), id); - - // Note that we do not need to add a dependency on the tracked struct - // as IDs that are reused increment their generation, invalidating any - // dependent queries directly. - // SAFETY: `data` is a valid pointer acquired from the table. - unsafe { self.lock_fields(data, zalsa.current_revision()) } - } - /// Returns all data corresponding to the tracked struct. pub fn entries<'db>(&'db self, zalsa: &'db Zalsa) -> impl Iterator> { zalsa @@ -1171,6 +1168,11 @@ where } } +// SAFETY: `IngredientImpl` only allocates pages containing `Value`. +unsafe impl crate::ingredient::TableIngredient for IngredientImpl { + type Slot = Value; +} + // SAFETY: `Value` is our private type branded over the unique configuration `C`. unsafe impl Slot for Value where diff --git a/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr b/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr index f392d9340..c94325aad 100644 --- a/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr +++ b/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr @@ -14,12 +14,12 @@ help: the trait `salsa::plumbing::TrackedStructInDb` is implemented for `MyTrack | 10 | #[salsa::tracked] | ^^^^^^^^^^^^^^^^^ -note: required by a bound in `salsa::function::specify::>::specify_and_record` +note: required by a bound in `salsa::function::specify::>::specify_and_record` --> src/function/specify.rs | - | pub fn specify_and_record<'db>(&'db self, db: &'db C::DbView, key: Id, value: C::Output<'db>) + | pub fn specify_and_record(&self, key: Id, value: C::Output<'db>) | ------------------ required by a bound in this associated function | where | C::Input<'db>: TrackedStructInDb, - | ^^^^^^^^^^^^^^^^^ required by this bound in `salsa::function::specify::>::specify_and_record` + | ^^^^^^^^^^^^^^^^^ required by this bound in `salsa::function::specify::>::specify_and_record` = note: this error originates in the macro `salsa::plumbing::setup_tracked_fn` which comes from the expansion of the attribute macro `salsa::tracked` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr b/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr index d9f07ab82..70dbcc3fe 100644 --- a/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr +++ b/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr @@ -14,12 +14,12 @@ help: the trait `salsa::plumbing::TrackedStructInDb` is implemented for `MyTrack | 10 | #[salsa::tracked] | ^^^^^^^^^^^^^^^^^ -note: required by a bound in `salsa::function::specify::>::specify_and_record` +note: required by a bound in `salsa::function::specify::>::specify_and_record` --> src/function/specify.rs | - | pub fn specify_and_record<'db>(&'db self, db: &'db C::DbView, key: Id, value: C::Output<'db>) + | pub fn specify_and_record(&self, key: Id, value: C::Output<'db>) | ------------------ required by a bound in this associated function | where | C::Input<'db>: TrackedStructInDb, - | ^^^^^^^^^^^^^^^^^ required by this bound in `salsa::function::specify::>::specify_and_record` + | ^^^^^^^^^^^^^^^^^ required by this bound in `salsa::function::specify::>::specify_and_record` = note: this error originates in the macro `salsa::plumbing::setup_tracked_fn` which comes from the expansion of the attribute macro `salsa::tracked` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/field_access_in_db.rs b/tests/field_access_in_db.rs new file mode 100644 index 000000000..6c263ffea --- /dev/null +++ b/tests/field_access_in_db.rs @@ -0,0 +1,24 @@ +#[salsa::input] +struct FirstInput { + #[returns(copy)] + value: u32, +} + +#[salsa::input] +struct SecondInput { + #[returns(copy)] + value: u32, +} + +#[test] +#[should_panic(expected = "page belongs to ingredient")] +fn field_access_rejects_id_owned_by_another_ingredient() { + let db = salsa::DatabaseImpl::default(); + let _first = FirstInput::new(&db, 1); + let second = SecondInput::new(&db, 2); + + let forged = + ::from_id(salsa::plumbing::AsId::as_id(&second)); + + forged.value(&db); +}