From b8643a5f702eacd304a7f6da95acb1821dc476d1 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 13 Aug 2022 00:42:40 -0400 Subject: [PATCH] distinguish fields from other specified values And make `QueryOrigin` finer grained while we are at it. --- .../salsa-2022-macros/src/tracked_struct.rs | 2 +- .../salsa-2022/src/function/accumulated.rs | 5 +++- components/salsa-2022/src/function/fetch.rs | 5 +++- .../src/function/maybe_changed_after.rs | 8 +++-- components/salsa-2022/src/function/specify.rs | 29 +++++++++++++++---- components/salsa-2022/src/function/store.rs | 2 +- .../salsa-2022/src/runtime/local_state.rs | 17 +++++++---- 7 files changed, 49 insertions(+), 19 deletions(-) diff --git a/components/salsa-2022-macros/src/tracked_struct.rs b/components/salsa-2022-macros/src/tracked_struct.rs index 273b4be7..b2dcc11f 100644 --- a/components/salsa-2022-macros/src/tracked_struct.rs +++ b/components/salsa-2022-macros/src/tracked_struct.rs @@ -133,7 +133,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(__db, __id, #value_field_names); + __ingredients.#value_field_indices.specify_field(__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 a98cbcf2..df3effc9 100644 --- a/components/salsa-2022/src/function/accumulated.rs +++ b/components/salsa-2022/src/function/accumulated.rs @@ -65,7 +65,10 @@ impl Stack { /// Extend the stack of queries with the dependencies from `origin`. fn extend(&mut self, origin: Option) { match origin { - None | Some(QueryOrigin::Assigned(_)) => {} + None + | Some(QueryOrigin::Assigned(_)) + | Some(QueryOrigin::BaseInput) + | Some(QueryOrigin::Field) => {} Some(QueryOrigin::Derived(edges)) | Some(QueryOrigin::DerivedUntracked(edges)) => { for DependencyIndex { ingredient_index, diff --git a/components/salsa-2022/src/function/fetch.rs b/components/salsa-2022/src/function/fetch.rs index e29e9091..a6b30dc9 100644 --- a/components/salsa-2022/src/function/fetch.rs +++ b/components/salsa-2022/src/function/fetch.rs @@ -96,7 +96,10 @@ where fn evict(&self, key: C::Key) { if let Some(memo) = self.memo_map.get(key) { match memo.revisions.origin { - QueryOrigin::Assigned(_) | QueryOrigin::DerivedUntracked(_) => { + QueryOrigin::Assigned(_) + | QueryOrigin::DerivedUntracked(_) + | QueryOrigin::BaseInput + | QueryOrigin::Field => { // 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/maybe_changed_after.rs b/components/salsa-2022/src/function/maybe_changed_after.rs index 390d941a..9f64e1da 100644 --- a/components/salsa-2022/src/function/maybe_changed_after.rs +++ b/components/salsa-2022/src/function/maybe_changed_after.rs @@ -162,7 +162,7 @@ where } match &old_memo.revisions.origin { - QueryOrigin::Assigned(Some(_)) => { + QueryOrigin::Assigned(_) => { // If the value was assigneed by another query, // and that query were up-to-date, // then we would have updated the `verified_at` field already. @@ -170,8 +170,10 @@ where // during this revision or is otherwise stale. return false; } - QueryOrigin::Assigned(None) => { - // This value was `set` by the mutator thread -- ie, it's a base input and it cannot be out of date. + 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. return true; } QueryOrigin::DerivedUntracked(_) => { diff --git a/components/salsa-2022/src/function/specify.rs b/components/salsa-2022/src/function/specify.rs index 1dbaf55e..25a4d45a 100644 --- a/components/salsa-2022/src/function/specify.rs +++ b/components/salsa-2022/src/function/specify.rs @@ -16,8 +16,13 @@ where /// Specifies the value of the function for the given key. /// This is a way to imperatively set the value of a function. /// It only works if the key is a tracked struct created in the current query. - pub fn specify<'db>(&self, db: &'db DynDb<'db, C>, key: C::Key, value: C::Value) - where + pub(crate) fn specify<'db>( + &self, + db: &'db DynDb<'db, C>, + key: C::Key, + value: C::Value, + origin: impl Fn(DatabaseKeyIndex) -> QueryOrigin, + ) where C::Key: TrackedStructInDb>, { let runtime = db.salsa_runtime(); @@ -55,7 +60,7 @@ where let mut revisions = QueryRevisions { changed_at: current_deps.changed_at, durability: current_deps.durability, - origin: QueryOrigin::Assigned(Some(active_query_key)), + origin: origin(active_query_key), }; if let Some(old_memo) = self.memo_map.get(key) { @@ -73,13 +78,26 @@ where self.insert_memo(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 beacuse 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); + } + /// 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) where C::Key: TrackedStructInDb>, { - self.specify(db, key, value); + self.specify(db, key, value, |database_key_index| { + QueryOrigin::Assigned(database_key_index) + }); // Record that the current query *specified* a value for this cell. let database_key_index = self.database_key_index(key); @@ -97,7 +115,6 @@ where key: C::Key, ) { let runtime = db.salsa_runtime(); - let current_revision = runtime.current_revision(); let memo = match self.memo_map.get(key) { Some(m) => m, @@ -107,7 +124,7 @@ where // If we are marking this as validated, it must be a value that was // assigneed by `executor`. match memo.revisions.origin { - QueryOrigin::Assigned(Some(by_query)) => assert_eq!(by_query, executor), + QueryOrigin::Assigned(by_query) => assert_eq!(by_query, executor), _ => panic!( "expected a query assigned by `{:?}`, not `{:?}`", executor.debug(db), diff --git a/components/salsa-2022/src/function/store.rs b/components/salsa-2022/src/function/store.rs index 04f5093d..a90b1245 100644 --- a/components/salsa-2022/src/function/store.rs +++ b/components/salsa-2022/src/function/store.rs @@ -28,7 +28,7 @@ where revisions: QueryRevisions { changed_at: revision, durability, - origin: QueryOrigin::Assigned(None), + origin: QueryOrigin::BaseInput, }, }; diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index 822fb250..06a675b4 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -56,11 +56,16 @@ impl QueryRevisions { /// Tracks the way that a memoized value for a query was created. #[derive(Debug, Clone)] pub enum QueryOrigin { - /// The value was assigned as the output of another query (e.g., using `specify`) - /// or is an input set by the mutator thread (e.g., using `set`). - /// The `DatabaseKeyIndex` is the identity of the assigning query - /// or `None` if this is an input set by the mutator thread. - Assigned(Option), + /// The value was assigned as the output of another query (e.g., using `specify`). + /// 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, /// The value was derived by executing a function /// and we were able to track ALL of that function's inputs. @@ -78,10 +83,10 @@ impl QueryOrigin { /// Indices for queries *written* by this query (or `&[]` if its value was assigned). pub(crate) fn outputs(&self) -> impl Iterator + '_ { let slice = match self { - QueryOrigin::Assigned(_) => &[], QueryOrigin::Derived(edges) | QueryOrigin::DerivedUntracked(edges) => { &edges.input_outputs[edges.separator as usize..] } + QueryOrigin::Assigned(_) | QueryOrigin::BaseInput | QueryOrigin::Field => &[], }; // the `QueryEdges` repr. invariant guarantees all tracked outputs are full 'dependency index' values,