generalize push to always have a &mut route

We also track whether reset is required at the ingredient level.
For tracked struct fields, we were not using `push_mut`,
and I think that was an oversight.

The plan is to do a "dependent ingredient" interlinking pass
once the database is constructed.
This commit is contained in:
Niko Matsakis 2022-08-14 03:26:27 -04:00
parent 60cdce22e9
commit 66ffae1bb9
14 changed files with 161 additions and 95 deletions

View file

@ -116,7 +116,7 @@ fn ingredients_for_impl(args: &Args, struct_ty: &syn::Type, data_ty: &syn::Type)
where
DB: salsa::DbWithJar<Self::Jar> + salsa::storage::JarFromJars<Self::Jar>,
{
let index = routes.push_mut(
let index = routes.push(
|jars| {
let jar = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars(jars);
<_ as salsa::storage::HasIngredientsFor<Self>>::ingredient(jar)

View file

@ -159,6 +159,11 @@ impl InputStruct {
let ingredients = <_ as salsa::storage::HasIngredientsFor<Self>>::ingredient(jar);
&ingredients.#all_field_indices
},
|jars| {
let jar = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars_mut(jars);
let ingredients = <_ as salsa::storage::HasIngredientsFor<Self>>::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<Self>>::ingredient(jar);
&ingredients.#input_index
},
|jars| {
let jar = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars_mut(jars);
let ingredients = <_ as salsa::storage::HasIngredientsFor<Self>>::ingredient_mut(jar);
&mut ingredients.#input_index
},
);
salsa::input::InputIngredient::new(index)
},

View file

@ -136,6 +136,10 @@ impl InternedStruct {
let jar = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars(jars);
<_ as salsa::storage::HasIngredientsFor<Self>>::ingredient(jar)
},
|jars| {
let jar = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars_mut(jars);
<_ as salsa::storage::HasIngredientsFor<Self>>::ingredient_mut(jar)
},
);
salsa::interned::InternedIngredient::new(index)
}

View file

@ -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 = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars(jars);
let ingredients =
<_ as salsa::storage::HasIngredientsFor<Self::Ingredients>>::ingredient(jar);
&ingredients.intern_map
});
let index = routes.push(
|jars| {
let jar = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars(jars);
let ingredients =
<_ as salsa::storage::HasIngredientsFor<Self::Ingredients>>::ingredient(jar);
&ingredients.intern_map
},
|jars| {
let jar = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars_mut(jars);
let ingredients =
<_ as salsa::storage::HasIngredientsFor<Self::Ingredients>>::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 = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars(jars);
let ingredients =
<_ as salsa::storage::HasIngredientsFor<Self::Ingredients>>::ingredient(jar);
&ingredients.function
});
let index = routes.push(
|jars| {
let jar = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars(jars);
let ingredients =
<_ as salsa::storage::HasIngredientsFor<Self::Ingredients>>::ingredient(jar);
&ingredients.function
},
|jars| {
let jar = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars_mut(jars);
let ingredients =
<_ as salsa::storage::HasIngredientsFor<Self::Ingredients>>::ingredient_mut(jar);
&mut ingredients.function
});
salsa::function::FunctionIngredient::new(index)
},
}

View file

@ -182,12 +182,17 @@ impl TrackedStruct {
let ingredients = <_ as salsa::storage::HasIngredientsFor<Self>>::ingredient(jar);
&ingredients.#value_field_indices
},
|jars| {
let jar = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars_mut(jars);
let ingredients = <_ as salsa::storage::HasIngredientsFor<Self>>::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 = <DB as salsa::storage::JarFromJars<Self::Jar>>::jar_from_jars(jars);
let ingredients = <_ as salsa::storage::HasIngredientsFor<Self>>::ingredient(jar);

View file

@ -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<DB: ?Sized, Data> MutIngredient<DB> for AccumulatorIngredient<Data>
impl<Data> IngredientRequiresReset for AccumulatorIngredient<Data>
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;
}

View file

@ -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<DB, C> MutIngredient<DB> for FunctionIngredient<C>
where
DB: ?Sized + DbWithJar<C::Jar>,
C: Configuration,
{
fn reset_for_new_revision(&mut self) {
std::mem::take(&mut self.deleted_entries);
}
}
impl<C> IngredientRequiresReset for FunctionIngredient<C>
where
C: Configuration,
{
const RESET_ON_NEW_REVISION: bool = true;
}

View file

@ -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};

View file

@ -33,13 +33,22 @@ pub trait Ingredient<DB: ?Sized> {
///
/// 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<DB: ?Sized>: Ingredient<DB> {
/// 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;
}

View file

@ -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<Id> IngredientRequiresReset for InputIngredient<Id>
where
Id: InputId,
{
const RESET_ON_NEW_REVISION: bool = false;
}

View file

@ -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<Id, Data> IngredientRequiresReset for InternedIngredient<Id, Data>
where
Id: InternedId,
Data: InternedData,
{
const RESET_ON_NEW_REVISION: bool = false;
}
pub struct IdentityInterner<Id: AsId> {

View file

@ -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<DB: HasJars> = dyn Fn(&DB::Jars) -> (&dyn Ingredient<DB>) + Se
#[allow(type_alias_bounds)]
#[allow(unused_parens)]
pub type DynMutRoute<DB: HasJars> =
dyn Fn(&mut DB::Jars) -> (&mut dyn MutIngredient<DB>) + Send + Sync;
dyn Fn(&mut DB::Jars) -> (&mut dyn Ingredient<DB>) + 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<DB: HasJars> {
/// 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<Box<DynRoute<DB>>>,
routes: Vec<(Box<DynRoute<DB>>, Box<DynMutRoute<DB>>)>,
/// 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<Box<DynMutRoute<DB>>>,
/// Indices of routes which need a 'reset' call.
needs_reset: Vec<IngredientIndex>,
}
impl<DB: HasJars> Routes<DB> {
@ -66,7 +62,7 @@ impl<DB: HasJars> Routes<DB> {
pub(super) fn new() -> Self {
Routes {
routes: vec![],
mut_routes: vec![],
needs_reset: vec![],
}
}
@ -77,52 +73,53 @@ impl<DB: HasJars> Routes<DB> {
///
/// # 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<I>(
&mut self,
route: impl (Fn(&DB::Jars) -> &dyn Ingredient<DB>) + 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<DB> + 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<DB>) + Send + Sync + 'static,
mut_route: impl (Fn(&mut DB::Jars) -> &mut dyn MutIngredient<DB>) + 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<DB> {
&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<Item = &dyn Fn(&mut DB::Jars) -> &mut dyn MutIngredient<DB>> + '_ {
self.mut_routes
.iter()
.map(|b| &**b as &dyn Fn(&mut DB::Jars) -> &mut dyn MutIngredient<DB>)
index: IngredientIndex,
) -> &dyn Fn(&mut DB::Jars) -> &mut dyn Ingredient<DB> {
&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<Item = &dyn Fn(&mut DB::Jars) -> &mut dyn Ingredient<DB>> + '_ {
self.needs_reset.iter().map(|&index| self.route_mut(index))
}
}

View file

@ -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();
}

View file

@ -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<DB: ?Sized, Id, Data> MutIngredient<DB> for TrackedStructIngredient<Id, Data>
where
Id: TrackedStructId,
Data: TrackedStructData,
{
fn reset_for_new_revision(&mut self) {
self.interned.clear_deleted_indices();
}
}
impl<Id, Data> IngredientRequiresReset for TrackedStructIngredient<Id, Data>
where
Id: TrackedStructId,
Data: TrackedStructData,
{
const RESET_ON_NEW_REVISION: bool = true;
}