Skip to content

Clear the textmap cache in Page.flush_cache() - #1397

Open
siddharthgaur1 wants to merge 1 commit into
jsvine:developfrom
siddharthgaur1:fix-flush-cache-textmap
Open

siddharthgaur1 wants to merge 1 commit into
jsvine:developfrom
siddharthgaur1:fix-flush-cache-textmap

Conversation

@siddharthgaur1

Copy link
Copy Markdown

Fixes #1395.

Problem

Page.flush_cache() iterates Page.cached_properties, which covers _layout and the object/edge caches — but not the lru_cache wrapping _get_textmap, which is created as an instance attribute in Page.__init__ and is the largest of a page's caches. So a flush left most of the memory in place.

Measured on tests/pdfs/chelsea_pdta.pdf (65 pages), calling extract_text() on every page under tracemalloc:

traced memory
no flush 184.61 MB
flush_cache() — before this PR 105.19 MB
close() 4.80 MB
flush_cache() — after this PR 4.80 MB

Note on the issue's diagnosis

The report attributes the retention to _layout surviving the flush and to page_obj, and proposes page.page_obj = None as the fix. Neither holds up:

  • _layout is cleared already. flush_cache reads self.cached_properties, which on a Page resolves to Container.cached_properties + ["_layout"], so the default flush includes it.
  • page.page_obj = None changes nothing measurable (105.19 MB → 105.19 MB). The entire difference is the textmap cache.

The retention the reporter measured is real; the cause is the textmap cache.

Change

  • Page.flush_cache() now clears the textmap cache after delegating to Container.flush_cache.
  • Page.close() did this explicitly already, so it now just delegates to flush_cache().
  • DerivedPage.__init__ builds its textmap cache before calling flush_cache() rather than after, since flush_cache() now touches that attribute. Without the reorder, constructing any cropped page raises AttributeError.

Users who want the old surgical behaviour can still pass an explicit list, e.g. page.flush_cache(["_objects"]) — the textmap cache is cleared either way, which matches what the method's name promises.

Tests

Adds test_flush_cache_clears_textmap, covering both an original page and a cropped one (the DerivedPage path that the reorder protects). It fails on the parent commit (assert 1 == 0) and passes with the change.

Full suite: 164 passed, 10 failed — all 10 failures are pre-existing on a clean develop in this environment (test_repair.py needs Ghostscript, test_convert.py hits Windows line-ending differences) and are unrelated to this change.

black, isort --profile black, flake8 and mypy all pass on the touched files.

Page.flush_cache() iterates Page.cached_properties, which covers _layout
and the object/edge caches but not the lru_cache wrapping _get_textmap.
That textmap cache is the largest of a page's caches, so flushing left
most of the memory in place.

Measured on tests/pdfs/chelsea_pdta.pdf (65 pages), extracting text from
every page and tracing with tracemalloc:

    no flush          184.61 MB
    flush_cache()     105.19 MB   <- before
    close()             4.80 MB
    flush_cache()       4.80 MB   <- after

Page.close() already cleared it explicitly, so it now simply delegates to
flush_cache(). DerivedPage.__init__ builds its textmap cache before
calling flush_cache(), rather than after, since flush_cache() now touches
that attribute.

Fixes jsvine#1395.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant