perf(calc): size the spill-index decision by the sheet's own changed cells

A batch spread over several sheets used its total length for every sheet, so a
one-cell sheet with many spills built the block index as if it had the whole
batch's lookups.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
vangel76
2026-10-30 00:04:25 +01:00
co-authored by Claude Sonnet 5.5
parent edfde80f95
commit bdbd29e623
2 changed files with 55 additions and 11 deletions
+33 -11
View File
@@ -22,6 +22,13 @@ use crate::spill_index::{SpillIndex, indexing_pays};
/// Changed cells (in one `cells_changed`) from which a sheet's spill areas may be indexed.
const INDEX_SPILLS_FROM: usize = 16;
/// Whether a sheet's spill areas are indexed for a batch of `batch` changed cells of which
/// `on_sheet` are on that sheet: the batch must be a paste, and registering the areas must cost
/// less than walking them once for each of the sheet's own changed cells.
fn index_spills<'a>(batch: usize, on_sheet: usize, areas: impl ExactSizeIterator<Item = &'a RangeRef>) -> bool {
batch > INDEX_SPILLS_FROM && areas.len() > 0 && indexing_pays(areas, on_sheet)
}
mod graph;
mod pass_host;
@@ -250,20 +257,22 @@ impl Calc {
// walked for each one, when registering them costs less than walking them would.
let mut areas = SpillIndex::default();
let mut strategy: HashMap<usize, bool> = HashMap::default();
let mut per_sheet: HashMap<usize, usize> = HashMap::default();
for k in changed {
*per_sheet.entry(k.0).or_insert(0) += 1;
}
let mut at = Vec::new();
for &k in changed {
if let Some(sh) = wb.sheet(k.0) {
let indexed = changed.len() > INDEX_SPILLS_FROM
&& !sh.spill_ranges.is_empty()
&& *strategy.entry(k.0).or_insert_with(|| {
let indexed = indexing_pays(sh.spill_ranges.values(), changed.len());
if indexed {
for (anchor, r) in &sh.spill_ranges {
areas.insert_rect((k.0, *anchor), *r, false);
}
let indexed = *strategy.entry(k.0).or_insert_with(|| {
let indexed = index_spills(changed.len(), per_sheet[&k.0], sh.spill_ranges.values());
if indexed {
for (anchor, r) in &sh.spill_ranges {
areas.insert_rect((k.0, *anchor), *r, false);
}
indexed
});
}
indexed
});
if indexed {
// Anchor order, as walking `spill_ranges` gives.
areas.meeting(k.0, RangeRef::new(k.1, k.1), &mut at);
@@ -556,7 +565,7 @@ pub(crate) mod tests {
use gridcraft_core::{CellRef, RangeRef, Value};
use gridcraft_model::{Cell, Formula, Workbook};
use super::{Calc, HashMap, Host, Intervals, Key, PassHost};
use super::{Calc, HashMap, Host, Intervals, Key, PassHost, index_spills};
/// xorshift64 for repeatable pseudo-random cases.
fn rng(state: &mut u64) -> u64 {
@@ -573,6 +582,19 @@ pub(crate) mod tests {
v
}
/// One changed cell on a sheet with many small spills does not index them just because the
/// batch (mostly on another sheet) is large; the sheet's own share of the batch decides.
#[test]
fn the_spill_index_is_chosen_from_the_sheets_own_changed_cells() {
let small: Vec<RangeRef> = (0..1_000).map(|i| RangeRef::new(CellRef::new(i * 3, 0), CellRef::new(i * 3 + 2, 0))).collect();
// Same areas, same batch of 5000: indexed when 5000 of them are on the sheet, walked when 1 is.
assert!(index_spills(5_000, 5_000, small.iter()));
assert!(!index_spills(5_000, 1, small.iter()));
// Not a paste, or nothing to index: never.
assert!(!index_spills(16, 16, small.iter()));
assert!(!index_spills(5_000, 5_000, [].iter()));
}
#[test]
fn intervals_match_a_linear_scan() {
let mut iv: Intervals<u32> = Intervals::default();
+22
View File
@@ -2371,3 +2371,25 @@ fn a_paste_over_small_and_tall_spills_blocks_those_it_touches() {
assert_eq!(t.get("I1"), Value::Error(CellError::Spill));
assert_eq!(t.num("E3"), 3.0);
}
#[test]
fn a_big_paste_on_one_sheet_and_one_cell_over_many_spills_on_another() {
let mut t = T::new();
t.wb.sheets.push(std::sync::Arc::new(gridcraft_model::Sheet::new("Data")));
for i in 0..1_500u32 {
t.wb.sheet_mut(1).unwrap().cells.set(CellRef::new(4 * i, 0), Cell::formula(Formula::new(&format!("=SEQUENCE(3,1,{})", 10 * i))));
}
t.calc.recalc_all(&mut t.wb);
assert_eq!(t.wb.sheet(1).unwrap().value(CellRef::new(4 * 700 + 2, 0)), Value::Number(7002.0));
// 5000 cells pasted on the first sheet, one over the middle of the 701st array on the second.
let mut changed: Vec<(usize, CellRef)> = (0..5_000u32).map(|i| (0, CellRef::new(i / 50, i % 50))).collect();
changed.push((1, CellRef::new(4 * 700 + 1, 0)));
for k in &changed {
t.wb.sheet_mut(k.0).unwrap().cells.set(k.1, Cell::value(Value::Number(1.0)));
}
t.calc.cells_changed(&mut t.wb, &changed);
let data = t.wb.sheet(1).unwrap();
assert_eq!(data.value(CellRef::new(4 * 700, 0)), Value::Error(CellError::Spill));
assert_eq!(data.value(CellRef::new(4 * 699 + 2, 0)), Value::Number(6992.0));
assert_eq!(data.value(CellRef::new(4 * 701 + 2, 0)), Value::Number(7012.0));
}