From 49ccac5d3df8e0af5f9f68931497c6f4feb17147 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 11 Aug 2022 00:28:34 -0400 Subject: [PATCH] track both inputs/outputs for each query Rename QueryInputs to QueryEdges and modify its fields to track both inputs and outputs. The size of the struct doesn't actually change, the separator comes out of padding. --- components/salsa-2022-macros/src/db.rs | 2 +- components/salsa-2022/src/accumulator.rs | 4 +- components/salsa-2022/src/function.rs | 4 +- .../salsa-2022/src/function/accumulated.rs | 6 +-- components/salsa-2022/src/function/fetch.rs | 2 +- components/salsa-2022/src/function/inputs.rs | 6 +-- .../src/function/maybe_changed_after.rs | 4 +- components/salsa-2022/src/function/specify.rs | 9 ++-- components/salsa-2022/src/function/store.rs | 7 +-- components/salsa-2022/src/ingredient.rs | 4 +- components/salsa-2022/src/input.rs | 4 +- components/salsa-2022/src/interned.rs | 4 +- .../salsa-2022/src/runtime/active_query.rs | 25 +++++++--- .../salsa-2022/src/runtime/local_state.rs | 49 ++++++++++++++++--- components/salsa-2022/src/storage.rs | 4 +- components/salsa-2022/src/tracked_struct.rs | 4 +- 16 files changed, 92 insertions(+), 46 deletions(-) diff --git a/components/salsa-2022-macros/src/db.rs b/components/salsa-2022-macros/src/db.rs index 94c729ef..0d1151fc 100644 --- a/components/salsa-2022-macros/src/db.rs +++ b/components/salsa-2022-macros/src/db.rs @@ -136,7 +136,7 @@ fn has_jars_dyn_impl(input: &syn::ItemStruct, storage: &syn::Ident) -> syn::Item fn inputs( &self, index: salsa::DatabaseKeyIndex, - ) -> Option { + ) -> Option { let ingredient = self.#storage.ingredient(index.ingredient_index()); ingredient.inputs(index.key_index()) } diff --git a/components/salsa-2022/src/accumulator.rs b/components/salsa-2022/src/accumulator.rs index 07e12b47..a685ef67 100644 --- a/components/salsa-2022/src/accumulator.rs +++ b/components/salsa-2022/src/accumulator.rs @@ -3,7 +3,7 @@ use crate::{ hash::FxDashMap, ingredient::{Ingredient, MutIngredient}, key::DependencyIndex, - runtime::{local_state::QueryInputs, StampedValue}, + runtime::{local_state::QueryEdges, StampedValue}, storage::HasJar, DatabaseKeyIndex, Durability, IngredientIndex, Revision, Runtime, }; @@ -78,7 +78,7 @@ where CycleRecoveryStrategy::Panic } - fn inputs(&self, _key_index: crate::Id) -> Option { + fn inputs(&self, _key_index: crate::Id) -> Option { None } } diff --git a/components/salsa-2022/src/function.rs b/components/salsa-2022/src/function.rs index fddd3a88..1ac8793a 100644 --- a/components/salsa-2022/src/function.rs +++ b/components/salsa-2022/src/function.rs @@ -8,7 +8,7 @@ use crate::{ ingredient::MutIngredient, jar::Jar, key::{DatabaseKeyIndex, DependencyIndex}, - runtime::local_state::QueryInputs, + runtime::local_state::QueryEdges, salsa_struct::SalsaStructInDb, Cycle, DbWithJar, Id, Revision, }; @@ -198,7 +198,7 @@ where C::CYCLE_STRATEGY } - fn inputs(&self, key_index: Id) -> Option { + fn inputs(&self, key_index: Id) -> Option { let key = C::key_from_id(key_index); self.inputs(key) } diff --git a/components/salsa-2022/src/function/accumulated.rs b/components/salsa-2022/src/function/accumulated.rs index e4243cab..ee80a823 100644 --- a/components/salsa-2022/src/function/accumulated.rs +++ b/components/salsa-2022/src/function/accumulated.rs @@ -1,7 +1,7 @@ use crate::{ hash::FxHashSet, key::DependencyIndex, - runtime::local_state::QueryInputs, + runtime::local_state::QueryEdges, storage::{HasJar, HasJarsDyn}, Database, DatabaseKeyIndex, }; @@ -55,7 +55,7 @@ impl Stack { self.v.pop() } - fn extend(&mut self, inputs: Option) { + fn extend(&mut self, inputs: Option) { let inputs = match inputs { None => return, Some(v) => v, @@ -64,7 +64,7 @@ impl Stack { for DependencyIndex { ingredient_index, key_index, - } in inputs.tracked.iter().copied() + } in inputs.inputs().iter().copied() { if let Some(key_index) = key_index { let i = DatabaseKeyIndex { diff --git a/components/salsa-2022/src/function/fetch.rs b/components/salsa-2022/src/function/fetch.rs index d8b82732..ed20b0cf 100644 --- a/components/salsa-2022/src/function/fetch.rs +++ b/components/salsa-2022/src/function/fetch.rs @@ -93,7 +93,7 @@ where if let Some(memo) = self.memo_map.get(key) { // Careful: we can't evict memos with untracked inputs // as their values cannot be reconstructed. - if memo.revisions.inputs.untracked { + if memo.revisions.edges.untracked { return; } diff --git a/components/salsa-2022/src/function/inputs.rs b/components/salsa-2022/src/function/inputs.rs index 2ad9d0b2..4cfc4c80 100644 --- a/components/salsa-2022/src/function/inputs.rs +++ b/components/salsa-2022/src/function/inputs.rs @@ -1,4 +1,4 @@ -use crate::runtime::local_state::QueryInputs; +use crate::runtime::local_state::QueryEdges; use super::{Configuration, FunctionIngredient}; @@ -6,7 +6,7 @@ impl FunctionIngredient where C: Configuration, { - pub(super) fn inputs(&self, key: C::Key) -> Option { - self.memo_map.get(key).map(|m| m.revisions.inputs.clone()) + pub(super) fn inputs(&self, key: C::Key) -> Option { + self.memo_map.get(key).map(|m| m.revisions.edges.clone()) } } diff --git a/components/salsa-2022/src/function/maybe_changed_after.rs b/components/salsa-2022/src/function/maybe_changed_after.rs index b576b770..abcde4b9 100644 --- a/components/salsa-2022/src/function/maybe_changed_after.rs +++ b/components/salsa-2022/src/function/maybe_changed_after.rs @@ -158,13 +158,13 @@ where return true; } - if old_memo.revisions.inputs.untracked { + if old_memo.revisions.edges.untracked { // Untracked inputs? Have to assume that it changed. return false; } let last_verified_at = old_memo.verified_at.load(); - for &input in old_memo.revisions.inputs.tracked.iter() { + for &input in old_memo.revisions.edges.inputs().iter() { if db.maybe_changed_after(input, last_verified_at) { return false; } diff --git a/components/salsa-2022/src/function/specify.rs b/components/salsa-2022/src/function/specify.rs index baa9e476..6c879707 100644 --- a/components/salsa-2022/src/function/specify.rs +++ b/components/salsa-2022/src/function/specify.rs @@ -1,7 +1,7 @@ use crossbeam::atomic::AtomicCell; use crate::{ - runtime::local_state::{QueryInputs, QueryRevisions}, + runtime::local_state::{QueryEdges, QueryRevisions}, tracked_struct::TrackedStructInDb, Database, }; @@ -49,16 +49,17 @@ where // // - a result that is verified in the current revision, because it was set, which will use the set value // - a result that is NOT verified and has untracked inputs, which will re-execute (and likely panic) - let inputs = QueryInputs { + let edges = QueryEdges { untracked: false, - tracked: runtime.empty_dependencies(), + separator: 0, + input_outputs: runtime.empty_dependencies(), }; let revision = runtime.current_revision(); let mut revisions = QueryRevisions { changed_at: current_deps.changed_at, durability: current_deps.durability, - inputs, + edges, }; if let Some(old_memo) = self.memo_map.get(key) { diff --git a/components/salsa-2022/src/function/store.rs b/components/salsa-2022/src/function/store.rs index 843afead..ece3f424 100644 --- a/components/salsa-2022/src/function/store.rs +++ b/components/salsa-2022/src/function/store.rs @@ -4,7 +4,7 @@ use crossbeam::atomic::AtomicCell; use crate::{ durability::Durability, - runtime::local_state::{QueryInputs, QueryRevisions}, + runtime::local_state::{QueryEdges, QueryRevisions}, Runtime, }; @@ -28,9 +28,10 @@ where revisions: QueryRevisions { changed_at: revision, durability, - inputs: QueryInputs { + edges: QueryEdges { untracked: false, - tracked: runtime.empty_dependencies(), + separator: 0, + input_outputs: runtime.empty_dependencies(), }, }, }; diff --git a/components/salsa-2022/src/ingredient.rs b/components/salsa-2022/src/ingredient.rs index 0b321c26..d2aef7b4 100644 --- a/components/salsa-2022/src/ingredient.rs +++ b/components/salsa-2022/src/ingredient.rs @@ -1,5 +1,5 @@ use crate::{ - cycle::CycleRecoveryStrategy, key::DependencyIndex, runtime::local_state::QueryInputs, Id, + cycle::CycleRecoveryStrategy, key::DependencyIndex, runtime::local_state::QueryEdges, Id, }; use super::Revision; @@ -21,7 +21,7 @@ pub trait Ingredient { fn maybe_changed_after(&self, db: &DB, input: DependencyIndex, revision: Revision) -> bool; /// What were the inputs (if any) that were used to create the value at `key_index`. - fn inputs(&self, key_index: Id) -> Option; + fn inputs(&self, key_index: Id) -> Option; } /// Optional trait for ingredients that wish to be notified when new revisions are diff --git a/components/salsa-2022/src/input.rs b/components/salsa-2022/src/input.rs index b61dd3ba..3fb7afc8 100644 --- a/components/salsa-2022/src/input.rs +++ b/components/salsa-2022/src/input.rs @@ -2,7 +2,7 @@ use crate::{ cycle::CycleRecoveryStrategy, ingredient::Ingredient, key::{DatabaseKeyIndex, DependencyIndex}, - runtime::{local_state::QueryInputs, Runtime}, + runtime::{local_state::QueryEdges, Runtime}, AsId, IngredientIndex, Revision, }; @@ -58,7 +58,7 @@ where CycleRecoveryStrategy::Panic } - fn inputs(&self, _key_index: crate::Id) -> Option { + fn inputs(&self, _key_index: crate::Id) -> Option { None } } diff --git a/components/salsa-2022/src/interned.rs b/components/salsa-2022/src/interned.rs index 26fc14c7..cf6c88fd 100644 --- a/components/salsa-2022/src/interned.rs +++ b/components/salsa-2022/src/interned.rs @@ -6,7 +6,7 @@ use std::marker::PhantomData; use crate::durability::Durability; use crate::id::AsId; use crate::key::DependencyIndex; -use crate::runtime::local_state::QueryInputs; +use crate::runtime::local_state::QueryEdges; use crate::runtime::Runtime; use super::hash::FxDashMap; @@ -194,7 +194,7 @@ where crate::cycle::CycleRecoveryStrategy::Panic } - fn inputs(&self, _key_index: crate::Id) -> Option { + fn inputs(&self, _key_index: crate::Id) -> Option { None } } diff --git a/components/salsa-2022/src/runtime/active_query.rs b/components/salsa-2022/src/runtime/active_query.rs index 04337858..7652ed27 100644 --- a/components/salsa-2022/src/runtime/active_query.rs +++ b/components/salsa-2022/src/runtime/active_query.rs @@ -8,7 +8,7 @@ use crate::{ Cycle, Revision, Runtime, }; -use super::local_state::{QueryInputs, QueryRevisions}; +use super::local_state::{QueryEdges, QueryRevisions}; #[derive(Debug)] pub(super) struct ActiveQuery { @@ -95,18 +95,27 @@ impl ActiveQuery { } pub(crate) fn revisions(&self, runtime: &Runtime) -> QueryRevisions { - let inputs = QueryInputs { + let separator = u32::try_from(self.dependencies.len()).unwrap(); + + let input_outputs = if self.dependencies.is_empty() && self.outputs.is_empty() { + runtime.empty_dependencies() + } else { + self.dependencies + .iter() + .copied() + .chain(self.outputs.iter().map(|&o| o.into())) + .collect() + }; + + let edges = QueryEdges { untracked: self.untracked_read, - tracked: if self.dependencies.is_empty() { - runtime.empty_dependencies() - } else { - self.dependencies.iter().copied().collect() - }, + separator, + input_outputs, }; QueryRevisions { changed_at: self.changed_at, - inputs, + edges, durability: self.durability, } } diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index 6c5b7730..4a56e901 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -40,7 +40,7 @@ pub(crate) struct QueryRevisions { pub(crate) durability: Durability, /// The inputs that went into our query, if we are tracking them. - pub(crate) inputs: QueryInputs, + pub(crate) edges: QueryEdges, } impl QueryRevisions { @@ -53,18 +53,53 @@ impl QueryRevisions { } } -/// Every input. +/// 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) +/// and output edges +/// (e.g., when Q0 specified the value for another query Q2). #[derive(Debug, Clone)] -pub struct QueryInputs { - /// Inputs that are fully known. - /// We track these even if there are unknown inputs so that the accumulator code - /// can walk all the inputs even for tracked functions that read untracked values. - pub(crate) tracked: Arc<[DependencyIndex]>, +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, + /// but we still use it for finding the transitive inputs to an accumulator. + /// + /// You can access the input/output list via the methods [`inputs`] and [`outputs`] respectively. + /// + /// 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. + pub(crate) input_outputs: Arc<[DependencyIndex]>, + + /// The index that separates inputs from outputs in the `tracked` field. + pub(crate) separator: u32, /// Where there any *unknown* inputs? pub(crate) untracked: bool, } +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] + } + + /// Returns the queries whose values were assigned while computing this memoized value. + /// + /// These will always be in sorted order. + pub(crate) fn outputs(&self) -> &[DependencyIndex] { + &self.input_outputs[self.separator as usize..] + } +} + impl Default for LocalState { fn default() -> Self { LocalState { diff --git a/components/salsa-2022/src/storage.rs b/components/salsa-2022/src/storage.rs index 29926d37..5384a76a 100644 --- a/components/salsa-2022/src/storage.rs +++ b/components/salsa-2022/src/storage.rs @@ -6,7 +6,7 @@ use crate::cycle::CycleRecoveryStrategy; use crate::ingredient::Ingredient; use crate::jar::Jar; use crate::key::DependencyIndex; -use crate::runtime::local_state::QueryInputs; +use crate::runtime::local_state::QueryEdges; use crate::runtime::Runtime; use crate::{Database, DatabaseKeyIndex, IngredientIndex}; @@ -178,7 +178,7 @@ pub trait HasJarsDyn { fn cycle_recovery_strategy(&self, input: IngredientIndex) -> CycleRecoveryStrategy; - fn inputs(&self, input: DatabaseKeyIndex) -> Option; + fn inputs(&self, input: DatabaseKeyIndex) -> Option; } pub trait HasIngredientsFor diff --git a/components/salsa-2022/src/tracked_struct.rs b/components/salsa-2022/src/tracked_struct.rs index 0b326775..c57819cf 100644 --- a/components/salsa-2022/src/tracked_struct.rs +++ b/components/salsa-2022/src/tracked_struct.rs @@ -3,7 +3,7 @@ use crate::{ ingredient::{Ingredient, MutIngredient}, interned::{InternedData, InternedId, InternedIngredient}, key::{DatabaseKeyIndex, DependencyIndex}, - runtime::{local_state::QueryInputs, Runtime}, + runtime::{local_state::QueryEdges, Runtime}, salsa_struct::SalsaStructInDb, Database, IngredientIndex, Revision, }; @@ -114,7 +114,7 @@ where <_ as Ingredient>::cycle_recovery_strategy(&self.interned) } - fn inputs(&self, _key_index: crate::Id) -> Option { + fn inputs(&self, _key_index: crate::Id) -> Option { None } }