Skip to content

Commit 0537864

Browse files
VeykrilVeykril
committed
Shrink QueryRevisions size by 3 words
1 parent 88a1d77 commit 0537864

15 files changed

+70
-98
lines changed

‎src/accumulator.rs

-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ use std::{
88

99
use accumulated::Accumulated;
1010
use accumulated::AnyAccumulated;
11-
use accumulated_map::AccumulatedMap;
1211

1312
use crate::{
1413
cycle::CycleRecoveryStrategy,
@@ -149,10 +148,6 @@ impl<A: Accumulator> Ingredient for IngredientImpl<A> {
149148
fn debug_name(&self) -> &'static str {
150149
A::DEBUG_NAME
151150
}
152-
153-
fn accumulated(&self, _db: &dyn Database, _key_index: Id) -> Option<&AccumulatedMap> {
154-
None
155-
}
156151
}
157152

158153
impl<A> std::fmt::Debug for IngredientImpl<A>

‎src/accumulator/accumulated_map.rs

+15-29
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::ops;
2+
13
use rustc_hash::FxHashMap;
24

35
use crate::IngredientIndex;
@@ -7,10 +9,6 @@ use super::{accumulated::Accumulated, Accumulator, AnyAccumulated};
79
#[derive(Default, Debug)]
810
pub struct AccumulatedMap {
911
map: FxHashMap<IngredientIndex, Box<dyn AnyAccumulated>>,
10-
11-
/// [`InputAccumulatedValues::Empty`] if any input read during the query's execution
12-
/// has any direct or indirect accumulated values.
13-
inputs: InputAccumulatedValues,
1412
}
1513

1614
impl AccumulatedMap {
@@ -21,21 +19,6 @@ impl AccumulatedMap {
2119
.accumulate(value);
2220
}
2321

24-
/// Adds the accumulated state of an input to this accumulated map.
25-
pub(crate) fn add_input(&mut self, input: InputAccumulatedValues) {
26-
if input.is_any() {
27-
self.inputs = InputAccumulatedValues::Any;
28-
}
29-
}
30-
31-
/// Returns whether an input of the associated query has any accumulated values.
32-
///
33-
/// Note: Use [`InputAccumulatedValues::from_map`] to check if the associated query itself
34-
/// or any of its inputs has accumulated values.
35-
pub(crate) fn inputs(&self) -> InputAccumulatedValues {
36-
self.inputs
37-
}
38-
3922
pub fn extend_with_accumulated<A: Accumulator>(
4023
&self,
4124
index: IngredientIndex,
@@ -50,6 +33,10 @@ impl AccumulatedMap {
5033
.unwrap()
5134
.extend_with_accumulated(output);
5235
}
36+
37+
pub fn is_empty(&self) -> bool {
38+
self.map.is_empty()
39+
}
5340
}
5441

5542
impl Clone for AccumulatedMap {
@@ -60,7 +47,6 @@ impl Clone for AccumulatedMap {
6047
.iter()
6148
.map(|(&key, value)| (key, value.cloned()))
6249
.collect(),
63-
inputs: self.inputs,
6450
}
6551
}
6652
}
@@ -70,7 +56,7 @@ impl Clone for AccumulatedMap {
7056
/// Knowning whether any input has accumulated values makes aggregating the accumulated values
7157
/// cheaper because we can skip over entire subtrees.
7258
#[derive(Copy, Clone, Debug, Default)]
73-
pub(crate) enum InputAccumulatedValues {
59+
pub enum InputAccumulatedValues {
7460
/// The query nor any of its inputs have any accumulated values.
7561
#[default]
7662
Empty,
@@ -80,14 +66,6 @@ pub(crate) enum InputAccumulatedValues {
8066
}
8167

8268
impl InputAccumulatedValues {
83-
pub(crate) fn from_map(accumulated: &AccumulatedMap) -> Self {
84-
if accumulated.map.is_empty() {
85-
accumulated.inputs
86-
} else {
87-
Self::Any
88-
}
89-
}
90-
9169
pub(crate) const fn is_any(self) -> bool {
9270
matches!(self, Self::Any)
9371
}
@@ -96,3 +74,11 @@ impl InputAccumulatedValues {
9674
matches!(self, Self::Empty)
9775
}
9876
}
77+
78+
impl ops::BitOrAssign for InputAccumulatedValues {
79+
fn bitor_assign(&mut self, rhs: Self) {
80+
if rhs.is_any() {
81+
*self = Self::Any;
82+
}
83+
}
84+
}

‎src/active_query.rs

+18-3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::ops::Not;
2+
13
use rustc_hash::FxHashMap;
24

35
use super::zalsa_local::{EdgeKind, QueryEdges, QueryOrigin, QueryRevisions};
@@ -53,6 +55,10 @@ pub(crate) struct ActiveQuery {
5355
/// Stores the values accumulated to the given ingredient.
5456
/// The type of accumulated value is erased but known to the ingredient.
5557
pub(crate) accumulated: AccumulatedMap,
58+
59+
/// [`InputAccumulatedValues::Empty`] if any input read during the query's execution
60+
/// has any direct or indirect accumulated values.
61+
pub(super) accumulated_inputs: InputAccumulatedValues,
5662
}
5763

5864
impl ActiveQuery {
@@ -67,6 +73,7 @@ impl ActiveQuery {
6773
disambiguator_map: Default::default(),
6874
tracked_struct_ids: Default::default(),
6975
accumulated: Default::default(),
76+
accumulated_inputs: Default::default(),
7077
}
7178
}
7279

@@ -80,7 +87,7 @@ impl ActiveQuery {
8087
self.input_outputs.insert((EdgeKind::Input, input));
8188
self.durability = self.durability.min(durability);
8289
self.changed_at = self.changed_at.max(revision);
83-
self.accumulated.add_input(accumulated);
90+
self.accumulated_inputs |= accumulated;
8491
}
8592

8693
pub(super) fn add_untracked_read(&mut self, changed_at: Revision) {
@@ -119,13 +126,21 @@ impl ActiveQuery {
119126
} else {
120127
QueryOrigin::Derived(edges)
121128
};
122-
129+
let accumulated = self
130+
.accumulated
131+
.is_empty()
132+
.not()
133+
.then(|| Box::new(self.accumulated));
123134
QueryRevisions {
124135
changed_at: self.changed_at,
125136
origin,
126137
durability: self.durability,
127138
tracked_struct_ids: self.tracked_struct_ids,
128-
accumulated: self.accumulated,
139+
accumulated_inputs: match &accumulated {
140+
Some(_) => InputAccumulatedValues::Any,
141+
None => self.accumulated_inputs,
142+
},
143+
accumulated,
129144
}
130145
}
131146

‎src/function.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::{any::Any, fmt, sync::Arc};
22

33
use crate::{
4-
accumulator::accumulated_map::AccumulatedMap,
4+
accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues},
55
cycle::CycleRecoveryStrategy,
66
ingredient::fmt_index,
77
key::DatabaseKeyIndex,
@@ -249,7 +249,7 @@ where
249249
&'db self,
250250
db: &'db dyn Database,
251251
key_index: Id,
252-
) -> Option<&'db AccumulatedMap> {
252+
) -> (Option<&'db AccumulatedMap>, InputAccumulatedValues) {
253253
let db = db.as_view::<C::DbView>();
254254
self.accumulated_map(db, key_index)
255255
}

‎src/function/accumulated.rs

+12-8
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use super::{Configuration, IngredientImpl};
2+
use crate::accumulator::accumulated_map::InputAccumulatedValues;
23
use crate::zalsa_local::QueryOrigin;
34
use crate::{
45
accumulator::{self, accumulated_map::AccumulatedMap},
@@ -55,13 +56,13 @@ where
5556

5657
let ingredient = zalsa.lookup_ingredient(k.ingredient_index);
5758
// Extend `output` with any values accumulated by `k`.
58-
if let Some(accumulated_map) = ingredient.accumulated(db, k.key_index) {
59+
let (accumulated_map, input) = ingredient.accumulated(db, k.key_index);
60+
if let Some(accumulated_map) = accumulated_map {
5961
accumulated_map.extend_with_accumulated(accumulator.index(), &mut output);
60-
61-
// Skip over the inputs because we know that the entire sub-graph has no accumulated values
62-
if accumulated_map.inputs().is_empty() {
63-
continue;
64-
}
62+
}
63+
// Skip over the inputs because we know that the entire sub-graph has no accumulated values
64+
if input.is_empty() {
65+
continue;
6566
}
6667

6768
// Find the inputs of `k` and push them onto the stack.
@@ -95,9 +96,12 @@ where
9596
&'db self,
9697
db: &'db C::DbView,
9798
key: Id,
98-
) -> Option<&'db AccumulatedMap> {
99+
) -> (Option<&'db AccumulatedMap>, InputAccumulatedValues) {
99100
// NEXT STEP: stash and refactor `fetch` to return an `&Memo` so we can make this work
100101
let memo = self.refresh_memo(db, key);
101-
Some(&memo.revisions.accumulated)
102+
(
103+
memo.revisions.accumulated.as_deref(),
104+
memo.revisions.accumulated_inputs,
105+
)
102106
}
103107
}

‎src/function/fetch.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use super::{memo::Memo, Configuration, IngredientImpl};
2-
use crate::accumulator::accumulated_map::InputAccumulatedValues;
32
use crate::{runtime::StampedValue, zalsa::ZalsaDatabase, AsDynDatabase as _, Id};
43

54
impl<C> IngredientImpl<C>
@@ -25,7 +24,7 @@ where
2524
self.database_key_index(id).into(),
2625
durability,
2726
changed_at,
28-
InputAccumulatedValues::from_map(&memo.revisions.accumulated),
27+
memo.revisions.accumulated_inputs,
2928
);
3029

