mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-09 03:49:54 -06:00
perf(model): cache formula trees when reading a workbook from JSON
A Formula read from JSON had no cached tree (`expr` is not saved), so every graph rebuild and every recalculation parsed each formula again. Reading one now parses its text once and keeps the tree, as Formula::new does. The saved text is kept as written. Fixes #3577 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
640fbf22f6
commit
4bba98fd50
@@ -11,6 +11,7 @@ use crate::style::StyleId;
|
||||
|
||||
/// A parsed formula. `text` is the canonical formula text without the leading `=`.
|
||||
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
|
||||
#[serde(from = "SavedFormula")]
|
||||
pub struct Formula {
|
||||
pub text: String,
|
||||
#[serde(skip)]
|
||||
@@ -39,15 +40,15 @@ impl Formula {
|
||||
pub fn from_expr(e: Expr) -> Formula {
|
||||
Formula { text: gridcraft_formula::print(&e), expr: Some(Arc::new(e)), array: None, dynamic: false }
|
||||
}
|
||||
/// The parsed expression, re-parsing after deserialization.
|
||||
/// The parsed expression, re-parsing when no tree is cached.
|
||||
pub fn expr(&self) -> Option<Expr> {
|
||||
self.expr.as_deref().cloned().or_else(|| gridcraft_formula::parse(&self.text).ok())
|
||||
}
|
||||
/// The parsed expression without copying it (re-parses after deserialization).
|
||||
/// The parsed expression without copying it (re-parses when no tree is cached).
|
||||
pub fn expr_arc(&self) -> Option<Arc<Expr>> {
|
||||
self.expr.clone().or_else(|| gridcraft_formula::parse(&self.text).ok().map(Arc::new))
|
||||
}
|
||||
/// Runs `f` on the parsed expression, borrowing the cached tree or, after deserialization,
|
||||
/// Runs `f` on the parsed expression, borrowing the cached tree or, when none is cached,
|
||||
/// a freshly parsed one that is dropped afterwards. Unlike [`expr`](Self::expr) it never copies
|
||||
/// the tree, and unlike [`expr_arc`](Self::expr_arc) it needs no reference count or `Arc`
|
||||
/// allocation. `None` when the text does not parse.
|
||||
@@ -59,6 +60,25 @@ impl Formula {
|
||||
}
|
||||
}
|
||||
|
||||
/// A formula as a saved workbook holds it: the tree is not saved. Reading one parses its text
|
||||
/// once and caches the tree, so a workbook opened from JSON does not parse every formula again on
|
||||
/// each recalculation. The text is kept as saved, and text that doesn't parse caches nothing, as
|
||||
/// [`Formula::new`] does.
|
||||
#[derive(Deserialize)]
|
||||
struct SavedFormula {
|
||||
text: String,
|
||||
array: Option<RangeRef>,
|
||||
#[serde(default)]
|
||||
dynamic: bool,
|
||||
}
|
||||
|
||||
impl From<SavedFormula> for Formula {
|
||||
fn from(f: SavedFormula) -> Formula {
|
||||
let expr = gridcraft_formula::parse(&f.text).ok().map(Arc::new);
|
||||
Formula { text: f.text, expr, array: f.array, dynamic: f.dynamic }
|
||||
}
|
||||
}
|
||||
|
||||
/// A static picture stored as cell content, independent of floating drawing anchors.
|
||||
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
|
||||
pub struct CellPicture {
|
||||
@@ -131,3 +151,59 @@ impl Cell {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use gridcraft_core::CellRef;
|
||||
|
||||
#[test]
|
||||
fn formulas_read_from_json_keep_their_text_and_cache_the_tree() -> Result<(), serde_json::Error> {
|
||||
let mut array = Formula::new("=A1:B2*2");
|
||||
array.array = Some(RangeRef::new(CellRef::new(0, 2), CellRef::new(1, 3)));
|
||||
array.dynamic = true;
|
||||
for f in [Formula::new("=SUM(A1:A3)+'My Sheet'!B2"), Formula::new("=LET(x,A1*2,x+1)"), array] {
|
||||
let back: Formula = serde_json::from_str(&serde_json::to_string(&f)?)?;
|
||||
assert!(back.expr.is_some(), "{}: the tree is cached after reading", f.text);
|
||||
assert_eq!(back, f);
|
||||
}
|
||||
// Saved text is kept as written, even when the canonical print differs, and its tree is
|
||||
// the one the text parses to.
|
||||
let back: Formula = serde_json::from_str(r#"{"text": "sum( a1 , 2 )", "array": null}"#)?;
|
||||
assert_eq!(back.text, "sum( a1 , 2 )");
|
||||
assert!(!back.dynamic);
|
||||
assert_eq!(back.expr.as_deref(), gridcraft_formula::parse("sum( a1 , 2 )").ok().as_ref());
|
||||
assert!(back.expr.is_some());
|
||||
// Text that doesn't parse caches nothing, as `Formula::new` does.
|
||||
let bad = Formula::new("=SUM(");
|
||||
let back: Formula = serde_json::from_str(&serde_json::to_string(&bad)?)?;
|
||||
assert_eq!(back, bad);
|
||||
assert!(back.expr.is_none());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_workbook_read_from_json_has_every_formula_tree() -> Result<(), Box<dyn std::error::Error>> {
|
||||
let mut wb = crate::Workbook::new();
|
||||
let mut array = Formula::new("=A1:A3*2");
|
||||
array.array = Some(RangeRef::new(CellRef::new(0, 3), CellRef::new(2, 3)));
|
||||
let sheet = wb.sheet_mut(0).ok_or("no sheet")?;
|
||||
sheet.cells.set(CellRef::new(0, 0), Cell::value(Value::Number(1.0)));
|
||||
sheet.cells.set(CellRef::new(0, 1), Cell::formula(Formula::new("=SUM(A1:A3)")));
|
||||
sheet.cells.set(CellRef::new(1, 1), Cell::formula(Formula::new("=IF(A1>0,\"yes\",\"no\")")));
|
||||
sheet.cells.set(CellRef::new(2, 1), Cell::formula(Formula::new("=SUM(")));
|
||||
sheet.cells.set(CellRef::new(0, 3), Cell::formula(array));
|
||||
let json = serde_json::to_string(&wb)?;
|
||||
let back: crate::Workbook = serde_json::from_str(&json)?;
|
||||
let (before, after) = (wb.sheet(0).ok_or("no sheet")?, back.sheet(0).ok_or("no sheet")?);
|
||||
let mut formulas = 0;
|
||||
for (c, cell) in before.cells.iter() {
|
||||
let (Some(f), Some(read)) = (&cell.formula, after.cell(c).and_then(|cell| cell.formula.as_ref())) else { continue };
|
||||
assert_eq!(read, f, "{c:?}: text, tree and flags as saved");
|
||||
assert_eq!(read.expr.is_some(), gridcraft_formula::parse(&f.text).is_ok(), "{c:?}: every formula that parses has its tree");
|
||||
formulas += 1;
|
||||
}
|
||||
assert_eq!(formulas, 4);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user