pjfanning opened a new pull request, #1329: URL: https://github.com/apache/poi/pull/1329
Follow-up to #1317. In the default mode (no `IStabilityClassifier`) every evaluated formula records the cells it read so that `notifyUpdateCell` can invalidate it. After #1317 this bookkeeping is what separates DEFAULT from TOTALLY_IMMUTABLE on the lookup benchmark (12.1 vs 3.8 MB/op, 32 vs 12 ms at 100 rows). ### What was allocated The record of a formula's inputs was built three times over: 1. `CellEvaluationFrame` collected the entries in a `HashSet` — one node per cell read (`SUM(A1:A1000)` = 1000 nodes); 2. `getSensitiveInputCells()` copied the set into an array; 3. `FormulaCellCacheEntry.setSensitiveInputCells` `clone()`d that array, and on re-evaluation built *another* `HashSet` to diff old inputs against new. Plus every `CellCacheEntry` — including every cached plain cell — eagerly allocated a `FormulaCellCacheEntrySet` (object + array), which under TOTALLY_IMMUTABLE nothing ever adds to. ### Change - The frame appends entries to a growable array (8, doubling; nothing allocated for formulas that read no cells) and hands the array over once — the entry retains it. - De-duplication (a formula reading the same cell twice: `A1*A1`, or a cell read directly and through an area) moves to registration time: `addConsumingCell` now returns whether the consumer was new, and `setSensitiveInputCells` keeps only the cells for which it was. Without this, `clearFormulaEntry` would unregister a duplicate twice and throw. - Old inputs are unregistered wholesale before the new ones are registered. Same end state as the old diff (set semantics), no `HashSet`; and the old inputs are almost always `null` at that point — they are cleared together with the cached value. - `CellCacheEntry._consumingCells` is created on the first registration. `FormulaCellCacheEntrySet.add` and `CellCacheEntry.addConsumingCell` changed from `void` to `boolean` — both package-private classes, source-compatible for every caller. ### Numbers (`XSSFLookupEvaluationBenchmark`, `-Pjmh.profilers=gc`) | rows | mode | alloc/op before | after | time before | after | |---|---|---|---|---|---| | 100 | DEFAULT | 12.12 MB | **6.34 MB** | 35.7 ms | **23.1 ms** | | 200 | DEFAULT | 29.00 MB | **13.89 MB** | 84.4 ms | **52.4 ms** | | 100 | TOTALLY_IMMUTABLE | 3.80 MB | 3.74 MB | 14.2 ms | 15.4 ms | | 200 | TOTALLY_IMMUTABLE | 7.28 MB | 7.05 MB | 36.1 ms | 33.1 ms | Allocation counts are exact; timings are back-to-back runs on a noisy machine (±10 ms), the direction is consistent. ### Tests - `TestCellCacheEntry`: on-demand consumer set (empty, register, duplicate registration returns false, unregister, unregister again throws); inputs listed several times are kept once, re-evaluation with a different input set unregisters from the dropped ones, `clearFormulaEntry` twice is harmless. - `TestEvaluationCache.testCellReadMoreThanOnce`: `B1*B1+SUM(B1:C1)+B1` end to end with the listener log — the change notification still reaches the formula and the entry is cleared without error; `A1+A1` reading a formula cell twice. - Formula-evaluation suites in both modules green (only the pre-existing `stackoverflow23114397` font-metrics failure). 🤖 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]
