fix(incremental): meter the whole-cache recompute and correct the step 5 notes (improvise-shbc)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
committed by
Edward Langley
parent
effca3f663
commit
4abeb7829d
@@ -221,10 +221,11 @@ taken where clause 4 and clause 7 leave a choice; evidence is in
|
||||
|
||||
## Step 5: formulas (improvise-shbc)
|
||||
|
||||
`Incremental` no longer calls `recompute_formulas`. It keeps the formula cache (`Model::formula_cell`,
|
||||
`set_formula_cell`, `evaluate_formula_cell`, the three small additions to `improvise-core`) and decides
|
||||
which cells of it to evaluate again; the evaluator that computes each value is core's own. The ADR
|
||||
(clauses 5 and 6) is followed in outline, and the places it is not are the reason for this section.
|
||||
`Incremental` calls `recompute_formulas` only for the documents item 2 calls wide. Otherwise it keeps
|
||||
the formula cache itself (`Model::formula_cell`, `set_formula_cell`, `clear_formula_cells` and the
|
||||
now-public `evaluate_formula_cell`, the small additions to `improvise-core`) and decides which cells of
|
||||
it to evaluate again; the evaluator that computes each value is core's own. The ADR (clauses 5 and 6)
|
||||
is followed in outline, and the places it is not are the reason for this section.
|
||||
|
||||
1. **A stem is the unit, because eager's cache is per stem.** `Stems` (`formulas.rs`) holds, per
|
||||
target category, how many stored cells give each stem (a cell minus that category and the
|
||||
|
||||
@@ -453,7 +453,7 @@ One shared text is that text, two are `mixed`, a number among text is `mixed`, a
|
||||
|
||||
**How the faults were injected.** Twenty-seven faults went into `src/formulas.rs` and
|
||||
`src/incremental.rs`, one at a time, each run against the whole crate and reverted before the next.
|
||||
A first pass left eight alive; five were a gap in the tests and were closed by the tests below, and
|
||||
A first pass left eight alive; six were a gap in the tests and were closed by the tests below, and
|
||||
the remaining two (`born-stems-not-evaluated`, `target-category-not-read`) cannot change an answer.
|
||||
What each fault was and the test that names it:
|
||||
- a cell that gives a stem is not enough to evaluate it / no reader is dirtied:
|
||||
@@ -596,3 +596,9 @@ sixteenth pass to see nothing move.
|
||||
### formulas::tests::what_a_formula_reads_includes_what_the_formulas_it_names_read
|
||||
|
||||
`Margin` names `Profit`, which names `Cost`; the reads of `Margin` include `Cost`.
|
||||
|
||||
### the_whole_cache_recompute_of_a_wide_document_is_metered
|
||||
|
||||
A document with formulas in two target categories falls back to `recompute_formulas`; the fallback was
|
||||
doing the work with the meter at zero (Copilot on PR #351). It now counts `Meter::recomputes`, and a
|
||||
document with one target category counts none.
|
||||
|
||||
@@ -225,6 +225,7 @@ impl Incremental {
|
||||
/// cells, recomputes every formula cell as eager does.
|
||||
fn evaluate(&mut self, stems: impl IntoIterator<Item = CellKey>) {
|
||||
if self.stems.is_wide() {
|
||||
self.work.recomputes += 1;
|
||||
return self.model.recompute_formulas(&self.none_cats);
|
||||
}
|
||||
for stem in stems {
|
||||
|
||||
@@ -24,6 +24,9 @@ pub struct Meter {
|
||||
pub formula_cells: u64,
|
||||
/// Times the stems were rebuilt from the store, as a formula edit does.
|
||||
pub rebuilds: u64,
|
||||
/// Whole-cache recomputes by the fallback for documents whose stems read each other. One is
|
||||
/// every stem's formulas for every pass, so it is counted as an event, not as cells.
|
||||
pub recomputes: u64,
|
||||
}
|
||||
|
||||
impl Meter {
|
||||
@@ -36,6 +39,7 @@ impl Meter {
|
||||
stems: self.stems + o.stems,
|
||||
formula_cells: self.formula_cells + o.formula_cells,
|
||||
rebuilds: self.rebuilds + o.rebuilds,
|
||||
recomputes: self.recomputes + o.recomputes,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -191,6 +191,33 @@ fn two_target_categories_read_each_others_stems_whatever_the_order_they_are_adde
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_whole_cache_recompute_of_a_wide_document_is_metered() {
|
||||
let wb = formula_workbook();
|
||||
let mut backend = Incremental::new(wb.model.clone(), none_of(&wb));
|
||||
let edit = bump("East", "A", "Revenue", 3.0);
|
||||
backend.apply(&edit);
|
||||
assert_eq!(
|
||||
backend.meter().recomputes,
|
||||
0,
|
||||
"one target category: by stem"
|
||||
);
|
||||
backend.apply(&formula_edit(14));
|
||||
let before = backend.meter();
|
||||
backend.apply(&edit);
|
||||
backend.apply(&bump("East", "A", "Revenue", 4.0));
|
||||
let spent = backend.meter();
|
||||
assert_eq!(
|
||||
spent.recomputes - before.recomputes,
|
||||
2,
|
||||
"Region and _Measure targets: wide"
|
||||
);
|
||||
assert_eq!(
|
||||
spent.stems, before.stems,
|
||||
"and no stem is evaluated on its own"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_name_in_another_category_reads_another_stems_cell() {
|
||||
let mut wb = formula_workbook();
|
||||
|
||||
@@ -67,6 +67,7 @@ fn since(before: Meter, after: Meter) -> Meter {
|
||||
stems: after.stems - before.stems,
|
||||
formula_cells: after.formula_cells - before.formula_cells,
|
||||
rebuilds: after.rebuilds - before.rebuilds,
|
||||
recomputes: after.recomputes - before.recomputes,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user