mirror of
https://github.com/storytold/gridcraft.git
synced 2026-10-24 10:46:50 -06:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
f8cec7dd6d
commit
13e730d42a
@@ -410,6 +410,13 @@ struct PassHost<'a> {
|
||||
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
|
||||
@@ -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<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 => {
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user