Merge PR #1340: calc: calculate long dependency chains with bounded stack

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,24 @@ struct PassHost<'a> {
     cycle: bool,
     cycles: Vec<Key>,
     depth: usize,
+    /// A formula needed at [`MAX_NESTED`] levels deep: the evaluations in progress are abandoned
+    /// and it is calculated first.
+    suspend: Option<Key>,
+    /// 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<Key>,
+    /// The formulas abandoned since the last suspension.
+    abandoned: Vec<Key>,
+    /// 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<Key>,
+    /// 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<Key>,
     /// Iterative calculation: a circular reference reads the cell's previous value instead of
     /// giving `#CIRC!`.
     iterate: bool,
@@ -425,6 +449,13 @@ impl<'a> PassHost<'a> {
             cycle: false,
             cycles: vec![],
             depth: 0,
+            suspend: None,
+            waiting: HashSet::default(),
+            abandoned: Vec::new(),
+            also: Vec::new(),
+            #[cfg(test)]
+            suspensions: 0,
+            cycle_seen: HashSet::default(),
             iterate: false,
             prev: HashMap::default(),
             stack: Vec::new(),
@@ -471,6 +502,50 @@ fn spills_as_before(sheet: &gridcraft_model::Sheet, anchor: CellRef, range: Rang
 }
 
 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<Key>)> = vec![(k, Vec::new())];
+        while let Some(&(top, _)) = work.last() {
+            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<Key> = 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 => {
+                    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();
@@ -482,14 +557,29 @@ 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. 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) {
             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);
@@ -501,6 +591,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 {
@@ -591,7 +692,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;
         }
@@ -984,7 +1085,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);
@@ -1345,6 +1446,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.

crates/calc/src/tests.rs

@@ -1286,6 +1286,94 @@ fn offset_extreme_arguments_return_ref() {
     }
 }
 
+/// 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);
+}
+
 #[test]
 fn median_large_values_recalculate_dependents() {
     let mut t = T::new();