From 97fc6a0920ae8c265a3f2a6bf522d5c661d8087d Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 6 May 2024 06:20:35 -0400 Subject: [PATCH] rework interning to have a Configuration This will permit GATs so that interned values can carry lifetimes. --- components/salsa-2022-macros/src/interned.rs | 25 ++++++++-- .../salsa-2022-macros/src/tracked_fn.rs | 50 +++++++++++++------ components/salsa-2022/src/interned.rs | 50 ++++++++++++------- components/salsa-2022/src/tracked_struct.rs | 4 ++ 4 files changed, 91 insertions(+), 38 deletions(-) diff --git a/components/salsa-2022-macros/src/interned.rs b/components/salsa-2022-macros/src/interned.rs index ecfcdbc6..2ce7594a 100644 --- a/components/salsa-2022-macros/src/interned.rs +++ b/components/salsa-2022-macros/src/interned.rs @@ -55,8 +55,10 @@ impl InternedStruct { fn generate_interned(&self) -> syn::Result { self.validate_interned()?; let id_struct = self.the_struct_id(); + let config_struct = self.config_struct(); let data_struct = self.data_struct(); - let ingredients_for_impl = self.ingredients_for_impl(); + let configuration_impl = self.configuration_impl(&data_struct.ident, &config_struct.ident); + let ingredients_for_impl = self.ingredients_for_impl(&config_struct.ident); let as_id_impl = self.as_id_impl(); let named_fields_impl = self.inherent_impl_for_named_fields(); let salsa_struct_in_db_impl = self.salsa_struct_in_db_impl(); @@ -64,6 +66,8 @@ impl InternedStruct { Ok(quote! { #id_struct + #config_struct + #configuration_impl #data_struct #ingredients_for_impl #as_id_impl @@ -113,6 +117,20 @@ impl InternedStruct { } } + fn configuration_impl( + &self, + data_struct: &syn::Ident, + config_struct: &syn::Ident, + ) -> syn::ItemImpl { + parse_quote_spanned!( + config_struct.span() => + + impl salsa::interned::Configuration for #config_struct { + type Data = #data_struct; + } + ) + } + /// If this is an interned struct, then generate methods to access each field, /// as well as a `new` method. fn inherent_impl_for_named_fields(&self) -> syn::ItemImpl { @@ -186,15 +204,14 @@ impl InternedStruct { /// Generates an impl of `salsa::storage::IngredientsFor`. /// /// For a memoized type, the only ingredient is an `InternedIngredient`. - fn ingredients_for_impl(&self) -> syn::ItemImpl { + fn ingredients_for_impl(&self, config_struct: &syn::Ident) -> syn::ItemImpl { let id_ident = self.the_ident(); let debug_name = crate::literal(id_ident); let jar_ty = self.jar_ty(); - let data_ident = self.data_ident(); parse_quote! { impl salsa::storage::IngredientsFor for #id_ident { type Jar = #jar_ty; - type Ingredients = salsa::interned::InternedIngredient<#data_ident>; + type Ingredients = salsa::interned::InternedIngredient<#config_struct>; fn create_ingredients( routes: &mut salsa::routes::Routes, diff --git a/components/salsa-2022-macros/src/tracked_fn.rs b/components/salsa-2022-macros/src/tracked_fn.rs index cfd3d9aa..8cee8aab 100644 --- a/components/salsa-2022-macros/src/tracked_fn.rs +++ b/components/salsa-2022-macros/src/tracked_fn.rs @@ -293,6 +293,7 @@ fn fn_struct(args: &FnArgs, item_fn: &syn::ItemFn) -> syn::Result<(syn::Type, To let struct_item_ident = &struct_item.ident; let config_ty: syn::Type = parse_quote!(#struct_item_ident); let configuration_impl = configuration.to_impl(&config_ty); + let interned_configuration_impl = interned_configuration_impl(item_fn, &config_ty); let ingredients_for_impl = ingredients_for_impl(args, item_fn, &config_ty); let item_impl = setter_impl(args, item_fn, &config_ty)?; @@ -301,22 +302,27 @@ fn fn_struct(args: &FnArgs, item_fn: &syn::ItemFn) -> syn::Result<(syn::Type, To quote! { #struct_item #configuration_impl + #interned_configuration_impl #ingredients_for_impl #item_impl }, )) } -/// Returns the key type for this tracked function. -/// This is a tuple of all the argument types (apart from the database). -fn key_tuple_ty(item_fn: &syn::ItemFn) -> syn::Type { +fn interned_configuration_impl(item_fn: &syn::ItemFn, config_ty: &syn::Type) -> syn::ItemImpl { let arg_tys = item_fn.sig.inputs.iter().skip(1).map(|arg| match arg { syn::FnArg::Receiver(_) => unreachable!(), syn::FnArg::Typed(pat_ty) => pat_ty.ty.clone(), }); + let intern_data_ty: syn::Type = parse_quote!( + (#(#arg_tys),*) + ); + parse_quote!( - (#(#arg_tys,)*) + impl salsa::interned::Configuration for #config_ty { + type Data = #intern_data_ty; + } ) } @@ -324,17 +330,15 @@ fn configuration_struct(item_fn: &syn::ItemFn) -> syn::ItemStruct { let fn_name = item_fn.sig.ident.clone(); let visibility = &item_fn.vis; - let salsa_struct_ty = salsa_struct_ty(item_fn); let intern_map: syn::Type = match function_type(item_fn) { FunctionType::Constant => { - parse_quote! { salsa::interned::IdentityInterner<()> } + parse_quote! { salsa::interned::IdentityInterner } } FunctionType::SalsaStruct => { - parse_quote! { salsa::interned::IdentityInterner<#salsa_struct_ty> } + parse_quote! { salsa::interned::IdentityInterner } } FunctionType::RequiresInterning => { - let key_ty = key_tuple_ty(item_fn); - parse_quote! { salsa::interned::InternedIngredient<#key_ty> } + parse_quote! { salsa::interned::InternedIngredient } } }; @@ -389,7 +393,24 @@ fn fn_configuration(args: &FnArgs, item_fn: &syn::ItemFn) -> Configuration { let fn_ty = item_fn.sig.ident.clone(); - let indices = (0..item_fn.sig.inputs.len() - 1).map(Literal::usize_unsuffixed); + // During recovery or execution, we are invoked with a `salsa::Id` + // that represents the interned value. We convert it back to a key + // which is either a single value (if there is one argument) + // or a tuple of values (if multiple arugments). `key_var` is the variable + // name that will store this result, and `key_splat` is a set of tokens + // that will convert it into one or multiple arguments (e.g., `key_var` if there + // is one argument or `key_var.0, key_var.1` if 2) that can be pasted into a function call. + let key_var = syn::Ident::new("__key", item_fn.span()); + let key_fields = item_fn.sig.inputs.len() - 1; + let key_splat = if key_fields == 1 { + quote!(#key_var) + } else { + let indices = (0..key_fields) + .map(Literal::usize_unsuffixed) + .collect::>(); + quote!(#(__key.#indices),*) + }; + let (cycle_strategy, recover_fn) = if let Some(recovery_fn) = &args.recovery_fn { // Create the `recover_from_cycle` function, which (a) maps from the interned id to the actual // keys and then (b) invokes the recover function itself. @@ -400,8 +421,8 @@ 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 = __ingredients.intern_map.data(__runtime, __id).clone(); - #recovery_fn(__db, __cycle, #(__key.#indices),*) + let #key_var = __ingredients.intern_map.data(__runtime, __id).clone(); + #recovery_fn(__db, __cycle, #key_splat) } }; (cycle_strategy, cycle_fullback) @@ -425,7 +446,6 @@ fn fn_configuration(args: &FnArgs, item_fn: &syn::ItemFn) -> Configuration { // Create the `execute` function, which (a) maps from the interned id to the actual // keys and then (b) invokes the function itself (which we embed within). - let indices = (0..item_fn.sig.inputs.len() - 1).map(Literal::usize_unsuffixed); let execute_fn = parse_quote! { fn execute<'db>(__db: &'db salsa::function::DynDb<'db, Self>, __id: salsa::Id) -> Self::Value<'db> { #inner_fn @@ -433,8 +453,8 @@ 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 = __ingredients.intern_map.data(__runtime, __id).clone(); - #inner_fn_name(__db, #(__key.#indices),*) + let #key_var = __ingredients.intern_map.data(__runtime, __id).clone(); + #inner_fn_name(__db, #key_splat) } }; diff --git a/components/salsa-2022/src/interned.rs b/components/salsa-2022/src/interned.rs index 7ee8493a..c7639aeb 100644 --- a/components/salsa-2022/src/interned.rs +++ b/components/salsa-2022/src/interned.rs @@ -18,25 +18,29 @@ use super::ingredient::Ingredient; use super::routes::IngredientIndex; use super::Revision; +pub trait Configuration { + type Data: InternedData; +} + pub trait InternedData: Sized + Eq + Hash + Clone {} impl InternedData for T {} /// The interned ingredient has the job of hashing values of type `Data` to produce an `Id`. /// It used to store interned structs but also to store the id fields of a tracked struct. /// Interned values endure until they are explicitly removed in some way. -pub struct InternedIngredient { +pub struct InternedIngredient { /// Index of this ingredient in the database (used to construct database-ids, etc). ingredient_index: IngredientIndex, /// Maps from data to the existing interned id for that data. /// /// Deadlock requirement: We access `value_map` while holding lock on `key_map`, but not vice versa. - key_map: FxDashMap, + key_map: FxDashMap, /// 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,14 +56,14 @@ pub struct InternedIngredient { /// references to that data floating about that are tied to the lifetime of some /// `&db` reference. This queue itself is not freed until we have an `&mut db` reference, /// guaranteeing that there are no more references to it. - deleted_entries: SegQueue>, + deleted_entries: SegQueue>, debug_name: &'static str, } -impl InternedIngredient +impl InternedIngredient where - Data: InternedData, + C: Configuration, { pub fn new(ingredient_index: IngredientIndex, debug_name: &'static str) -> Self { Self { @@ -78,7 +82,7 @@ where /// * `id` is the interned id /// * `b` is a boolean, `true` indicates this fn call added `data` to the interning table; /// `false` indicates it was already present - pub(crate) fn intern_full(&self, runtime: &Runtime, data: Data) -> (Id, bool) { + pub(crate) fn intern_full(&self, runtime: &Runtime, data: C::Data) -> (Id, bool) { runtime.report_tracked_read( DependencyIndex::for_table(self.ingredient_index), Durability::MAX, @@ -109,7 +113,7 @@ where } } - pub fn intern(&self, runtime: &Runtime, data: Data) -> Id { + pub fn intern(&self, runtime: &Runtime, data: C::Data) -> Id { self.intern_full(runtime, data).0 } @@ -125,7 +129,7 @@ where } #[track_caller] - pub fn data<'db>(&'db self, runtime: &'db Runtime, id: Id) -> &'db Data { + pub fn data<'db>(&'db self, runtime: &'db Runtime, id: Id) -> &'db C::Data { runtime.report_tracked_read( DependencyIndex::for_table(self.ingredient_index), Durability::MAX, @@ -187,9 +191,9 @@ where } } -impl Ingredient for InternedIngredient +impl Ingredient for InternedIngredient where - Data: InternedData, + C: Configuration, { fn ingredient_index(&self) -> IngredientIndex { self.ingredient_index @@ -247,28 +251,36 @@ where } } -impl IngredientRequiresReset for InternedIngredient +impl IngredientRequiresReset for InternedIngredient where - Data: InternedData, + C: Configuration, { const RESET_ON_NEW_REVISION: bool = false; } -pub struct IdentityInterner { - data: PhantomData, +pub struct IdentityInterner +where + C: Configuration, + C::Data: AsId, +{ + data: PhantomData, } -impl IdentityInterner { +impl IdentityInterner +where + C: Configuration, + C::Data: AsId, +{ #[allow(clippy::new_without_default)] pub fn new() -> Self { IdentityInterner { data: PhantomData } } - pub fn intern(&self, _runtime: &Runtime, id: Id) -> crate::Id { + pub fn intern(&self, _runtime: &Runtime, id: C::Data) -> crate::Id { id.as_id() } - pub fn data(&self, _runtime: &Runtime, id: crate::Id) -> (Id,) { - (Id::from_id(id),) + pub fn data(&self, _runtime: &Runtime, id: crate::Id) -> C::Data { + ::from_id(id) } } diff --git a/components/salsa-2022/src/tracked_struct.rs b/components/salsa-2022/src/tracked_struct.rs index 38f16b68..4240a428 100644 --- a/components/salsa-2022/src/tracked_struct.rs +++ b/components/salsa-2022/src/tracked_struct.rs @@ -110,6 +110,10 @@ struct TrackedStructKey { data_hash: u64, } +impl crate::interned::Configuration for TrackedStructKey { + type Data = TrackedStructKey; +} + // ANCHOR: TrackedStructValue #[derive(Debug)] pub struct TrackedStructValue