3130
value

‎src/function/memo.rs

+12-6
Original file line numberDiff line numberDiff line change
@@ -76,23 +76,25 @@ impl<C: Configuration> IngredientImpl<C> {
7676
}
7777
QueryOrigin::Derived(_) => {
7878
// QueryRevisions: !Clone to discourage cloning, we need it here though
79-
let QueryRevisions {
79+
let &QueryRevisions {
8080
changed_at,
8181
durability,
82-
origin,
83-
tracked_struct_ids,
84-
accumulated,
82+
ref origin,
83+
ref tracked_struct_ids,
84+
ref accumulated,
85+
accumulated_inputs,
8586
} = &memo.revisions;
8687
// Re-assemble the memo but with the value set to `None`
8788
Arc::new(Memo::new(
8889
None,
8990
memo.verified_at.load(),
9091
QueryRevisions {
91-
changed_at: *changed_at,
92-
durability: *durability,
92+
changed_at,
93+
durability,
9394
origin: origin.clone(),
9495
tracked_struct_ids: tracked_struct_ids.clone(),
9596
accumulated: accumulated.clone(),
97+
accumulated_inputs,
9698
},
9799
))
98100
}
@@ -115,6 +117,10 @@ pub(super) struct Memo<V> {
115117
pub(super) revisions: QueryRevisions,
116118
}
117119

120+
// Memo's are stored a lot, make sure there size is doesn't randomly increase.
121+
// #[cfg(test)]
122+
const _: [(); std::mem::size_of::<Memo<()>>()] = [(); std::mem::size_of::<[usize; 12]>()];
123+
118124
impl<V> Memo<V> {
119125
pub(super) fn new(value: Option<V>, revision_now: Revision, revisions: QueryRevisions) -> Self {
120126
Memo {

‎src/function/specify.rs

+1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ where
7070
origin: QueryOrigin::Assigned(active_query_key),
7171
tracked_struct_ids: Default::default(),
7272
accumulated: Default::default(),
73+
accumulated_inputs: Default::default(),
7374
};
7475

7576
if let Some(old_memo) = self.get_memo_from_table_for(zalsa, key) {

‎src/ingredient.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::{
44
};
55

66
use crate::{
7-
accumulator::accumulated_map::AccumulatedMap,
7+
accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues},
88
cycle::CycleRecoveryStrategy,
99
zalsa::{IngredientIndex, MemoIngredientIndex},
1010
zalsa_local::QueryOrigin,
@@ -74,7 +74,10 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
7474
&'db self,
7575
db: &'db dyn Database,
7676
key_index: Id,
77-
) -> Option<&'db AccumulatedMap>;
77+
) -> (Option<&'db AccumulatedMap>, InputAccumulatedValues) {
78+
_ = (db, key_index);
79+
(None, InputAccumulatedValues::Any)
80+
}
7881

7982
/// Invoked when the value `output_key` should be marked as valid in the current revision.
8083
/// This occurs because the value for `executor`, which generated it, was marked as valid

‎src/input.rs

-8
Original file line numberDiff line numberDiff line change
@@ -275,14 +275,6 @@ impl<C: Configuration> Ingredient for IngredientImpl<C> {
275275
fn debug_name(&self) -> &'static str {
276276
C::DEBUG_NAME
277277
}
278-
279-
fn accumulated<'db>(
280-
&'db self,
281-
_db: &'db dyn Database,
282-
_key_index: Id,
283-
) -> Option<&'db crate::accumulator::accumulated_map::AccumulatedMap> {
284-
None
285-
}
286278
}
287279

