fix(engine): bound Undo history by retained cell storage (#1227)

This commit is contained in:
derechopiano
2026-10-14 08:23:57 +01:00
parent 84acf6293b
commit 72db972de3
5 changed files with 144 additions and 9 deletions
+6 -4
View File
@@ -601,11 +601,12 @@ fn undo(s: &mut Session, p: &Json) -> Result<Json> {
let now = d.wb.clone();
for _ in 0..steps {
let Some(e) = d.undo.pop() else { break };
let cur = crate::HistoryEntry { label: e.label.clone(), wb: d.wb.clone(), selection: d.selection.clone() };
d.redo.push(cur);
let cur_wb = d.wb.clone();
let cur_selection = d.selection.clone();
label = e.label.clone();
d.wb = e.wb;
d.selection = e.selection;
crate::push_history(&mut d.redo, label.clone(), cur_wb, &d.wb, cur_selection);
}
super::extra::keep_watch_list(&now, &mut d.wb);
d.calc.rebuild(&d.wb);
@@ -620,11 +621,12 @@ fn redo(s: &mut Session, p: &Json) -> Result<Json> {
let now = d.wb.clone();
for _ in 0..steps {
let Some(e) = d.redo.pop() else { break };
let cur = crate::HistoryEntry { label: e.label.clone(), wb: d.wb.clone(), selection: d.selection.clone() };
d.undo.push(cur);
let cur_wb = d.wb.clone();
let cur_selection = d.selection.clone();
label = e.label.clone();
d.wb = e.wb;
d.selection = e.selection;
crate::push_history(&mut d.undo, label.clone(), cur_wb, &d.wb, cur_selection);
}
super::extra::keep_watch_list(&now, &mut d.wb);
d.calc.rebuild(&d.wb);
+40 -4
View File
@@ -61,6 +61,8 @@ pub struct HistoryEntry {
pub label: String,
pub wb: Arc<Workbook>,
pub selection: Selection,
/// Estimate of storage this snapshot retains beyond the next workbook version.
pub retained_bytes: usize,
}
/// The clipboard: a block of cells (with formulas and styles) or marching-ants reference.
@@ -116,6 +118,43 @@ pub struct DocState {
static NEXT_UID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1);
const UNDO_LIMIT: usize = 100;
const UNDO_BYTES_LIMIT: usize = 128 * 1024 * 1024;
fn history_retained_bytes(older: &Workbook, newer: &Workbook) -> usize {
let mut bytes = std::mem::size_of::<Workbook>();
for (index, sheet) in older.sheets.iter().enumerate() {
// Moving or inserting sheets must not charge unchanged sheets at a new index.
if newer.sheets.iter().any(|next| Arc::ptr_eq(sheet, next)) {
continue;
}
bytes = bytes.saturating_add(std::mem::size_of::<gridcraft_model::Sheet>());
let empty = gridcraft_model::CellStore::new();
let next = newer.sheets.get(index).map_or(&empty, |next| &next.cells);
bytes = bytes.saturating_add(sheet.cells.retained_bytes_against(next));
}
bytes
}
fn push_history(history: &mut Vec<HistoryEntry>, label: String, wb: Arc<Workbook>, next: &Workbook, selection: Selection) {
push_history_with_budget(history, label, wb, next, selection, UNDO_BYTES_LIMIT);
}
fn push_history_with_budget(
history: &mut Vec<HistoryEntry>,
label: String,
wb: Arc<Workbook>,
next: &Workbook,
selection: Selection,
byte_budget: usize,
) {
let retained_bytes = history_retained_bytes(&wb, next);
history.push(HistoryEntry { label, wb, selection, retained_bytes });
let mut total = history.iter().fold(0usize, |sum, entry| sum.saturating_add(entry.retained_bytes));
// Always preserve the latest action, even if that one edit exceeds the budget.
while history.len() > 1 && (history.len() > UNDO_LIMIT || total > byte_budget) {
total = total.saturating_sub(history.remove(0).retained_bytes);
}
}
impl DocState {
pub fn new(mut wb: Workbook, path: Option<String>, title: String) -> DocState {
@@ -350,10 +389,7 @@ impl Session {
&& let Some(d) = self.docs.iter_mut().find(|d| d.uid == uid)
&& !Arc::ptr_eq(&d.wb, &wb)
{
d.undo.push(HistoryEntry { label: spec.label.trim_end_matches('…').to_string(), wb, selection: sel });
if d.undo.len() > UNDO_LIMIT {
d.undo.remove(0);
}
push_history(&mut d.undo, spec.label.trim_end_matches('…').to_string(), wb, &d.wb, sel);
d.redo.clear();
d.revision += 1;
}
+44
View File
@@ -253,6 +253,50 @@ fn undo_redo() {
assert!(s.doc().unwrap().is_dirty());
}
#[test]
fn bulk_formula_history_is_bounded_and_latest_edit_still_undoes() {
use std::sync::Arc;
use gridcraft_model::{Cell, Formula, Workbook};
let mut current = Arc::new(Workbook::new());
let mut history = Vec::new();
let budget = 64 * 1024;
for version in 0..5 {
let previous = current.clone();
let mut next = (*previous).clone();
let sheet = next.sheet_mut(0).unwrap();
for row in 0..128 {
sheet.cells.set(CellRef::new(row, 0), Cell::formula(Formula::new(&format!("=B{}+{}", row + 1, version))));
}
current = Arc::new(next);
crate::push_history_with_budget(&mut history, format!("edit {version}"), previous, &current, crate::Selection::default(), budget);
}
assert!(history.len() < 5, "bulk edits should evict old steps before reaching the count limit");
assert_eq!(history.last().unwrap().label, "edit 4");
assert!(history.len() == 1 || history.iter().map(|e| e.retained_bytes).sum::<usize>() <= budget);
let mut session = s();
let doc = session.doc_mut().unwrap();
doc.wb = current;
doc.undo = history;
session.execute("edit.undo", json!({})).unwrap();
assert_eq!(session.doc().unwrap().wb.active().unwrap().cells.get(CellRef::new(0, 0)).unwrap().formula.as_ref().unwrap().text, "B1+3");
session.execute("edit.redo", json!({})).unwrap();
assert_eq!(session.doc().unwrap().wb.active().unwrap().cells.get(CellRef::new(0, 0)).unwrap().formula.as_ref().unwrap().text, "B1+4");
}
#[test]
fn small_edits_keep_the_full_undo_depth() {
let mut session = s();
for number in 0..100 {
session.execute("cell.set", json!({"cell": "A1", "input": number.to_string()})).unwrap();
}
assert_eq!(session.doc().unwrap().undo.len(), 100);
session.execute("edit.undo", json!({"steps": 100})).unwrap();
assert_eq!(v(&session, "A1"), Value::Empty);
}
#[test]
fn shape_text_edit_undo_redo_and_noop() {
let mut s = s();
+49
View File
@@ -154,6 +154,55 @@ impl CellStore {
}
out
}
/// Approximate heap storage kept alive by this snapshot's bands that `newer` no longer
/// shares. This is for bounding Undo history, not for reporting process memory: BTreeMap
/// nodes and allocator overhead vary by platform. Shared formulas and values are counted
/// only when the newer band no longer holds the same Arc.
pub fn retained_bytes_against(&self, newer: &CellStore) -> usize {
use std::mem::size_of;
let mut bytes = 0usize;
for (key, band) in &self.bands {
let next = newer.bands.get(key);
if next.is_some_and(|b| Arc::ptr_eq(b, band)) {
continue;
}
bytes = bytes.saturating_add(size_of::<Band>());
for (row, cols) in &band.rows {
bytes = bytes.saturating_add(size_of::<u32>() + 3 * size_of::<usize>());
for (col, cell) in cols {
// Cells and BTreeMap nodes are duplicated with a cloned band even when
// their formula/value Arcs are still shared with the next snapshot.
bytes = bytes.saturating_add(size_of::<u32>() + size_of::<Cell>() + 3 * size_of::<usize>());
let next_cell = next.and_then(|b| b.rows.get(row)).and_then(|r| r.get(col));
if let Some(formula) = &cell.formula
&& !next_cell.and_then(|c| c.formula.as_ref()).is_some_and(|f| Arc::ptr_eq(f, formula))
{
bytes = bytes.saturating_add(size_of::<crate::cell::Formula>() + formula.text.len());
if let Some(expr) = &formula.expr {
expr.walk(&mut |_| bytes = bytes.saturating_add(size_of::<gridcraft_formula::Expr>()));
}
}
match &cell.value {
gridcraft_core::Value::Text(text)
if !next_cell.and_then(|c| c.value.as_text()).is_some_and(|t| std::ptr::eq(t, text.as_ref())) =>
{
bytes = bytes.saturating_add(text.len());
}
gridcraft_core::Value::Array(array)
if !next_cell.is_some_and(|c| matches!(&c.value, gridcraft_core::Value::Array(a) if Arc::ptr_eq(a, array))) =>
{
bytes = bytes.saturating_add(
size_of::<gridcraft_core::Array>() + array.data.len().saturating_mul(size_of::<gridcraft_core::Value>()),
);
}
_ => {}
}
}
}
}
bytes
}
/// Removes every cell in `r` and returns them.
pub fn take_range(&mut self, r: RangeRef) -> Vec<(CellRef, Cell)> {
let keys: Vec<CellRef> = self.iter_range(r).map(|(c, _)| c).collect();
+5 -1
View File
@@ -1,6 +1,6 @@
# Architecture
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-10 · **Change:** major (created from the code on main; supersedes the local-only plan/architecture.md for committed docs) · **Target:** Microsoft Excel (Microsoft 365)
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-14 · **Change:** minor (bound Undo's retained cell storage) · **Target:** Microsoft Excel (Microsoft 365)
How GridCraft is built today. Rules for contributors are in [`AGENTS.md`](../AGENTS.md).
@@ -35,6 +35,9 @@ including tests, 2026-10-10.
`data.sortAscending`) plus primitives (`cell.set`, `selection.set`, `range.setValues`).
`Session::execute` catches escaped panics and restores the workbook (never-crash guard).
2. **Edits** change the model through copy-on-write bands; undo keeps the previous snapshot.
Undo retains at most 100 steps and trims older steps when its estimated retained cell storage
exceeds 128 MiB. The newest step remains undoable even if that step alone exceeds the budget.
The estimate is a history policy, not a measure or hard cap of process memory.
3. **Recalc** marks dependents of changed cells dirty through the dependency graph (with range
nodes for range dependents), evaluates in topological order, spills dynamic arrays and reports
`#SPILL!`/cycles. Volatile functions are always dirty.
@@ -68,4 +71,5 @@ rpm, tarball), FreeBSD, and web (WASM; WebGPU or WebGL2). Release workflows in
| Date | Change | Summary |
|---|---|---|
| 2026-10-14 | minor | Document Undo's retained-cell-storage budget and latest-step guarantee |
| 2026-10-10 | major | Created from the code on main: crates, layers, data flow, agent control, known gaps |