Merge pull request #636 from Veykril/veykril/push-accumulated

Shrink `QueryRevisions` size by 3 words

src/accumulator.rs

@@ -8,7 +8,6 @@ use std::{
 
 use accumulated::Accumulated;
 use accumulated::AnyAccumulated;
-use accumulated_map::AccumulatedMap;
 
 use crate::{
     cycle::CycleRecoveryStrategy,
@@ -144,10 +143,6 @@ impl<A: Accumulator> Ingredient for IngredientImpl<A> {
     fn debug_name(&self) -> &'static str {
         A::DEBUG_NAME
     }
-
-    fn accumulated(&self, _db: &dyn Database, _key_index: Id) -> Option<&AccumulatedMap> {
-        None
-    }
 }
 
 impl<A> std::fmt::Debug for IngredientImpl<A>

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<IngredientIndex, Box<dyn AnyAccumulated>>,
-
-    /// [`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<A: Accumulator>(
         &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;
+        }
+    }
+}

src/active_query.rs

@@ -1,3 +1,5 @@
+use std::ops::Not;
+
 use rustc_hash::FxHashMap;
 
 use super::zalsa_local::{QueryEdges, QueryOrigin, QueryRevisions};
@@ -55,6 +57,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 {
@@ -69,6 +75,7 @@ impl ActiveQuery {
             disambiguator_map: Default::default(),
             tracked_struct_ids: Default::default(),
             accumulated: Default::default(),
+            accumulated_inputs: Default::default(),
         }
     }
 
@@ -82,7 +89,7 @@ impl ActiveQuery {
         self.input_outputs.insert(QueryEdge::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) {
@@ -114,13 +121,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,
         }
     }
 

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,
@@ -242,7 +242,7 @@ where
         &'db self,
         db: &'db dyn Database,
         key_index: Id,
-    ) -> Option<&'db AccumulatedMap> {
+    ) -> (Option<&'db AccumulatedMap>, InputAccumulatedValues) {
         let db = db.as_view::<C::DbView>();
         self.accumulated_map(db, key_index)
     }

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,
+        )
     }
 }

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<C> IngredientImpl<C>
@@ -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

src/function/memo.rs

@@ -76,23 +76,25 @@ impl<C: Configuration> IngredientImpl<C> {
                     }
                     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<V> {
     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::<Memo<std::num::NonZeroUsize>>()] =
+    [(); std::mem::size_of::<[usize; 12]>()];
+
 impl<V> Memo<V> {
     pub(super) fn new(value: Option<V>, revision_now: Revision, revisions: QueryRevisions) -> Self {
         Memo {

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) {

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

src/input.rs

@@ -267,14 +267,6 @@ impl<C: Configuration> Ingredient for IngredientImpl<C> {
     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<C: Configuration> std::fmt::Debug for IngredientImpl<C> {

src/input/input_field.rs

@@ -90,14 +90,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<C> std::fmt::Debug for FieldIngredientImpl<C>

src/interned.rs

@@ -308,14 +308,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<C> std::fmt::Debug for IngredientImpl<C>

src/tracked_struct.rs

@@ -622,14 +622,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<C> std::fmt::Debug for IngredientImpl<C>

src/tracked_struct/tracked_field.rs

@@ -111,14 +111,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<C> std::fmt::Debug for FieldIngredientImpl<C>