perf: win back formula-entry and XLSX-save time lost since 23829d8c (#2654)

Measured with crates/engine/examples/bench.rs (perf/xtask-bench), 100k rows, M2 Pro:
enter.formulas 465 -> 420 ms (23829d8c: 375), xlsx.save 540 -> 488 ms (466).

- number_to_text: the round-up-to-1E+15 check (f29ac7d9e) formatted every number with
  exact {:.14e} and parsed it back; formula printing calls it once per literal. Run it
  only for magnitudes that can round up (>= 9.9999999999999e14). Same output; boundary
  tests added.
- Expr clone/map, xlsx prefix_params: a binary node whose left operand is not a chain
  no longer allocates a spine Vec (2aeb21d4d); map rewrites the operands in their boxes.
  Expr::walk reserves room for a typical formula up front.
- grow_autofilter checks the sheet has an AutoFilter before looking up the cell.
- retained_bytes_against looks up the newer band's row once per row, not per cell.
- The journal measures an entry's JSON size with a counting writer instead of building
  the text (a big paste's params are MBs).

The rest of the enter.formulas gap and all of edit.fanout's (~+12%) is Undo's byte
budget (72db972de, #1227): sizing each snapshot and freeing evicted ones, an intended
cost of bounded memory.

Co-authored-by: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Nihilist Narwhal
2026-11-14 16:57:27 +01:00
committed by GitHub
co-authored by nihilist-narwhal Claude Opus 5.5
parent a8a51e3cf7
commit c2aba7a3f4
6 changed files with 53 additions and 8 deletions
+6 -2
View File
@@ -454,8 +454,9 @@ pub fn number_to_text(n: f64) -> String {
if !(1e-9..1e15).contains(&a) && a.round() != a || a >= 1e15 {
return sci_text(n, 15);
}
// A value just below 1e15 can round up to 1e15 at 15 significant digits.
if format!("{:.14e}", a).parse::<f64>().is_ok_and(|r| r >= 1e15) {
// A value just below 1e15 can round up to 1e15 at 15 significant digits. Only that narrow
// band pays for the exact formatting: it runs for every number literal a formula prints.
if a >= 9.999_999_999_999_9e14 && format!("{:.14e}", a).parse::<f64>().is_ok_and(|r| r >= 1e15) {
return sci_text(n, 15);
}
// 15 significant digits, trailing zeros trimmed.
@@ -506,6 +507,9 @@ mod tests {
assert_eq!(number_to_text(999999999999999.9), "1E+15");
assert_eq!(number_to_text(-999999999999999.9), "-1E+15");
assert_eq!(number_to_text(999999999999999.0), "999999999999999");
assert_eq!(number_to_text(999999999999999.5), "1E+15");
assert_eq!(number_to_text(999999999999999.4), "999999999999999");
assert_eq!(number_to_text(999999999999989.9), "999999999999990");
}
#[test]
+4 -3
View File
@@ -166,14 +166,15 @@ fn filter_blank(v: &Value) -> bool {
/// A table's own filter is left to the table, and a row typed after a blank row does not count.
/// A formula counts before it is calculated: this runs inside the edit, ahead of the recalc.
pub(crate) fn grow_autofilter(sh: &mut Sheet, at: CellRef) {
if !sh.cells.get(at).is_some_and(|c| !filter_blank(&c.value) || c.formula.is_some()) {
return;
}
// The AutoFilter checks come first: they are cheap, and most sheets have none.
let Some(af) = sh.autofilter.as_ref() else { return };
let r = af.range;
if r.end.row.checked_add(1) != Some(at.row) || at.col < r.start.col || at.col > r.end.col || sh.tables.iter().any(|t| t.range == r) {
return;
}
if !sh.cells.get(at).is_some_and(|c| !filter_blank(&c.value) || c.formula.is_some()) {
return;
}
let grown = RangeRef::new(r.start, CellRef::new(at.row, r.end.col));
if let Some(af) = sh.autofilter.as_mut() {
af.range = grown;
+18 -1
View File
@@ -305,6 +305,19 @@ fn trim_to_budget(history: &mut Vec<HistoryEntry>, byte_budget: usize) {
}
}
/// A writer that only counts the bytes written to it.
struct ByteCounter(usize);
impl std::io::Write for ByteCounter {
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
self.0 = self.0.saturating_add(buf.len());
Ok(buf.len())
}
fn flush(&mut self) -> std::io::Result<()> {
Ok(())
}
}
impl DocState {
/// A document for `wb`, which came from where `trust` says: [`Trust::FromFile`] for anything
/// read from bytes (a file, a download, a recovery copy), [`Trust::Local`] only for a workbook
@@ -638,7 +651,11 @@ impl Session {
/// Journals a command, then trims the oldest entries past either cap. A recording in progress
/// keeps pointing at the same entry (the steps the trim dropped are lost).
fn push_journal(&mut self, id: &str, params: Json) {
let size = |e: &(String, Json)| serde_json::to_vec(e).map_or(0, |v| v.len());
// Counted as it is written, without building the text: a paste's params can be MBs.
let size = |e: &(String, Json)| {
let mut counter = ByteCounter(0);
serde_json::to_writer(&mut counter, e).map_or(0, |()| counter.0)
};
// `journal` is public: if it was changed from outside, measure it again.
if self.journal_sizes.len() != self.journal.len() {
self.journal_sizes = self.journal.iter().map(size).collect();
+22 -1
View File
@@ -215,7 +215,9 @@ impl Expr {
/// Visits every node depth-first, parents before children, left to right.
pub fn walk<'a>(&'a self, f: &mut dyn FnMut(&'a Expr)) {
// An explicit stack: a long operator chain must not recurse once per operand.
let mut pending = vec![self];
// Room for a typical formula up front, so walking one costs one allocation, not several.
let mut pending = Vec::with_capacity(8);
pending.push(self);
while let Some(e) = pending.pop() {
f(e);
match e {
@@ -236,6 +238,19 @@ impl Expr {
}
/// Rewrites every node bottom-up, children left to right before their parent.
pub fn map(self, f: &mut dyn FnMut(Expr) -> Expr) -> Expr {
// One operator on a non-chain operand (`A1*2`), the common case, needs no spine: rewrite
// the operands in their boxes.
if let Expr::Binary(op, mut a, mut b) = self {
if !matches!(*a, Expr::Binary(..)) {
*a = std::mem::replace(&mut *a, Expr::Missing).map(f);
*b = std::mem::replace(&mut *b, Expr::Missing).map(f);
return f(Expr::Binary(op, a, b));
}
return Expr::Binary(op, a, b).map_chain(f);
}
self.map_chain(f)
}
fn map_chain(self, f: &mut dyn FnMut(Expr) -> Expr) -> Expr {
// Unwind the left spine of a long operator chain into a list, then rebuild it from the
// innermost operand outwards, in the order a recursive rewrite would visit the nodes.
let mut spine = Vec::new();
@@ -283,6 +298,12 @@ impl Expr {
impl Clone for Expr {
fn clone(&self) -> Self {
// One operator on a non-chain operand needs no spine.
if let Expr::Binary(op, a, b) = self
&& !matches!(**a, Expr::Binary(..))
{
return Expr::Binary(*op, a.clone(), b.clone());
}
// Copy the left spine of an operator chain in a loop (see [`Expr`]).
let mut spine = Vec::new();
let mut node = self;
+2 -1
View File
@@ -235,11 +235,12 @@ impl CellStore {
bytes = bytes.saturating_add(size_of::<Band>());
for (row, cols) in &band.rows {
bytes = bytes.saturating_add(size_of::<u32>() + 3 * size_of::<usize>());
let next_row = next.and_then(|b| b.rows.get(row));
for (col, cell) in cols {
// Cells and BTreeMap nodes are duplicated with a cloned band even when
// their formula/value Arcs are still shared with the next snapshot.
bytes = bytes.saturating_add(size_of::<u32>() + size_of::<Cell>() + 3 * size_of::<usize>());
let next_cell = next.and_then(|b| b.rows.get(row)).and_then(|r| r.get(col));
let next_cell = next_row.and_then(|r| r.get(col));
if let Some(formula) = &cell.formula
&& !next_cell.and_then(|c| c.formula.as_ref()).is_some_and(|f| Arc::ptr_eq(f, formula))
{
+1
View File
@@ -47,6 +47,7 @@ fn prefix_params(e: Expr, scope: &[String]) -> Expr {
Expr::Name(n) if scope.iter().any(|s| s.eq_ignore_ascii_case(&n)) => Expr::Name(format!("{PARAM_PREFIX}{n}")),
Expr::Unary(op, x) => Expr::Unary(op, sub(x, scope)),
Expr::Paren(x) => Expr::Paren(sub(x, scope)),
Expr::Binary(op, a, b) if !matches!(*a, Expr::Binary(..)) => Expr::Binary(op, sub(a, scope), sub(b, scope)),
Expr::Binary(op, a, b) => {
// A long chain such as A1+A2+… nests on its left: follow it in a loop, not one
// level of recursion per operand.