From d0b5fd4f8664b6e5fcde39524e18af6a41fdf011 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 22 Dec 2024 18:10:30 +0100 Subject: [PATCH] Shrink `QueryRevisions` size by 3 words --- src/accumulator.rs | 5 ---- src/accumulator/accumulated_map.rs | 44 ++++++++++------------------- src/active_query.rs | 21 ++++++++++++-- src/function.rs | 4 +-- src/function/accumulated.rs | 12 +++++--- src/function/fetch.rs | 3 +- src/function/memo.rs | 4 +++ src/function/specify.rs | 1 + src/ingredient.rs | 7 +++-- src/input.rs | 8 ------ src/input/input_field.rs | 8 ------ src/interned.rs | 8 ------ src/key.rs | 11 ++++++-- src/tracked_struct.rs | 8 ------ src/tracked_struct/tracked_field.rs | 8 ------ src/zalsa_local.rs | 5 +++- 16 files changed, 66 insertions(+), 91 deletions(-) diff --git a/src/accumulator.rs b/src/accumulator.rs index b9355419a..b23fd9a23 100644 --- a/src/accumulator.rs +++ b/src/accumulator.rs @@ -8,7 +8,6 @@ use std::{ use accumulated::Accumulated; use accumulated::AnyAccumulated; -use accumulated_map::AccumulatedMap; use crate::{ cycle::CycleRecoveryStrategy, @@ -145,10 +144,6 @@ impl Ingredient for IngredientImpl { fn debug_name(&self) -> &'static str { A::DEBUG_NAME } - - fn accumulated(&self, _db: &dyn Database, _key_index: Id) -> Option<&AccumulatedMap> { - None - } } impl std::fmt::Debug for IngredientImpl diff --git a/src/accumulator/accumulated_map.rs b/src/accumulator/accumulated_map.rs index 5c9e92543..288c482fb 100644 --- a/src/accumulator/accumulated_map.rs +++ b/src/accumulator/accumulated_map.rs @@ -1,3 +1,5 @@ +use std::ops; + use rustc_hash::FxHashMap; use crate::IngredientIndex; @@ -7,10 +9,6 @@ use super::{accumulated::Accumulated, Accumulator, AnyAccumulated}; #[derive(Default, Debug)] pub struct AccumulatedMap { map: FxHashMap>, - - /// [`InputAccumulatedValues::Empty`] if any input read during the query's execution - /// has any direct or indirect accumulated values. - inputs: InputAccumulatedValues, } impl AccumulatedMap { @@ -21,21 +19,6 @@ impl AccumulatedMap { .accumulate(value); } - /// Adds the accumulated state of an input to this accumulated map. - pub(crate) fn add_input(&mut self, input: InputAccumulatedValues) { - if input.is_any() { - self.inputs = InputAccumulatedValues::Any; - } - } - - /// Returns whether an input of the associated query has any accumulated values. - /// - /// Note: Use [`InputAccumulatedValues::from_map`] to check if the associated query itself - /// or any of its inputs has accumulated values. - pub(crate) fn inputs(&self) -> InputAccumulatedValues { - self.inputs - } - pub fn extend_with_accumulated( &self, index: IngredientIndex, @@ -50,6 +33,10 @@ impl AccumulatedMap { .unwrap() .extend_with_accumulated(output); } + + pub fn is_empty(&self) -> bool { + self.map.is_empty() + } } impl Clone for AccumulatedMap { @@ -60,7 +47,6 @@ impl Clone for AccumulatedMap { .iter() .map(|(&key, value)| (key, value.cloned())) .collect(), - inputs: self.inputs, } } } @@ -70,7 +56,7 @@ impl Clone for AccumulatedMap { /// Knowning whether any input has accumulated values makes aggregating the accumulated values /// cheaper because we can skip over entire subtrees. #[derive(Copy, Clone, Debug, Default)] -pub(crate) enum InputAccumulatedValues { +pub enum InputAccumulatedValues { /// The query nor any of its inputs have any accumulated values. #[default] Empty, @@ -80,14 +66,6 @@ pub(crate) enum InputAccumulatedValues { } impl InputAccumulatedValues { - pub(crate) fn from_map(accumulated: &AccumulatedMap) -> Self { - if accumulated.map.is_empty() { - accumulated.inputs - } else { - Self::Any - } - } - pub(crate) const fn is_any(self) -> bool { matches!(self, Self::Any) } @@ -96,3 +74,11 @@ impl InputAccumulatedValues { matches!(self, Self::Empty) } } + +impl ops::BitOrAssign for InputAccumulatedValues { + fn bitor_assign(&mut self, rhs: Self) { + if rhs.is_any() { + *self = Self::Any; + } + } +} diff --git a/src/active_query.rs b/src/active_query.rs index 71781a385..3459f7779 100644 --- a/src/active_query.rs +++ b/src/active_query.rs @@ -1,3 +1,5 @@ +use std::ops::Not; + use rustc_hash::FxHashMap; use super::zalsa_local::{EdgeKind, QueryEdges, QueryOrigin, QueryRevisions}; @@ -54,6 +56,10 @@ pub(crate) struct ActiveQuery { /// Stores the values accumulated to the given ingredient. /// The type of accumulated value is erased but known to the ingredient. pub(crate) accumulated: AccumulatedMap, + + /// [`InputAccumulatedValues::Empty`] if any input read during the query's execution + /// has any direct or indirect accumulated values. + pub(super) accumulated_inputs: InputAccumulatedValues, } impl ActiveQuery { @@ -68,6 +74,7 @@ impl ActiveQuery { disambiguator_map: Default::default(), tracked_struct_ids: Default::default(), accumulated: Default::default(), + accumulated_inputs: Default::default(), } } @@ -81,7 +88,7 @@ impl ActiveQuery { self.input_outputs.insert((EdgeKind::Input, input)); self.durability = self.durability.min(durability); self.changed_at = self.changed_at.max(revision); - self.accumulated.add_input(accumulated); + self.accumulated_inputs |= accumulated; } pub(super) fn add_untracked_read(&mut self, changed_at: Revision) { @@ -120,13 +127,21 @@ impl ActiveQuery { } else { QueryOrigin::Derived(edges) }; - + let accumulated = self + .accumulated + .is_empty() + .not() + .then(|| Box::new(self.accumulated)); QueryRevisions { changed_at: self.changed_at, origin, durability: self.durability, tracked_struct_ids: self.tracked_struct_ids, - accumulated: self.accumulated, + accumulated_inputs: match &accumulated { + Some(_) => InputAccumulatedValues::Any, + None => self.accumulated_inputs, + }, + accumulated, } } diff --git a/src/function.rs b/src/function.rs index 07f13d496..03108137c 100644 --- a/src/function.rs +++ b/src/function.rs @@ -1,7 +1,7 @@ use std::{any::Any, fmt, sync::Arc}; use crate::{ - accumulator::accumulated_map::AccumulatedMap, + accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues}, cycle::CycleRecoveryStrategy, ingredient::fmt_index, key::DatabaseKeyIndex, @@ -249,7 +249,7 @@ where &'db self, db: &'db dyn Database, key_index: Id, - ) -> Option<&'db AccumulatedMap> { + ) -> (Option<&'db AccumulatedMap>, InputAccumulatedValues) { let db = db.as_view::(); self.accumulated_map(db, key_index) } diff --git a/src/function/accumulated.rs b/src/function/accumulated.rs index d8b0fcfc9..c83a809af 100644 --- a/src/function/accumulated.rs +++ b/src/function/accumulated.rs @@ -1,4 +1,5 @@ use super::{Configuration, IngredientImpl}; +use crate::accumulator::accumulated_map::InputAccumulatedValues; use crate::zalsa_local::QueryOrigin; use crate::{ accumulator::{self, accumulated_map::AccumulatedMap}, @@ -54,11 +55,11 @@ where } // Extend `output` with any values accumulated by `k`. - if let Some(accumulated_map) = k.accumulated(db) { + if let (Some(accumulated_map), input) = k.accumulated(db) { accumulated_map.extend_with_accumulated(accumulator.index(), &mut output); // Skip over the inputs because we know that the entire sub-graph has no accumulated values - if accumulated_map.inputs().is_empty() { + if input.is_empty() { continue; } } @@ -97,9 +98,12 @@ where &'db self, db: &'db C::DbView, key: Id, - ) -> Option<&'db AccumulatedMap> { + ) -> (Option<&'db AccumulatedMap>, InputAccumulatedValues) { // NEXT STEP: stash and refactor `fetch` to return an `&Memo` so we can make this work let memo = self.refresh_memo(db, key); - Some(&memo.revisions.accumulated) + ( + memo.revisions.accumulated.as_deref(), + memo.revisions.accumulated_inputs, + ) } } diff --git a/src/function/fetch.rs b/src/function/fetch.rs index f6d495dff..d67d84123 100644 --- a/src/function/fetch.rs +++ b/src/function/fetch.rs @@ -1,5 +1,4 @@ use super::{memo::Memo, Configuration, IngredientImpl}; -use crate::accumulator::accumulated_map::InputAccumulatedValues; use crate::{runtime::StampedValue, zalsa::ZalsaDatabase, AsDynDatabase as _, Id}; impl IngredientImpl @@ -25,7 +24,7 @@ where self.database_key_index(id).into(), durability, changed_at, - InputAccumulatedValues::from_map(&memo.revisions.accumulated), + memo.revisions.accumulated_inputs, ); value diff --git a/src/function/memo.rs b/src/function/memo.rs index f982dc339..bc95d439d 100644 --- a/src/function/memo.rs +++ b/src/function/memo.rs @@ -101,6 +101,10 @@ pub(super) struct Memo { pub(super) revisions: QueryRevisions, } +// Memo's are stored a lot, make sure there size is doesn't randomly increase. +// #[cfg(test)] +const _: [(); std::mem::size_of::>()] = [(); std::mem::size_of::<[usize; 12]>()]; + impl Memo { pub(super) fn new(value: Option, revision_now: Revision, revisions: QueryRevisions) -> Self { Memo { diff --git a/src/function/specify.rs b/src/function/specify.rs index 9eccad65b..e85600f7f 100644 --- a/src/function/specify.rs +++ b/src/function/specify.rs @@ -70,6 +70,7 @@ where origin: QueryOrigin::Assigned(active_query_key), tracked_struct_ids: Default::default(), accumulated: Default::default(), + accumulated_inputs: Default::default(), }; if let Some(old_memo) = self.get_memo_from_table_for(zalsa, key) { diff --git a/src/ingredient.rs b/src/ingredient.rs index 383fdc6b7..88fd2879b 100644 --- a/src/ingredient.rs +++ b/src/ingredient.rs @@ -4,7 +4,7 @@ use std::{ }; use crate::{ - accumulator::accumulated_map::AccumulatedMap, + accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues}, cycle::CycleRecoveryStrategy, zalsa::{IngredientIndex, MemoIngredientIndex}, zalsa_local::QueryOrigin, @@ -51,7 +51,10 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync { &'db self, db: &'db dyn Database, key_index: Id, - ) -> Option<&'db AccumulatedMap>; + ) -> (Option<&'db AccumulatedMap>, InputAccumulatedValues) { + _ = (db, key_index); + (None, InputAccumulatedValues::Any) + } /// Invoked when the value `output_key` should be marked as valid in the current revision. /// This occurs because the value for `executor`, which generated it, was marked as valid diff --git a/src/input.rs b/src/input.rs index fd3726def..5e3420d5d 100644 --- a/src/input.rs +++ b/src/input.rs @@ -267,14 +267,6 @@ impl Ingredient for IngredientImpl { fn debug_name(&self) -> &'static str { C::DEBUG_NAME } - - fn accumulated<'db>( - &'db self, - _db: &'db dyn Database, - _key_index: Id, - ) -> Option<&'db crate::accumulator::accumulated_map::AccumulatedMap> { - None - } } impl std::fmt::Debug for IngredientImpl { diff --git a/src/input/input_field.rs b/src/input/input_field.rs index fd3082256..505aaf4f0 100644 --- a/src/input/input_field.rs +++ b/src/input/input_field.rs @@ -96,14 +96,6 @@ where fn debug_name(&self) -> &'static str { C::FIELD_DEBUG_NAMES[self.field_index] } - - fn accumulated<'db>( - &'db self, - _db: &'db dyn Database, - _key_index: Id, - ) -> Option<&'db crate::accumulator::accumulated_map::AccumulatedMap> { - None - } } impl std::fmt::Debug for FieldIngredientImpl diff --git a/src/interned.rs b/src/interned.rs index ba58b512d..13fc0b7ac 100644 --- a/src/interned.rs +++ b/src/interned.rs @@ -274,14 +274,6 @@ where fn debug_name(&self) -> &'static str { C::DEBUG_NAME } - - fn accumulated<'db>( - &'db self, - _db: &'db dyn Database, - _key_index: Id, - ) -> Option<&'db crate::accumulator::accumulated_map::AccumulatedMap> { - None - } } impl std::fmt::Debug for IngredientImpl diff --git a/src/key.rs b/src/key.rs index 92e63541d..dc9aa8260 100644 --- a/src/key.rs +++ b/src/key.rs @@ -1,6 +1,8 @@ use crate::{ - accumulator::accumulated_map::AccumulatedMap, cycle::CycleRecoveryStrategy, - zalsa::IngredientIndex, Database, Id, + accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues}, + cycle::CycleRecoveryStrategy, + zalsa::IngredientIndex, + Database, Id, }; /// An integer that uniquely identifies a particular query instance within the @@ -100,7 +102,10 @@ impl DatabaseKeyIndex { self.ingredient_index.cycle_recovery_strategy(db) } - pub(crate) fn accumulated(self, db: &dyn Database) -> Option<&AccumulatedMap> { + pub(crate) fn accumulated( + self, + db: &dyn Database, + ) -> (Option<&AccumulatedMap>, InputAccumulatedValues) { db.zalsa() .lookup_ingredient(self.ingredient_index) .accumulated(db, self.key_index) diff --git a/src/tracked_struct.rs b/src/tracked_struct.rs index 367cc36cc..cfc9cfd40 100644 --- a/src/tracked_struct.rs +++ b/src/tracked_struct.rs @@ -631,14 +631,6 @@ where } fn reset_for_new_revision(&mut self) {} - - fn accumulated<'db>( - &'db self, - _db: &'db dyn Database, - _key_index: Id, - ) -> Option<&'db crate::accumulator::accumulated_map::AccumulatedMap> { - None - } } impl std::fmt::Debug for IngredientImpl diff --git a/src/tracked_struct/tracked_field.rs b/src/tracked_struct/tracked_field.rs index ff1909397..d5c214ade 100644 --- a/src/tracked_struct/tracked_field.rs +++ b/src/tracked_struct/tracked_field.rs @@ -112,14 +112,6 @@ where fn debug_name(&self) -> &'static str { C::FIELD_DEBUG_NAMES[self.field_index] } - - fn accumulated<'db>( - &'db self, - _db: &'db dyn Database, - _key_index: Id, - ) -> Option<&'db crate::accumulator::accumulated_map::AccumulatedMap> { - None - } } impl std::fmt::Debug for FieldIngredientImpl diff --git a/src/zalsa_local.rs b/src/zalsa_local.rs index 6988d6537..7f1d7b27d 100644 --- a/src/zalsa_local.rs +++ b/src/zalsa_local.rs @@ -355,7 +355,10 @@ pub(crate) struct QueryRevisions { /// only entries that appeared in the new revision. pub(super) tracked_struct_ids: FxHashMap, - pub(super) accumulated: AccumulatedMap, + pub(super) accumulated: Option>, + /// [`InputAccumulatedValues::Empty`] if any input read during the query's execution + /// has any direct or indirect accumulated values. + pub(super) accumulated_inputs: InputAccumulatedValues, } impl QueryRevisions {