Improve Memory efficiency - #824
Conversation
parseData() used to decode every indirect object into a fully built raw object graph - deeply nested arrays that inflate the source roughly 8-10x - and return the whole thing before Parser turned any of it into PDFObjects. At that moment the entire raw graph and the PDF string were held simultaneously, which is the peak for most documents. Replace parseData() with parseHeaderAndXref() plus a getObjectsStream() generator. Parser now builds each PDFObject and releases the raw structure before pulling the next one, so the full raw graph is never materialized, and the PDF string is freed before text extraction. Backward compatibility note: the public RawDataParser::parseData() method is removed (it had no remaining callers). Third party consumers (I don't think there are any) that called it directly should switch to parseHeaderAndXref() + getObjectsStream(), or iterate the latter into an array for the old [$xref, $objects] return shape. Measured peak memory (getText() over the whole file), vs master: samples/bugs/PullRequest457.pdf 147 MB -> 88 MB (-40%) samples/DocumentWithLotsOfObjects.pdf 99 MB -> 62 MB (-38%) Documents whose peak occurs during text extraction rather than parsing (e.g. Issue356.pdf, Issue391.pdf) are unchanged. Extracted text is identical in all cases.
getSectionsText() ran the formatted content stream through a single preg_split() into an array of every line, then kept only the handful needed for text positioning. A vector-graphics-heavy page can format to hundreds of thousands of lines while yielding almost no text, so that array was the peak of the whole text-extraction phase. Split the formatted stream in, line-aligned 1 MB chunks instead. Each chunk is then preg_split(), filtered and then discarded. Where the source stream is smaller than 1 MB, there is no change in behaviour. Output is byte-for-byte identical, and there is virtually no throughput cost. Measured peak memory (parseFile + getText on the whole file) vs master, on a graphics heavy document: samples/bugs/Issue356.pdf 79.4 MB -> 59.2 MB (-25%) Other documents are unchanged (PullRequest457.pdf 146.9 MB, DocumentWithLotsOfObjects.pdf 99.1 MB, Issue391.pdf 10.4 MB).
Add Config::setContentSpooling(): when enabled, each parsed object's decoded stream content is written to a single temporary file (ContentSpool) and read back on demand via PDFObject::getContent(), instead of being held in memory for the document's lifetime. The temp file is removed automatically once the document is destroyed. This lowers the memory retained after parsing and during text extraction; extracted text and document details are byte-for-byte identical with the option on or off (verified across the sample suite). Measured memory retained after parsing (getText output identical), vs master: samples/bugs/PullRequest457.pdf 71 MB -> 30 MB (-58%) samples/DocumentWithLotsOfObjects.pdf 55 MB -> 32 MB (-41%) samples/bugs/Issue356.pdf 25 MB -> 2 MB (-93%) samples/bugs/Issue391.pdf 5 MB -> 2 MB (-62%) Note: on documents whose peak occurs while the raw object graph is being built, spooling alone does not lower the peak - it pairs with incremental object construction, which removes that earlier peak.
* mem/pipeline-objects: Build PDF objects incrementally to lower peak memory
* mem/stream-sections: Chunked getSectionsText() to lower peak memory
Combine the three memory-reduction branches: - mem/pipeline-objects : stream raw objects one at a time (getObjectsStream) - mem/stream-sections : chunked getSectionsText() - mem/content-spooling : opt-in spooling of decoded stream content to disk Peak memory was measured by parsing each sample PDF and running getText() in a fresh PHP process (memory_get_peak_usage(true), memory_limit=-1), in three configs: master, the merged branch with spooling off, and with spooling on (Config::setContentSpooling(true)). Extracted text is byte-identical across all three configs. sample (size) master off spool d-spool bugs/PullRequest457.pdf (16 MiB) 150.9 94.9 57.4 -62% DocumentWithLotsOfObjects.pdf (6.1 MiB) 101.9 65.9 39.9 -61% bugs/Issue356.pdf (5.9 MiB) 95.9 77.9 63.6 -34% bugs/Issue267_array_access.pdf (2.0 MiB) 14.0 8.0 6.0 -57% bugs/Issue391.pdf (0.9 MiB) 16.0 14.0 12.0 -25% ImproperFontFallback.pdf (0.7 MiB) 24.0 10.0 10.0 -58% bugs/Issue585.pdf (0.5 MiB) 22.0 10.0 10.0 -55% (figures in MiB; d-spool = spool-on vs master) The in-memory streaming changes alone (off) already cut peak memory by roughly 35-60% on large documents, by never holding the full raw-object array or all decoded streams at once. Spooling reclaims most of the remaining decoded-stream footprint, roughly halving it again on the largest files. On documents whose peak is dominated by something other than retained decoded streams (e.g. ImproperFontFallback, Issue585) the streaming pass captures the win and spooling adds nothing further. Documents below ~0.4 MiB stay within the allocator's granularity (2-4 MiB) and show no measurable change. bugs/Issue104a.pdf (108 KiB) peaks at ~203 MiB in all three configs; its cost is in glyph/text handling, not stream decoding, and is unaffected by these changes.
| * @return array{0: array, 1: string} [$xref, $pdfData] | ||
| * | ||
| * @throws EmptyPdfException if empty PDF data given | ||
| * @throws MissingPdfHeaderException if PDF data missing `%PDF-` header | ||
| */ | ||
| public function parseData(string $data): array | ||
| public function parseHeaderAndXref(string $data): array |
There was a problem hiding this comment.
Thank you for your PR. Just a quick note: It might be worth further discussing if RawDataParser (and other internal classes) are considered part of the public API but regardless, changing a function and its return value is something we need to talk about. Could you elaborate on that change please? I am not sure yet if I am OK with this change.
There was a problem hiding this comment.
Basically parsedata was split into two parts: the header parsing and the object parsing. the latter now yields one object at a time - enabling more efficient memory handling.
if you consider parsedata a public api, I can readd it wrapping the two new methods - it's like 5 lines or so. but it will be unused by the library itself.
|
Could you please close this PR and split it up into 3 PRs which reflect the structure you talked about? This would help me to decide if its manageable or not. |
|
Okay I submitted all three commits as separate PRs. TBH I think they only make sense in combination and if you want them all three, you land here again ;-) I did not readd the parseData() method yet. Let me know if you want it here or on #827 or not at all. I let you close whichever PRs you don't want. |
Type of pull request
About
This branch combines three independent, complementary changes that lower peak memory consumption when parsing PDFs while keeping extracted text and document details byte-for-byte identical.
Each commit message contains the measured improvement of the commit on its own against master. Each individual commit should be easy to review (not many changes). My repository contains a branch for each of these commits separately if you'd rather only merge one or two of them - the commits slightly overlap, this PR addresses the merge conflicts for you.
The commit message of the final merge (2e56042) has memory measurements of all three combined, resulting in gains between roughly 35 to 60%.
Checklist for code / configuration changes
tests/PHPUnit/Unit/ContentSpoolTest.php, plusConfigunit + integration tests asserting spool-on/off produce identical output. Full suite passessetContentSpoolingrow + section indoc/CustomConfig.md.