Skip to content

Commit d1a042b

Browse files
authored
refactor: Introduce MemoHeader to reduce monomorphization (#1142)
* refactor: erase configuration from memo storage * refactor: clarify cycle completion outcomes * refactor: preserve comments during memo extraction * refactor: remove memo revision forwarding method * refactor: simplify query completion and validation * fix: reduce hot memo validation cache misses * fix: preserve behavior during memo extraction
1 parent a7d212d commit d1a042b

12 files changed

Lines changed: 664 additions & 437 deletions

src/function.rs

Lines changed: 127 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ where
269269
mut memo: memo::Memo<'db, C>,
270270
memo_ingredient_index: MemoIngredientIndex,
271271
) -> &'db memo::Memo<'db, C> {
272-
if let Some(tracked_struct_ids) = memo.revisions.tracked_struct_ids_mut() {
272+
if let Some(tracked_struct_ids) = memo.header.revisions.tracked_struct_ids_mut() {
273273
tracked_struct_ids.shrink_to_fit();
274274
}
275275

@@ -343,30 +343,8 @@ where
343343
return;
344344
};
345345

346-
let origin = memo.revisions.origin();
347-
348-
visited_edges.insert(edge);
349-
350-
// Collect the minimum dependency tree.
351-
for edge in origin.edges() {
352-
// Avoid forming cycles.
353-
if visited_edges.contains(&edge) {
354-
continue;
355-
}
356-
357-
// Avoid flattening edges that we're going to serialize directly.
358-
if serialized_edges.contains(&edge) {
359-
continue;
360-
}
361-
362-
let dependency = zalsa.lookup_ingredient(edge.key().ingredient_index());
363-
dependency.collect_minimum_serialized_edges(
364-
zalsa,
365-
edge,
366-
serialized_edges,
367-
visited_edges,
368-
)
369-
}
346+
memo.header
347+
.collect_minimum_serialized_edges(zalsa, edge, serialized_edges, visited_edges);
370348
}
371349

372350
/// Returns `final` if the memo has the `verified_final` flag set.
@@ -381,21 +359,7 @@ where
381359
let memo =
382360
self.get_memo_from_table_for(zalsa, input, self.memo_ingredient_index(zalsa, input))?;
383361

384-
let iteration = memo.revisions.iteration();
385-
let verified_final = memo.revisions.verified_final.load(Ordering::Relaxed);
386-
387-
Some(if verified_final {
388-
ProvisionalStatus::Final {
389-
iteration,
390-
verified_at: memo.verified_at.load(),
391-
}
392-
} else {
393-
ProvisionalStatus::Provisional {
394-
iteration,
395-
verified_at: memo.verified_at.load(),
396-
cycle_heads: memo.cycle_heads(),
397-
}
398-
})
362+
Some(memo.header.provisional_status())
399363
}
400364

401365
fn set_cycle_iteration_count(&self, zalsa: &Zalsa, input: Id, iteration: IterationStamp) {
@@ -405,8 +369,8 @@ where
405369
return;
406370
};
407371

408-
memo.revisions
409-
.set_iteration_count(Self::database_key_index(self, input), iteration);
372+
memo.header
373+
.set_cycle_iteration_count(self.database_key_index(input), iteration);
410374
}
411375

412376
fn finalize_cycle_head(&self, zalsa: &Zalsa, input: Id) {
@@ -416,7 +380,7 @@ where
416380
return;
417381
};
418382

419-
memo.revisions.verified_final.store(true, Ordering::Release);
383+
memo.header.finalize_cycle_head();
420384
}
421385

422386
fn flatten_cycle_head_dependencies(
@@ -431,41 +395,13 @@ where
431395
return;
432396
};
433397

434-
let database_key_index = self.database_key_index(id);
435-
436-
// Only flatten dependencies of provisional queries, because only those
437-
// contain cyclic dependencies.
438-
if !memo.may_be_provisional() {
439-
flattened_input_outputs.insert(QueryEdge::input(database_key_index));
440-
return;
441-
}
442-
443-
// There's nothing to do if we've visited this query before.
444-
if !seen.insert(database_key_index) {
445-
return;
446-
}
447-
448-
let inputs = memo.revisions.origin().inputs();
449-
450-
match C::CYCLE_STRATEGY {
451-
// For queries with cycle handling, simply extend the input/outputs, because
452-
// they already flattened their own dependencies when completing the query.
453-
CycleRecoveryStrategy::FallbackImmediate | CycleRecoveryStrategy::Fixpoint => {
454-
flattened_input_outputs.extend(inputs.map(QueryEdge::input));
455-
}
456-
// For regular queries, recurse
457-
CycleRecoveryStrategy::Panic => {
458-
for input in inputs {
459-
let ingredient = zalsa.lookup_ingredient(input.ingredient_index());
460-
ingredient.flatten_cycle_head_dependencies(
461-
zalsa,
462-
input.key_index(),
463-
flattened_input_outputs,
464-
seen,
465-
);
466-
}
467-
}
468-
}
398+
memo.header.flatten_cycle_head_dependencies(
399+
zalsa,
400+
self.database_key_index(id),
401+
C::CYCLE_STRATEGY,
402+
flattened_input_outputs,
403+
seen,
404+
);
469405
}
470406

