Release the underlying PDFPage object when a page is closed - #1396
Open
afonsojanu wants to merge 1 commit into
Open
afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
flush_cache() clears the derived caches (_layout, _rect_edges, etc.) but leaves page.page_obj, the raw pdfminer PDFPage, referenced. That object holds resource and content-stream data of its own, so a caller who iterates over many pages calling extract_text() then flush_cache() per page (rather than holding the whole document open) ends up retaining most of that memory anyway. page.close() is the one place nothing should read from the page afterward, so it's the safe spot to drop page_obj too, unlike flush_cache() itself, which other properties like layout and annots may still need mid-lifecycle. Confirmed with a small benchmark against one of the repo's own multi-KB test PDFs, reopening and reprocessing it 20 times: peak traced memory roughly halved once page_obj is cleared alongside the existing caches. Fixes jsvine#1395.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1395.
page.close()clears the derived caches viaflush_cache()and clears theget_textmapLRU cache, but it never lets go ofpage.page_obj, the raw pdfminerPDFPage. That object carries its own resource/content-stream data, so anyone processing many pages one at a time (extract text, flush, move on, rather than holding the whole document open) ends up retaining most of that memory regardless offlush_cache().I didn't touch
flush_cache()itself for this, since a couple of properties (layout,annots) still readpage_objmid-lifecycle and callingflush_cache()doesn't mean a page is done being used.close()is the one spot where nothing should touch the page again afterward, so it's the safe place to release it.Measured the actual effect with a small script against one of the repo's own multi-page test PDFs, reopening and reprocessing it 20 times and tracking peak memory with
tracemalloc: clearingpage_objalongside the existing caches roughly halved peak retained memory in that run.Added a regression test in
tests/test_issues.pyconfirmingpage.page_objisNoneafterclose()(fails on unmodifieddevelop, passes with the fix). Ran the full suite: everything passes except the six pre-existingtest_repair.pyfailures from Ghostscript not being installed in my sandbox, unrelated to this change and failing identically on unmodifieddevelop.black,isort, andflake8are clean on the two files I touched (there's a pre-existingisortcomplaint onpage.py's import block that predates this change).