From 1241fbf2609bb2d60acfda6cc31bfb864c2212cc Mon Sep 17 00:00:00 2001 From: Harrison Booth Date: Wed, 14 Oct 2026 13:08:42 -0500 Subject: [PATCH 1/2] calc: calculate long dependency chains with bounded stack A formula that reads one later in row-major order (further down, or on a later sheet) is calculated on demand, inside the formula that reads it. Each level costs stack, so real models with chains of about 1,000 such formulas overflowed the 1 MB main-thread stack on Windows and crashed on open. Past 2,000 levels the chain gave #CIRC! instead. On-demand evaluation now goes at most 64 formulas deep. A formula needed deeper suspends the evaluations in progress: it is calculated from the top, then the suspended ones again, reading the results kept so far. Chains of any length calculate, with the same results. A suspended formula read again is a circular reference, as a formula in progress is, so loops are found at the same cell as before. Regression tests build the chains in code: 20,000 formulas each reading the row below, the same hopping between two sheets, a 5,000-formula loop, and a loop closing into the middle of a deep chain. Co-Authored-By: Claude Opus 5.5 --- crates/calc/src/recalc.rs | 81 +++++++++++++++++++++++++++++++++-- crates/calc/src/tests.rs | 88 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 4 deletions(-) diff --git a/crates/calc/src/recalc.rs b/crates/calc/src/recalc.rs index b391821d7..634e391ba 100644 --- a/crates/calc/src/recalc.rs +++ b/crates/calc/src/recalc.rs @@ -60,6 +60,12 @@ const WIDE: u32 = 64; /// Insertions an [`Intervals`] keeps aside (scanned by every query) before merging them in. const PENDING: usize = 32; +/// Formulas a formula may calculate on demand one inside the other (each needing one another's +/// value before it is done). Each level takes stack, so a chain deeper than this is suspended and +/// calculated from the top instead (see [`PassHost::calculate`]): long dependency chains calculate +/// whatever order they come in, with bounded stack. +const MAX_NESTED: usize = 64; + /// Formula evaluations iterative calculation may spend on one recalculation (passes × formulas /// evaluated again), however high Maximum Iterations is. const MAX_ITERATION_WORK: usize = 10_000_000; @@ -395,6 +401,17 @@ struct PassHost<'a> { cycle: bool, cycles: Vec, depth: usize, + /// A formula needed at [`MAX_NESTED`] levels deep: the evaluations in progress are abandoned + /// and it is calculated first. + suspend: Option, + /// Formulas whose evaluation was abandoned for another one, to be calculated again after it. + /// They all need it, so reading one of them while it is calculated is a circular reference, + /// as reading a formula in progress is. + waiting: HashSet, + /// The formulas abandoned since the last suspension. + abandoned: Vec, + /// The formulas in `cycles`. + cycle_seen: HashSet, /// Iterative calculation: a circular reference reads the cell's previous value instead of /// giving `#CIRC!`. iterate: bool, @@ -425,6 +442,10 @@ impl<'a> PassHost<'a> { cycle: false, cycles: vec![], depth: 0, + suspend: None, + waiting: HashSet::default(), + abandoned: Vec::new(), + cycle_seen: HashSet::default(), iterate: false, prev: HashMap::default(), stack: Vec::new(), @@ -457,6 +478,36 @@ fn spill_blocked(host: &PassHost<'_>, (sheet, anchor): Key, a: &Array) -> bool { } impl PassHost<'_> { + /// Calculates `k` (and the formulas it reads) with bounded stack: when [`Self::compute`] would + /// go deeper than [`MAX_NESTED`], it suspends what is in progress, the formula it needed is + /// calculated from here, then the suspended ones again (reading the results kept so far). + fn calculate(&mut self, k: Key) { + // Formulas to calculate, innermost last, each with the ones abandoned to calculate the + // next. + let mut work: Vec<(Key, Vec)> = vec![(k, Vec::new())]; + while let Some(&(top, _)) = work.last() { + self.compute(top); + match self.suspend.take() { + Some(next) => { + let held = std::mem::take(&mut self.abandoned); + self.waiting.extend(held.iter().copied()); + if let Some(entry) = work.last_mut() { + entry.1 = held; + } + work.push((next, Vec::new())); + } + None => { + work.pop(); + if let Some(entry) = work.last_mut() { + for a in entry.1.drain(..) { + self.waiting.remove(&a); + } + } + } + } + } + } + fn compute(&mut self, k: Key) -> Value { if let Some(v) = self.results.get(&k) { let v = v.clone(); @@ -468,14 +519,25 @@ impl PassHost<'_> { let Some(expr) = self.exprs.get(&k) else { return self.wb.sheet(k.0).map(|s| s.value(k.1)).unwrap_or_default(); }; - if self.in_progress.contains(&k) || self.depth > 2000 { + if self.suspend.is_some() { + // Unwinding to `calculate`: the value is thrown away. + return Value::Error(CellError::Calc); + } + if self.in_progress.contains(&k) || self.waiting.contains(&k) { self.cycle = true; if self.iterate { return self.circular_read(k); } - self.cycles.push(k); + // Once each, though a suspended evaluation meets it again. + if self.cycle_seen.insert(k) { + self.cycles.push(k); + } return Value::Error(CellError::Circ); } + if self.depth >= MAX_NESTED { + self.suspend = Some(k); + return Value::Error(CellError::Calc); + } self.in_progress.insert(k); self.depth += 1; let outer_taint = std::mem::replace(&mut self.taint, false); @@ -487,6 +549,17 @@ impl PassHost<'_> { let mut ev = Evaluator::new(self, k.0, k.1); ev.value(&expr) }; + if self.suspend.is_some() { + // Abandoned: `calculate` evaluates this formula again once the one it needs is done. + self.depth -= 1; + self.in_progress.remove(&k); + self.abandoned.push(k); + if self.iterate { + self.stack.pop(); + } + self.taint = outer_taint; + return Value::Error(CellError::Calc); + } // A legacy array formula fills the range it was entered in. let legacy = self.wb.sheet(k.0).and_then(|s| s.cell(k.1)).and_then(|c| c.formula.as_ref()).and_then(|f| f.array); let v = match legacy { @@ -577,7 +650,7 @@ impl PassHost<'_> { self.pending = set.clone(); self.spills.retain(|a, _| !set.contains(a)); for k in &again { - self.compute(*k); + self.calculate(*k); } passes += 1; } @@ -969,7 +1042,7 @@ impl Calc { let mut host = PassHost::new(wb, &exprs, &mut rng, now); host.iterate = wb.calc.iterative; for k in &dirty { - host.compute(*k); + host.calculate(*k); } if host.iterate && !host.members.is_empty() { let passes = host.iterate_loops(&dirty, wb.calc.max_iterations, wb.calc.max_change); diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 196c57de7..ef83116e2 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -1266,3 +1266,91 @@ fn offset_extreme_arguments_return_ref() { assert_eq!(crate::evaluate(&wb, 0, c("D1"), formula), Value::Number(expected), "{formula}"); } } + +/// A formula reading one further down (or on a later sheet) is calculated on demand, inside the +/// formula that reads it. Chains of thousands of such formulas, common in financial models, used +/// to overflow the stack (or give `#CIRC!` past 2,000 levels); they now calculate in full. +#[test] +fn long_dependency_chains_against_row_order_calculate() { + const N: u32 = 20_000; + let mut t = T::new(); + { + let sh = t.wb.sheet_mut(0).unwrap(); + for r in 1..N { + sh.cells.set(c(&format!("A{r}")), Cell::formula(Formula::new(&format!("=A{}+1", r + 1)))); + } + sh.cells.set(c(&format!("A{N}")), Cell::value(Value::Number(1.0))); + } + t.calc.recalc_all(&mut t.wb); + assert_eq!(t.num("A1"), N as f64); + assert_eq!(t.num("A2"), (N - 1) as f64); + assert!(t.calc.circular.is_empty()); + // An edit at the far end recalculates the whole chain. + t.set(&format!("A{N}"), "10"); + assert_eq!(t.num("A1"), (N + 9) as f64); + assert!(t.calc.circular.is_empty()); + + // Hopping between two sheets at every step, through a range read (SUM) as well. + let mut t = T::new(); + t.wb.sheets.push(std::sync::Arc::new(gridcraft_model::Sheet::new("Data"))); + for r in 1..N { + t.wb.sheet_mut(0).unwrap().cells.set(c(&format!("A{r}")), Cell::formula(Formula::new(&format!("=Data!A{r}+1")))); + let next = format!("=SUM(Sheet1!A{}:A{})", r + 1, r + 1); + t.wb.sheet_mut(1).unwrap().cells.set(c(&format!("A{r}")), Cell::formula(Formula::new(&next))); + } + t.wb.sheet_mut(0).unwrap().cells.set(c(&format!("A{N}")), Cell::value(Value::Number(0.0))); + t.calc.recalc_all(&mut t.wb); + assert_eq!(t.num("A1"), (N - 1) as f64); + assert_eq!(t.wb.sheet(1).unwrap().value(c("A1")), Value::Number((N - 2) as f64)); + assert!(t.calc.circular.is_empty()); +} + +/// A loop longer than the on-demand nesting limit is still a circular reference (and still ends). +#[test] +fn long_circular_chain_is_circular() { + const N: u32 = 5_000; + let mut t = T::new(); + { + let sh = t.wb.sheet_mut(0).unwrap(); + for r in 1..N { + sh.cells.set(c(&format!("A{r}")), Cell::formula(Formula::new(&format!("=A{}+1", r + 1)))); + } + sh.cells.set(c(&format!("A{N}")), Cell::formula(Formula::new("=A1+1"))); + sh.cells.set(c("B1"), Cell::formula(Formula::new("=1+2"))); + } + t.calc.recalc_all(&mut t.wb); + assert_eq!(t.get("A1"), Value::Error(CellError::Circ)); + assert_eq!(t.get(&format!("A{N}")), Value::Error(CellError::Circ)); + assert_eq!(t.num("B1"), 3.0); + assert!(!t.calc.circular.is_empty()); + // Each formula is listed once at most. + let mut seen = t.calc.circular.clone(); + seen.sort_by_key(|(s, c)| (*s, c.row, c.col)); + seen.dedup(); + assert_eq!(seen.len(), t.calc.circular.len()); + // With iterative calculation the loop settles to numbers instead (no hang, no crash). + t.wb.calc.iterative = true; + t.wb.calc.max_iterations = 3; + t.calc.recalc_all(&mut t.wb); + assert!(t.get("A1").as_f64().is_some()); +} + +/// A loop closing back into the middle of a chain deeper than the on-demand nesting limit is found +/// where calculating it in one go finds it: at the formula read while it was being calculated. +#[test] +fn deep_loop_is_found_where_it_closes() { + let mut t = T::new(); + { + let sh = t.wb.sheet_mut(0).unwrap(); + for r in 1..300 { + sh.cells.set(c(&format!("A{r}")), Cell::formula(Formula::new(&format!("=A{}+1", r + 1)))); + } + sh.cells.set(c("A300"), Cell::formula(Formula::new("=A30+1"))); + // Reads the loop through IFERROR, so its value shows where the loop was cut. + sh.cells.set(c("B1"), Cell::formula(Formula::new("=IFERROR(A1,-1)"))); + } + t.calc.recalc_all(&mut t.wb); + assert_eq!(t.calc.circular, vec![(0, c("A30"))]); + assert_eq!(t.get("A1"), Value::Error(CellError::Circ)); + assert_eq!(t.num("B1"), -1.0); +} From 13e730d42a54eea14f4b53dc9c8294c004a96ef5 Mon Sep 17 00:00:00 2001 From: Harrison Booth Date: Mon, 19 Oct 2026 13:36:56 -0500 Subject: [PATCH 2/2] calc: calculate every formula read at the depth limit in one suspension A formula at the nesting limit reading many uncalculated formulas (for example SUM over a wide range) suspended the abandoned evaluations once per cell read, restarting them each time. The other formulas read at the limit are now collected and calculated together with the suspended one, before the abandoned evaluations restart. Results, circular-reference and iteration behaviour are unchanged. Adds a regression test (3000 suspensions before, 1 after). Co-Authored-By: Claude Sonnet 5.5 --- crates/calc/src/recalc.rs | 59 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/crates/calc/src/recalc.rs b/crates/calc/src/recalc.rs index 634e391ba..86a97b472 100644 --- a/crates/calc/src/recalc.rs +++ b/crates/calc/src/recalc.rs @@ -410,6 +410,13 @@ struct PassHost<'a> { waiting: HashSet, /// The formulas abandoned since the last suspension. abandoned: Vec, + /// Other formulas read at [`MAX_NESTED`] levels deep by the evaluations being abandoned. They + /// are calculated along with the suspended one, rather than one per restart (a wide range + /// read at the limit would restart the abandoned evaluations once per cell in it). + also: Vec, + /// Suspensions so far (tests check that a wide read at the limit needs one, not one per cell). + #[cfg(test)] + suspensions: usize, /// The formulas in `cycles`. cycle_seen: HashSet, /// Iterative calculation: a circular reference reads the cell's previous value instead of @@ -445,6 +452,9 @@ impl<'a> PassHost<'a> { suspend: None, waiting: HashSet::default(), abandoned: Vec::new(), + also: Vec::new(), + #[cfg(test)] + suspensions: 0, cycle_seen: HashSet::default(), iterate: false, prev: HashMap::default(), @@ -489,11 +499,25 @@ impl PassHost<'_> { self.compute(top); match self.suspend.take() { Some(next) => { + #[cfg(test)] + { + self.suspensions += 1; + } let held = std::mem::take(&mut self.abandoned); self.waiting.extend(held.iter().copied()); if let Some(entry) = work.last_mut() { entry.1 = held; } + // The other formulas read at the limit are calculated after `next`, before the + // held evaluations restart. Each has nothing held of its own, so the held ones + // are released only when the last of them is done. + let mut seen: HashSet = HashSet::default(); + seen.insert(next); + for s in std::mem::take(&mut self.also) { + if seen.insert(s) { + work.push((s, Vec::new())); + } + } work.push((next, Vec::new())); } None => { @@ -520,7 +544,11 @@ impl PassHost<'_> { return self.wb.sheet(k.0).map(|s| s.value(k.1)).unwrap_or_default(); }; if self.suspend.is_some() { - // Unwinding to `calculate`: the value is thrown away. + // Unwinding to `calculate`: the value is thrown away. A formula read at the limit is + // calculated with the suspended one, so the evaluations are not restarted for each. + if self.depth >= MAX_NESTED && !self.in_progress.contains(&k) && !self.waiting.contains(&k) { + self.also.push(k); + } return Value::Error(CellError::Calc); } if self.in_progress.contains(&k) || self.waiting.contains(&k) { @@ -1395,6 +1423,35 @@ pub(crate) mod tests { assert_eq!(deps, vec![at("DA9"), at("DA9")]); } + /// A formula at the nesting limit reading thousands of formulas not calculated yet suspends the + /// evaluations in progress once, not once per formula read. + #[test] + fn wide_read_at_the_nesting_limit_suspends_once() { + const WIDTH: u32 = 3000; + let mut wb = Workbook::new(); + let mut exprs = HashMap::default(); + let mut put = |wb: &mut Workbook, at: String, text: String| { + let at = CellRef::parse(&at).unwrap(); + wb.sheet_mut(0).unwrap().cells.set(at, Cell::formula(Formula::new(&text))); + exprs.insert((0, at), std::sync::Arc::new(gridcraft_formula::parse(&text[1..]).unwrap())); + }; + // A1 reads A2, ..., A64 (the 64th formula deep) reads a wide range of formulas. + for r in 1..64 { + put(&mut wb, format!("A{r}"), format!("=A{}+1", r + 1)); + } + put(&mut wb, "A64".to_string(), format!("=SUM(B1:B{WIDTH})")); + for r in 1..=WIDTH { + put(&mut wb, format!("B{r}"), format!("={r}*2")); + } + let mut rng = 1u64; + let mut host = PassHost::new(&wb, &exprs, &mut rng, 0.0); + host.calculate((0, CellRef::parse("A1").unwrap())); + let total = (WIDTH * (WIDTH + 1)) as f64; + assert_eq!(host.results.get(&(0, CellRef::parse("A1").unwrap())), Some(&Value::Number(total + 63.0))); + assert!(host.cycles.is_empty()); + assert!(host.suspensions <= 3, "{} suspensions", host.suspensions); + } + /// For tests of `PassHost::visit_range`, which can't be reached from outside this module: the /// values it hands over for `range` while the formulas in `pending` (cell, formula) have not been /// calculated yet, or `None` when it declines the block.