wire up salsa struct seletion, test it

We now delete entities and data associated with them!
Neat!
This commit is contained in:
Niko Matsakis 2022-08-15 13:13:54 +03:00
parent 66ffae1bb9
commit 689751b243
12 changed files with 355 additions and 12 deletions

View file

@ -150,6 +150,11 @@ fn has_jars_dyn_impl(input: &syn::ItemStruct, storage: &syn::Ident) -> syn::Item
let ingredient = self.#storage.ingredient(stale_output.ingredient_index());
ingredient.remove_stale_output(self, executor, stale_output.key_index());
}
fn salsa_struct_deleted(&self, ingredient: salsa::IngredientIndex, id: salsa::Id) {
let ingredient = self.#storage.ingredient(ingredient);
ingredient.salsa_struct_deleted(self, id);
}
}
}
}

View file

@ -100,6 +100,10 @@ where
fn reset_for_new_revision(&mut self) {
panic!("unexpected reset on accumulator")
}
fn salsa_struct_deleted(&self, _db: &DB, _id: crate::Id) {
panic!("unexpected call: accumulator is not registered as a dependent fn");
}
}
impl<Data> IngredientRequiresReset for AccumulatorIngredient<Data>

View file

@ -83,6 +83,12 @@ pub enum EventKind {
/// Key for the query that is no longer output
output_key: DatabaseKeyIndex,
},
/// Tracked structs or memoized data were discarded (freed).
DidDiscard {
/// Value being discarded.
key: DatabaseKeyIndex,
},
}
impl fmt::Debug for EventKind {
@ -113,6 +119,9 @@ impl fmt::Debug for EventKind {
.field("execute_key", &execute_key)
.field("output_key", &output_key)
.finish(),
EventKind::DidDiscard { key } => {
fmt.debug_struct("DidDiscard").field("key", &key).finish()
}
}
}
}
@ -148,6 +157,10 @@ where
.field("execute_key", &execute_key.debug(db))
.field("output_key", &output_key.debug(db))
.finish(),
EventKind::DidDiscard { key } => fmt
.debug_struct("DidDiscard")
.field("key", &key.debug(db))
.finish(),
}
}
}

View file

@ -10,7 +10,7 @@ use crate::{
key::{DatabaseKeyIndex, DependencyIndex},
runtime::local_state::QueryOrigin,
salsa_struct::SalsaStructInDb,
Cycle, DbWithJar, Id, Revision,
Cycle, DbWithJar, Event, EventKind, Id, Revision,
};
use super::{ingredient::Ingredient, routes::IngredientIndex, AsId};
@ -244,6 +244,26 @@ where
fn reset_for_new_revision(&mut self) {
std::mem::take(&mut self.deleted_entries);
}
fn salsa_struct_deleted(&self, db: &DB, id: crate::Id) {
// Remove any data keyed by `id`, since `id` no longer
// exists in this revision.
let id: C::Key = C::key_from_id(id);
if let Some(origin) = self.delete_memo(id) {
let key = self.database_key_index(id);
db.salsa_event(Event {
runtime_id: db.salsa_runtime().id(),
kind: EventKind::DidDiscard { key },
});
// Anything that was output by this memoized execution
// is now itself stale.
for stale_output in origin.outputs() {
db.remove_stale_output(key, stale_output)
}
}
}
}
impl<C> IngredientRequiresReset for FunctionIngredient<C>

View file

@ -1,3 +1,5 @@
use crate::runtime::local_state::QueryOrigin;
use super::{Configuration, FunctionIngredient};
impl<C> FunctionIngredient<C>
@ -6,9 +8,13 @@ where
{
/// Removes the memoized value for `key` from the memo-map.
/// Pushes the memo onto `deleted_entries` to ensure that any references into that memo which were handed out remain valid.
pub(super) fn delete_memo(&self, key: C::Key) {
pub(super) fn delete_memo(&self, key: C::Key) -> Option<QueryOrigin> {
if let Some(memo) = self.memo_map.remove(key) {
let origin = memo.load().revisions.origin.clone();
self.deleted_entries.push(memo);
Some(origin)
} else {
None
}
}
}

View file

@ -34,6 +34,12 @@ 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);
/// Informs the ingredient `self` that the salsa struct with id `id` has been deleted.
/// This gives `self` a chance to remove any memoized data dependent on `id`.
/// To receive this callback, `self` must register itself as a dependent function using
/// [`SalsaStructInDb::register_dependent_fn`](`crate::salsa_struct::SalsaStructInDb::register_dependent_fn`).
fn salsa_struct_deleted(&self, db: &DB, id: Id);
/// 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.

