PI: Optimize retrieval of named destinations in reader - #3442

@stefan6419846 I need some help with this. ruff and Python both are complaining about Dict in my added code, claiming it's not declared. It is imported on line 39, though. And other code uses it in exactly the same way, without ruff or Python complaining. It works for me in both Python and ruff locally. What is going on?

Please try to rebase on the latest main - your base appears to be outdated and the corresponding import is gone in main.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.64%. Comparing base (885049b) to head (5139db4).
⚠️ Report is 139 commits behind head on main.

@@           Coverage Diff           @@
##             main    #3442   +/-   ##
=======================================
  Coverage   97.64%   97.64%           
=======================================
  Files          55       55           
  Lines       10227    10234    +7     
  Branches     1878     1880    +2     
=======================================
+ Hits         9986     9993    +7     
  Misses        137      137           
  Partials      104      104           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

That worked. Thank you! The PR should be ready now.

Thanks for the PR. This code seems to assume that the named destinations never change. Is this really a valid assumption or will this create unexpected behavior in specific cases?

It does assume the destinations never change. Originally I put this cache on the shared superclass for readers and writers, but then had the same realization as you. So now this code is only in the PdfReader class. That works for the purposes of the link rewriting code etc, but it does mean that getting named destinations from ´PdfWriter` is as slow as it used to be.

Thanks for the explanation - indeed, in the reader it should not be an issue. Are we able to add a test for this?

Hmmm. What sort of tests are you looking for? Tests of changing the named destinations in the reader? I guess not, since that's not supposed to be possible. Tests of changing the named destinations in the writer? It doesn't use this code, but I guess I could add. Or just tests of reading named destinations at all? Those might already exist for all I know, but I could check, once I know what you're thinking.

Sorry for not being explicit enough - I am referring to the fact that these are cached. One possible approach would be to check the time for handling many destinations the first and the second time, although I am open for other tests as well.

We really need this optimization. We just had a publishing job time out because of this issue. When I try that publishing job on my laptop, the job got to page 68 while I was having lunch. So in roughly half an hour. It's not really workable.

With this optimization it does the entire 1100-page document in 8 minutes and 24 seconds. If we assume 100% constant speed pr page then this is a speedup by a factor of 60.

What's holding this up is that I don't understand what tests you want. Please, can you explain so that I know what I need to do to get this code into the project?

What's holding this up is that I don't understand what tests you want. Please, can you explain so that I know what I need to do to get this code into the project?

I want a test that shows the caching. This could be a speed-based test for both retrievals or one based upon mocking:

from unittest import mock


def test_get_named_destinations__caching():
    url = "..."  # TODO: Fill.
    name = "PDF32000_2008.pdf"
    reader = PdfReader(BytesIO(get_data_from_url(name=name)))

    destinations = list(reader.named_destinations)
    assert len(destinations) > 10

    with mock.patch("pypdf._doc_common.PdfDocCommon._get_named_destinations") as get_mock:
        destinations2 = list(reader.named_destinations)
    assert destinations2 == destinations
    get_mock.assert_not_called()

@stefan6419846 Please have another look now.