288280
impl<C: Configuration> std::fmt::Debug for IngredientImpl<C> {

‎src/input/input_field.rs

-8
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,6 @@ where
9696
fn debug_name(&self) -> &'static str {
9797
C::FIELD_DEBUG_NAMES[self.field_index]
9898
}
99-
100-
fn accumulated<'db>(
101-
&'db self,
102-
_db: &'db dyn Database,
103-
_key_index: Id,
104-
) -> Option<&'db crate::accumulator::accumulated_map::AccumulatedMap> {
105-
None
106-
}
10799
}
108100

109101
impl<C> std::fmt::Debug for FieldIngredientImpl<C>

‎src/interned.rs

-8
Original file line numberDiff line numberDiff line change
@@ -279,14 +279,6 @@ where
279279
fn debug_name(&self) -> &'static str {
280280
C::DEBUG_NAME
281281
}
282-
283-
fn accumulated<'db>(
284-
&'db self,
285-
_db: &'db dyn Database,
286-
_key_index: Id,
287-
) -> Option<&'db crate::accumulator::accumulated_map::AccumulatedMap> {
288-
None
289-
}
290282
}
291283

292284
impl<C> std::fmt::Debug for IngredientImpl<C>

‎src/tracked_struct.rs

-8
Original file line numberDiff line numberDiff line change
@@ -636,14 +636,6 @@ where
636636
}
637637

638638
fn reset_for_new_revision(&mut self) {}
639-
640-
fn accumulated<'db>(
641-
&'db self,
642-
_db: &'db dyn Database,
643-
_key_index: Id,
644-
) -> Option<&'db crate::accumulator::accumulated_map::AccumulatedMap> {
645-
None
646-
}
647639
}
648640

649641
impl<C> std::fmt::Debug for IngredientImpl<C>

‎src/tracked_struct/tracked_field.rs

-8
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,6 @@ where
112112
fn debug_name(&self) -> &'static str {
113113
C::FIELD_DEBUG_NAMES[self.field_index]
114114
}
115-
116-
fn accumulated<'db>(
117-
&'db self,
118-
_db: &'db dyn Database,
119-
_key_index: Id,
120-
) -> Option<&'db crate::accumulator::accumulated_map::AccumulatedMap> {
121-
None
122-
}
123115
}
124116

125117
impl<C> std::fmt::Debug for FieldIngredientImpl<C>

0 commit comments

Comments
 (0)