pjfanning opened a new pull request, #1241: URL: https://github.com/apache/poi/pull/1241
Two small, independent performance fixes found while reviewing poi-ooxml. (The third item from that review, `ContentType.equals`, is in #1240.) ### Cache the `TransformerFactory` in `XMLHelper` `XMLHelper.newTransformer()` called `TransformerFactory.newInstance()` on every invocation, redoing the JAXP provider lookup and re-applying the security configuration each time. It runs once per part that has relationships on every OPC save (`ZipPartMarshaller.marshallRelationshipPart`), plus once for the content types part, and is also used by the HWPF/HSSF converters. The `DocumentBuilderFactory` and `SAXParserFactory` in the same class are already cached this way; the `TransformerFactory` was the odd one out. Measured on JDK 17 with the built-in `TransformerFactoryImpl` (2000 iterations, warmed up): | | single-threaded | 8 threads | |---|---|---| | current: `newInstance()` per call | 83.1 µs/op | 124.7 µs/op | | cached + `synchronized` (this PR) | 8.1 µs/op | 16.9 µs/op | | cached + `ThreadLocal` | 12.8 µs/op | 4.1 µs/op | `newInstance()` plus the configuration is ~87% of the cost, which is what caching removes. `TransformerFactory` is not guaranteed thread-safe and the implementation is pluggable via system property, so the creation call is synchronized rather than assumed safe. The lock was free when uncontended in the measurements above (the difference against an unsynchronized cached factory was within noise), and it guards only the identity-transformer creation, not the transform itself. A `ThreadLocal` is ~4x better than the lock under 8-way contention, but both are well clear of the status quo, the absolute numbers are small next to the surrounding document save, and a library-held `ThreadLocal` retaining a pluggable factory is a known classloader-leak hazard when POI runs inside an application server. Happy to switch if you would rather have the contended throughput. ### Don't fetch comments the extractor is going to discard `XSSFExcelExtractor` called `cell.getCellComment()` for every cell and only then checked `includeCellComments`. With the default `includeCellComments = false`, that is a comments-table lookup per cell — plus a linear VML shape scan for every cell that does have a comment (`XSSFVMLDrawing.findCommentShape`) — purely to throw the result away. Moved the call inside the guard. `TestXMLHelper`, the poi-ooxml extractor and openxml4j suites (272 tests), and the poi-scratchpad converter suites (1507 tests, the heaviest `newTransformer()` users) pass locally. 🤖 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]