View file

@ -84,6 +84,12 @@ where
fn reset_for_new_revision(&mut self) {
panic!("unexpected call to `reset_for_new_revision`")
}
fn salsa_struct_deleted(&self, _db: &DB, _id: crate::Id) {
panic!(
"unexpected call: input ingredients do not register for salsa struct deletion events"
);
}
}
impl<Id> IngredientRequiresReset for InputIngredient<Id>

View file

@ -225,6 +225,10 @@ where
// items, but those are only used for tracked struct ingredients.
panic!("unexpected call to `reset_for_new_revision`")
}
fn salsa_struct_deleted(&self, _db: &DB, _id: crate::Id) {
panic!("unexpected call: interned ingredients do not register for salsa struct deletion events");
}
}
impl<Id, Data> IngredientRequiresReset for InternedIngredient<Id, Data>

View file

@ -8,7 +8,7 @@ use crate::jar::Jar;
use crate::key::DependencyIndex;
use crate::runtime::local_state::QueryOrigin;
use crate::runtime::Runtime;
use crate::{Database, DatabaseKeyIndex, IngredientIndex};
use crate::{Database, DatabaseKeyIndex, Id, IngredientIndex};
use super::routes::Routes;
use super::{ParallelDatabase, Revision};
@ -182,7 +182,19 @@ pub trait HasJarsDyn {
fn mark_validated_output(&self, executor: DatabaseKeyIndex, output: DatabaseKeyIndex);
/// Invoked when `executor` used to output `stale_output` but no longer does.
/// This method routes that into a call to the [`remove_stale_output`](`crate::ingredient::Ingredient::remove_stale_output`)
/// method on the ingredient for `stale_output`.
fn remove_stale_output(&self, executor: DatabaseKeyIndex, stale_output: DatabaseKeyIndex);
/// Informs `ingredient` that the salsa struct with id `id` has been deleted.
/// This means that `id` will not be used in this revision and hence
/// any memoized values keyed by that struct can be discarded.
///
/// In order to receive this callback, `ingredient` must have registered itself
/// as a dependent function using
/// [`SalsaStructInDb::register_dependent_fn`](`crate::salsa_struct::SalsaStructInDb::register_dependent_fn`).
fn salsa_struct_deleted(&self, ingredient: IngredientIndex, id: Id);
}
pub trait HasIngredientsFor<I>

View file

