pjfanning opened a new pull request, #1300: URL: https://github.com/apache/poi/pull/1300
Fixes https://bz.apache.org/bugzilla/show_bug.cgi?id=70139 ## Problem Inserting rows one at a time into a sheet with a few thousand rows (`shiftRows(2500, last, 1)` + `createRow`) is orders of magnitude slower than in POI 4.0.x. Since the fix for bug 64516, `XSSFSheet.rebuildRows()`: 1. sorts all `CTRow`s and calls `sheetData.setRowArray(...)` — an XmlBeans deep copy of every row element, and 2. recreates every `XSSFRow` (and thereby every `XSSFCell`) from the XML. It runs **twice** per `shiftRows` call (inside `shiftCommentsAndRows` and again at the end), and twice in `shiftColumns` where no row is renumbered at all. It also silently invalidates the `XSSFRow`/`XSSFCell` instances the caller holds. The XML reorder is only needed when a shift makes rows jump over other rows (e.g. `shiftRows(2, 2, -2)`, bugs 59733/64516). Ordinary inserts and deletes keep the `CTRow` order intact — rows are only renumbered. ## Fix - `rebuildRows()` walks `_rows` once. It is a no-op if nothing was renumbered; if the rows are still in order it only refreshes the keys of the `_rows` map, keeping the existing row and cell instances; the XML reorder plus recreation is kept for the out-of-order case. - Recreating the rows used to re-register shared and array formulas via `onReadCell` as a side effect. This is now done explicitly by `rebuildFormulaBookkeeping()` at the end of `shiftRows`/`shiftColumns`, after the formulas were shifted. - That exposed a latent bug in `XSSFRowColShifter.updateRowFormulas`: when the sheet's shared-formula registry holds the *live* formula of a re-homed master (after `onDeleteFormula` deleted the old master) instead of a detached copy, the ref was shifted twice and ended up as `#REF!` (`TestXSSFBugs.testBug65306`). The recreation used to paper over that; it is now guarded with `sf != f`. ## Measurements Reporter's benchmark shape (6000 rows with heights only, 10 single-row inserts at row 2500, min of 6 runs): trunk **279 ms** → this PR **~40 ms**. The remainder is XmlBeans `getR`/`setR` per row, as in POI 4. ## Tests - `TestXSSFSheetShiftRows.testShiftRowsKeepsRowAndCellInstances` — row/cell instances survive a shift, sheetData stays ordered, round-trips. - `TestXSSFSheetShiftRows.testShiftRowsOverOtherRowsKeepsSheetDataInOrder` — the bug 64516 path still reorders the XML. - `TestXSSFSheetShiftRows.testShiftSharedFormulasTwice` — shared-formula bookkeeping across successive shifts. - `xssf.usermodel.*`, `xssf.streaming.*`, `ss.*` in `poi-ooxml` pass; `TestXSSFBugs.stackoverflow23114397`, `TestSXSSFBugs.stackoverflow23114397` and `TestSXSSFSheetAutoSizeColumn` fail on my machine on unmodified trunk too (font-metric dependent) and are unrelated. 🤖 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]
