Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 79 additions & 67 deletions crates/workbook/benches/baselines.json
Original file line number Diff line number Diff line change
@@ -1,127 +1,139 @@
{
"recorded_at": "2026-08-28",
"recorded_at": "2026-09-03",
"recorded_on": "Apple M1 Max (10 core), macOS 14.4, rustc 1.94.1, release profile; best of 5 full bench runs",
"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.",
"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.",
"reference": "calibration/hash_alloc",
"reference_best_ns_recorded": 3270207,
"reference_best_ns_recorded": 3164388,
"regression_pct": 150,
"improvement_pct": 40,
"benchmarks": {
"full_recalc/independent/100": {
"ref_units": 0.1436,
"best_ns_recorded": 469632
"ref_units": 0.1391,
"best_ns_recorded": 440310
},
"full_recalc/independent/1000": {
"ref_units": 1.3395,
"best_ns_recorded": 4380494
"ref_units": 1.2879,
"best_ns_recorded": 4075270
},
"full_recalc/independent/5000": {
"ref_units": 6.9069,
"best_ns_recorded": 22587145
"ref_units": 7.0781,
"best_ns_recorded": 22397889
},
"full_recalc/chain/1000": {
"ref_units": 1.196,
"best_ns_recorded": 3822395
"ref_units": 1.2234,
"best_ns_recorded": 3871375
},
"full_recalc/chain/5000": {
"ref_units": 6.5853,
"best_ns_recorded": 21046666
"ref_units": 6.7395,
"best_ns_recorded": 21326255
},
"full_recalc/row_totals/500": {
"ref_units": 1.3837,
"best_ns_recorded": 4524877
"ref_units": 1.4669,
"best_ns_recorded": 4641820
},
"full_recalc/row_totals/2000": {
"ref_units": 6.0191,
"best_ns_recorded": 19683565
"ref_units": 6.3515,
"best_ns_recorded": 20098562
},
"full_recalc/block_subtotals/2000": {
"ref_units": 1.2934,
"best_ns_recorded": 4229585
"ref_units": 1.3706,
"best_ns_recorded": 4337191
},
"full_recalc/block_subtotals/10000": {
"ref_units": 8.0703,
"best_ns_recorded": 26391430
"ref_units": 8.2032,
"best_ns_recorded": 25957993
},
"full_recalc/multi_sheet/200x50": {
"ref_units": 13.8314,
"best_ns_recorded": 45612155
"ref_units": 13.9405,
"best_ns_recorded": 44113229
},
"full_recalc/multi_sheet_long_names/200x50": {
"ref_units": 13.2369,
"best_ns_recorded": 43651501
"ref_units": 13.3224,
"best_ns_recorded": 42157171
},
"full_recalc/multi_sheet_cross_refs/200x50": {
"ref_units": 13.4762,
"best_ns_recorded": 44440765
"ref_units": 15.199,
"best_ns_recorded": 48095548
},
"full_recalc/tall_sparse/20000": {
"ref_units": 39.9981,
"best_ns_recorded": 130802021
"ref_units": 40.8564,
"best_ns_recorded": 129285553
},
"depgraph_build/tall_sparse/20000": {
"ref_units": 9.9993,
"best_ns_recorded": 32699704
"ref_units": 9.9017,
"best_ns_recorded": 31332860
},
"depgraph_build/row_totals/2000": {
"ref_units": 0.9512,
"best_ns_recorded": 3110508
"ref_units": 0.9418,
"best_ns_recorded": 2980216
},
"depgraph_build/multi_sheet_cross_refs/200x50": {
"ref_units": 3.2537,
"best_ns_recorded": 10729783
"ref_units": 3.1996,
"best_ns_recorded": 10124723
},
"incremental_recalc/independent_edit_root/100": {
"ref_units": 0.0605,
"best_ns_recorded": 192758
"ref_units": 0.0566,
"best_ns_recorded": 179180
},
"incremental_recalc/independent_edit_root/1000": {
"ref_units": 0.3549,
"best_ns_recorded": 1130495
"ref_units": 0.2876,
"best_ns_recorded": 910143
},
"incremental_recalc_cold/independent_edit_root/100": {
"ref_units": 0.0919,
"best_ns_recorded": 290789
},
"incremental_recalc_cold/independent_edit_root/1000": {
"ref_units": 0.736,
"best_ns_recorded": 2329101
},
"incremental_recalc/multi_sheet_edit_root/200x50": {
"ref_units": 2.5644,
"best_ns_recorded": 8114804
},
"incremental_recalc/block_subtotals_edit_root/10000": {
"ref_units": 1.2095,
"best_ns_recorded": 3823493
"ref_units": 0.7581,
"best_ns_recorded": 2398977
},
"incremental_recalc/chain_edit_root/1000": {
"ref_units": 1.1638,
"best_ns_recorded": 3719666
"ref_units": 1.0771,
"best_ns_recorded": 3408477
},
"incremental_recalc/chain_edit_root/5000": {
"ref_units": 6.3891,
"best_ns_recorded": 20419766
"ref_units": 5.9662,
"best_ns_recorded": 18879251
},
"incremental_recalc/chain_edit_leaf/1000": {
"ref_units": 0.3183,
"best_ns_recorded": 1017376
"ref_units": 0.2646,
"best_ns_recorded": 837166
},
"incremental_recalc/chain_edit_leaf/5000": {
"ref_units": 1.6274,
"best_ns_recorded": 5201330
"ref_units": 1.3121,
"best_ns_recorded": 4152095
},
"incremental_recalc/multi_sheet_edit_root/200x50": {
"ref_units": 3.2604,
"best_ns_recorded": 10751809
"incremental_recalc/all_literal_edit/120000cells": {
"ref_units": 3.8168,
"best_ns_recorded": 12077798
},
"from_json/500rows": {
"ref_units": 0.2833,
"best_ns_recorded": 926457
"incremental_recalc/row_totals_volatile_seed/100": {
"ref_units": 0.0793,
"best_ns_recorded": 250884
},
"to_json/500rows": {
"ref_units": 0.2216,
"best_ns_recorded": 724738
"incremental_recalc/row_totals_volatile_seed/10000": {
"ref_units": 7.5273,
"best_ns_recorded": 23819279
},
"incremental_recalc_cold/independent_edit_root/100": {
"ref_units": 0.0933,
"best_ns_recorded": 297138
"incremental_recalc/row_totals_volatile_seed/100000": {
"ref_units": 96.2568,
"best_ns_recorded": 304593843
},
"incremental_recalc_cold/independent_edit_root/1000": {
"ref_units": 0.7459,
"best_ns_recorded": 2375688
"from_json/500rows": {
"ref_units": 0.2837,
"best_ns_recorded": 897782
},
"incremental_recalc/all_literal_edit/120000cells": {
"ref_units": 3.7609,
"best_ns_recorded": 12543118
"to_json/500rows": {
"ref_units": 0.2194,
"best_ns_recorded": 694132
}
}
}
47 changes: 41 additions & 6 deletions crates/workbook/benches/workbook_perf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,12 +499,14 @@ fn bench_incremental_recalc(c: &mut Criterion) {
// Read the two together, not the ratio between them. That ratio is roughly
// flat (3.7x at n=1000, 3.9x at n=5000 when recorded), because both cases
// pay the same per-formula-cell fixed cost every incremental recalc owes —
// `snapshot_formula_values`, the volatile sweep over `formula_cells()`, and
// `seed_spill_sensitive`, each `O(formula cells)` by construction. At
// n=5000 the leaf case's time is almost entirely that floor rather than its
// one-cell recompute, which is precisely what makes it worth recording: no
// other incremental benchmark holds the dirty set fixed while the workbook
// grows, so nothing else here can see that floor at all.
// `snapshot_formula_values` and `seed_spill_sensitive`, each
// `O(formula cells)` by construction (the volatile sweep over
// `formula_cells()` was a third such term until issue #983 made it a
// cached lookup instead). At n=5000 the leaf case's time is almost
// entirely that remaining floor rather than its one-cell recompute, which
// is precisely what makes it worth recording: no other incremental
// benchmark holds the dirty set fixed while the workbook grows, so
// nothing else here can see that floor at all.
//
// The regression signal is the *gap*: a leaf closure that silently widened
// to the whole chain would jump the leaf case onto the root case's number
Expand Down Expand Up @@ -579,6 +581,39 @@ fn bench_incremental_recalc(c: &mut Criterion) {
});
});
group.finish();

// Reproduces the exact shape issue #983 measured: a formula-heavy
// document with zero actually-volatile functions, timing a single
// unrelated literal write. Before the fix this scaled linearly with
// formula count (367ms at n=100,000, dirty closure verified at 1)
// because the volatile-seeding loop re-derived every formula cell's
// volatility from scratch on every call.
//
// The fix makes *that term* O(1) in formula count, but the numbers this
// group records are not themselves flat: `snapshot_formula_values` and
// `seed_spill_sensitive` are two other O(formula cells) passes this same
// call still pays on every incremental recalc (see `chain_edit_leaf`'s
// comment above, which documents the same floor), and issue #983 does not
// touch either. Don't read a still-scaling number here as the fix having
// failed — the isolated volatile-rescan term it actually targets is gone;
// what remains is the other two terms, tracked separately.
let mut group = c.benchmark_group("incremental_recalc/row_totals_volatile_seed");
group.sample_size(10);
group.measurement_time(Duration::from_secs(5));
for n in [100u32, 10_000, 100_000] {
let mut template = build_row_totals(n);
template.recalc(&ctx);
group.bench_with_input(BenchmarkId::from_parameter(n), &n, |b, _| {
b.iter(|| {
let mut wb = template.clone();
let a1 = Address::new(1, 1).unwrap();
wb.set("Sheet1", a1, CellInput::Literal(Value::Number(99.0)))
.unwrap();
wb.recalc_incremental(&ctx, &[("Sheet1".to_string(), a1)])
});
});
}
group.finish();
}

fn bench_from_json(c: &mut Criterion) {
Expand Down
5 changes: 5 additions & 0 deletions crates/workbook/src/graph_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ pub struct CachedGraph {
pub(crate) order: Vec<CellRef>,
/// The cycle set from that same pass.
pub(crate) cycle: BTreeSet<CellRef>,
/// Every formula cell whose formula text names a volatile function
/// (`Workbook::is_volatile`), for this exact graph — computed once at
/// build time so `recalc_incremental`'s seeding step never re-derives it
/// per call (issue #983).
pub(crate) volatile: BTreeSet<CellRef>,
}

impl CachedGraph {
Expand Down
23 changes: 18 additions & 5 deletions crates/workbook/src/recalc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,23 @@ impl Workbook {
// times per incremental recalc (the spill widen loop) while ordering
// the same graph every time.
let (order, cycle) = graph.evaluation_order();
// Likewise derived from the graph and cached alongside it: a formula
// cell's volatility cannot change without the formula text changing,
// which already invalidates this cache — so this is the one place
// `is_volatile` needs to run, once per formula cell per graph build,
// instead of once per formula cell per incremental recalc (issue
// #983).
let sheets = SheetIndex::build(self);
let volatile: BTreeSet<CellRef> = graph
.formula_cells()
.filter(|cell| self.is_volatile(&sheets, cell))
.cloned()
.collect();
let entry = Arc::new(CachedGraph {
graph,
order,
cycle,
volatile,
});
self.store_cached_graph(entry.clone());
entry
Expand Down Expand Up @@ -412,11 +425,11 @@ impl Workbook {
}
}

// (b) Volatile cells are always dirty (scope ADR Decision 3).
for cell in graph.formula_cells() {
if self.is_volatile(&sheets, cell) {
frontier.insert(cell.clone());
}
// (b) Volatile cells are always dirty (scope ADR Decision 3). The set
// is cached on the graph at build time (issue #983) — no
// re-derivation here.
for cell in &cached.volatile {
frontier.insert(cell.clone());
}

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