diff --git a/components/salsa-2022-macros/src/interned.rs b/components/salsa-2022-macros/src/interned.rs index 976a8d57..b5863414 100644 --- a/components/salsa-2022-macros/src/interned.rs +++ b/components/salsa-2022-macros/src/interned.rs @@ -82,7 +82,7 @@ impl InternedStruct { fn validate_interned(&self) -> syn::Result<()> { self.disallow_id_fields("interned")?; - self.require_db_lifetime()?; + self.require_no_generics()?; Ok(()) } @@ -160,17 +160,17 @@ impl InternedStruct { if field.is_clone_field() { parse_quote_spanned! { field_get_name.span() => #field_vis fn #field_get_name(self, db: &#db_dyn_ty) -> #field_ty { - let (jar, runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(db); + let (jar, _runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(db); let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #the_ident #type_generics >>::ingredient(jar); - std::clone::Clone::clone(&ingredients.data(runtime, self.0).#field_name) + std::clone::Clone::clone(&ingredients.data(self.0).#field_name) } } } else { parse_quote_spanned! { field_get_name.span() => #field_vis fn #field_get_name<'db>(self, db: &'db #db_dyn_ty) -> &'db #field_ty { - let (jar, runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(db); + let (jar, _runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(db); let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #the_ident #type_generics >>::ingredient(jar); - &ingredients.data(runtime, self.0).#field_name + &ingredients.data(self.0).#field_name } } } @@ -190,7 +190,7 @@ impl InternedStruct { let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #the_ident #type_generics >>::ingredient(jar); Self(ingredients.intern(runtime, #data_ident { #(#field_names,)* - })) + }).salsa_id()) } }; diff --git a/components/salsa-2022-macros/src/tracked_fn.rs b/components/salsa-2022-macros/src/tracked_fn.rs index 0cdafecb..bc0ed847 100644 --- a/components/salsa-2022-macros/src/tracked_fn.rs +++ b/components/salsa-2022-macros/src/tracked_fn.rs @@ -428,7 +428,7 @@ fn fn_configuration(args: &FnArgs, item_fn: &syn::ItemFn) -> Configuration { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db); let __ingredients = <_ as salsa::storage::HasIngredientsFor<#fn_ty>>::ingredient(__jar); - let #key_var = __ingredients.intern_map.data(__runtime, __id).clone(); + let #key_var = __ingredients.intern_map.data(__id).clone(); #recovery_fn(__db, __cycle, #key_splat) } }; @@ -460,7 +460,7 @@ fn fn_configuration(args: &FnArgs, item_fn: &syn::ItemFn) -> Configuration { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db); let __ingredients = <_ as salsa::storage::HasIngredientsFor<#fn_ty>>::ingredient(__jar); - let #key_var = __ingredients.intern_map.data(__runtime, __id).clone(); + let #key_var = __ingredients.intern_map.data(__id).clone(); #inner_fn_name(__db, #key_splat) } }; @@ -652,7 +652,7 @@ fn ref_getter_fn( { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(#db_var); let __ingredients = <_ as salsa::storage::HasIngredientsFor<#config_ty>>::ingredient(__jar); - let __key = __ingredients.intern_map.intern(__runtime, (#(#arg_names),*)); + let __key = __ingredients.intern_map.intern_id(__runtime, (#(#arg_names),*)); __ingredients.function.fetch(#db_var, __key) } }; @@ -696,7 +696,7 @@ fn setter_fn( { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar_mut(#db_var); let __ingredients = <_ as salsa::storage::HasIngredientsFor<#config_ty>>::ingredient_mut(__jar); - let __key = __ingredients.intern_map.intern(__runtime, (#(#arg_names),*)); + let __key = __ingredients.intern_map.intern_id(__runtime, (#(#arg_names),*)); __ingredients.function.store(__runtime, __key, #value_arg, salsa::Durability::LOW) } }, @@ -765,7 +765,7 @@ fn specify_fn( let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(#db_var); let __ingredients = <_ as salsa::storage::HasIngredientsFor<#config_ty>>::ingredient(__jar); - let __key = __ingredients.intern_map.intern(__runtime, (#(#arg_names),*)); + let __key = __ingredients.intern_map.intern_id(__runtime, (#(#arg_names),*)); __ingredients.function.specify_and_record(#db_var, __key, #value_arg) } }, @@ -877,7 +877,7 @@ fn accumulated_fn( { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(#db_var); let __ingredients = <_ as salsa::storage::HasIngredientsFor<#config_ty>>::ingredient(__jar); - let __key = __ingredients.intern_map.intern(__runtime, (#(#arg_names),*)); + let __key = __ingredients.intern_map.intern_id(__runtime, (#(#arg_names),*)); __ingredients.function.accumulated::<__A>(#db_var, __key) } }; diff --git a/components/salsa-2022/src/interned.rs b/components/salsa-2022/src/interned.rs index b912ed86..71f9417b 100644 --- a/components/salsa-2022/src/interned.rs +++ b/components/salsa-2022/src/interned.rs @@ -7,6 +7,7 @@ use crate::durability::Durability; use crate::id::AsId; use crate::ingredient::{fmt_index, IngredientRequiresReset}; use crate::key::DependencyIndex; +use crate::plumbing::transmute_lifetime; use crate::runtime::local_state::QueryOrigin; use crate::runtime::Runtime; use crate::{DatabaseKeyIndex, Id}; @@ -38,7 +39,7 @@ pub struct InternedIngredient { /// Maps from an interned id to its data. /// /// Deadlock requirement: We access `value_map` while holding lock on `key_map`, but not vice versa. - value_map: FxDashMap>>, + value_map: FxDashMap>>, /// counter for the next id. counter: AtomicCell, @@ -52,6 +53,15 @@ pub struct InternedIngredient { debug_name: &'static str, } +/// Struct storing the interned fields. +pub struct InternedValue +where + C: Configuration, +{ + id: Id, + fields: C::Data<'static>, +} + impl InternedIngredient where C: Configuration, @@ -71,12 +81,16 @@ where unsafe { std::mem::transmute(data) } } - unsafe fn from_internal_data<'db>(&'db self, data: &C::Data<'static>) -> &'db C::Data<'db> { - unsafe { std::mem::transmute(data) } + pub fn intern_id<'db>(&'db self, runtime: &'db Runtime, data: C::Data<'db>) -> crate::Id { + self.intern(runtime, data).salsa_id() } - /// Intern data to a unique id. - pub fn intern<'db>(&'db self, runtime: &'db Runtime, data: C::Data<'db>) -> Id { + /// Intern data to a unique reference. + pub fn intern<'db>( + &'db self, + runtime: &'db Runtime, + data: C::Data<'db>, + ) -> &'db InternedValue { runtime.report_tracked_read( DependencyIndex::for_table(self.ingredient_index), Durability::MAX, @@ -86,56 +100,57 @@ where // Optimisation to only get read lock on the map if the data has already // been interned. let internal_data = unsafe { self.to_internal_data(data) }; - if let Some(id) = self.key_map.get(&internal_data) { - return *id; + if let Some(guard) = self.key_map.get(&internal_data) { + let id = *guard; + drop(guard); + return self.interned_value(id); } match self.key_map.entry(internal_data.clone()) { // Data has been interned by a racing call, use that ID instead - dashmap::mapref::entry::Entry::Occupied(entry) => *entry.get(), + dashmap::mapref::entry::Entry::Occupied(entry) => { + let id = *entry.get(); + drop(entry); + self.interned_value(id) + } // We won any races so should intern the data dashmap::mapref::entry::Entry::Vacant(entry) => { let next_id = self.counter.fetch_add(1); let next_id = Id::from_id(crate::id::Id::from_u32(next_id)); - let old_value = self.value_map.insert(next_id, Box::new(internal_data)); - assert!( - old_value.is_none(), - "next_id is guaranteed to be unique, bar overflow" - ); + let value = self + .value_map + .entry(next_id) + .or_insert(Box::new(InternedValue { + id: next_id, + fields: internal_data, + })); + // SAFETY: Items are only removed from the `value_map` with an `&mut self` reference. + let value_ref = unsafe { transmute_lifetime(self, &**value) }; + drop(value); entry.insert(next_id); - next_id + value_ref } } } + pub fn interned_value<'db>(&'db self, id: Id) -> &'db InternedValue { + let r = self.value_map.get(&id).unwrap(); + + // SAFETY: Items are only removed from the `value_map` with an `&mut self` reference. + unsafe { transmute_lifetime(self, &**r) } + } + + pub fn data<'db>(&'db self, id: Id) -> &'db C::Data<'db> { + self.interned_value(id).data() + } + pub fn reset(&mut self, revision: Revision) { assert!(revision > self.reset_at); self.reset_at = revision; self.key_map.clear(); self.value_map.clear(); } - - #[track_caller] - pub fn data<'db>(&'db self, runtime: &'db Runtime, id: Id) -> &'db C::Data<'db> { - runtime.report_tracked_read( - DependencyIndex::for_table(self.ingredient_index), - Durability::MAX, - self.reset_at, - ); - - let data = match self.value_map.get(&id) { - Some(d) => d, - None => { - panic!("no data found for id `{:?}`", id) - } - }; - - // Unsafety clause: - // - // * Values are only removed or altered when we have `&mut self` - unsafe { self.from_internal_data(&data) } - } } impl Ingredient for InternedIngredient @@ -223,11 +238,28 @@ where IdentityInterner { data: PhantomData } } - pub fn intern<'db>(&'db self, _runtime: &'db Runtime, id: C::Data<'db>) -> crate::Id { + pub fn intern_id<'db>(&'db self, _runtime: &'db Runtime, id: C::Data<'db>) -> crate::Id { id.as_id() } - pub fn data<'db>(&'db self, _runtime: &'db Runtime, id: crate::Id) -> C::Data<'db> { + pub fn data<'db>(&'db self, id: crate::Id) -> C::Data<'db> { >::from_id(id) } } + +impl InternedValue +where + C: Configuration, +{ + pub fn salsa_id(&self) -> Id { + self.id + } + + pub fn data<'db>(&'db self) -> &'db C::Data<'db> { + unsafe { self.to_self_ref(&self.fields) } + } + + unsafe fn to_self_ref<'db>(&'db self, fields: &'db C::Data<'static>) -> &'db C::Data<'db> { + unsafe { std::mem::transmute(fields) } + } +}