diff --git a/crates/engine/examples/fill_handle_perf.rs b/crates/engine/examples/fill_handle_perf.rs new file mode 100644 index 000000000..5e9ea423c --- /dev/null +++ b/crates/engine/examples/fill_handle_perf.rs @@ -0,0 +1,50 @@ +//! Times the fill-handle double-click scan against a densely-filled neighbour column: the old +//! row-by-row `value_ref` loop from `grid.rs` vs. `Sheet::last_contiguous_nonempty`. +//! `cargo run --release -p gridcraft-engine --example fill_handle_perf -- [rows]` + +#![allow(clippy::disallowed_methods)] // a native-only benchmark: wasm never runs it + +use std::time::Instant; + +use gridcraft_engine::Session; +use gridcraft_engine::core::CellRef; +use serde_json::json; + +fn main() { + let rows: u32 = std::env::args().nth(1).and_then(|s| s.parse().ok()).unwrap_or(1_000_000); + let mut s = Session::new(); + s.new_workbook(); + let t = Instant::now(); + let chunk = 10_000u32; + let mut r = 0u32; + while r < rows { + let n = chunk.min(rows - r); + let vals: Vec = (0..n).map(|i| json!([(r + i) as f64])).collect(); + s.execute("range.setValues", json!({"range": format!("A{}", r + 1), "values": vals})).expect("set"); + r += n; + } + println!("fill A1:A{rows}: {:?}", t.elapsed()); + + let d = s.doc().expect("doc"); + let sh = d.wb.active().expect("sheet"); + let neighbour = 0u32; // column A + let cap = (rows - 1).min(999_999); + + let t = Instant::now(); + let mut last = 0u32; + let mut iters = 0u64; + while last + 1 < gridcraft_engine::core::MAX_ROWS && sh.value_ref(CellRef::new(last + 1, neighbour)).is_some_and(|v| !v.is_empty()) && last < cap + { + last += 1; + iters += 1; + } + let old = t.elapsed(); + println!("old row-by-row scan: {iters} iterations, {old:?} ({:.1} ns/iter)", old.as_nanos() as f64 / iters.max(1) as f64); + + let t = Instant::now(); + let new_last = sh.last_contiguous_nonempty(neighbour, 1, cap); + let new_t = t.elapsed(); + println!("new last_contiguous_nonempty: last={new_last}, {new_t:?} ({:.2}x faster)", old.as_secs_f64() / new_t.as_secs_f64().max(1e-12)); + + assert_eq!(last, new_last, "old and new scans must agree"); +} diff --git a/crates/model/src/sheet.rs b/crates/model/src/sheet.rs index b404b891c..9833024b5 100644 --- a/crates/model/src/sheet.rs +++ b/crates/model/src/sheet.rs @@ -183,6 +183,39 @@ impl Sheet { _ => self.spill.get(&c), } } + /// Last row (inclusive) of the maximal contiguous run of non-empty values in `col`, starting + /// at `from_row` and capped at `cap` (inclusive). `from_row - 1` when `from_row` itself is + /// empty (or `from_row > cap`), same as a cell-by-cell `value_ref` scan would find, but + /// without calling `value_ref` once per row: `CellStore::contiguous_value_run` walks each + /// band's row map directly for the common case (a run of stored cells), only falling back to + /// a per-row `spill` check where the stored run stops short of `cap`. Used by the fill-handle + /// double-click, where `col` can be densely populated for very long stretches. + pub fn last_contiguous_nonempty(&self, col: u32, from_row: u32, cap: u32) -> u32 { + if from_row > cap { + return from_row.saturating_sub(1); + } + let mut row = from_row; + loop { + if row > cap { + return cap; + } + let c = CellRef::new(row, col); + match self.cells.contiguous_value_run(c, cap) { + Some(last) => row = last + 1, + None => { + // A stored cell decides the row on its own, even with an empty value (see + // `contiguous_value_run`'s doc comment): only an unstored row falls through + // to `spill`, exactly like `value_ref`. + let qualifies = self.cells.get(c).is_none() && self.spill.get(&c).is_some_and(|v| !v.is_empty()); + if qualifies { + row += 1; + } else { + return row.saturating_sub(1); + } + } + } + } + } pub fn style_id(&self, c: CellRef) -> StyleId { if let Some(cell) = self.cells.get(c) { return cell.style; @@ -411,6 +444,7 @@ fn line_at(pos: f64, default: f64, custom: &BTreeMap, max: u32) - #[cfg(test)] mod tests { use super::*; + use crate::cell::Formula; #[test] fn line_index_matches_linear() { @@ -427,6 +461,103 @@ mod tests { } } + /// Reference implementation: one `value_ref` call per row, exactly the loop + /// `last_contiguous_nonempty` replaces (from `grid.rs`'s fill-handle double-click). + fn last_contiguous_nonempty_naive(s: &Sheet, col: u32, from_row: u32, cap: u32) -> u32 { + let mut row = from_row; + while row <= cap && s.value_ref(CellRef::new(row, col)).is_some_and(|v| !v.is_empty()) { + row += 1; + } + row.saturating_sub(1).max(from_row.saturating_sub(1)) + } + + /// Deterministic xorshift32 so failures reproduce without pulling in a `rand` dependency + /// just for this test. + struct Xorshift32(u32); + impl Xorshift32 { + fn next(&mut self) -> u32 { + self.0 ^= self.0 << 13; + self.0 ^= self.0 >> 17; + self.0 ^= self.0 << 5; + self.0 + } + fn below(&mut self, n: u32) -> u32 { + self.next() % n.max(1) + } + } + + #[test] + fn last_contiguous_nonempty_matches_naive() { + let mut rng = Xorshift32(0x9e3779b9); + for trial in 0..300u32 { + let mut s = Sheet::new("S"); + let rows = rng.below(400); + // A mix of directly-stored values, formulas (value empty but formula present), + // spill-only cells, blank-but-stored cells, and gaps, like a real sheet. + for r in 0..rows { + match rng.below(5) { + 0 => s.set_value(CellRef::new(r, 0), Value::Number(r as f64)), + 1 => s.set_cell(CellRef::new(r, 0), Cell { formula: Some(Arc::new(Formula::new("1"))), ..Default::default() }), + 2 => { + s.spill.insert(CellRef::new(r, 0), Value::Number(r as f64)); + } + 3 => { + // Stored but blank: does not count (matches `has_value`/`value_ref`). + s.set_value(CellRef::new(r, 0), Value::Text("".into())); + } + _ => {} // gap + } + } + for _ in 0..5 { + let from_row = rng.below(420); + let cap = from_row.saturating_add(rng.below(450)); + let got = s.last_contiguous_nonempty(0, from_row, cap); + let want = last_contiguous_nonempty_naive(&s, 0, from_row, cap); + assert_eq!(got, want, "trial {trial}: col 0, from_row {from_row}, cap {cap}, rows up to {rows}"); + } + } + } + + #[test] + fn last_contiguous_nonempty_dense_run() { + let mut s = Sheet::new("S"); + for r in 0..2000u32 { + s.set_value(CellRef::new(r, 2), Value::Number(r as f64)); + } + assert_eq!(s.last_contiguous_nonempty(2, 0, 1_999), 1999); + assert_eq!(s.last_contiguous_nonempty(2, 0, 1_000), 1000); // capped + assert_eq!(s.last_contiguous_nonempty(2, 500, 1_999), 1999); + assert_eq!(s.last_contiguous_nonempty(2, 2000, 3000), 1999); // nothing at from_row + assert_eq!(s.last_contiguous_nonempty(2, 2001, 3000), 2000); // from_row > cap: from_row - 1 + } + + #[test] + fn last_contiguous_nonempty_formula_with_empty_value_ends_the_run() { + let mut s = Sheet::new("S"); + s.set_value(CellRef::new(0, 1), Value::Number(1.0)); + s.set_value(CellRef::new(1, 1), Value::Number(2.0)); + // row 2: a formula (e.g. `=""`) whose cached value is empty. A stored cell always + // decides the row on its own, exactly like `value_ref`: this does NOT count, same as + // `value_ref(...).is_some_and(|v| !v.is_empty())` would say for it, even though it has + // a formula (unlike `CellStore::has`, used for Ctrl+arrow navigation, which would). + s.set_cell(CellRef::new(2, 1), Cell { formula: Some(Arc::new(Formula::new("\"\""))), ..Default::default() }); + s.set_value(CellRef::new(3, 1), Value::Number(4.0)); // would extend the run if row 2 didn't stop it + assert_eq!(s.last_contiguous_nonempty(1, 0, 100), 1); + assert_eq!(s.last_contiguous_nonempty(1, 2, 100), 1); // from_row itself is the stop + } + + #[test] + fn last_contiguous_nonempty_spill_extends_an_unstored_row() { + let mut s = Sheet::new("S"); + s.set_value(CellRef::new(0, 1), Value::Number(1.0)); + // row 1: nothing stored, but a spill value from an array formula anchored elsewhere. + s.spill.insert(CellRef::new(1, 1), Value::Number(2.0)); + s.set_value(CellRef::new(2, 1), Value::Number(3.0)); + // row 3: a genuine gap (nothing stored, no spill). + assert_eq!(s.last_contiguous_nonempty(1, 0, 100), 2); + assert_eq!(s.last_contiguous_nonempty(1, 3, 100), 2); // from_row itself is the gap + } + #[test] fn geometry() { let mut s = Sheet::new("S"); diff --git a/crates/model/src/store.rs b/crates/model/src/store.rs index d6a9c4561..658b898bf 100644 --- a/crates/model/src/store.rs +++ b/crates/model/src/store.rs @@ -13,6 +13,10 @@ use crate::cell::Cell; const BAND: u32 = 64; +fn has_value(cell: &Cell) -> bool { + !cell.value.is_empty() || cell.formula.is_some() +} + #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] struct Band { /// row → (col → cell) @@ -118,7 +122,45 @@ impl CellStore { } /// Last stored row in column `col` at or above `below_or_at` (for Ctrl+arrow navigation). pub fn has(&self, c: CellRef) -> bool { - self.get(c).is_some_and(|cell| !cell.value.is_empty() || cell.formula.is_some()) + self.get(c).is_some_and(has_value) + } + /// Last row (inclusive, capped at `cap`) of the maximal run of contiguous rows at or after + /// `from.row` that store a cell in `from.col` with a non-empty *value* in `from.col`. `None` + /// when `from` itself doesn't qualify. + /// + /// Deliberately narrower than `has`/`get` (which also count a formula with no cached value + /// yet, for Ctrl+arrow navigation): this matches `Sheet::value_ref`, whose callers test the + /// `Value` it returns with `!v.is_empty()` regardless of whether a formula produced it — a + /// formula cell with an empty cached value (`=""`, or not yet recalculated) is "empty" to + /// them. A stored cell always decides the row on its own either way (`value_ref` never falls + /// through to `spill` once a cell is stored there, whether or not its value is empty), so + /// `last_contiguous_nonempty` only needs to check `spill` where this returns `None`, i.e. + /// no cell is stored at `from.col` for that row at all. + /// + /// Walks each band's row map with `BTreeMap::range` (one in-order pass per band) instead of + /// one `get` per row, so a long, densely-populated run costs roughly one tree-traversal step + /// per row rather than one binary search per row. + pub fn contiguous_value_run(&self, from: CellRef, cap: u32) -> Option { + let mut last = None; + let mut expect = from.row; + while expect <= cap { + let band_key = expect / BAND; + let Some(band) = self.bands.get(&band_key) else { break }; + let band_hi = ((band_key + 1) * BAND - 1).min(cap); + let mut advanced = false; + for (r, cols) in band.rows.range(expect..=band_hi) { + if *r != expect || cols.get(&from.col).is_none_or(|c| c.value.is_empty()) { + return last; + } + last = Some(expect); + expect += 1; + advanced = true; + } + if !advanced { + break; + } + } + last } /// Positions whose cells differ between `self` and `other` (stored on one side only, or not /// equal). Bands both stores share are skipped, so comparing an edited copy with the store diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 106bc6684..8067cd58c 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -2005,11 +2005,8 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & // Fill down as far as the neighbouring column goes. let src = sel.current(); let neighbour = if src.start.col > 0 { src.start.col - 1 } else { src.end.col + 1 }; - let mut last = src.end.row; - while last + 1 < MAX_ROWS && sh.value_ref(CellRef::new(last + 1, neighbour)).is_some_and(|v| !v.is_empty()) && last - src.end.row < 1_000_000 - { - last += 1; - } + let cap = src.end.row.saturating_add(1_000_000).min(MAX_ROWS - 1); + let last = if src.end.row < cap { sh.last_contiguous_nonempty(neighbour, src.end.row + 1, cap) } else { src.end.row }; if last > src.end.row { let target = RangeRef::new(src.start, CellRef::new(last, src.end.col)); let _ = app.run("edit.autoFill", json!({"source": src.a1(), "target": target.a1()}));