471407
fn cycle_converged(&self, zalsa: &Zalsa, input: Id) -> bool {
@@ -475,7 +411,7 @@ where
475411
return true;
476412
};
477413

478-
memo.revisions.cycle_converged()
414+
memo.header.cycle_converged()
479415
}
480416

481417
fn mark_as_transfer_target(&self, key_index: Id) -> Option<SyncOwner> {
@@ -621,6 +557,117 @@ where
621557
}
622558
}
623559

560+
impl memo::MemoHeader {
561+
fn collect_minimum_serialized_edges(
562+
&self,
563+
zalsa: &Zalsa,
564+
edge: QueryEdge,
565+
serialized_edges: &mut FxIndexSet<QueryEdge>,
566+
visited_edges: &mut FxHashSet<QueryEdge>,
567+
) {
568+
visited_edges.insert(edge);
569+
570+
// Collect the minimum dependency tree.
571+
for edge in self.origin().edges() {
572+
// Avoid forming cycles.
573+
if visited_edges.contains(&edge) {
574+
continue;
575+
}
576+
577+
// Avoid flattening edges that we're going to serialize directly.
578+
if serialized_edges.contains(&edge) {
579+
continue;
580+
}
581+
582+
let dependency = zalsa.lookup_ingredient(edge.key().ingredient_index());
583+
dependency.collect_minimum_serialized_edges(
584+
zalsa,
585+
edge,
586+
serialized_edges,
587+
visited_edges,
588+
);
589+
}
590+
}
591+
592+
fn provisional_status(&self) -> ProvisionalStatus<'_> {
593+
let iteration = self.revisions.iteration();
594+
let verified_at = self.verified_at.load();
595+
596+
if self.revisions.verified_final.load(Ordering::Relaxed) {
597+
ProvisionalStatus::Final {
598+
iteration,
599+
verified_at,
600+
}
601+
} else {
602+
ProvisionalStatus::Provisional {
603+
iteration,
604+
verified_at,
605+
cycle_heads: self.cycle_heads(),
606+
}
607+
}
608+
}
609+
610+
fn set_cycle_iteration_count(
611+
&self,
612+
database_key_index: DatabaseKeyIndex,
613+
iteration: IterationStamp,
614+
) {
615+
self.revisions
616+
.set_iteration_count(database_key_index, iteration);
617+
}
618+
619+
fn finalize_cycle_head(&self) {
620+
self.revisions.verified_final.store(true, Ordering::Release);
621+
}
622+
623+
fn flatten_cycle_head_dependencies(
624+
&self,
625+
zalsa: &Zalsa,
626+
database_key_index: DatabaseKeyIndex,
627+
cycle_recovery_strategy: CycleRecoveryStrategy,
628+
flattened_input_outputs: &mut FxIndexSet<QueryEdge>,
629+
seen: &mut FxHashSet<DatabaseKeyIndex>,
630+
) {
631+
// Only flatten dependencies of provisional queries, because only those
632+
// contain cyclic dependencies.
633+
if !self.may_be_provisional() {
634+
flattened_input_outputs.insert(QueryEdge::input(database_key_index));
635+
return;
636+
}
637+
638+
// There's nothing to do if we've visited this query before.
639+
if !seen.insert(database_key_index) {
640+
return;
641+
}
642+
643+
let inputs = self.origin().inputs();
644+
645+
match cycle_recovery_strategy {
646+
// For queries with cycle handling, simply extend the input/outputs, because
647+
// they already flattened their own dependencies when completing the query.
648+
CycleRecoveryStrategy::FallbackImmediate | CycleRecoveryStrategy::Fixpoint => {
649+
flattened_input_outputs.extend(inputs.map(QueryEdge::input));
650+
}
651+
// For regular queries, recurse
652+
CycleRecoveryStrategy::Panic => {
653+
for input in inputs {
654+
let ingredient = zalsa.lookup_ingredient(input.ingredient_index());
655+
ingredient.flatten_cycle_head_dependencies(
656+
zalsa,
657+
input.key_index(),
658+
flattened_input_outputs,
659+
seen,
660+
);
661+
}
662+
}
663+
}
664+
}
665+
666+
fn cycle_converged(&self) -> bool {
667+
self.revisions.cycle_converged()
668+
}
669+
}
670+
624671
impl<C> std::fmt::Debug for IngredientImpl<C>
625672
where
626673
C: Configuration,
@@ -699,7 +746,7 @@ mod persistence {
699746

700747
if let Some(memo) = memo.filter(|memo| memo.should_serialize()) {
701748
// Flatten the dependencies of this query down to the base inputs.
702-
let flattened_origin = match memo.revisions.origin() {
749+
let flattened_origin = match memo.header.origin() {
703750
QueryOriginRef::Derived(edges) => {
704751
collect_minimum_serialized_edges(
705752
zalsa,

src/function/accumulated.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,8 @@ where
9696
// NEXT STEP: stash and refactor `fetch` to return an `&Memo` so we can make this work
9797
let memo = self.refresh_memo(db, zalsa, zalsa_local, key);
9898
(
99-
memo.revisions.accumulated(),
100-
memo.revisions.accumulated_inputs.load(),
99+
memo.header.revisions.accumulated(),
100+
memo.header.revisions.accumulated_inputs.load(),
101101
)
102102
}
103103
}

src/function/backdate.rs

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::Backtrace;
22
use crate::DatabaseKeyIndex;
3-
use crate::function::memo::Memo;
3+
use crate::function::memo::{Memo, MemoHeader};
44
use crate::function::{Configuration, IngredientImpl};
55
use crate::zalsa_local::QueryRevisions;
66
use std::fmt;
@@ -19,39 +19,44 @@ where
1919
revisions: &mut QueryRevisions,
2020
value: &C::Output<'db>,
2121
) {
22+
if old_memo.header.can_backdate(revisions)
23+
&& old_memo
24+
.value
25+
.as_ref()
26+
.is_some_and(|old_value| C::values_equal(old_value, value))
27+
{
28+
old_memo.header.backdate(index, revisions);
29+
}
30+
}
31+
}
32+
33+
impl MemoHeader {
34+
fn can_backdate(&self, revisions: &QueryRevisions) -> bool {
2235
// We've seen issues where queries weren't re-validated when backdating provisional values
2336
// in ty. This is more of a bandaid because we're close to a release and don't have the time to prove
2437
// right now whether backdating could be made safe for queries participating in queries.
2538
// TODO: Write a test that demonstrates that backdating queries participating in a cycle isn't safe
2639
// OR write many tests showing that it is (and fixing the case where it didn't correctly account for today).
27-
if !revisions.cycle_heads().is_empty() || old_memo.may_be_provisional() {
28-
return;
29-
}
30-
31-
if let Some(old_value) = &old_memo.value {
40+
revisions.cycle_heads().is_empty()
41+
&& !self.may_be_provisional()
3242
// Careful: if the value became less durable than it
3343
// used to be, that is a "breaking change" that our
3444
// consumers must be aware of. Becoming *more* durable
3545
// is not. See the test `durable_to_less_durable`.
36-
if revisions.durability >= old_memo.revisions.durability
37-
&& C::values_equal(old_value, value)
38-
{
39-
crate::tracing::debug!(
40-
"{index:?} value is equal, back-dating to {:?}",
41-
old_memo.revisions.changed_at,
42-
);
46+
&& revisions.durability >= self.revisions.durability
47+
}
4348

44-
if old_memo.revisions.changed_at > revisions.changed_at {
45-
report_backdate_violation(
46-
index,
47-
old_memo.revisions.changed_at,
48-
revisions.changed_at,
49-
);
50-
}
49+
fn backdate(&self, index: DatabaseKeyIndex, revisions: &mut QueryRevisions) {
50+
crate::tracing::debug!(
51+
"{index:?} value is equal, back-dating to {:?}",
52+
self.revisions.changed_at,
53+
);
5154

52-
revisions.changed_at = old_memo.revisions.changed_at;
53-
}
55+
if self.revisions.changed_at > revisions.changed_at {
56+
report_backdate_violation(index, self.revisions.changed_at, revisions.changed_at);
5457
}
58+
59+
revisions.changed_at = self.revisions.changed_at;
5560
}
5661
}
5762

0 commit comments

Comments
 (0)