From 4c4096e39bb4b2563eb4b644b5cdd4177289ee8b Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Tue, 13 Sep 2022 08:18:19 +0800 Subject: [PATCH 01/19] panic when reading fields of tracked structs from older revisions --- .../src/function/maybe_changed_after.rs | 14 ++++-- ...of-tracked-structs-from-older-revisions.rs | 46 +++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs diff --git a/components/salsa-2022/src/function/maybe_changed_after.rs b/components/salsa-2022/src/function/maybe_changed_after.rs index 2888b451..391d7068 100644 --- a/components/salsa-2022/src/function/maybe_changed_after.rs +++ b/components/salsa-2022/src/function/maybe_changed_after.rs @@ -170,12 +170,18 @@ where // during this revision or is otherwise stale. return false; } - QueryOrigin::BaseInput | QueryOrigin::Field => { - // BaseInput: This value was `set` by the mutator thread -- ie, it's a base input and it cannot be out of date. - // Field: This value is the value of a field of some tracked struct S. It is always updated whenever S is created. - // So if a query has access to S, then they will have an up-to-date value. + QueryOrigin::BaseInput => { + // This value was `set` by the mutator thread -- ie, it's a base input and it cannot be out of date. return true; } + QueryOrigin::Field => { + // This value is the value of a field of some tracked struct S. + // The fact that we are here means that we are accessing fields from old revisions, which is not allowed. + panic!( + "accessing fields of tracked struct {:?} from older revisions", + std::any::type_name::<::SalsaStruct>() + ) + } QueryOrigin::DerivedUntracked(_) => { // Untracked inputs? Have to assume that it changed. return false; diff --git a/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs b/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs new file mode 100644 index 00000000..92304133 --- /dev/null +++ b/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs @@ -0,0 +1,46 @@ +use test_log::test; + +#[salsa::jar(db = Db)] +struct Jar(MyInput, MyTracked, tracked_fn); + +trait Db: salsa::DbWithJar {} + +#[salsa::input(jar = Jar)] +struct MyInput { + field: u32, +} + +#[salsa::tracked(jar = Jar)] +struct MyTracked { + field: u32, +} + +#[salsa::tracked(jar = Jar)] +fn tracked_fn(db: &dyn Db, input: MyInput) -> MyTracked { + MyTracked::new(db, input.field(db) / 2) +} + +#[salsa::db(Jar)] +#[derive(Default)] +struct Database { + storage: salsa::Storage, +} + +impl salsa::Database for Database {} + +impl Db for Database {} + +#[test] +#[should_panic(expected = "accessing fields of tracked struct")] +fn execute() { + let mut db = Database::default(); + + let input = MyInput::new(&mut db, 22); + let tracked = tracked_fn(&db, input); + + // modify the input and change the revision + input.set_field(&mut db).to(24); + + // panic when reading fields of tracked structs from older revisions + tracked.field(&db); +} From 99cfca5799c180764797027a81d161b73c15c321 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Tue, 13 Sep 2022 11:32:05 +0800 Subject: [PATCH 02/19] add the fields of tracked struct to the output of queries --- components/salsa-2022/src/function/specify.rs | 3 +++ salsa-2022-tests/tests/deletion.rs | 1 + 2 files changed, 4 insertions(+) diff --git a/components/salsa-2022/src/function/specify.rs b/components/salsa-2022/src/function/specify.rs index 8c0fe19c..17a703d2 100644 --- a/components/salsa-2022/src/function/specify.rs +++ b/components/salsa-2022/src/function/specify.rs @@ -100,6 +100,8 @@ where C::Key: TrackedStructInDb>, { self.specify(db, key, value, |_| QueryOrigin::Field); + let database_key_index = self.database_key_index(key); + db.runtime().add_output(database_key_index.into()); } /// Specify the value for `key` *and* record that we did so. @@ -138,6 +140,7 @@ where // assigneed by `executor`. match memo.revisions.origin { QueryOrigin::Assigned(by_query) => assert_eq!(by_query, executor), + QueryOrigin::Field => {} _ => panic!( "expected a query assigned by `{:?}`, not `{:?}`", executor.debug(db), diff --git a/salsa-2022-tests/tests/deletion.rs b/salsa-2022-tests/tests/deletion.rs index 2860cb39..52ecd772 100644 --- a/salsa-2022-tests/tests/deletion.rs +++ b/salsa-2022-tests/tests/deletion.rs @@ -104,6 +104,7 @@ fn basic() { db.assert_logs(expect![[r#" [ "intermediate_result(MyInput(Id { value: 1 }))", + "salsa_event(WillDiscardStaleOutput { execute_key: create_tracked_structs(0), output_key: field(2) })", "salsa_event(WillDiscardStaleOutput { execute_key: create_tracked_structs(0), output_key: MyTracked(2) })", "salsa_event(DidDiscard { key: MyTracked(2) })", "salsa_event(DidDiscard { key: field(2) })", From 1ae6a7bbc52f8a45f295bb998c9be3684ee53edb Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 17 Sep 2022 07:09:39 +0800 Subject: [PATCH 03/19] use `Assigned` insread of `Field` and remove `Field` --- components/salsa-2022-macros/src/tracked_struct.rs | 2 +- components/salsa-2022/src/function/accumulated.rs | 3 +-- .../salsa-2022/src/function/maybe_changed_after.rs | 8 -------- components/salsa-2022/src/function/memo.rs | 3 +-- components/salsa-2022/src/function/specify.rs | 14 -------------- components/salsa-2022/src/runtime/local_state.rs | 6 +----- 6 files changed, 4 insertions(+), 32 deletions(-) diff --git a/components/salsa-2022-macros/src/tracked_struct.rs b/components/salsa-2022-macros/src/tracked_struct.rs index fd826d07..98f36cf9 100644 --- a/components/salsa-2022-macros/src/tracked_struct.rs +++ b/components/salsa-2022-macros/src/tracked_struct.rs @@ -158,7 +158,7 @@ impl TrackedStruct { let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient(__jar); let __id = __ingredients.#struct_index.new_struct(__runtime, (#(#id_field_names,)*)); #( - __ingredients.#value_field_indices.specify_field(__db, __id, #value_field_names); + __ingredients.#value_field_indices.specify_and_record(__db, __id, #value_field_names); )* __id } diff --git a/components/salsa-2022/src/function/accumulated.rs b/components/salsa-2022/src/function/accumulated.rs index 58ef6ee5..ccb3a2a0 100644 --- a/components/salsa-2022/src/function/accumulated.rs +++ b/components/salsa-2022/src/function/accumulated.rs @@ -67,8 +67,7 @@ impl Stack { match origin { None | Some(QueryOrigin::Assigned(_)) - | Some(QueryOrigin::BaseInput) - | Some(QueryOrigin::Field) => {} + | Some(QueryOrigin::BaseInput) => {} Some(QueryOrigin::Derived(edges)) | Some(QueryOrigin::DerivedUntracked(edges)) => { for DependencyIndex { ingredient_index, diff --git a/components/salsa-2022/src/function/maybe_changed_after.rs b/components/salsa-2022/src/function/maybe_changed_after.rs index 391d7068..77f8e0e5 100644 --- a/components/salsa-2022/src/function/maybe_changed_after.rs +++ b/components/salsa-2022/src/function/maybe_changed_after.rs @@ -174,14 +174,6 @@ where // This value was `set` by the mutator thread -- ie, it's a base input and it cannot be out of date. return true; } - QueryOrigin::Field => { - // This value is the value of a field of some tracked struct S. - // The fact that we are here means that we are accessing fields from old revisions, which is not allowed. - panic!( - "accessing fields of tracked struct {:?} from older revisions", - std::any::type_name::<::SalsaStruct>() - ) - } QueryOrigin::DerivedUntracked(_) => { // Untracked inputs? Have to assume that it changed. return false; diff --git a/components/salsa-2022/src/function/memo.rs b/components/salsa-2022/src/function/memo.rs index 73c7c2c4..7c77ce8c 100644 --- a/components/salsa-2022/src/function/memo.rs +++ b/components/salsa-2022/src/function/memo.rs @@ -55,8 +55,7 @@ impl MemoMap { match memo.revisions.origin { QueryOrigin::Assigned(_) | QueryOrigin::DerivedUntracked(_) - | QueryOrigin::BaseInput - | QueryOrigin::Field => { + | QueryOrigin::BaseInput => { // Careful: Cannot evict memos whose values were // assigned as output of another query // or those with untracked inputs diff --git a/components/salsa-2022/src/function/specify.rs b/components/salsa-2022/src/function/specify.rs index 17a703d2..0c215383 100644 --- a/components/salsa-2022/src/function/specify.rs +++ b/components/salsa-2022/src/function/specify.rs @@ -91,19 +91,6 @@ where self.insert_memo(db, key, memo); } - /// Specify the value for `key` but do not record it is an output. - /// This is used for the value fields declared on a tracked struct. - /// They are different from other calls to specify because we KNOW they will be given a value by construction, - /// so recording them as an explicit output (and checking them for validity, etc) is pure overhead. - pub fn specify_field<'db>(&self, db: &'db DynDb<'db, C>, key: C::Key, value: C::Value) - where - C::Key: TrackedStructInDb>, - { - self.specify(db, key, value, |_| QueryOrigin::Field); - let database_key_index = self.database_key_index(key); - db.runtime().add_output(database_key_index.into()); - } - /// Specify the value for `key` *and* record that we did so. /// Used for explicit calls to `specify`, but not needed for pre-declared tracked struct fields. pub fn specify_and_record<'db>(&self, db: &'db DynDb<'db, C>, key: C::Key, value: C::Value) @@ -140,7 +127,6 @@ where // assigneed by `executor`. match memo.revisions.origin { QueryOrigin::Assigned(by_query) => assert_eq!(by_query, executor), - QueryOrigin::Field => {} _ => panic!( "expected a query assigned by `{:?}`, not `{:?}`", executor.debug(db), diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index 0897722e..e5359cb9 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -60,10 +60,6 @@ pub enum QueryOrigin { /// The `DatabaseKeyIndex` is the identity of the assigning query. Assigned(DatabaseKeyIndex), - /// This is the value field of a tracked struct. - /// These are different from `Assigned` because we know they will always be assigned a value and hence are never "out of date". - Field, - /// This value was set as a base input to the computation. BaseInput, @@ -86,7 +82,7 @@ impl QueryOrigin { QueryOrigin::Derived(edges) | QueryOrigin::DerivedUntracked(edges) => { &edges.input_outputs[edges.separator as usize..] } - QueryOrigin::Assigned(_) | QueryOrigin::BaseInput | QueryOrigin::Field => &[], + QueryOrigin::Assigned(_) | QueryOrigin::BaseInput => &[], }; slice.iter().copied() From a1db7d4e8404eb567e4ad4184dae896dce08ad6e Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 17 Sep 2022 12:26:20 +0000 Subject: [PATCH 04/19] add a test --- .../tests/tracked_fn_read_own_specify.rs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 salsa-2022-tests/tests/tracked_fn_read_own_specify.rs diff --git a/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs b/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs new file mode 100644 index 00000000..638a0056 --- /dev/null +++ b/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs @@ -0,0 +1,48 @@ +#![allow(warnings)] + +#[salsa::jar(db = Db)] +struct Jar(MyInput, MyTracked, tracked_fn, tracked_fn_extra); + +trait Db: salsa::DbWithJar {} + +#[salsa::input(jar = Jar)] +struct MyInput { + field: u32, +} + +#[salsa::tracked(jar = Jar)] +struct MyTracked { + field: u32, +} + +#[salsa::tracked(jar = Jar)] +fn tracked_fn(db: &dyn Db, input: MyInput) -> u32 { + let t = MyTracked::new(db, input.field(db) * 2); + tracked_fn_extra::specify(db, t, 2222); + tracked_fn_extra(db, t) +} + +#[salsa::tracked(jar = Jar, specify)] +fn tracked_fn_extra(_db: &dyn Db, _input: MyTracked) -> u32 { + 0 +} + +#[salsa::db(Jar)] +#[derive(Default)] +struct Database { + storage: salsa::Storage, +} + +impl salsa::Database for Database {} + +impl Db for Database {} + +#[test] +fn execute() { + let mut db = Database::default(); + let input = MyInput::new(&mut db, 22); + assert_eq!(tracked_fn(&db, input), 2222); + + let input2 = MyInput::new(&mut db, 44); + assert_eq!(tracked_fn(&db, input), 2222); +} From c219944699ea6622175a41af9d10ee4dd20a2ed2 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sun, 18 Sep 2022 00:20:20 +0000 Subject: [PATCH 05/19] merge input/output lists into one list --- .../salsa-2022/src/runtime/active_query.rs | 52 ++++++------------- .../salsa-2022/src/runtime/local_state.rs | 45 ++++++++++------ 2 files changed, 46 insertions(+), 51 deletions(-) diff --git a/components/salsa-2022/src/runtime/active_query.rs b/components/salsa-2022/src/runtime/active_query.rs index 87fa8b46..298b5afe 100644 --- a/components/salsa-2022/src/runtime/active_query.rs +++ b/components/salsa-2022/src/runtime/active_query.rs @@ -1,5 +1,3 @@ -use std::collections::BTreeSet; - use crate::{ durability::Durability, hash::{FxIndexMap, FxIndexSet}, @@ -8,7 +6,7 @@ use crate::{ Cycle, Revision, Runtime, }; -use super::local_state::{QueryEdges, QueryOrigin, QueryRevisions}; +use super::local_state::{QueryEdges, QueryOrigin, QueryRevisions, EdgeKind}; #[derive(Debug)] pub(super) struct ActiveQuery { @@ -22,8 +20,13 @@ pub(super) struct ActiveQuery { /// untracked read, this will be set to the most recent revision. pub(super) changed_at: Revision, - /// Set of subqueries that were accessed thus far. - pub(super) dependencies: FxIndexSet, + /// Inputs: Set of subqueries that were accessed thus far. + /// Outputs: Tracks values written by this query. Could be... + /// + /// * tracked structs created + /// * invocations of `specify` + /// * accumulators pushed to + pub(super) input_outputs: FxIndexSet<(EdgeKind, DependencyIndex)>, /// True if there was an untracked read. pub(super) untracked_read: bool, @@ -35,16 +38,6 @@ pub(super) struct ActiveQuery { /// hash is added to this map. If it is not present, then the disambiguator is 0. /// Otherwise it is 1 more than the current value (which is incremented). pub(super) disambiguator_map: FxIndexMap, - - /// Tracks values written by this query. Could be... - /// - /// * tracked structs created - /// * invocations of `specify` - /// * accumulators pushed to - /// - /// We use a btree-set because we want to be able to - /// extract the keys in sorted order. - pub(super) outputs: BTreeSet, } impl ActiveQuery { @@ -53,11 +46,10 @@ impl ActiveQuery { database_key_index, durability: Durability::MAX, changed_at: Revision::start(), - dependencies: FxIndexSet::default(), + input_outputs: FxIndexSet::default(), untracked_read: false, cycle: None, disambiguator_map: Default::default(), - outputs: Default::default(), } } @@ -67,7 +59,7 @@ impl ActiveQuery { durability: Durability, revision: Revision, ) { - self.dependencies.insert(input); + self.input_outputs.insert((EdgeKind::Input, input)); self.durability = self.durability.min(durability); self.changed_at = self.changed_at.max(revision); } @@ -86,29 +78,19 @@ impl ActiveQuery { /// Adds a key to our list of outputs. pub(super) fn add_output(&mut self, key: DependencyIndex) { - self.outputs.insert(key); + self.input_outputs.insert((EdgeKind::Output, key)); } /// True if the given key was output by this query. pub(super) fn is_output(&self, key: DatabaseKeyIndex) -> bool { let key: DependencyIndex = key.into(); - self.outputs.contains(&key) + self.input_outputs.contains(&(EdgeKind::Output, key)) } pub(crate) fn revisions(&self, runtime: &Runtime) -> QueryRevisions { - let separator = self.dependencies.len(); + let input_outputs = self.input_outputs.iter().copied().collect(); - let input_outputs = if self.dependencies.is_empty() && self.outputs.is_empty() { - runtime.empty_dependencies() - } else { - self.dependencies - .iter() - .copied() - .chain(self.outputs.iter().copied()) - .collect() - }; - - let edges = QueryEdges::new(separator, input_outputs); + let edges = QueryEdges::new(input_outputs); let origin = if self.untracked_read { QueryOrigin::DerivedUntracked(edges) @@ -129,7 +111,7 @@ impl ActiveQuery { self.changed_at = self.changed_at.max(other.changed_at); self.durability = self.durability.min(other.durability); self.untracked_read |= other.untracked_read; - self.dependencies.extend(other.dependencies.iter().copied()); + self.input_outputs.extend(other.input_outputs.iter().copied()); } /// Removes the participants in `cycle` from my dependencies. @@ -137,7 +119,7 @@ impl ActiveQuery { pub(super) fn remove_cycle_participants(&mut self, cycle: &Cycle) { for p in cycle.participant_keys() { let p: DependencyIndex = p.into(); - self.dependencies.remove(&p); + self.input_outputs.remove(&(EdgeKind::Input, p)); } } @@ -146,7 +128,7 @@ impl ActiveQuery { pub(crate) fn take_inputs_from(&mut self, cycle_query: &ActiveQuery) { self.changed_at = cycle_query.changed_at; self.durability = cycle_query.durability; - self.dependencies = cycle_query.dependencies.clone(); + self.input_outputs = cycle_query.input_outputs.clone(); } pub(super) fn disambiguate(&mut self, hash: u64) -> Disambiguator { diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index e5359cb9..8dbfd60a 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -76,19 +76,25 @@ pub enum QueryOrigin { } impl QueryOrigin { - /// Indices for queries *written* by this query (or `&[]` if its value was assigned). + /// Indices for queries *written* by this query (or `vec![]` if its value was assigned). pub(crate) fn outputs(&self) -> impl Iterator + '_ { let slice = match self { QueryOrigin::Derived(edges) | QueryOrigin::DerivedUntracked(edges) => { - &edges.input_outputs[edges.separator as usize..] + edges.outputs() } - QueryOrigin::Assigned(_) | QueryOrigin::BaseInput => &[], + QueryOrigin::Assigned(_) | QueryOrigin::BaseInput => vec![], }; - slice.iter().copied() + slice.into_iter() } } +#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] +pub enum EdgeKind { + Input, + Output, +} + /// The edges between a memoized value and other queries in the dependency graph. /// These edges include both dependency edges /// e.g., when creating the memoized value for Q0 executed another function Q1) @@ -98,8 +104,6 @@ impl QueryOrigin { pub struct QueryEdges { /// The list of outgoing edges from this node. /// This list combines *both* inputs and outputs. - /// The inputs are defined from the indices `0..S` where - /// `S` is the value of the `separator` field. /// /// Note that we always track input dependencies even when there are untracked reads. /// Untracked reads mean that we can't verify values, so we don't use the list of inputs for that, @@ -110,26 +114,35 @@ pub struct QueryEdges { /// Important: /// /// * The inputs must be in **execution order** for the red-green algorithm to work. - /// * The outputs must be in **sorted order** so that we can easily "diff" them between revisions. - input_outputs: Arc<[DependencyIndex]>, - - /// The index that separates inputs from outputs in the `tracked` field. - separator: u32, + input_outputs: Arc<[(EdgeKind, DependencyIndex)]>, } impl QueryEdges { /// Returns the (tracked) inputs that were executed in computing this memoized value. /// /// These will always be in execution order. - pub(crate) fn inputs(&self) -> &[DependencyIndex] { - &self.input_outputs[0..self.separator as usize] + pub(crate) fn inputs(&self) -> Vec { + self.input_outputs + .iter() + .filter(|(edge_kind, _)| *edge_kind == EdgeKind::Input) + .map(|(_, dependency_index)| *dependency_index) + .collect() + } + + /// Returns the (tracked) inputs that were executed in computing this memoized value. + /// + /// These will always be in execution order. + pub(crate) fn outputs(&self) -> Vec { + self.input_outputs + .iter() + .filter(|(edge_kind, _)| *edge_kind == EdgeKind::Output) + .map(|(_, dependency_index)| *dependency_index) + .collect() } /// Creates a new `QueryEdges`; the values given for each field must meet struct invariants. - pub(crate) fn new(separator: usize, input_outputs: Arc<[DependencyIndex]>) -> Self { - debug_assert!(separator <= input_outputs.len()); + pub(crate) fn new(input_outputs: Arc<[(EdgeKind, DependencyIndex)]>) -> Self { Self { - separator: u32::try_from(separator).unwrap(), input_outputs, } } From 98d1be065020a64fba9967cf43c3ee4f5a4df2ef Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sun, 18 Sep 2022 01:58:17 +0000 Subject: [PATCH 06/19] mark the outputs as valid as we encounter them in deep_verify_memo --- .../src/function/maybe_changed_after.rs | 15 ++++++++---- .../salsa-2022/src/runtime/local_state.rs | 2 +- ...ot-work-if-the-key-is-a-salsa-input.stderr | 24 +++++++++---------- ...work-if-the-key-is-a-salsa-interned.stderr | 24 +++++++++---------- ...of-tracked-structs-from-older-revisions.rs | 2 +- 5 files changed, 37 insertions(+), 30 deletions(-) diff --git a/components/salsa-2022/src/function/maybe_changed_after.rs b/components/salsa-2022/src/function/maybe_changed_after.rs index 77f8e0e5..2b93f657 100644 --- a/components/salsa-2022/src/function/maybe_changed_after.rs +++ b/components/salsa-2022/src/function/maybe_changed_after.rs @@ -5,7 +5,7 @@ use crate::{ debug::DebugWithDb, key::DatabaseKeyIndex, runtime::{ - local_state::{ActiveQueryGuard, QueryOrigin}, + local_state::{ActiveQueryGuard, QueryOrigin, EdgeKind}, StampedValue, }, storage::HasJarsDyn, @@ -186,9 +186,16 @@ where // valid, then some later input I1 might never have executed at all, so verifying // it is still up to date is meaningless. let last_verified_at = old_memo.verified_at.load(); - for &input in edges.inputs().iter() { - if db.maybe_changed_after(input, last_verified_at) { - return false; + for &(edge_kind, dependency_index) in edges.input_outputs.iter() { + match edge_kind { + EdgeKind::Input => { + if db.maybe_changed_after(dependency_index, last_verified_at) { + return false; + } + }, + EdgeKind::Output => { + db.mark_validated_output(database_key_index, dependency_index); + }, } } } diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index 8dbfd60a..ce36c454 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -114,7 +114,7 @@ pub struct QueryEdges { /// Important: /// /// * The inputs must be in **execution order** for the red-green algorithm to work. - input_outputs: Arc<[(EdgeKind, DependencyIndex)]>, + pub input_outputs: Arc<[(EdgeKind, DependencyIndex)]>, } impl QueryEdges { diff --git a/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr index 5c52c233..4f8730cb 100644 --- a/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr +++ b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr @@ -1,14 +1,14 @@ error[E0277]: the trait bound `MyInput: TrackedStructInDb` is not satisfied - --> tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs:21:28 - | -20 | #[salsa::tracked(jar = Jar, specify)] - | ------------------------------------- required by a bound introduced by this call -21 | fn tracked_fn(db: &dyn Db, input: MyInput) -> MyTracked { - | ^^^^^ the trait `TrackedStructInDb` is not implemented for `MyInput` - | - = help: the trait `TrackedStructInDb` is implemented for `MyTracked` + --> tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs:21:28 + | +20 | #[salsa::tracked(jar = Jar, specify)] + | ------------------------------------- required by a bound introduced by this call +21 | fn tracked_fn(db: &dyn Db, input: MyInput) -> MyTracked { + | ^^^^^ the trait `TrackedStructInDb` is not implemented for `MyInput` + | + = help: the trait `TrackedStructInDb` is implemented for `MyTracked` note: required by a bound in `function::specify::>::specify_and_record` - --> $WORKSPACE/components/salsa-2022/src/function/specify.rs - | - | C::Key: TrackedStructInDb>, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `function::specify::>::specify_and_record` + --> $WORKSPACE/components/salsa-2022/src/function/specify.rs + | + | C::Key: TrackedStructInDb>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `function::specify::>::specify_and_record` diff --git a/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr index 771ae492..6568a82c 100644 --- a/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr +++ b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr @@ -1,14 +1,14 @@ error[E0277]: the trait bound `MyInterned: TrackedStructInDb` is not satisfied - --> tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.rs:22:28 - | -21 | #[salsa::tracked(jar = Jar, specify)] - | ------------------------------------- required by a bound introduced by this call -22 | fn tracked_fn(db: &dyn Db, input: MyInterned) -> MyTracked { - | ^^^^^ the trait `TrackedStructInDb` is not implemented for `MyInterned` - | - = help: the trait `TrackedStructInDb` is implemented for `MyTracked` + --> tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.rs:22:28 + | +21 | #[salsa::tracked(jar = Jar, specify)] + | ------------------------------------- required by a bound introduced by this call +22 | fn tracked_fn(db: &dyn Db, input: MyInterned) -> MyTracked { + | ^^^^^ the trait `TrackedStructInDb` is not implemented for `MyInterned` + | + = help: the trait `TrackedStructInDb` is implemented for `MyTracked` note: required by a bound in `function::specify::>::specify_and_record` - --> $WORKSPACE/components/salsa-2022/src/function/specify.rs - | - | C::Key: TrackedStructInDb>, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `function::specify::>::specify_and_record` + --> $WORKSPACE/components/salsa-2022/src/function/specify.rs + | + | C::Key: TrackedStructInDb>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `function::specify::>::specify_and_record` diff --git a/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs b/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs index 92304133..b0d17ffc 100644 --- a/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs +++ b/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs @@ -31,7 +31,7 @@ impl salsa::Database for Database {} impl Db for Database {} #[test] -#[should_panic(expected = "accessing fields of tracked struct")] +#[should_panic(expected = "`execute` method for field")] fn execute() { let mut db = Database::default(); From 961c4ce1541f6cde6b8aa04baa0cc23ae6e13eef Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sun, 18 Sep 2022 02:41:54 +0000 Subject: [PATCH 07/19] update some tests and run cargo fmt --- components/salsa-2022/src/function/accumulated.rs | 4 +--- components/salsa-2022/src/function/maybe_changed_after.rs | 6 +++--- components/salsa-2022/src/runtime/active_query.rs | 5 +++-- components/salsa-2022/src/runtime/local_state.rs | 8 ++------ salsa-2022-tests/tests/accumulate-reuse-workaround.rs | 1 + salsa-2022-tests/tests/deletion-cascade.rs | 1 + salsa-2022-tests/tests/deletion.rs | 2 +- .../tests/specify_tracked_fn_in_rev_1_but_not_2.rs | 6 ++++++ 8 files changed, 18 insertions(+), 15 deletions(-) diff --git a/components/salsa-2022/src/function/accumulated.rs b/components/salsa-2022/src/function/accumulated.rs index ccb3a2a0..b44d737b 100644 --- a/components/salsa-2022/src/function/accumulated.rs +++ b/components/salsa-2022/src/function/accumulated.rs @@ -65,9 +65,7 @@ impl Stack { /// Extend the stack of queries with the dependencies from `origin`. fn extend(&mut self, origin: Option) { match origin { - None - | Some(QueryOrigin::Assigned(_)) - | Some(QueryOrigin::BaseInput) => {} + None | Some(QueryOrigin::Assigned(_)) | Some(QueryOrigin::BaseInput) => {} Some(QueryOrigin::Derived(edges)) | Some(QueryOrigin::DerivedUntracked(edges)) => { for DependencyIndex { ingredient_index, diff --git a/components/salsa-2022/src/function/maybe_changed_after.rs b/components/salsa-2022/src/function/maybe_changed_after.rs index 2b93f657..c407bca6 100644 --- a/components/salsa-2022/src/function/maybe_changed_after.rs +++ b/components/salsa-2022/src/function/maybe_changed_after.rs @@ -5,7 +5,7 @@ use crate::{ debug::DebugWithDb, key::DatabaseKeyIndex, runtime::{ - local_state::{ActiveQueryGuard, QueryOrigin, EdgeKind}, + local_state::{ActiveQueryGuard, EdgeKind, QueryOrigin}, StampedValue, }, storage::HasJarsDyn, @@ -192,10 +192,10 @@ where if db.maybe_changed_after(dependency_index, last_verified_at) { return false; } - }, + } EdgeKind::Output => { db.mark_validated_output(database_key_index, dependency_index); - }, + } } } } diff --git a/components/salsa-2022/src/runtime/active_query.rs b/components/salsa-2022/src/runtime/active_query.rs index 298b5afe..cf972d39 100644 --- a/components/salsa-2022/src/runtime/active_query.rs +++ b/components/salsa-2022/src/runtime/active_query.rs @@ -6,7 +6,7 @@ use crate::{ Cycle, Revision, Runtime, }; -use super::local_state::{QueryEdges, QueryOrigin, QueryRevisions, EdgeKind}; +use super::local_state::{EdgeKind, QueryEdges, QueryOrigin, QueryRevisions}; #[derive(Debug)] pub(super) struct ActiveQuery { @@ -111,7 +111,8 @@ impl ActiveQuery { self.changed_at = self.changed_at.max(other.changed_at); self.durability = self.durability.min(other.durability); self.untracked_read |= other.untracked_read; - self.input_outputs.extend(other.input_outputs.iter().copied()); + self.input_outputs + .extend(other.input_outputs.iter().copied()); } /// Removes the participants in `cycle` from my dependencies. diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index ce36c454..3c8cb008 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -79,9 +79,7 @@ impl QueryOrigin { /// Indices for queries *written* by this query (or `vec![]` if its value was assigned). pub(crate) fn outputs(&self) -> impl Iterator + '_ { let slice = match self { - QueryOrigin::Derived(edges) | QueryOrigin::DerivedUntracked(edges) => { - edges.outputs() - } + QueryOrigin::Derived(edges) | QueryOrigin::DerivedUntracked(edges) => edges.outputs(), QueryOrigin::Assigned(_) | QueryOrigin::BaseInput => vec![], }; @@ -142,9 +140,7 @@ impl QueryEdges { /// Creates a new `QueryEdges`; the values given for each field must meet struct invariants. pub(crate) fn new(input_outputs: Arc<[(EdgeKind, DependencyIndex)]>) -> Self { - Self { - input_outputs, - } + Self { input_outputs } } } diff --git a/salsa-2022-tests/tests/accumulate-reuse-workaround.rs b/salsa-2022-tests/tests/accumulate-reuse-workaround.rs index e960db1b..f06ebb06 100644 --- a/salsa-2022-tests/tests/accumulate-reuse-workaround.rs +++ b/salsa-2022-tests/tests/accumulate-reuse-workaround.rs @@ -89,5 +89,6 @@ fn test1() { [ "accumulated(List(Id { value: 1 }))", "compute(List(Id { value: 1 }))", + "compute(List(Id { value: 2 }))", ]"#]]); } diff --git a/salsa-2022-tests/tests/deletion-cascade.rs b/salsa-2022-tests/tests/deletion-cascade.rs index 65fc0a6b..c04473b2 100644 --- a/salsa-2022-tests/tests/deletion-cascade.rs +++ b/salsa-2022-tests/tests/deletion-cascade.rs @@ -125,6 +125,7 @@ fn basic() { "salsa_event(DidDiscard { key: MyTracked(5) })", "salsa_event(DidDiscard { key: field(5) })", "salsa_event(DidDiscard { key: copy_field(5) })", + "salsa_event(WillDiscardStaleOutput { execute_key: create_tracked_structs(0), output_key: field(2) })", "final_result(MyInput(Id { value: 1 }))", ]"#]]); } diff --git a/salsa-2022-tests/tests/deletion.rs b/salsa-2022-tests/tests/deletion.rs index 52ecd772..e732d40c 100644 --- a/salsa-2022-tests/tests/deletion.rs +++ b/salsa-2022-tests/tests/deletion.rs @@ -104,11 +104,11 @@ fn basic() { db.assert_logs(expect![[r#" [ "intermediate_result(MyInput(Id { value: 1 }))", - "salsa_event(WillDiscardStaleOutput { execute_key: create_tracked_structs(0), output_key: field(2) })", "salsa_event(WillDiscardStaleOutput { execute_key: create_tracked_structs(0), output_key: MyTracked(2) })", "salsa_event(DidDiscard { key: MyTracked(2) })", "salsa_event(DidDiscard { key: field(2) })", "salsa_event(DidDiscard { key: contribution_from_struct(2) })", + "salsa_event(WillDiscardStaleOutput { execute_key: create_tracked_structs(0), output_key: field(2) })", "final_result(MyInput(Id { value: 1 }))", ]"#]]); } diff --git a/salsa-2022-tests/tests/specify_tracked_fn_in_rev_1_but_not_2.rs b/salsa-2022-tests/tests/specify_tracked_fn_in_rev_1_but_not_2.rs index c799d4ab..db6a8322 100644 --- a/salsa-2022-tests/tests/specify_tracked_fn_in_rev_1_but_not_2.rs +++ b/salsa-2022-tests/tests/specify_tracked_fn_in_rev_1_but_not_2.rs @@ -209,6 +209,7 @@ fn test_run_0_then_5_then_20() { [ "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", + "Event { runtime_id: RuntimeId { counter: 0 }, kind: DidValidateMemoizedValue { database_key: input(0) } }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillExecute { database_key: create_tracked(0) } }", "create_tracked(MyInput(Id { value: 1 }))", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", @@ -228,6 +229,7 @@ fn test_run_0_then_5_then_20() { [ "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", + "Event { runtime_id: RuntimeId { counter: 0 }, kind: DidValidateMemoizedValue { database_key: input(0) } }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillExecute { database_key: create_tracked(0) } }", "create_tracked(MyInput(Id { value: 1 }))", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillDiscardStaleOutput { execute_key: create_tracked(0), output_key: maybe_specified(0) } }", @@ -280,6 +282,7 @@ fn test_run_0_then_5_then_10_then_20() { [ "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", + "Event { runtime_id: RuntimeId { counter: 0 }, kind: DidValidateMemoizedValue { database_key: input(0) } }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillExecute { database_key: create_tracked(0) } }", "create_tracked(MyInput(Id { value: 1 }))", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", @@ -299,6 +302,7 @@ fn test_run_0_then_5_then_10_then_20() { [ "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", + "Event { runtime_id: RuntimeId { counter: 0 }, kind: DidValidateMemoizedValue { database_key: input(0) } }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillExecute { database_key: create_tracked(0) } }", "create_tracked(MyInput(Id { value: 1 }))", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillDiscardStaleOutput { execute_key: create_tracked(0), output_key: maybe_specified(0) } }", @@ -320,6 +324,7 @@ fn test_run_0_then_5_then_10_then_20() { [ "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", + "Event { runtime_id: RuntimeId { counter: 0 }, kind: DidValidateMemoizedValue { database_key: input(0) } }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillExecute { database_key: create_tracked(0) } }", "create_tracked(MyInput(Id { value: 1 }))", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", @@ -364,6 +369,7 @@ fn test_run_5_then_20() { [ "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillCheckCancellation }", + "Event { runtime_id: RuntimeId { counter: 0 }, kind: DidValidateMemoizedValue { database_key: input(0) } }", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillExecute { database_key: create_tracked(0) } }", "create_tracked(MyInput(Id { value: 1 }))", "Event { runtime_id: RuntimeId { counter: 0 }, kind: WillDiscardStaleOutput { execute_key: create_tracked(0), output_key: maybe_specified(0) } }", From d71892c0472496065e4dc6baf10826fad7eb7a21 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Fri, 23 Sep 2022 23:58:53 +0000 Subject: [PATCH 08/19] update diff output --- .../salsa-2022/src/function/diff_outputs.rs | 32 +++++++------------ 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/components/salsa-2022/src/function/diff_outputs.rs b/components/salsa-2022/src/function/diff_outputs.rs index 9c03bd94..c8f8fc63 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, + hash::FxHashSet, key::DependencyIndex, runtime::local_state::QueryRevisions, + storage::HasJarsDyn, Database, DatabaseKeyIndex, Event, EventKind, }; use super::{memo::Memo, Configuration, DynDb, FunctionIngredient}; @@ -18,25 +18,17 @@ where old_memo: &Memo, revisions: &QueryRevisions, ) { - let mut old_outputs = old_memo.revisions.origin.outputs().peekable(); - let mut new_outputs = revisions.origin.outputs().peekable(); + // Iterate over the outputs of the `old_memo` and put them into a hashset + let mut old_outputs = FxHashSet::default(); + old_memo.revisions.origin.outputs().for_each(|i| { + old_outputs.insert(i); + }); - // two list are in sorted order, we can merge them in linear time. - while let (Some(&old_output), Some(&new_output)) = (old_outputs.peek(), new_outputs.peek()) - { - #[allow(clippy::comparison_chain)] - if old_output < new_output { - // Output that was generated but is no longer. - Self::report_stale_output(db, key, old_output); - old_outputs.next(); - } else if new_output < old_output { - // This is a new output that was not generated before. - // No action needed. - new_outputs.next(); - } else { - // Output generated both times. - old_outputs.next(); - new_outputs.next(); + // Iterate over the outputs of the current query + // and remove elements from `old_outputs` when we find them + for new_output in revisions.origin.outputs() { + if old_outputs.contains(&new_output) { + old_outputs.remove(&new_output); } } From a2be847e1a915fe9711e533fce04d2ff1fa0204e Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 24 Sep 2022 00:20:03 +0000 Subject: [PATCH 09/19] remove unused variables and functions --- components/salsa-2022/src/function/execute.rs | 2 +- components/salsa-2022/src/runtime.rs | 4 ---- components/salsa-2022/src/runtime/active_query.rs | 4 ++-- components/salsa-2022/src/runtime/local_state.rs | 5 ++--- components/salsa-2022/src/runtime/shared_state.rs | 8 ++------ 5 files changed, 7 insertions(+), 16 deletions(-) diff --git a/components/salsa-2022/src/function/execute.rs b/components/salsa-2022/src/function/execute.rs index c05ad328..6857e579 100644 --- a/components/salsa-2022/src/function/execute.rs +++ b/components/salsa-2022/src/function/execute.rs @@ -71,7 +71,7 @@ where } } }; - let mut revisions = active_query.pop(runtime); + let mut revisions = active_query.pop(); // We assume that query is side-effect free -- that is, does // not mutate the "inputs" to the query system. Sanity check diff --git a/components/salsa-2022/src/runtime.rs b/components/salsa-2022/src/runtime.rs index 710ec67a..9bc7f12b 100644 --- a/components/salsa-2022/src/runtime.rs +++ b/components/salsa-2022/src/runtime.rs @@ -97,10 +97,6 @@ impl Runtime { self.local_state.active_query() } - pub(crate) fn empty_dependencies(&self) -> Arc<[DependencyIndex]> { - self.shared_state.empty_dependencies.clone() - } - pub fn snapshot(&self) -> Self { if self.local_state.query_in_progress() { panic!("it is not legal to `snapshot` during a query (see salsa-rs/salsa#80)"); diff --git a/components/salsa-2022/src/runtime/active_query.rs b/components/salsa-2022/src/runtime/active_query.rs index cf972d39..a97bf62f 100644 --- a/components/salsa-2022/src/runtime/active_query.rs +++ b/components/salsa-2022/src/runtime/active_query.rs @@ -3,7 +3,7 @@ use crate::{ hash::{FxIndexMap, FxIndexSet}, key::{DatabaseKeyIndex, DependencyIndex}, tracked_struct::Disambiguator, - Cycle, Revision, Runtime, + Cycle, Revision, }; use super::local_state::{EdgeKind, QueryEdges, QueryOrigin, QueryRevisions}; @@ -87,7 +87,7 @@ impl ActiveQuery { self.input_outputs.contains(&(EdgeKind::Output, key)) } - pub(crate) fn revisions(&self, runtime: &Runtime) -> QueryRevisions { + pub(crate) fn revisions(&self) -> QueryRevisions { let input_outputs = self.input_outputs.iter().copied().collect(); let edges = QueryEdges::new(input_outputs); diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index 3c8cb008..3d0bb3c5 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -6,7 +6,6 @@ use crate::key::DependencyIndex; use crate::runtime::Revision; use crate::tracked_struct::Disambiguator; use crate::Cycle; -use crate::Runtime; use std::cell::RefCell; use std::sync::Arc; @@ -339,14 +338,14 @@ impl ActiveQueryGuard<'_> { /// which summarizes the other queries that were accessed during this /// query's execution. #[inline] - pub(crate) fn pop(self, runtime: &Runtime) -> QueryRevisions { + pub(crate) fn pop(self) -> QueryRevisions { // Extract accumulated inputs. let popped_query = self.complete(); // If this frame were a cycle participant, it would have unwound. assert!(popped_query.cycle.is_none()); - popped_query.revisions(runtime) + popped_query.revisions() } /// If the active query is registered as a cycle participant, remove and diff --git a/components/salsa-2022/src/runtime/shared_state.rs b/components/salsa-2022/src/runtime/shared_state.rs index f7971150..14149001 100644 --- a/components/salsa-2022/src/runtime/shared_state.rs +++ b/components/salsa-2022/src/runtime/shared_state.rs @@ -1,9 +1,9 @@ -use std::sync::{atomic::AtomicUsize, Arc}; +use std::sync::atomic::AtomicUsize; use crossbeam::atomic::AtomicCell; use parking_lot::Mutex; -use crate::{durability::Durability, key::DependencyIndex, revision::AtomicRevision}; +use crate::{durability::Durability, revision::AtomicRevision}; use super::dependency_graph::DependencyGraph; @@ -13,9 +13,6 @@ pub(super) struct SharedState { /// Stores the next id to use for a snapshotted runtime (starts at 1). pub(super) next_id: AtomicUsize, - /// Vector we can clone - pub(super) empty_dependencies: Arc<[DependencyIndex]>, - /// Set to true when the current revision has been canceled. /// This is done when we an input is being changed. The flag /// is set back to false once the input has been changed. @@ -47,7 +44,6 @@ impl SharedState { fn with_durabilities(durabilities: usize) -> Self { SharedState { next_id: AtomicUsize::new(1), - empty_dependencies: None.into_iter().collect(), revision_canceled: Default::default(), revisions: (0..durabilities).map(|_| AtomicRevision::start()).collect(), dependency_graph: Default::default(), From 559c02ba809e8ad5ee9cce479e685eb9a8554514 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 24 Sep 2022 00:41:55 +0000 Subject: [PATCH 10/19] update a test --- .../tests/tracked_fn_read_own_specify.rs | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs b/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs index 638a0056..b31c6a54 100644 --- a/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs +++ b/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs @@ -1,9 +1,11 @@ -#![allow(warnings)] +use expect_test::expect; +use salsa::{Database as SalsaDatabase, DebugWithDb}; +use salsa_2022_tests::{HasLogger, Logger}; #[salsa::jar(db = Db)] struct Jar(MyInput, MyTracked, tracked_fn, tracked_fn_extra); -trait Db: salsa::DbWithJar {} +trait Db: salsa::DbWithJar + HasLogger {} #[salsa::input(jar = Jar)] struct MyInput { @@ -17,13 +19,15 @@ struct MyTracked { #[salsa::tracked(jar = Jar)] fn tracked_fn(db: &dyn Db, input: MyInput) -> u32 { + db.push_log(format!("tracked_fn({:?})", input.debug(db))); let t = MyTracked::new(db, input.field(db) * 2); tracked_fn_extra::specify(db, t, 2222); tracked_fn_extra(db, t) } #[salsa::tracked(jar = Jar, specify)] -fn tracked_fn_extra(_db: &dyn Db, _input: MyTracked) -> u32 { +fn tracked_fn_extra(db: &dyn Db, input: MyTracked) -> u32 { + db.push_log(format!("tracked_fn_extra({:?})", input.debug(db))); 0 } @@ -31,18 +35,34 @@ fn tracked_fn_extra(_db: &dyn Db, _input: MyTracked) -> u32 { #[derive(Default)] struct Database { storage: salsa::Storage, + logger: Logger, } impl salsa::Database for Database {} impl Db for Database {} +impl HasLogger for Database { + fn logger(&self) -> &Logger { + &self.logger + } +} + #[test] fn execute() { let mut db = Database::default(); - let input = MyInput::new(&mut db, 22); + let input = MyInput::new(&db, 22); assert_eq!(tracked_fn(&db, input), 2222); + db.assert_logs(expect![[r#" + [ + "tracked_fn(MyInput { [salsa id]: 0 })", + ]"#]]); - let input2 = MyInput::new(&mut db, 44); + // A "synthetic write" causes the system to act *as though* some + // input of durability `durability` has changed. + db.synthetic_write(salsa::Durability::LOW); + + // Re-run the query on the original input. Nothing re-executes! assert_eq!(tracked_fn(&db, input), 2222); + db.assert_logs(expect!["[]"]); } From 1214610451e221e9036be38aa9bb52298c5c4c12 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 24 Sep 2022 01:02:29 +0000 Subject: [PATCH 11/19] remove inputs method of QueryEdges --- .../salsa-2022/src/function/accumulated.rs | 20 ++++++++----------- .../salsa-2022/src/runtime/local_state.rs | 15 ++------------ 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/components/salsa-2022/src/function/accumulated.rs b/components/salsa-2022/src/function/accumulated.rs index b44d737b..1a159b65 100644 --- a/components/salsa-2022/src/function/accumulated.rs +++ b/components/salsa-2022/src/function/accumulated.rs @@ -1,7 +1,6 @@ use crate::{ hash::FxHashSet, - key::DependencyIndex, - runtime::local_state::QueryOrigin, + runtime::local_state::{EdgeKind, QueryOrigin}, storage::{HasJar, HasJarsDyn}, DatabaseKeyIndex, }; @@ -67,18 +66,15 @@ impl Stack { match origin { None | Some(QueryOrigin::Assigned(_)) | Some(QueryOrigin::BaseInput) => {} Some(QueryOrigin::Derived(edges)) | Some(QueryOrigin::DerivedUntracked(edges)) => { - for DependencyIndex { - ingredient_index, - key_index, - } in edges.inputs().iter().copied() + for (_, dependency_index) in edges + .input_outputs + .iter() + .filter(|edge| edge.0 == EdgeKind::Input) + .copied() { - if let Some(key_index) = key_index { - let i = DatabaseKeyIndex { - ingredient_index, - key_index, - }; + if let Ok(i) = DatabaseKeyIndex::try_from(dependency_index) { if self.s.insert(i) { - self.v.push(i); + self.v.push(i) } } } diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index 3d0bb3c5..caa3ba91 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -76,7 +76,7 @@ pub enum QueryOrigin { impl QueryOrigin { /// Indices for queries *written* by this query (or `vec![]` if its value was assigned). - pub(crate) fn outputs(&self) -> impl Iterator + '_ { + pub(crate) fn outputs(&self) -> impl Iterator { let slice = match self { QueryOrigin::Derived(edges) | QueryOrigin::DerivedUntracked(edges) => edges.outputs(), QueryOrigin::Assigned(_) | QueryOrigin::BaseInput => vec![], @@ -115,18 +115,7 @@ pub struct QueryEdges { } impl QueryEdges { - /// Returns the (tracked) inputs that were executed in computing this memoized value. - /// - /// These will always be in execution order. - pub(crate) fn inputs(&self) -> Vec { - self.input_outputs - .iter() - .filter(|(edge_kind, _)| *edge_kind == EdgeKind::Input) - .map(|(_, dependency_index)| *dependency_index) - .collect() - } - - /// Returns the (tracked) inputs that were executed in computing this memoized value. + /// Returns the (tracked) outputs that were executed in computing this memoized value. /// /// These will always be in execution order. pub(crate) fn outputs(&self) -> Vec { From e7c7f386fe4b6c87fb610389cf32d89613551bf8 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sun, 25 Sep 2022 00:24:16 +0000 Subject: [PATCH 12/19] fix accumulator: clear the outdated accumulated values --- components/salsa-2022/src/accumulator.rs | 6 ++++-- components/salsa-2022/src/function/specify.rs | 3 ++- components/salsa-2022/src/runtime.rs | 2 +- components/salsa-2022/src/runtime/active_query.rs | 3 +-- components/salsa-2022/src/runtime/local_state.rs | 2 +- salsa-2022-tests/tests/accumulate-reuse-workaround.rs | 1 - 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/components/salsa-2022/src/accumulator.rs b/components/salsa-2022/src/accumulator.rs index 3898f9d5..69ea998c 100644 --- a/components/salsa-2022/src/accumulator.rs +++ b/components/salsa-2022/src/accumulator.rs @@ -61,8 +61,10 @@ impl AccumulatorIngredient { produced_at: current_revision, }); - // This is the first push in a new revision. Reset. - if accumulated_values.produced_at != current_revision { + // When we call `push' in a query, we will add the accumulator to the output of the query. + // If we find here that this accumulator is not the output of the query, + // we can say that the accumulated values we stored for this query is out of date. + if !runtime.is_output_of_active_query(self.dependency_index()) { accumulated_values.values.truncate(0); accumulated_values.produced_at = current_revision; } diff --git a/components/salsa-2022/src/function/specify.rs b/components/salsa-2022/src/function/specify.rs index 0c215383..1904cc63 100644 --- a/components/salsa-2022/src/function/specify.rs +++ b/components/salsa-2022/src/function/specify.rs @@ -46,7 +46,8 @@ where // // Now, if We invoke Q3 first, We get one result for Q2, but if We invoke Q4 first, We get a different value. That's no good. let database_key_index = key.database_key_index(db); - if !runtime.is_output_of_active_query(database_key_index) { + let dependency_index = database_key_index.into(); + if !runtime.is_output_of_active_query(dependency_index) { panic!("can only use `specfiy` on entities created during current query"); } diff --git a/components/salsa-2022/src/runtime.rs b/components/salsa-2022/src/runtime.rs index 9bc7f12b..a5e36648 100644 --- a/components/salsa-2022/src/runtime.rs +++ b/components/salsa-2022/src/runtime.rs @@ -149,7 +149,7 @@ impl Runtime { } /// Check whether `entity` is contained the list of outputs written by the current query. - pub(super) fn is_output_of_active_query(&self, entity: DatabaseKeyIndex) -> bool { + pub(super) fn is_output_of_active_query(&self, entity: DependencyIndex) -> bool { self.local_state.is_output(entity) } diff --git a/components/salsa-2022/src/runtime/active_query.rs b/components/salsa-2022/src/runtime/active_query.rs index a97bf62f..5ffcae6a 100644 --- a/components/salsa-2022/src/runtime/active_query.rs +++ b/components/salsa-2022/src/runtime/active_query.rs @@ -82,8 +82,7 @@ impl ActiveQuery { } /// True if the given key was output by this query. - pub(super) fn is_output(&self, key: DatabaseKeyIndex) -> bool { - let key: DependencyIndex = key.into(); + pub(super) fn is_output(&self, key: DependencyIndex) -> bool { self.input_outputs.contains(&(EdgeKind::Output, key)) } diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index caa3ba91..272ca934 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -190,7 +190,7 @@ impl LocalState { }) } - pub(super) fn is_output(&self, entity: DatabaseKeyIndex) -> bool { + pub(super) fn is_output(&self, entity: DependencyIndex) -> bool { self.with_query_stack(|stack| { if let Some(top_query) = stack.last_mut() { top_query.is_output(entity) diff --git a/salsa-2022-tests/tests/accumulate-reuse-workaround.rs b/salsa-2022-tests/tests/accumulate-reuse-workaround.rs index f06ebb06..e960db1b 100644 --- a/salsa-2022-tests/tests/accumulate-reuse-workaround.rs +++ b/salsa-2022-tests/tests/accumulate-reuse-workaround.rs @@ -89,6 +89,5 @@ fn test1() { [ "accumulated(List(Id { value: 1 }))", "compute(List(Id { value: 1 }))", - "compute(List(Id { value: 2 }))", ]"#]]); } From cc6ab647fa28d37c12995bf7f6c69547dbd57a05 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sun, 25 Sep 2022 01:21:39 +0000 Subject: [PATCH 13/19] refactor outputs method of LocalState --- .../salsa-2022/src/function/accumulated.rs | 9 ++----- .../salsa-2022/src/runtime/local_state.rs | 27 ++++++++++++------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/components/salsa-2022/src/function/accumulated.rs b/components/salsa-2022/src/function/accumulated.rs index 1a159b65..9ee8b9e3 100644 --- a/components/salsa-2022/src/function/accumulated.rs +++ b/components/salsa-2022/src/function/accumulated.rs @@ -1,6 +1,6 @@ use crate::{ hash::FxHashSet, - runtime::local_state::{EdgeKind, QueryOrigin}, + runtime::local_state::QueryOrigin, storage::{HasJar, HasJarsDyn}, DatabaseKeyIndex, }; @@ -66,12 +66,7 @@ impl Stack { match origin { None | Some(QueryOrigin::Assigned(_)) | Some(QueryOrigin::BaseInput) => {} Some(QueryOrigin::Derived(edges)) | Some(QueryOrigin::DerivedUntracked(edges)) => { - for (_, dependency_index) in edges - .input_outputs - .iter() - .filter(|edge| edge.0 == EdgeKind::Input) - .copied() - { + for dependency_index in edges.inputs() { if let Ok(i) = DatabaseKeyIndex::try_from(dependency_index) { if self.s.insert(i) { self.v.push(i) diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index 272ca934..19506aaa 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -76,13 +76,13 @@ pub enum QueryOrigin { impl QueryOrigin { /// Indices for queries *written* by this query (or `vec![]` if its value was assigned). - pub(crate) fn outputs(&self) -> impl Iterator { - let slice = match self { - QueryOrigin::Derived(edges) | QueryOrigin::DerivedUntracked(edges) => edges.outputs(), - QueryOrigin::Assigned(_) | QueryOrigin::BaseInput => vec![], - }; - - slice.into_iter() + pub(crate) fn outputs(&self) -> Box + '_> { + match self { + QueryOrigin::Derived(edges) | QueryOrigin::DerivedUntracked(edges) => { + Box::new(edges.outputs()) + } + QueryOrigin::Assigned(_) | QueryOrigin::BaseInput => Box::new(vec![].into_iter()), + } } } @@ -115,15 +115,24 @@ pub struct QueryEdges { } impl QueryEdges { + /// Returns the (tracked) inputs that were executed in computing this memoized value. + /// + /// These will always be in execution order. + pub(crate) fn inputs(&self) -> impl Iterator + '_ { + self.input_outputs + .iter() + .filter(|(edge_kind, _)| *edge_kind == EdgeKind::Input) + .map(|(_, dependency_index)| *dependency_index) + } + /// Returns the (tracked) outputs that were executed in computing this memoized value. /// /// These will always be in execution order. - pub(crate) fn outputs(&self) -> Vec { + pub(crate) fn outputs(&self) -> impl Iterator + '_ { self.input_outputs .iter() .filter(|(edge_kind, _)| *edge_kind == EdgeKind::Output) .map(|(_, dependency_index)| *dependency_index) - .collect() } /// Creates a new `QueryEdges`; the values given for each field must meet struct invariants. From a8e16a72d248e39498c2249591167aee4e8735ad Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sun, 25 Sep 2022 01:23:36 +0000 Subject: [PATCH 14/19] update test: don't need mut reference when create inputs --- salsa-2022-tests/tests/deletion-cascade.rs | 2 +- ...elds-of-tracked-structs-from-older-revisions.rs | 2 +- salsa-2022-tests/tests/singleton.rs | 14 +++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/salsa-2022-tests/tests/deletion-cascade.rs b/salsa-2022-tests/tests/deletion-cascade.rs index c04473b2..d6343203 100644 --- a/salsa-2022-tests/tests/deletion-cascade.rs +++ b/salsa-2022-tests/tests/deletion-cascade.rs @@ -91,7 +91,7 @@ fn basic() { let mut db = Database::default(); // Creates 3 tracked structs - let input = MyInput::new(&mut db, 3); + let input = MyInput::new(&db, 3); assert_eq!(final_result(&db, input), 2 * 2 + 2); db.assert_logs(expect![[r#" [ diff --git a/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs b/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs index b0d17ffc..8288f948 100644 --- a/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs +++ b/salsa-2022-tests/tests/panic-when-reading-fields-of-tracked-structs-from-older-revisions.rs @@ -35,7 +35,7 @@ impl Db for Database {} fn execute() { let mut db = Database::default(); - let input = MyInput::new(&mut db, 22); + let input = MyInput::new(&db, 22); let tracked = tracked_fn(&db, input); // modify the input and change the revision diff --git a/salsa-2022-tests/tests/singleton.rs b/salsa-2022-tests/tests/singleton.rs index 6d8de8b9..275a2d3c 100644 --- a/salsa-2022-tests/tests/singleton.rs +++ b/salsa-2022-tests/tests/singleton.rs @@ -39,8 +39,8 @@ impl HasLogger for Database { #[test] fn basic() { - let mut db = Database::default(); - let input1 = MyInput::new(&mut db, 3, 4); + let db = Database::default(); + let input1 = MyInput::new(&db, 3, 4); let input2 = MyInput::get(&db); assert_eq!(input1, input2); @@ -52,20 +52,20 @@ fn basic() { #[test] #[should_panic] fn twice() { - let mut db = Database::default(); - let input1 = MyInput::new(&mut db, 3, 4); + let db = Database::default(); + let input1 = MyInput::new(&db, 3, 4); let input2 = MyInput::get(&db); assert_eq!(input1, input2); // should panic here - _ = MyInput::new(&mut db, 3, 5); + _ = MyInput::new(&db, 3, 5); } #[test] fn debug() { - let mut db = Database::default(); - let input = MyInput::new(&mut db, 3, 4); + let db = Database::default(); + let input = MyInput::new(&db, 3, 4); let actual = format!("{:?}", input.debug(&db)); let expected = expect![[r#"MyInput { [salsa id]: 0, id_field: 4 }"#]]; expected.assert_eq(&actual); From 6239fc217a5fe3d3113237047730f03f37b02767 Mon Sep 17 00:00:00 2001 From: zhou fan <1247714429@qq.com> Date: Mon, 26 Sep 2022 17:09:19 +0800 Subject: [PATCH 15/19] add a comment Co-authored-by: Niko Matsakis --- .../src/function/maybe_changed_after.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/components/salsa-2022/src/function/maybe_changed_after.rs b/components/salsa-2022/src/function/maybe_changed_after.rs index c407bca6..c8d454b9 100644 --- a/components/salsa-2022/src/function/maybe_changed_after.rs +++ b/components/salsa-2022/src/function/maybe_changed_after.rs @@ -194,6 +194,22 @@ where } } EdgeKind::Output => { + // Subtle: Mark outputs as validated now, even though we may + // later find an input that requires us to re-execute the function. + // Even if it re-execute, the function will wind up writing the same value, + // since all prior inputs were green. It's important to do this during + // this loop, because it's possible that one of our input queries will + // re-execute and may read one of our earlier outputs + // (e.g., in a scenario where we do something like + // `e = Entity::new(..); query(e);` and `query` reads a field of `e`). + // + // NB. Accumulators are also outputs, but the above logic doesn't + // quite apply to them. Since multiple values are pushed, the first value + // may be unchanged, but later values could be different. + // In that case, however, the data accumulated + // by this function cannot be read until this function is marked green, + // so even if we mark them as valid here, the function will re-execute + // and overwrite the contents. db.mark_validated_output(database_key_index, dependency_index); } } From 409ed367c2944ba078e16aeb52234e0b456bda53 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Mon, 26 Sep 2022 09:11:16 +0000 Subject: [PATCH 16/19] using flat-map trick --- components/salsa-2022/src/runtime/local_state.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index 19506aaa..417872f6 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -76,13 +76,12 @@ pub enum QueryOrigin { impl QueryOrigin { /// Indices for queries *written* by this query (or `vec![]` if its value was assigned). - pub(crate) fn outputs(&self) -> Box + '_> { - match self { - QueryOrigin::Derived(edges) | QueryOrigin::DerivedUntracked(edges) => { - Box::new(edges.outputs()) - } - QueryOrigin::Assigned(_) | QueryOrigin::BaseInput => Box::new(vec![].into_iter()), - } + pub(crate) fn outputs(&self) -> impl Iterator + '_ { + let opt_edges = match self { + QueryOrigin::Derived(edges) | QueryOrigin::DerivedUntracked(edges) => Some(edges), + QueryOrigin::Assigned(_) | QueryOrigin::BaseInput => None, + }; + opt_edges.into_iter().flat_map(|edges| edges.outputs()) } } From 8f85d4ee279832d44842682adf2e7c1d460cfbea Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Mon, 26 Sep 2022 09:15:10 +0000 Subject: [PATCH 17/19] cargo fmt --- components/salsa-2022/src/function/maybe_changed_after.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/salsa-2022/src/function/maybe_changed_after.rs b/components/salsa-2022/src/function/maybe_changed_after.rs index c8d454b9..5ab74d60 100644 --- a/components/salsa-2022/src/function/maybe_changed_after.rs +++ b/components/salsa-2022/src/function/maybe_changed_after.rs @@ -200,8 +200,8 @@ where // since all prior inputs were green. It's important to do this during // this loop, because it's possible that one of our input queries will // re-execute and may read one of our earlier outputs - // (e.g., in a scenario where we do something like - // `e = Entity::new(..); query(e);` and `query` reads a field of `e`). + // (e.g., in a scenario where we do something like + // `e = Entity::new(..); query(e);` and `query` reads a field of `e`). // // NB. Accumulators are also outputs, but the above logic doesn't // quite apply to them. Since multiple values are pushed, the first value From 589bbeb65ea53183c3a4e85810c3f6d189018030 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Mon, 26 Sep 2022 10:51:23 +0000 Subject: [PATCH 18/19] Revert "remove unused variables and functions" This reverts commit a2be847e1a915fe9711e533fce04d2ff1fa0204e. --- components/salsa-2022/src/function/execute.rs | 2 +- components/salsa-2022/src/runtime.rs | 4 ++++ components/salsa-2022/src/runtime/active_query.rs | 4 ++-- components/salsa-2022/src/runtime/local_state.rs | 5 +++-- components/salsa-2022/src/runtime/shared_state.rs | 8 ++++++-- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/components/salsa-2022/src/function/execute.rs b/components/salsa-2022/src/function/execute.rs index 6857e579..c05ad328 100644 --- a/components/salsa-2022/src/function/execute.rs +++ b/components/salsa-2022/src/function/execute.rs @@ -71,7 +71,7 @@ where } } }; - let mut revisions = active_query.pop(); + let mut revisions = active_query.pop(runtime); // We assume that query is side-effect free -- that is, does // not mutate the "inputs" to the query system. Sanity check diff --git a/components/salsa-2022/src/runtime.rs b/components/salsa-2022/src/runtime.rs index a5e36648..73c9af53 100644 --- a/components/salsa-2022/src/runtime.rs +++ b/components/salsa-2022/src/runtime.rs @@ -97,6 +97,10 @@ impl Runtime { self.local_state.active_query() } + pub(crate) fn empty_dependencies(&self) -> Arc<[DependencyIndex]> { + self.shared_state.empty_dependencies.clone() + } + pub fn snapshot(&self) -> Self { if self.local_state.query_in_progress() { panic!("it is not legal to `snapshot` during a query (see salsa-rs/salsa#80)"); diff --git a/components/salsa-2022/src/runtime/active_query.rs b/components/salsa-2022/src/runtime/active_query.rs index 5ffcae6a..0707505a 100644 --- a/components/salsa-2022/src/runtime/active_query.rs +++ b/components/salsa-2022/src/runtime/active_query.rs @@ -3,7 +3,7 @@ use crate::{ hash::{FxIndexMap, FxIndexSet}, key::{DatabaseKeyIndex, DependencyIndex}, tracked_struct::Disambiguator, - Cycle, Revision, + Cycle, Revision, Runtime, }; use super::local_state::{EdgeKind, QueryEdges, QueryOrigin, QueryRevisions}; @@ -86,7 +86,7 @@ impl ActiveQuery { self.input_outputs.contains(&(EdgeKind::Output, key)) } - pub(crate) fn revisions(&self) -> QueryRevisions { + pub(crate) fn revisions(&self, runtime: &Runtime) -> QueryRevisions { let input_outputs = self.input_outputs.iter().copied().collect(); let edges = QueryEdges::new(input_outputs); diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index 417872f6..91f26ed7 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -6,6 +6,7 @@ use crate::key::DependencyIndex; use crate::runtime::Revision; use crate::tracked_struct::Disambiguator; use crate::Cycle; +use crate::Runtime; use std::cell::RefCell; use std::sync::Arc; @@ -335,14 +336,14 @@ impl ActiveQueryGuard<'_> { /// which summarizes the other queries that were accessed during this /// query's execution. #[inline] - pub(crate) fn pop(self) -> QueryRevisions { + pub(crate) fn pop(self, runtime: &Runtime) -> QueryRevisions { // Extract accumulated inputs. let popped_query = self.complete(); // If this frame were a cycle participant, it would have unwound. assert!(popped_query.cycle.is_none()); - popped_query.revisions() + popped_query.revisions(runtime) } /// If the active query is registered as a cycle participant, remove and diff --git a/components/salsa-2022/src/runtime/shared_state.rs b/components/salsa-2022/src/runtime/shared_state.rs index 14149001..f7971150 100644 --- a/components/salsa-2022/src/runtime/shared_state.rs +++ b/components/salsa-2022/src/runtime/shared_state.rs @@ -1,9 +1,9 @@ -use std::sync::atomic::AtomicUsize; +use std::sync::{atomic::AtomicUsize, Arc}; use crossbeam::atomic::AtomicCell; use parking_lot::Mutex; -use crate::{durability::Durability, revision::AtomicRevision}; +use crate::{durability::Durability, key::DependencyIndex, revision::AtomicRevision}; use super::dependency_graph::DependencyGraph; @@ -13,6 +13,9 @@ pub(super) struct SharedState { /// Stores the next id to use for a snapshotted runtime (starts at 1). pub(super) next_id: AtomicUsize, + /// Vector we can clone + pub(super) empty_dependencies: Arc<[DependencyIndex]>, + /// Set to true when the current revision has been canceled. /// This is done when we an input is being changed. The flag /// is set back to false once the input has been changed. @@ -44,6 +47,7 @@ impl SharedState { fn with_durabilities(durabilities: usize) -> Self { SharedState { next_id: AtomicUsize::new(1), + empty_dependencies: None.into_iter().collect(), revision_canceled: Default::default(), revisions: (0..durabilities).map(|_| AtomicRevision::start()).collect(), dependency_graph: Default::default(), From f48ca09a65354ea11be6e57484d4d1d64a4a8e1d Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Mon, 26 Sep 2022 10:58:21 +0000 Subject: [PATCH 19/19] add empty dependencies back --- components/salsa-2022/src/runtime.rs | 7 +++++-- components/salsa-2022/src/runtime/active_query.rs | 6 +++++- components/salsa-2022/src/runtime/shared_state.rs | 4 ++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/components/salsa-2022/src/runtime.rs b/components/salsa-2022/src/runtime.rs index 73c9af53..ad2cc879 100644 --- a/components/salsa-2022/src/runtime.rs +++ b/components/salsa-2022/src/runtime.rs @@ -12,7 +12,10 @@ use crate::{ Cancelled, Cycle, Database, Event, EventKind, Revision, }; -use self::{dependency_graph::DependencyGraph, local_state::ActiveQueryGuard}; +use self::{ + dependency_graph::DependencyGraph, + local_state::{ActiveQueryGuard, EdgeKind}, +}; use super::{tracked_struct::Disambiguator, IngredientIndex}; @@ -97,7 +100,7 @@ impl Runtime { self.local_state.active_query() } - pub(crate) fn empty_dependencies(&self) -> Arc<[DependencyIndex]> { + pub(crate) fn empty_dependencies(&self) -> Arc<[(EdgeKind, DependencyIndex)]> { self.shared_state.empty_dependencies.clone() } diff --git a/components/salsa-2022/src/runtime/active_query.rs b/components/salsa-2022/src/runtime/active_query.rs index 0707505a..ebe33a48 100644 --- a/components/salsa-2022/src/runtime/active_query.rs +++ b/components/salsa-2022/src/runtime/active_query.rs @@ -87,7 +87,11 @@ impl ActiveQuery { } pub(crate) fn revisions(&self, runtime: &Runtime) -> QueryRevisions { - let input_outputs = self.input_outputs.iter().copied().collect(); + let input_outputs = if self.input_outputs.is_empty() { + runtime.empty_dependencies() + } else { + self.input_outputs.iter().copied().collect() + }; let edges = QueryEdges::new(input_outputs); diff --git a/components/salsa-2022/src/runtime/shared_state.rs b/components/salsa-2022/src/runtime/shared_state.rs index f7971150..9a05b52e 100644 --- a/components/salsa-2022/src/runtime/shared_state.rs +++ b/components/salsa-2022/src/runtime/shared_state.rs @@ -5,7 +5,7 @@ use parking_lot::Mutex; use crate::{durability::Durability, key::DependencyIndex, revision::AtomicRevision}; -use super::dependency_graph::DependencyGraph; +use super::{dependency_graph::DependencyGraph, local_state::EdgeKind}; /// State that will be common to all threads (when we support multiple threads) #[derive(Debug)] @@ -14,7 +14,7 @@ pub(super) struct SharedState { pub(super) next_id: AtomicUsize, /// Vector we can clone - pub(super) empty_dependencies: Arc<[DependencyIndex]>, + pub(super) empty_dependencies: Arc<[(EdgeKind, DependencyIndex)]>, /// Set to true when the current revision has been canceled. /// This is done when we an input is being changed. The flag