diff --git a/components/salsa-2022-macros/src/accumulator.rs b/components/salsa-2022-macros/src/accumulator.rs index 8b1eddae..cfc32c9e 100644 --- a/components/salsa-2022-macros/src/accumulator.rs +++ b/components/salsa-2022-macros/src/accumulator.rs @@ -116,7 +116,7 @@ fn ingredients_for_impl(args: &Args, struct_ty: &syn::Type, data_ty: &syn::Type) where DB: salsa::DbWithJar + salsa::storage::JarFromJars, { - let index = routes.push_mut( + let index = routes.push( |jars| { let jar = >::jar_from_jars(jars); <_ as salsa::storage::HasIngredientsFor>::ingredient(jar) diff --git a/components/salsa-2022-macros/src/input.rs b/components/salsa-2022-macros/src/input.rs index fbcf1f30..a09794c9 100644 --- a/components/salsa-2022-macros/src/input.rs +++ b/components/salsa-2022-macros/src/input.rs @@ -159,6 +159,11 @@ impl InputStruct { let ingredients = <_ as salsa::storage::HasIngredientsFor>::ingredient(jar); &ingredients.#all_field_indices }, + |jars| { + let jar = >::jar_from_jars_mut(jars); + let ingredients = <_ as salsa::storage::HasIngredientsFor>::ingredient_mut(jar); + &mut ingredients.#all_field_indices + }, ); salsa::input_field::InputFieldIngredient::new(index) }, @@ -170,6 +175,11 @@ impl InputStruct { let ingredients = <_ as salsa::storage::HasIngredientsFor>::ingredient(jar); &ingredients.#input_index }, + |jars| { + let jar = >::jar_from_jars_mut(jars); + let ingredients = <_ as salsa::storage::HasIngredientsFor>::ingredient_mut(jar); + &mut ingredients.#input_index + }, ); salsa::input::InputIngredient::new(index) }, diff --git a/components/salsa-2022-macros/src/interned.rs b/components/salsa-2022-macros/src/interned.rs index 3ca076f7..2d5dc0ff 100644 --- a/components/salsa-2022-macros/src/interned.rs +++ b/components/salsa-2022-macros/src/interned.rs @@ -136,6 +136,10 @@ impl InternedStruct { let jar = >::jar_from_jars(jars); <_ as salsa::storage::HasIngredientsFor>::ingredient(jar) }, + |jars| { + let jar = >::jar_from_jars_mut(jars); + <_ as salsa::storage::HasIngredientsFor>::ingredient_mut(jar) + }, ); salsa::interned::InternedIngredient::new(index) } diff --git a/components/salsa-2022-macros/src/tracked_fn.rs b/components/salsa-2022-macros/src/tracked_fn.rs index fb031cb2..8c0884e1 100644 --- a/components/salsa-2022-macros/src/tracked_fn.rs +++ b/components/salsa-2022-macros/src/tracked_fn.rs @@ -205,12 +205,20 @@ fn ingredients_for_impl( let intern_map: syn::Expr = if requires_interning(item_fn) { parse_quote! { { - let index = routes.push(|jars| { - let jar = >::jar_from_jars(jars); - let ingredients = - <_ as salsa::storage::HasIngredientsFor>::ingredient(jar); - &ingredients.intern_map - }); + let index = routes.push( + |jars| { + let jar = >::jar_from_jars(jars); + let ingredients = + <_ as salsa::storage::HasIngredientsFor>::ingredient(jar); + &ingredients.intern_map + }, + |jars| { + let jar = >::jar_from_jars_mut(jars); + let ingredients = + <_ as salsa::storage::HasIngredientsFor>::ingredient_mut(jar); + &mut ingredients.intern_map + } + ); salsa::interned::InternedIngredient::new(index) } } @@ -233,12 +241,19 @@ fn ingredients_for_impl( intern_map: #intern_map, function: { - let index = routes.push(|jars| { - let jar = >::jar_from_jars(jars); - let ingredients = - <_ as salsa::storage::HasIngredientsFor>::ingredient(jar); - &ingredients.function - }); + let index = routes.push( + |jars| { + let jar = >::jar_from_jars(jars); + let ingredients = + <_ as salsa::storage::HasIngredientsFor>::ingredient(jar); + &ingredients.function + }, + |jars| { + let jar = >::jar_from_jars_mut(jars); + let ingredients = + <_ as salsa::storage::HasIngredientsFor>::ingredient_mut(jar); + &mut ingredients.function + }); salsa::function::FunctionIngredient::new(index) }, } diff --git a/components/salsa-2022-macros/src/tracked_struct.rs b/components/salsa-2022-macros/src/tracked_struct.rs index 7d87abc5..b689ff6d 100644 --- a/components/salsa-2022-macros/src/tracked_struct.rs +++ b/components/salsa-2022-macros/src/tracked_struct.rs @@ -182,12 +182,17 @@ impl TrackedStruct { let ingredients = <_ as salsa::storage::HasIngredientsFor>::ingredient(jar); &ingredients.#value_field_indices }, + |jars| { + let jar = >::jar_from_jars_mut(jars); + let ingredients = <_ as salsa::storage::HasIngredientsFor>::ingredient_mut(jar); + &mut ingredients.#value_field_indices + }, ); salsa::function::FunctionIngredient::new(index) }, )* { - let index = routes.push_mut( + let index = routes.push( |jars| { let jar = >::jar_from_jars(jars); let ingredients = <_ as salsa::storage::HasIngredientsFor>::ingredient(jar); diff --git a/components/salsa-2022/src/accumulator.rs b/components/salsa-2022/src/accumulator.rs index 6b326e1c..77a4b8f3 100644 --- a/components/salsa-2022/src/accumulator.rs +++ b/components/salsa-2022/src/accumulator.rs @@ -1,7 +1,7 @@ use crate::{ cycle::CycleRecoveryStrategy, hash::FxDashMap, - ingredient::{Ingredient, MutIngredient}, + ingredient::{Ingredient, IngredientRequiresReset}, key::DependencyIndex, runtime::{local_state::QueryOrigin, StampedValue}, storage::HasJar, @@ -96,14 +96,15 @@ where // FIXME drop((executor, stale_output_key)); } + + fn reset_for_new_revision(&mut self) { + panic!("unexpected reset on accumulator") + } } -impl MutIngredient for AccumulatorIngredient +impl IngredientRequiresReset for AccumulatorIngredient where Data: Clone, { - fn reset_for_new_revision(&mut self) { - // FIXME: We could certainly drop things here if we knew which ones - // to drop. There's a fixed point algorithm we could be doing. - } + const RESET_ON_NEW_REVISION: bool = false; } diff --git a/components/salsa-2022/src/function.rs b/components/salsa-2022/src/function.rs index d889bb2e..e3cef148 100644 --- a/components/salsa-2022/src/function.rs +++ b/components/salsa-2022/src/function.rs @@ -1,11 +1,11 @@ use std::sync::Arc; use arc_swap::ArcSwap; -use crossbeam::{atomic::AtomicCell, epoch::Atomic, queue::SegQueue}; +use crossbeam::{atomic::AtomicCell, queue::SegQueue}; use crate::{ cycle::CycleRecoveryStrategy, - ingredient::MutIngredient, + ingredient::IngredientRequiresReset, jar::Jar, key::{DatabaseKeyIndex, DependencyIndex}, runtime::local_state::QueryOrigin, @@ -240,14 +240,15 @@ where // but not in rev 2. We don't do anything in this case, we just leave the (now stale) memo. // Since its `verified_at` field has not changed, it will be considered dirty if it is invoked. } -} -impl MutIngredient for FunctionIngredient -where - DB: ?Sized + DbWithJar, - C: Configuration, -{ fn reset_for_new_revision(&mut self) { std::mem::take(&mut self.deleted_entries); } } + +impl IngredientRequiresReset for FunctionIngredient +where + C: Configuration, +{ + const RESET_ON_NEW_REVISION: bool = true; +} diff --git a/components/salsa-2022/src/function/diff_outputs.rs b/components/salsa-2022/src/function/diff_outputs.rs index db456636..617371ba 100644 --- a/components/salsa-2022/src/function/diff_outputs.rs +++ b/components/salsa-2022/src/function/diff_outputs.rs @@ -1,6 +1,6 @@ use crate::{ - key::DependencyIndex, runtime::local_state::QueryRevisions, storage::HasJarsDyn, Database, - DatabaseKeyIndex, Event, EventKind, + runtime::local_state::QueryRevisions, storage::HasJarsDyn, Database, DatabaseKeyIndex, Event, + EventKind, }; use super::{memo::Memo, Configuration, DynDb, FunctionIngredient}; diff --git a/components/salsa-2022/src/ingredient.rs b/components/salsa-2022/src/ingredient.rs index 04f376da..57e0d876 100644 --- a/components/salsa-2022/src/ingredient.rs +++ b/components/salsa-2022/src/ingredient.rs @@ -33,13 +33,22 @@ pub trait Ingredient { /// /// This hook is used to clear out the stale value so others cannot read it. fn remove_stale_output(&self, db: &DB, executor: DatabaseKeyIndex, stale_output_key: Id); -} -/// Optional trait for ingredients that wish to be notified when new revisions are -/// about to occur. If ingredients wish to receive these method calls, -/// they need to indicate that by invoking [`Routes::push_mut`] during initialization. -pub trait MutIngredient: Ingredient { - /// Invoked when a new revision is about to start. This gives ingredients - /// a chance to flush data and so forth. + /// Invoked when a new revision is about to start. + /// This moment is important because it means that we have an `&mut`-reference to the database, + /// and hence any pre-existing `&`-references must have expired. + /// Many ingredients, given an `&'db`-reference to the database, + /// use unsafe code to return `&'db`-references to internal values. + /// The backing memory for those values can only be freed once an `&mut`-reference to the database is created. + /// + /// **Important:** to actually receive resets, the ingredient must set + /// [`IngredientRequiresReset::RESET_ON_NEW_REVISION`] to true. fn reset_for_new_revision(&mut self); } + +/// Defines a const indicating if an ingredient needs to be reset each round. +/// This const probably *should* be a member of `Ingredient` trait but then `Ingredient` would not be dyn-safe. +pub trait IngredientRequiresReset { + /// If this is true, then `reset_for_new_revision` will be called every new revision. + const RESET_ON_NEW_REVISION: bool; +} diff --git a/components/salsa-2022/src/input.rs b/components/salsa-2022/src/input.rs index f4529fbc..87551557 100644 --- a/components/salsa-2022/src/input.rs +++ b/components/salsa-2022/src/input.rs @@ -1,6 +1,6 @@ use crate::{ cycle::CycleRecoveryStrategy, - ingredient::Ingredient, + ingredient::{Ingredient, IngredientRequiresReset}, key::{DatabaseKeyIndex, DependencyIndex}, runtime::{local_state::QueryOrigin, Runtime}, AsId, IngredientIndex, Revision, @@ -80,4 +80,15 @@ where executor, stale_output_key ); } + + fn reset_for_new_revision(&mut self) { + panic!("unexpected call to `reset_for_new_revision`") + } +} + +impl IngredientRequiresReset for InputIngredient +where + Id: InputId, +{ + const RESET_ON_NEW_REVISION: bool = false; } diff --git a/components/salsa-2022/src/interned.rs b/components/salsa-2022/src/interned.rs index 13a6f7d8..87e85336 100644 --- a/components/salsa-2022/src/interned.rs +++ b/components/salsa-2022/src/interned.rs @@ -5,6 +5,7 @@ use std::marker::PhantomData; use crate::durability::Durability; use crate::id::AsId; +use crate::ingredient::IngredientRequiresReset; use crate::key::DependencyIndex; use crate::runtime::local_state::QueryOrigin; use crate::runtime::Runtime; @@ -217,6 +218,21 @@ where executor, stale_output_key ); } + + fn reset_for_new_revision(&mut self) { + // Interned ingredients do not, normally, get deleted except when they are "reset" en masse. + // There ARE methods (e.g., `clear_deleted_entries` and `remove`) for deleting individual + // items, but those are only used for tracked struct ingredients. + panic!("unexpected call to `reset_for_new_revision`") + } +} + +impl IngredientRequiresReset for InternedIngredient +where + Id: InternedId, + Data: InternedData, +{ + const RESET_ON_NEW_REVISION: bool = false; } pub struct IdentityInterner { diff --git a/components/salsa-2022/src/routes.rs b/components/salsa-2022/src/routes.rs index 308f7217..02699f13 100644 --- a/components/salsa-2022/src/routes.rs +++ b/components/salsa-2022/src/routes.rs @@ -1,7 +1,6 @@ -use super::{ - ingredient::{Ingredient, MutIngredient}, - storage::HasJars, -}; +use crate::ingredient::IngredientRequiresReset; + +use super::{ingredient::Ingredient, storage::HasJars}; /// An ingredient index identifies a particular [`Ingredient`] in the database. /// The database contains a number of jars, and each jar contains a number of ingredients. @@ -39,7 +38,7 @@ pub type DynRoute = dyn Fn(&DB::Jars) -> (&dyn Ingredient) + Se #[allow(type_alias_bounds)] #[allow(unused_parens)] pub type DynMutRoute = - dyn Fn(&mut DB::Jars) -> (&mut dyn MutIngredient) + Send + Sync; + dyn Fn(&mut DB::Jars) -> (&mut dyn Ingredient) + Send + Sync; /// The "routes" structure is used to navigate the database. /// The database contains a number of jars, and each jar contains a number of ingredients. @@ -52,13 +51,10 @@ pub struct Routes { /// Vector indexed by ingredient index. Yields the `DynRoute`, /// a function which can be applied to the `DB::Jars` to yield /// the `dyn Ingredient. - routes: Vec>>, + routes: Vec<(Box>, Box>)>, - /// Vector if "mut routes". This vector is used to give callbacks - /// when new revisions are trigged. It is not indexed by ingredient - /// index as not every ingredient needs a callback; instead, you - /// just iterate over it and invoke each fn to get a `MutIngredient`. - mut_routes: Vec>>, + /// Indices of routes which need a 'reset' call. + needs_reset: Vec, } impl Routes { @@ -66,7 +62,7 @@ impl Routes { pub(super) fn new() -> Self { Routes { routes: vec![], - mut_routes: vec![], + needs_reset: vec![], } } @@ -77,52 +73,53 @@ impl Routes { /// /// # Parameters /// + /// * `requires_reset` -- if true, the [`Ingredient::reset_for_new_revision`] method will be called on this ingredient + /// at each new revision. See that method for more information. /// * `route` -- a closure which, given a database, will identify the ingredient. /// This closure will be invoked to dispatch calls to `maybe_changed_after`. - /// * `mut_route` -- an optional closure which identifies the ingredient in a mut + /// * `mut_route` -- a closure which identifies the ingredient in a mut /// database. - pub fn push( + pub fn push( &mut self, - route: impl (Fn(&DB::Jars) -> &dyn Ingredient) + Send + Sync + 'static, - ) -> IngredientIndex { + route: impl (Fn(&DB::Jars) -> &I) + Send + Sync + 'static, + mut_route: impl (Fn(&mut DB::Jars) -> &mut I) + Send + Sync + 'static, + ) -> IngredientIndex + where + I: Ingredient + IngredientRequiresReset + 'static, + { let len = self.routes.len(); - self.routes.push(Box::new(route)); + self.routes.push(( + Box::new(move |jars| route(jars)), + Box::new(move |jars| mut_route(jars)), + )); let index = IngredientIndex::from(len); - index - } - /// As [`Self::push`] but for an ingredient that wants a callback whenever - /// a new revision is published. - /// Many ingredients will, given an `&'db dyn Db` reference, - /// return a `&'db V` reference to one of their values. - /// This forces those values to live as long as the `&dyn Db` is valid. - /// This callback is invoked when a new revision begins. - /// It allows those values that were accessed to be freed. - /// We know it is safe to free them because, when a new revision starts, - /// we have an `&mut dyn Db`, and hence the `&dyn Db` lifetime must have ended. - pub fn push_mut( - &mut self, - route: impl (Fn(&DB::Jars) -> &dyn Ingredient) + Send + Sync + 'static, - mut_route: impl (Fn(&mut DB::Jars) -> &mut dyn MutIngredient) + Send + Sync + 'static, - ) -> IngredientIndex { - let index = self.push(route); - self.mut_routes.push(Box::new(mut_route)); + if I::RESET_ON_NEW_REVISION { + self.needs_reset.push(index); + } + index } /// Given an ingredient index, return the "route" - /// (a function that, given a `Jars`, returns the ingredient). + /// (a function that, given a `&Jars`, returns the ingredient). pub fn route(&self, index: IngredientIndex) -> &dyn Fn(&DB::Jars) -> &dyn Ingredient { - &self.routes[index.as_usize()] + &self.routes[index.as_usize()].0 } - /// Returns the "mutable routes" for the purposes of giving callbacks when a new revision is published. - /// Not all ingredients need callbacks, so this returns an iterator of just those that do. - pub fn mut_routes( + /// Given an ingredient index, return the "mut route" + /// (a function that, given an `&mut Jars`, returns the ingredient). + pub fn route_mut( &self, - ) -> impl Iterator &mut dyn MutIngredient> + '_ { - self.mut_routes - .iter() - .map(|b| &**b as &dyn Fn(&mut DB::Jars) -> &mut dyn MutIngredient) + index: IngredientIndex, + ) -> &dyn Fn(&mut DB::Jars) -> &mut dyn Ingredient { + &self.routes[index.as_usize()].1 + } + + /// Returns the mut routes for ingredients that need to be reset at the start of each revision. + pub fn reset_routes( + &self, + ) -> impl Iterator &mut dyn Ingredient> + '_ { + self.needs_reset.iter().map(|&index| self.route_mut(index)) } } diff --git a/components/salsa-2022/src/storage.rs b/components/salsa-2022/src/storage.rs index dff46b4b..fc13474a 100644 --- a/components/salsa-2022/src/storage.rs +++ b/components/salsa-2022/src/storage.rs @@ -94,7 +94,7 @@ where let routes = self.routes.clone(); let shared = Arc::get_mut(&mut self.shared).unwrap(); - for route in routes.mut_routes() { + for route in routes.reset_routes() { route(&mut shared.jars).reset_for_new_revision(); } diff --git a/components/salsa-2022/src/tracked_struct.rs b/components/salsa-2022/src/tracked_struct.rs index 025c11e5..489da3d4 100644 --- a/components/salsa-2022/src/tracked_struct.rs +++ b/components/salsa-2022/src/tracked_struct.rs @@ -1,12 +1,7 @@ -use std::sync::Arc; - -use arc_swap::{ArcSwap, ArcSwapOption, AsRaw}; - use crate::{ cycle::CycleRecoveryStrategy, - ingredient::{Ingredient, MutIngredient}, + ingredient::{Ingredient, IngredientRequiresReset}, ingredient_list::IngredientList, - input, interned::{InternedData, InternedId, InternedIngredient}, key::{DatabaseKeyIndex, DependencyIndex}, runtime::{local_state::QueryOrigin, Runtime}, @@ -155,14 +150,16 @@ where // FIXME -- we can delete this entity drop((executor, key)); } -} -impl MutIngredient for TrackedStructIngredient -where - Id: TrackedStructId, - Data: TrackedStructData, -{ fn reset_for_new_revision(&mut self) { self.interned.clear_deleted_indices(); } } + +impl IngredientRequiresReset for TrackedStructIngredient +where + Id: TrackedStructId, + Data: TrackedStructData, +{ + const RESET_ON_NEW_REVISION: bool = true; +}