@ -6,7 +6,7 @@ use crate::{
key::{DatabaseKeyIndex, DependencyIndex},
runtime::{local_state::QueryOrigin, Runtime},
salsa_struct::SalsaStructInDb,
Database, IngredientIndex, Revision,
Database, Event, IngredientIndex, Revision,
};
pub trait TrackedStructId: InternedId {}
@ -104,9 +104,17 @@ where
/// Using this method on an entity id that MAY be used in the current revision will lead to
/// unspecified results (but not UB). See [`InternedIngredient::delete_index`] for more
/// discussion and important considerations.
pub(crate) fn delete_entities(&self, _runtime: &Runtime, ids: impl Iterator<Item = Id>) {
for id in ids {
self.interned.delete_index(id);
pub(crate) fn delete_entity(&self, db: &dyn crate::Database, id: Id) {
db.salsa_event(Event {
runtime_id: db.salsa_runtime().id(),
kind: crate::EventKind::DidDiscard {
key: self.database_key_index(id),
},
});
self.interned.delete_index(id);
for dependent_fn in self.dependent_fns.iter() {
db.salsa_struct_deleted(dependent_fn, id.as_id());
}
}
@ -122,6 +130,7 @@ impl<DB: ?Sized, Id, Data> Ingredient<DB> for TrackedStructIngredient<Id, Data>
where
Id: TrackedStructId,
Data: TrackedStructData,
DB: crate::Database,
{
fn maybe_changed_after(&self, db: &DB, input: DependencyIndex, revision: Revision) -> bool {
self.interned.maybe_changed_after(db, input, revision)
@ -142,18 +151,25 @@ where
fn remove_stale_output(
&self,
_db: &DB,
executor: DatabaseKeyIndex,
db: &DB,
_executor: DatabaseKeyIndex,
stale_output_key: crate::Id,
) {
let key: Id = Id::from_id(stale_output_key);
// FIXME -- we can delete this entity
drop((executor, key));
// This method is called when, in prior revisions,
// `executor` creates a tracked struct `salsa_output_key`,
// but it did not in the current revision.
// In that case, we can delete `stale_output_key` and any data associated with it.
let stale_output_key: Id = Id::from_id(stale_output_key);
self.delete_entity(db.as_salsa_database(), stale_output_key);
}
fn reset_for_new_revision(&mut self) {
self.interned.clear_deleted_indices();
}
fn salsa_struct_deleted(&self, _db: &DB, _id: crate::Id) {
panic!("unexpected call: interned ingredients do not register for salsa struct deletion events");
}
}
impl<Id, Data> IngredientRequiresReset for TrackedStructIngredient<Id, Data>

View file

@ -0,0 +1,134 @@
//! Delete cascade:
//!
//! * when we delete memoized data, also delete outputs from that data
use salsa::DebugWithDb;
use salsa_2022_tests::{HasLogger, Logger};
use expect_test::expect;
use test_log::test;
#[salsa::jar(db = Db)]
struct Jar(
MyInput,
MyTracked,
final_result,
create_tracked_structs,
contribution_from_struct,
copy_field,
);
trait Db: salsa::DbWithJar<Jar> + HasLogger {}
#[salsa::input]
struct MyInput {
field: u32,
}
#[salsa::tracked]
fn final_result(db: &dyn Db, input: MyInput) -> u32 {
db.push_log(format!("final_result({:?})", input));
let mut sum = 0;
for tracked_struct in create_tracked_structs(db, input) {
sum += contribution_from_struct(db, tracked_struct);
}
sum
}
#[salsa::tracked]
struct MyTracked {
field: u32,
}
#[salsa::tracked]
fn create_tracked_structs(db: &dyn Db, input: MyInput) -> Vec<MyTracked> {
db.push_log(format!("intermediate_result({:?})", input));
(0..input.field(db))
.map(|i| MyTracked::new(db, i))
.collect()
}
#[salsa::tracked]
fn contribution_from_struct(db: &dyn Db, tracked: MyTracked) -> u32 {
let m = MyTracked::new(db, tracked.field(db));
copy_field(db, m) * 2
}
#[salsa::tracked]
fn copy_field(db: &dyn Db, tracked: MyTracked) -> u32 {
tracked.field(db)
}
#[salsa::db(Jar)]
#[derive(Default)]
struct Database {
storage: salsa::Storage<Self>,
logger: Logger,
}
impl salsa::Database for Database {
fn salsa_event(&self, event: salsa::Event) {
match event.kind {
salsa::EventKind::WillDiscardStaleOutput { .. }
| salsa::EventKind::DidDiscard { .. } => {
self.push_log(format!("salsa_event({:?})", event.kind.debug(self)));
}
_ => {}
}
}
fn salsa_runtime(&self) -> &salsa::Runtime {
self.storage.runtime()
}
}
impl Db for Database {}
impl HasLogger for Database {
fn logger(&self) -> &Logger {
&self.logger
}
}
#[test]
fn basic() {
let mut db = Database::default();
// Creates 3 tracked structs
let input = MyInput::new(&mut db, 3);
assert_eq!(final_result(&db, input), 2 * 2 + 1 * 2 + 0 * 2);
db.assert_logs(expect![[r#"
[
"final_result(MyInput(Id { value: 1 }))",
"intermediate_result(MyInput(Id { value: 1 }))",
]"#]]);
// Creates only 2 tracked structs in this revision, should delete 1
//
// Expect to see 6 DidDiscard events. Three from the primary struct:
//
// * the struct itself
// * the struct's field
// * the `contribution_from_struct` result
//
// and then 3 more from the struct created by `contribution_from_struct`:
//
// * the struct itself
// * the struct's field
// * the `copy_field` result
input.set_field(&mut db, 2);
assert_eq!(final_result(&db, input), 1 * 2 + 0 * 2);
db.assert_logs(expect![[r#"
[
"intermediate_result(MyInput(Id { value: 1 }))",
"salsa_event(WillDiscardStaleOutput { execute_key: DependencyIndex { ingredient_index: IngredientIndex(5), key_index: Some(Id { value: 1 }) }, output_key: DependencyIndex { ingredient_index: IngredientIndex(3), key_index: Some(Id { value: 3 }) } })",
"salsa_event(DidDiscard { key: DependencyIndex { ingredient_index: IngredientIndex(3), key_index: Some(Id { value: 3 }) } })",
"salsa_event(DidDiscard { key: DependencyIndex { ingredient_index: IngredientIndex(2), key_index: Some(Id { value: 3 }) } })",
"salsa_event(DidDiscard { key: DependencyIndex { ingredient_index: IngredientIndex(6), key_index: Some(Id { value: 3 }) } })",
"salsa_event(DidDiscard { key: DependencyIndex { ingredient_index: IngredientIndex(3), key_index: Some(Id { value: 6 }) } })",
"salsa_event(DidDiscard { key: DependencyIndex { ingredient_index: IngredientIndex(2), key_index: Some(Id { value: 6 }) } })",
"salsa_event(DidDiscard { key: DependencyIndex { ingredient_index: IngredientIndex(7), key_index: Some(Id { value: 6 }) } })",
"final_result(MyInput(Id { value: 1 }))",
]"#]]);
}

View file

@ -0,0 +1,117 @@
//! Basic deletion test:
//!
//! * entities not created in a revision are deleted, as is any memoized data keyed on them.
use salsa::DebugWithDb;
use salsa_2022_tests::{HasLogger, Logger};
use expect_test::expect;
use test_log::test;
#[salsa::jar(db = Db)]
struct Jar(
MyInput,
MyTracked,
final_result,
create_tracked_structs,
contribution_from_struct,
);
trait Db: salsa::DbWithJar<Jar> + HasLogger {}
#[salsa::input]
struct MyInput {
field: u32,
}
#[salsa::tracked]
fn final_result(db: &dyn Db, input: MyInput) -> u32 {
db.push_log(format!("final_result({:?})", input));
let mut sum = 0;
for tracked_struct in create_tracked_structs(db, input) {
sum += contribution_from_struct(db, tracked_struct);
}
sum
}
#[salsa::tracked]
struct MyTracked {
field: u32,
}
#[salsa::tracked]
fn create_tracked_structs(db: &dyn Db, input: MyInput) -> Vec<MyTracked> {
db.push_log(format!("intermediate_result({:?})", input));
(0..input.field(db))
.map(|i| MyTracked::new(db, i))
.collect()
}
#[salsa::tracked]
fn contribution_from_struct(db: &dyn Db, tracked: MyTracked) -> u32 {
tracked.field(db) * 2
}
#[salsa::db(Jar)]
#[derive(Default)]
struct Database {
storage: salsa::Storage<Self>,
logger: Logger,
}
impl salsa::Database for Database {
fn salsa_event(&self, event: salsa::Event) {
match event.kind {
salsa::EventKind::WillDiscardStaleOutput { .. }
| salsa::EventKind::DidDiscard { .. } => {
self.push_log(format!("salsa_event({:?})", event.kind.debug(self)));
}
_ => {}
}
}
fn salsa_runtime(&self) -> &salsa::Runtime {
self.storage.runtime()
}
}
impl Db for Database {}
impl HasLogger for Database {
fn logger(&self) -> &Logger {
&self.logger
}
}
#[test]
fn basic() {
let mut db = Database::default();
// Creates 3 tracked structs
let input = MyInput::new(&mut db, 3);
assert_eq!(final_result(&db, input), 2 * 2 + 1 * 2 + 0 * 2);
db.assert_logs(expect![[r#"
[
"final_result(MyInput(Id { value: 1 }))",
"intermediate_result(MyInput(Id { value: 1 }))",
]"#]]);
// Creates only 2 tracked structs in this revision, should delete 1
//
// Expect to see 3 DidDiscard events--
//
// * the struct itself
// * the struct's field
// * the `contribution_from_struct` result
input.set_field(&mut db, 2);
assert_eq!(final_result(&db, input), 1 * 2 + 0 * 2);
db.assert_logs(expect![[r#"
[
"intermediate_result(MyInput(Id { value: 1 }))",
"salsa_event(WillDiscardStaleOutput { execute_key: DependencyIndex { ingredient_index: IngredientIndex(5), key_index: Some(Id { value: 1 }) }, output_key: DependencyIndex { ingredient_index: IngredientIndex(3), key_index: Some(Id { value: 3 }) } })",
"salsa_event(DidDiscard { key: DependencyIndex { ingredient_index: IngredientIndex(3), key_index: Some(Id { value: 3 }) } })",
"salsa_event(DidDiscard { key: DependencyIndex { ingredient_index: IngredientIndex(2), key_index: Some(Id { value: 3 }) } })",
"salsa_event(DidDiscard { key: DependencyIndex { ingredient_index: IngredientIndex(6), key_index: Some(Id { value: 3 }) } })",
"final_result(MyInput(Id { value: 1 }))",
]"#]]);
}