pjfanning opened a new pull request, #1242: URL: https://github.com/apache/poi/pull/1242
XmlBeans indexed accessors walk the child list from the start, so `getXxxArray(i)` is O(i) and iterating the lazy `getXxxList()` is O(n²). Measured on xmlbeans 5.4.0, a single full pass over one element's children: | n | `getCArray()` pass | `getCList()` iteration | `getCArray(i)` loop | |---|---|---|---| | 1,000 | 0.31 ms | 8.4 ms | 2.7 ms | | 16,000 | 1.4 ms | 6,062 ms | 1,410 ms | So `getXxxArray()` is the cheap way to scan, and the problem is repeated or indexed access. Three places paid for it. ### `XSSFRow.fixupCTCells()` Runs on save for any row whose cells were created out of column order. It deep-copied every `CTCell` and reordered via indexed `setCArray(i, …)`/`getCArray(i)`, i.e. O(cells²) per affected row. It now reorders the existing beans with `XmlCursor.moveXml()` (linear) and re-attaches each `XSSFCell` from one `getCArray()` call. Document-level timings, before → after: n=16000 reversed 3009 ms → 27 ms; last cell moved to front 1357 ms → 17 ms; last two swapped 21 ms → 4 ms; first/last swapped 20 ms → 21 ms. Worth recording for future readers: a single `setCArray(CTCell[])` is *not* the fix. `XmlComplexContentImpl.arraySetterHelper` keeps the common prefix then appends copies and removes the originals by index, so a divergence at index 0 forces the whole tail to be rebuilt — measured 12x *slower* than the current code for a first/last swap, while only 2x faster for a full reversal. Verified by diffing the serialised worksheet XML of the old and new implementations over 6400 generated cases (row widths 1-8, every subset size, randomised permutations, with and without a trailing `extLst` that must stay last): zero differences. ### `XDDFDataSourcesFactory` The six anonymous `XDDFDataSource` implementations defined `getPointAt(index)` as `data.getPtArray(index)`, so `fillNumericalCache`/`XDDFChart.fillSheet` reading every point was O(n²). Each source already holds a private deep copy taken at construction and nothing mutates it, so the point array is now materialised once and indexed directly. `getPointCount()` and `getDataRangeReference()` are unchanged. ### `ColumnHelper` Four read-only loops iterated `getColList()`, including `cleanColumns()`, which runs when every sheet is loaded. They now iterate `getColArray()`, turning each O(n²) pass into a single walk. ### Deliberately not included `CalculationChain.removeItem()` is still O(m·n) for m removals. An index cache cannot be invalidated reliably: `getCTCalcChain()` hands out the live bean, the sheet id of an entry is positional (inherited from the previous entry when absent, so any insert/remove/`setI` shifts everything after it), and duplicate `(sheetId, ref)` entries must keep first-match-only semantics. Fixing it properly needs a batch-removal entry point plus caller changes, which belongs in its own change. `ColumnHelper.getColumn1Based` still re-walks per call; removing that needs cached lookup state that callers outside the class would invalidate, since they obtain and mutate the `CTCols`/`CTCol` beans directly. Full `poi-ooxml` suite run: the only failures are `TestXSSFBugs.stackoverflow23114397`, `TestSXSSFBugs.stackoverflow23114397` and `TestSXSSFSheetAutoSizeColumn[1]`, which fail identically on unmodified trunk on this machine (font-metric dependent). 🤖 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]
