Skip to content

Commit f39263d

Browse files
authored
Merge pull request #987 from truecalc/perf/983-volatile-cache
perf(workbook): cache formula-cell volatility with the dependency graph
2 parents 06284df + 6e6e3b1 commit f39263d

5 files changed

Lines changed: 345 additions & 78 deletions

File tree

Lines changed: 79 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,127 +1,139 @@
11
{
2-
"recorded_at": "2026-08-28",
2+
"recorded_at": "2026-09-03",
33
"recorded_on": "Apple M1 Max (10 core), macOS 14.4, rustc 1.94.1, release profile; best of 5 full bench runs",
4-
"note": "ref_units = min(benchmark time) / min(reference time), each the best of 5 full bench runs, not necessarily the same run - best-of-5 minima can (and do) come from different runs. This is a deliberate choice, not an oversight: best-of-5 biases the floor in the safe direction, and the resulting bias in ref_units is small and one-directional (recorded baselines read slightly inflated, so future runs read slightly more negative). The gate compares ratios, not nanoseconds, so a baseline recorded on one machine still means something on another. best_ns_recorded and reference_best_ns_recorded are informational only - nothing is checked against them. The bands are two-sided on purpose: see .github/scripts/check_perf_regression.py. The five multi_sheet entries were recorded 2026-08-29 on the same machine, best of 3 runs. The four incremental_recalc(_cold)/independent_edit_root entries were re-recorded the same day, also best of 3, because this change moves them: Sheet1 was being folded ~16 times per formula cell even in a single-sheet workbook before this change (16.50 folds per extra formula cell, vs 0 after), so the single-sheet incremental family got legitimately faster (paired runs on one machine: base 216,815/212,730/215,348 ns vs head 196,590/196,909/195,420 ns on independent_edit_root/100, about -8%; independent_edit_root/1000 and the _cold variants moved similarly). The previously recorded independent_edit_root/100 baseline (best_ns_recorded 321,871) was already stale relative to both the base and head commits on this machine (both read ~200,000-300,000 ns) - this change added the last several points on top of that pre-existing drift, it did not cause the whole gap. Every other entry is unchanged from the 2026-08-28 recording and remains within run-to-run noise of the base commit. The six chain entries (full_recalc/chain and incremental_recalc/chain_edit_root|chain_edit_leaf, each at 1000 and 5000) were added 2026-08-29 and recorded on the machine named above, best of 5 full bench runs - the count this file's recorded_on documents, not the best-of-3 the two exceptions above used. All five runs measured one unmodified binary, and their reference minimum was 3,196,028 ns - not the reference_best_ns_recorded field above, which belongs to the 2026-08-28 set - so verify these six as best_ns_recorded / 3196028, the same way the 2026-08-29 entries above divide by their own run set's minimum. They are new benchmarks, so no existing entry moved when they were added; every other number here is unchanged from its earlier recording. The incremental_recalc/all_literal_edit/120000cells entry (issue #984) was added 2026-09-03 on the machine named above, best of 5 full bench runs of the two benchmarks alone (not the whole suite) - its reference minimum was 3,335,120 ns, again not the reference_best_ns_recorded field above; verify it as best_ns_recorded / 3335120. It is a new benchmark, so no existing entry moved when it was added.",
4+
"note": "Single-run re-record after rebasing #983 (volatile-cell cache) onto main post-#984 (spill-anchor cache) merge \u2014 every entry comes from ONE cargo bench run of the fully-combined code (rustc 1.98.0 (88d9e12ae 2026-08-18), release profile), not a hand-merge of two separate recordings. Includes both PRs new groups: incremental_recalc/row_totals_volatile_seed/{100,10000,100000} (#983) and incremental_recalc/all_literal_edit/120000cells (#984). best_ns_recorded and reference_best_ns_recorded are informational only - nothing is checked against them. The bands are two-sided on purpose: see .github/scripts/check_perf_regression.py.",
55
"reference": "calibration/hash_alloc",
6-
"reference_best_ns_recorded": 3270207,
6+
"reference_best_ns_recorded": 3164388,
77
"regression_pct": 150,
88
"improvement_pct": 40,
99
"benchmarks": {
1010
"full_recalc/independent/100": {
11-
"ref_units": 0.1436,
12-
"best_ns_recorded": 469632
11+
"ref_units": 0.1391,
12+
"best_ns_recorded": 440310
1313
},
1414
"full_recalc/independent/1000": {
15-
"ref_units": 1.3395,
16-
"best_ns_recorded": 4380494
15+
"ref_units": 1.2879,
16+
"best_ns_recorded": 4075270
1717
},
1818
"full_recalc/independent/5000": {
19-
"ref_units": 6.9069,
20-
"best_ns_recorded": 22587145
19+
"ref_units": 7.0781,
20+
"best_ns_recorded": 22397889
2121
},
2222
"full_recalc/chain/1000": {
23-
"ref_units": 1.196,
24-
"best_ns_recorded": 3822395
23+
"ref_units": 1.2234,
24+
"best_ns_recorded": 3871375
2525
},
2626
"full_recalc/chain/5000": {
27-
"ref_units": 6.5853,
28-
"best_ns_recorded": 21046666
27+
"ref_units": 6.7395,
28+
"best_ns_recorded": 21326255
2929
},
3030
"full_recalc/row_totals/500": {
31-
"ref_units": 1.3837,
32-
"best_ns_recorded": 4524877
31+
"ref_units": 1.4669,
32+
"best_ns_recorded": 4641820
3333
},
3434
"full_recalc/row_totals/2000": {
35-
"ref_units": 6.0191,
36-
"best_ns_recorded": 19683565
35+
"ref_units": 6.3515,
36+
"best_ns_recorded": 20098562
3737
},
3838
"full_recalc/block_subtotals/2000": {
39-
"ref_units": 1.2934,
40-
"best_ns_recorded": 4229585
39+
"ref_units": 1.3706,
40+
"best_ns_recorded": 4337191
4141
},
4242
"full_recalc/block_subtotals/10000": {
43-
"ref_units": 8.0703,
44-
"best_ns_recorded": 26391430
43+
"ref_units": 8.2032,
44+
"best_ns_recorded": 25957993
4545
},
4646
"full_recalc/multi_sheet/200x50": {
47-
"ref_units": 13.8314,
48-
"best_ns_recorded": 45612155
47+
"ref_units": 13.9405,
48+
"best_ns_recorded": 44113229
4949
},
5050
"full_recalc/multi_sheet_long_names/200x50": {
51-
"ref_units": 13.2369,
52-
"best_ns_recorded": 43651501
51+
"ref_units": 13.3224,
52+
"best_ns_recorded": 42157171
5353
},
5454
"full_recalc/multi_sheet_cross_refs/200x50": {
55-
"ref_units": 13.4762,
56-
"best_ns_recorded": 44440765
55+
"ref_units": 15.199,
56+
"best_ns_recorded": 48095548
5757
},
5858
"full_recalc/tall_sparse/20000": {
59-
"ref_units": 39.9981,
60-
"best_ns_recorded": 130802021
59+
"ref_units": 40.8564,
60+
"best_ns_recorded": 129285553
6161
},
6262
"depgraph_build/tall_sparse/20000": {
63-
"ref_units": 9.9993,
64-
"best_ns_recorded": 32699704
63+
"ref_units": 9.9017,
64+
"best_ns_recorded": 31332860
6565
},
6666
"depgraph_build/row_totals/2000": {
67-
"ref_units": 0.9512,
68-
"best_ns_recorded": 3110508
67+
"ref_units": 0.9418,
68+
"best_ns_recorded": 2980216
6969
},
7070
"depgraph_build/multi_sheet_cross_refs/200x50": {
71-
"ref_units": 3.2537,
72-
"best_ns_recorded": 10729783
71+
"ref_units": 3.1996,
72+
"best_ns_recorded": 10124723
7373
},
7474
"incremental_recalc/independent_edit_root/100": {
75-
"ref_units": 0.0605,
76-
"best_ns_recorded": 192758
75+
"ref_units": 0.0566,
76+
"best_ns_recorded": 179180
7777
},
7878
"incremental_recalc/independent_edit_root/1000": {
79-
"ref_units": 0.3549,
80-
"best_ns_recorded": 1130495
79+
"ref_units": 0.2876,
80+
"best_ns_recorded": 910143
81+
},
82+
"incremental_recalc_cold/independent_edit_root/100": {
83+
"ref_units": 0.0919,
84+
"best_ns_recorded": 290789
85+
},
86+
"incremental_recalc_cold/independent_edit_root/1000": {
87+
"ref_units": 0.736,
88+
"best_ns_recorded": 2329101
89+
},
90+
"incremental_recalc/multi_sheet_edit_root/200x50": {
91+
"ref_units": 2.5644,
92+
"best_ns_recorded": 8114804
8193
},
8294
"incremental_recalc/block_subtotals_edit_root/10000": {
83-
"ref_units": 1.2095,
84-
"best_ns_recorded": 3823493
95+
"ref_units": 0.7581,
96+
"best_ns_recorded": 2398977
8597
},
8698
"incremental_recalc/chain_edit_root/1000": {
87-
"ref_units": 1.1638,
88-
"best_ns_recorded": 3719666
99+
"ref_units": 1.0771,
100+
"best_ns_recorded": 3408477
89101
},
90102
"incremental_recalc/chain_edit_root/5000": {
91-
"ref_units": 6.3891,
92-
"best_ns_recorded": 20419766
103+
"ref_units": 5.9662,
104+
"best_ns_recorded": 18879251
93105
},
94106
"incremental_recalc/chain_edit_leaf/1000": {
95-
"ref_units": 0.3183,
96-
"best_ns_recorded": 1017376
107+
"ref_units": 0.2646,
108+
"best_ns_recorded": 837166
97109
},
98110
"incremental_recalc/chain_edit_leaf/5000": {
99-
"ref_units": 1.6274,
100-
"best_ns_recorded": 5201330
111+
"ref_units": 1.3121,
112+
"best_ns_recorded": 4152095
101113
},
102-
"incremental_recalc/multi_sheet_edit_root/200x50": {
103-
"ref_units": 3.2604,
104-
"best_ns_recorded": 10751809
114+
"incremental_recalc/all_literal_edit/120000cells": {
115+
"ref_units": 3.8168,
116+
"best_ns_recorded": 12077798
105117
},
106-
"from_json/500rows": {
107-
"ref_units": 0.2833,
108-
"best_ns_recorded": 926457
118+
"incremental_recalc/row_totals_volatile_seed/100": {
119+
"ref_units": 0.0793,
120+
"best_ns_recorded": 250884
109121
},
110-
"to_json/500rows": {
111-
"ref_units": 0.2216,
112-
"best_ns_recorded": 724738
122+
"incremental_recalc/row_totals_volatile_seed/10000": {
123+
"ref_units": 7.5273,
124+
"best_ns_recorded": 23819279
113125
},
114-
"incremental_recalc_cold/independent_edit_root/100": {
115-
"ref_units": 0.0933,
116-
"best_ns_recorded": 297138
126+
"incremental_recalc/row_totals_volatile_seed/100000": {
127+
"ref_units": 96.2568,
128+
"best_ns_recorded": 304593843
117129
},
118-
"incremental_recalc_cold/independent_edit_root/1000": {
119-
"ref_units": 0.7459,
120-
"best_ns_recorded": 2375688
130+
"from_json/500rows": {
131+
"ref_units": 0.2837,
132+
"best_ns_recorded": 897782
121133
},
122-
"incremental_recalc/all_literal_edit/120000cells": {
123-
"ref_units": 3.7609,
124-
"best_ns_recorded": 12543118
134+
"to_json/500rows": {
135+
"ref_units": 0.2194,
136+
"best_ns_recorded": 694132
125137
}
126138
}
127139
}

‎crates/workbook/benches/workbook_perf.rs‎

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -499,12 +499,14 @@ fn bench_incremental_recalc(c: &mut Criterion) {
499499
// Read the two together, not the ratio between them. That ratio is roughly
500500
// flat (3.7x at n=1000, 3.9x at n=5000 when recorded), because both cases
501501
// pay the same per-formula-cell fixed cost every incremental recalc owes —
502-
// `snapshot_formula_values`, the volatile sweep over `formula_cells()`, and
503-
// `seed_spill_sensitive`, each `O(formula cells)` by construction. At
504-
// n=5000 the leaf case's time is almost entirely that floor rather than its
505-
// one-cell recompute, which is precisely what makes it worth recording: no
506-
// other incremental benchmark holds the dirty set fixed while the workbook
507-
// grows, so nothing else here can see that floor at all.
502+
// `snapshot_formula_values` and `seed_spill_sensitive`, each
503+
// `O(formula cells)` by construction (the volatile sweep over
504+
// `formula_cells()` was a third such term until issue #983 made it a
505+
// cached lookup instead). At n=5000 the leaf case's time is almost
506+
// entirely that remaining floor rather than its one-cell recompute, which
507+
// is precisely what makes it worth recording: no other incremental
508+
// benchmark holds the dirty set fixed while the workbook grows, so
509+
// nothing else here can see that floor at all.
508510
//
509511
// The regression signal is the *gap*: a leaf closure that silently widened
510512
// to the whole chain would jump the leaf case onto the root case's number
@@ -579,6 +581,39 @@ fn bench_incremental_recalc(c: &mut Criterion) {
579581
});
580582
});
581583
group.finish();
584+
585+
// Reproduces the exact shape issue #983 measured: a formula-heavy
586+
// document with zero actually-volatile functions, timing a single
587+
// unrelated literal write. Before the fix this scaled linearly with
588+
// formula count (367ms at n=100,000, dirty closure verified at 1)
589+
// because the volatile-seeding loop re-derived every formula cell's
590+
// volatility from scratch on every call.
591+
//
592+
// The fix makes *that term* O(1) in formula count, but the numbers this
593+
// group records are not themselves flat: `snapshot_formula_values` and
594+
// `seed_spill_sensitive` are two other O(formula cells) passes this same
595+
// call still pays on every incremental recalc (see `chain_edit_leaf`'s
596+
// comment above, which documents the same floor), and issue #983 does not
597+
// touch either. Don't read a still-scaling number here as the fix having
598+
// failed — the isolated volatile-rescan term it actually targets is gone;
599+
// what remains is the other two terms, tracked separately.
600+
let mut group = c.benchmark_group("incremental_recalc/row_totals_volatile_seed");
601+
group.sample_size(10);
602+
group.measurement_time(Duration::from_secs(5));
603+
for n in [100u32, 10_000, 100_000] {
604+
let mut template = build_row_totals(n);
605+
template.recalc(&ctx);
606+
group.bench_with_input(BenchmarkId::from_parameter(n), &n, |b, _| {
607+
b.iter(|| {
608+
let mut wb = template.clone();
609+
let a1 = Address::new(1, 1).unwrap();
610+
wb.set("Sheet1", a1, CellInput::Literal(Value::Number(99.0)))
611+
.unwrap();
612+
wb.recalc_incremental(&ctx, &[("Sheet1".to_string(), a1)])
613+
});
614+
});
615+
}
616+
group.finish();
582617
}
583618

584619
fn bench_from_json(c: &mut Criterion) {

‎crates/workbook/src/graph_cache.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ pub struct CachedGraph {
8585
pub(crate) order: Vec<CellRef>,
8686
/// The cycle set from that same pass.
8787
pub(crate) cycle: BTreeSet<CellRef>,
88+
/// Every formula cell whose formula text names a volatile function
89+
/// (`Workbook::is_volatile`), for this exact graph — computed once at
90+
/// build time so `recalc_incremental`'s seeding step never re-derives it
91+
/// per call (issue #983).
92+
pub(crate) volatile: BTreeSet<CellRef>,
8893
}
8994

9095
impl CachedGraph {

‎crates/workbook/src/recalc.rs‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -305,10 +305,23 @@ impl Workbook {
305305
// times per incremental recalc (the spill widen loop) while ordering
306306
// the same graph every time.
307307
let (order, cycle) = graph.evaluation_order();
308+
// Likewise derived from the graph and cached alongside it: a formula
309+
// cell's volatility cannot change without the formula text changing,
310+
// which already invalidates this cache — so this is the one place
311+
// `is_volatile` needs to run, once per formula cell per graph build,
312+
// instead of once per formula cell per incremental recalc (issue
313+
// #983).
314+
let sheets = SheetIndex::build(self);
315+
let volatile: BTreeSet<CellRef> = graph
316+
.formula_cells()
317+
.filter(|cell| self.is_volatile(&sheets, cell))
318+
.cloned()
319+
.collect();
308320
let entry = Arc::new(CachedGraph {
309321
graph,
310322
order,
311323
cycle,
324+
volatile,
312325
});
313326
self.store_cached_graph(entry.clone());
314327
entry
@@ -412,11 +425,11 @@ impl Workbook {
412425
}
413426
}
414427

415-
// (b) Volatile cells are always dirty (scope ADR Decision 3).
416-
for cell in graph.formula_cells() {
417-
if self.is_volatile(&sheets, cell) {
418-
frontier.insert(cell.clone());
419-
}
428+
// (b) Volatile cells are always dirty (scope ADR Decision 3). The set
429+
// is cached on the graph at build time (issue #983) — no
430+
// re-derivation here.
431+
for cell in &cached.volatile {
432+
frontier.insert(cell.clone());
420433
}
421434

422435
// Spill-occupancy seeding (issue #591). A cell's spill footprint or

0 commit comments

Comments
 (0)