From eb4340b546b37f6a6cad2f4cf4cd159b0673dc99 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 | 20 +++++++------ src/function/fetch.rs | 3 +- src/function/memo.rs | 19 +++++++++---- src/function/specify.rs | 1 + src/ingredient.rs | 7 +++-- src/input.rs | 8 ------ src/input/input_field.rs | 8 ------ src/interned.rs | 8 ------ src/tracked_struct.rs | 8 ------ src/tracked_struct/tracked_field.rs | 8 ------ src/zalsa_local.rs | 5 +++- 15 files changed, 71 insertions(+), 98 deletions(-) diff --git a/src/accumulator.rs b/src/accumulator.rs index 8cb38e2e..d30575d2 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, @@ -149,10 +148,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 5c9e9254..288c482f 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 a586cde0..a2dc8fcd 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}; @@ -53,6 +55,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 accumulated values. + pub(super) accumulated_inputs: InputAccumulatedValues, } impl ActiveQuery { @@ -67,6 +73,7 @@ impl ActiveQuery { disambiguator_map: Default::default(), tracked_struct_ids: Default::default(), accumulated: Default::default(), + accumulated_inputs: Default::default(), } } @@ -80,7 +87,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) { @@ -119,13 +126,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 b06be486..82704969 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 588e42d1..67664c02 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}, @@ -55,13 +56,13 @@ where let ingredient = zalsa.lookup_ingredient(k.ingredient_index); // Extend `output` with any values accumulated by `k`. - if let Some(accumulated_map) = ingredient.accumulated(db, k.key_index) { + let (accumulated_map, input) = ingredient.accumulated(db, k.key_index); + if let Some(accumulated_map) = accumulated_map { 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() { - continue; - } + } + // Skip over the inputs because we know that the entire sub-graph has no accumulated values + if input.is_empty() { + continue; } // Find the inputs of `k` and push them onto the stack. @@ -95,9 +96,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 65aeaea6..1b806f55 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 304c6c31..0ea70bb1 100644 --- a/src/function/memo.rs +++ b/src/function/memo.rs @@ -76,23 +76,25 @@ impl IngredientImpl { } QueryOrigin::Derived(_) => { // QueryRevisions: !Clone to discourage cloning, we need it here though - let QueryRevisions { + let &QueryRevisions { changed_at, durability, - origin, - tracked_struct_ids, - accumulated, + ref origin, + ref tracked_struct_ids, + ref accumulated, + accumulated_inputs, } = &memo.revisions; // Re-assemble the memo but with the value set to `None` Arc::new(Memo::new( None, memo.verified_at.load(), QueryRevisions { - changed_at: *changed_at, - durability: *durability, + changed_at, + durability, origin: origin.clone(), tracked_struct_ids: tracked_struct_ids.clone(), accumulated: accumulated.clone(), + accumulated_inputs, }, )) } @@ -115,6 +117,11 @@ pub(super) struct Memo { pub(super) revisions: QueryRevisions, } +// Memo's are stored a lot, make sure their 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 9eccad65..e85600f7 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 8a46205d..1bae2172 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, @@ -74,7 +74,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 eac540ab..92d1bc0e 100644 --- a/src/input.rs +++ b/src/input.rs @@ -275,14 +275,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 fd308225..505aaf4f 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 62ef4fe6..7d0b7874 100644 --- a/src/interned.rs +++ b/src/interned.rs @@ -279,14 +279,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/tracked_struct.rs b/src/tracked_struct.rs index 97d3f968..ecffccbf 100644 --- a/src/tracked_struct.rs +++ b/src/tracked_struct.rs @@ -636,14 +636,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 ff190939..d5c214ad 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 3fb28e7e..0376d0e0 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 {