Copilot commented on code in PR #3067:
URL: https://github.com/apache/tika/pull/3067#discussion_r3860503252
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/detect/microsoft/POIFSContainerDetector.java:
##########
@@ -583,16 +597,54 @@ private Set<String> getTopLevelNames(TikaInputStream
stream) throws IOException
}
}
+ /**
+ * Opens the OLE2 container from memory instead of spooling it to a temp
file. Returns
+ * null when the object is too large for the in-memory limit or POI cannot
load it from a
+ * stream (the caller then spools, as before).
+ */
+ private Set<String> getTopLevelNamesInMemory(TikaInputStream stream,
ParseContext context)
+ throws IOException {
+ CacheMemoryBudget budget = context == null ? null :
context.get(CacheMemoryBudget.class);
+ long limit = budget == null ? DEFAULT_IN_MEMORY_POIFS :
+ Math.min(MAX_IN_MEMORY_POIFS, budget.getMaxBytes());
+ // attach the budget first: without it the drain below caches only the
per-object
+ // default (1MB) in memory and spills the rest before we can even
check the size
+ stream.enableRewind(budget);
+ // the channel is served from memory while the content fits the
cache/budget
+ try (SeekableByteChannel channel = stream.getSeekableByteChannel()) {
+ long size = channel.size();
+ if (size > limit) {
+ return null;
+ }
+ if (budget != null) {
+ if (budget.tryReserve(size) == 0) {
+ return null;
+ }
+ stream.addCloseableResource(() -> budget.release(size));
+ }
+ POIFSFileSystem fs = new
POIFSFileSystem(Channels.newInputStream(channel));
+ stream.setOpenContainer(fs);
+ return getTopLevelNames(fs.getRoot());
Review Comment:
When POI's stream loader throws (IOException/RuntimeException), this method
returns null and falls back to spooling, but the CacheMemoryBudget reservation
added at `stream.addCloseableResource(() -> budget.release(size))` is still
held until the TikaInputStream is closed. That can artificially reduce the
shared budget and force unrelated caches to spill even though the in-memory
POIFS path was abandoned.
Consider only registering the release callback after successfully
constructing the POIFSFileSystem, and explicitly releasing the reservation on
failure before returning null.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/parser/pdf/PDFParser.java:
##########
@@ -315,10 +321,25 @@ private void scanXRefOffsets(PDFParserConfig localConfig,
List<StartXRefOffset> xRefOffsets = new ArrayList<>();
//TODO -- can we use the PDFBox parser's RandomAccessRead
//so that we don't have to reopen from file?
- try (RandomAccessRead ra =
- new
RandomAccessReadBufferedFile(tikaInputStream.getFile())) {
- StartXRefScanner xRefScanner = new StartXRefScanner(ra);
- xRefOffsets.addAll(xRefScanner.scan());
+ // In-memory input is scanned from memory rather than spooled to a
file for this
+ // pass. Rewind support is enabled first so the later parse (and a
renderer's
+ // getPath(), if rendering is on) can re-read the content from the
cache: the old
+ // getFile() spool used to provide that as a side effect.
+ boolean fileBacked = tikaInputStream.hasFile();
+ try {
+ if (!fileBacked) {
+
tikaInputStream.enableRewind(parseContext.get(CacheMemoryBudget.class));
+ }
+ try (RandomAccessRead ra = fileBacked ?
+ new
RandomAccessReadBufferedFile(tikaInputStream.getFile()) :
+ new
RandomAccessReadBuffer(CloseShieldInputStream.wrap(tikaInputStream))) {
+ StartXRefScanner xRefScanner = new StartXRefScanner(ra);
Review Comment:
For non-file-backed input, this path switches to
`RandomAccessReadBuffer(...)`, which is an in-memory random-access
implementation. For large PDFs provided via stream-backed TikaInputStream, this
can substantially increase peak heap usage (potentially duplicating the PDF
bytes), whereas the previous `RandomAccessReadBufferedFile` path kept the scan
largely off-heap/disk-backed.
It may be worth adding a size/budget-based cutoff (e.g., fall back to
spooling and `RandomAccessReadBufferedFile` when the PDF exceeds the
CacheMemoryBudget or some fixed threshold) to avoid unexpected heap pressure on
large inputs.
--
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]
