diff --git a/crates/calc/src/recalc.rs b/crates/calc/src/recalc.rs index 1ee6df243..8e97eeeea 100644 --- a/crates/calc/src/recalc.rs +++ b/crates/calc/src/recalc.rs @@ -558,6 +558,48 @@ pub(crate) mod tests { v } + /// A 1,000 x 1,000 Python result laid out from A1 in a pass, with the same values already + /// spilled on the sheet from the last calculation, as a full recalculation finds them. + fn big_python_spill() -> (Workbook, HashMap) { + let n = 1_000usize; + let data: Vec = (0..n * n).map(|i| Value::Number(i as f64)).collect(); + let array = gridcraft_core::Array::new(n, n, data.clone()).unwrap(); + let mut wb = Workbook::new(); + let sh = wb.sheet_mut(0).unwrap(); + let anchor = CellRef::new(0, 0); + let py = gridcraft_model::PyCode::new("big"); + sh.set_cell(anchor, Cell { value: Value::Number(0.0), py: Some(std::sync::Arc::new(py)), ..Cell::default() }); + let area = RangeRef::new(anchor, CellRef::new(n as u32 - 1, n as u32 - 1)); + sh.spill_ranges.insert(anchor, area); + for (i, c) in area.iter().enumerate().skip(1) { + sh.spill.insert(c, data[i].clone()); + } + let mut python = HashMap::default(); + python.insert((0, anchor), Value::Array(std::sync::Arc::new(array))); + (wb, python) + } + + #[test] + fn a_read_beside_a_large_spill_walks_none_of_it() { + let (wb, python) = big_python_spill(); + let exprs = HashMap::default(); + let mut rng = 1u64; + let mut host = PassHost::new(&wb, &exprs, &mut rng, 0.0).with_python(python); + assert_eq!(host.cell_value(0, CellRef::new(0, 0)), Value::Number(0.0)); + // Beside the spill (column ALM, the 1,001st), over the rows it covers. + let beside = RangeRef::parse("ALM1:ALM1000").unwrap(); + host.spill_visits = 0; + assert!(host.range_values(0, beside).iter().all(Value::is_empty)); + assert!(host.visit_range(0, beside, &mut |_| true), "a disjoint read takes the fast path"); + assert_eq!(host.spill_visits, 0, "nothing of the spill is walked"); + // A read inside it still sees the spilled values, walking only the cells it reads. + let inside = RangeRef::parse("B2:C3").unwrap(); + host.spill_visits = 0; + assert_eq!(host.range_values(0, inside), [1_001.0, 1_002.0, 2_001.0, 2_002.0].map(Value::Number).to_vec()); + assert!(host.spill_visits <= 8, "{}", host.spill_visits); + assert!(!host.visit_range(0, inside, &mut |_| true), "an overlapping read leaves the block to range_values"); + } + #[test] fn intervals_match_a_linear_scan() { let mut iv: Intervals = Intervals::default(); diff --git a/crates/calc/src/recalc/pass_host.rs b/crates/calc/src/recalc/pass_host.rs index 42cc6a834..5909369fd 100644 --- a/crates/calc/src/recalc/pass_host.rs +++ b/crates/calc/src/recalc/pass_host.rs @@ -45,6 +45,10 @@ pub(super) struct PassHost<'a> { /// Suspensions so far (tests check that a wide read at the limit needs one, not one per cell). #[cfg(test)] pub(super) suspensions: usize, + /// Spilled values looked at by range reads (tests check that a read outside a large spill + /// looks at none of it). + #[cfg(test)] + pub(super) spill_visits: usize, /// The formulas in `cycles`. cycle_seen: HashSet, /// Iterative calculation: a circular reference reads the cell's previous value instead of @@ -91,6 +95,8 @@ impl<'a> PassHost<'a> { also: Vec::new(), #[cfg(test)] suspensions: 0, + #[cfg(test)] + spill_visits: 0, cycle_seen: HashSet::default(), iterate: false, prev: HashMap::default(), @@ -151,6 +157,42 @@ pub(super) fn spills_as_before(sheet: &gridcraft_model::Sheet, anchor: CellRef, .all(|(i, c)| c == anchor || (!sheet.cells.has(c) && sheet.spill.get(&c) == Some(arr.data.get(i).unwrap_or(&Value::Empty)))) } +/// The area an array anchored at `anchor` covers. +fn array_area(anchor: CellRef, a: &Array) -> RangeRef { + let end = + CellRef::new(anchor.row.saturating_add((a.rows as u32).saturating_sub(1)), anchor.col.saturating_add((a.cols as u32).saturating_sub(1))); + RangeRef::new(anchor, end) +} + +/// The spilled values `sheet` stores inside `range`, passed to `f` until it returns false. Values +/// outside the range are not walked: with fewer rows in the range than spilled values, each row +/// is looked up on its own; otherwise one ordered scan visits each spilled value at most once. +/// Returns how many values were looked at. +fn stored_spill_in<'s>(sheet: &'s gridcraft_model::Sheet, range: RangeRef, f: &mut dyn FnMut(CellRef, &'s Value) -> bool) -> usize { + let mut seen = 0usize; + if sheet.spill.is_empty() { + return 0; + } + if (range.height() as usize) < sheet.spill.len() { + for row in range.start.row..=range.end.row { + for (c, v) in sheet.spill.range(CellRef::new(row, range.start.col)..=CellRef::new(row, range.end.col)) { + seen += 1; + if !f(*c, v) { + return seen; + } + } + } + } else { + for (c, v) in sheet.spill.range(range.start..=range.end) { + seen += 1; + if range.contains(*c) && !f(*c, v) { + return seen; + } + } + } + seen +} + 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 @@ -505,15 +547,18 @@ impl Host for PassHost<'_> { *slot = cell.value.clone(); } } - for (c, v) in sh.spill.range(range.start..=range.end) { - if range.contains(*c) { - let i = (c.row - range.start.row) as usize * w + (c.col - range.start.col) as usize; - if let Some(slot) = out.get_mut(i) - && slot.is_empty() - { - *slot = v.clone(); - } + let _seen = stored_spill_in(sh, range, &mut |c, v| { + let i = (c.row - range.start.row) as usize * w + (c.col - range.start.col) as usize; + if let Some(slot) = out.get_mut(i) + && slot.is_empty() + { + *slot = v.clone(); } + true + }); + #[cfg(test)] + { + self.spill_visits += _seen; } for (i, c) in formulas { let v = self.cell_value(sheet, c); @@ -522,26 +567,31 @@ impl Host for PassHost<'_> { } } if !self.spills.is_empty() { - let new: Vec<(usize, Value)> = self - .spills - .iter() - .filter(|(a, _)| a.0 == sheet) - .flat_map(|(a, arr)| { - let mut v = Vec::new(); - for r in 0..arr.rows { - for cc in 0..arr.cols { - let c = CellRef::new(a.1.row.saturating_add(r as u32), a.1.col.saturating_add(cc as u32)); - if c != a.1 && range.contains(c) { - v.push(( - (c.row - range.start.row) as usize * w + (c.col - range.start.col) as usize, - arr.get(r, cc).cloned().unwrap_or_default(), - )); - } + // Only the part of each array inside the range is walked: a large array beside a small + // read costs nothing. + let mut new: Vec<(usize, Value)> = Vec::new(); + for (a, arr) in self.spills.iter().filter(|(a, _)| a.0 == sheet) { + let area = array_area(a.1, arr); + if !area.intersects(&range) { + continue; + } + let (r0, r1) = (area.start.row.max(range.start.row), area.end.row.min(range.end.row)); + let (c0, c1) = (area.start.col.max(range.start.col), area.end.col.min(range.end.col)); + for row in r0..=r1 { + for col in c0..=c1 { + let c = CellRef::new(row, col); + if c == a.1 { + continue; } + let v = arr.get((row - a.1.row) as usize, (col - a.1.col) as usize).cloned().unwrap_or_default(); + new.push(((row - range.start.row) as usize * w + (col - range.start.col) as usize, v)); } - v - }) - .collect(); + } + } + #[cfg(test)] + { + self.spill_visits += new.len(); + } // Iterating: an array spilled by a formula that read a previous value. if self.iterate && self.spills.iter().any(|(a, arr)| { @@ -575,7 +625,17 @@ impl Host for PassHost<'_> { } // Spilled values sit between the stored cells (and the formulas just calculated may have // produced some): leave such blocks to `range_values`. - if self.spills.keys().any(|a| a.0 == sheet) || sh.spill.range(range.start..=range.end).any(|(c, _)| range.contains(*c)) { + let arrays = self.spills.iter().any(|(a, arr)| a.0 == sheet && array_area(a.1, arr).intersects(&range)); + let mut stored = false; + let _seen = stored_spill_in(sh, range, &mut |_, _| { + stored = true; + false + }); + #[cfg(test)] + { + self.spill_visits += _seen; + } + if arrays || stored { return false; } for (c, cell) in sh.cells.iter_range(range) {