pjfanning opened a new pull request, #1318: URL: https://github.com/apache/poi/pull/1318
Follow-up to #1317. `XSSFEvaluationWorkbook.getFormulaTokens` parsed the formula text (`cell.getCellFormula(this)` → `FormulaParser.parse`) on every call, so every re-evaluation of a formula after a `notifyUpdateCell` re-parsed it, and every cell of a shared formula group paid parse → shift → render → parse. ## What changed 1. **Per-evaluator token cache.** `XSSFEvaluationSheet` keeps the parsed `Ptg[]` per `XSSFCell` (identity-keyed) for the life of the evaluator. `clearAllCachedResultValues()` drops everything; a new `EvaluationSheet.notifyUpdateCell(row, col)` default hook — propagated from `WorkbookEvaluator.notifyUpdateCell`, exactly like the `notifyDeleteCell` hook from #1307 — drops the entry of a notified cell, so `notifySetFormula` reaches it. This is scoped to the evaluator's lifetime, like the result cache, so it needs no invalidation on the usermodel side and changes nothing for evaluate-once callers (each formula was parsed once per evaluator anyway). 2. **One parse per shared formula group.** A cell of a shared group takes the master's tokens shifted to its own position via `SharedFormula.convertSharedFormulas` (the same code HSSF uses), instead of going through `XSSFCell.getCellFormula()`'s render + re-parse. The master tokens are cached keyed by the master `CTCellFormula` object the sheet currently holds for the group — POI installs a new object when the group changes (`onDeleteFormula` promotes the next cell; `setCellFormula` on the master re-registers a copy), so a changed group is re-derived without any extra bookkeeping. Formulas containing a structured table reference (`[`) keep the old path, as their parse depends on the row. The `WorkbookEvaluator` only reads the arrays it gets, and `convertSharedFormulas` copies the reference tokens it shifts, so handing the same `Ptg[]` out repeatedly is safe. ## Tests `TestXSSFFormulaTokenCache` (on the shared-formula variant of the fixture): the same parse is handed out until the cell is notified, then a new one; the whole chain `FormulaEvaluator.notifySetFormula` → `WorkbookEvaluator` → `EvaluationSheet.notifyUpdateCell`; shared group cells' tokens render to exactly the formula `getCellFormula()` reports; a changed master is parsed again by a fresh evaluator. All existing `TestXSSFFormulaEvaluator*` (incl. the `*SharedFormulas` variants), `TestFormulaEvaluatorOnXSSF`, `TestMultiSheetFormulaEvaluatorOnXSSF`, `TestXSSFSheetShiftRows`, `TestXSSFBugs`, `TestSXSSFFormulaEvaluation`, `ss.formula.*` (both modules), `TestHSSFFormulaEvaluator*` pass locally (the usual pre-existing `stackoverflow23114397` font-metrics failure aside). ## Benchmark New `HSSF|XSSFFormulaReevaluationBenchmark`: one evaluator kept for the workbook, the quantity of the next data row is bumped and notified, every formula is read again (`fe.evaluate`) — only the row's chain, the running total below it and the summary block are recomputed, the rest is served from the result cache. Back-to-back, `-Pjmh.profilers=gc`: | | rows | trunk | this PR | |---|---|---|---| | XSSF ms/op | 100 | 5.25 ± 1.36 | **1.77 ± 1.17** | | XSSF B/op | 100 | 4.28 MB | **0.43 MB** | | XSSF ms/op | 200 | 10.12 ± 0.88 | **3.34 ± 1.65** | | XSSF B/op | 200 | 8.27 MB | **0.81 MB** | | HSSF ms/op (reference) | 100 / 200 | — | 1.03 / 2.26 | A re-evaluation in XSSF now costs about what it costs in HSSF, and allocates a tenth of what it did. The full-pass benchmarks (`*FormulaEvaluationBenchmark`) are unaffected by design: with a fresh evaluator per pass every formula is parsed once either way. One thing I noticed and left alone: `XSSFCell.setCellFormula` on the *master* of a shared group keeps the group and re-registers it with the new text, so the other cells of the group change meaning too (`D2 := A2*100` makes `D3` report `A3*100`). Excel would un-share the master and leave the others as they were. `aChangedSharedMasterIsParsedAgain` documents the current behaviour rather than asserting it's right — worth a separate look if you agree it's a bug. `changes.xml` left for you. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
