Skip to content

Remove the debug timing loop from Annot.update() - #5087

Open
brokkoli71 wants to merge 1 commit into
pymupdf:mainfrom
brokkoli71:drop-annot-update-timing-test
Open

Remove the debug timing loop from Annot.update()#5087
brokkoli71 wants to merge 1 commit into
pymupdf:mainfrom
brokkoli71:drop-annot-update-timing-test

Conversation

@brokkoli71

Copy link
Copy Markdown

Annot.update() unconditionally calls Annot.update_timing_test(), a static method that counts to 30,000 in pure Python and returns a sum the caller discards:

# src/__init__.py:1628, inside Annot.update()
Annot.update_timing_test()

# src/__init__.py:1891
@staticmethod
def update_timing_test():
    total = 0
    for i in range( 30*1000):
        total += i
    return total

It looks like a benchmark that was committed by accident. It is present in every release from 1.23.0 through 1.28.2 and in current main, and absent from 1.22.x, so it appears to have come in with the src/__init__.py rewrite.

Cost

Measured with PyMuPDF 1.27.2.3 on Python 3.14:

  • 0.68 ms per Annot.update() call.
  • Writing the 212 ink annotations of one 806-page document takes 420 ms, of which 144 ms is this loop. The remaining doc.save() for the same file is 86 ms — so the accidental loop costs more than saving the document.

Because it is pure Python it also holds the GIL, so it cannot be hidden on a worker thread. In an interactive application that writes annotations on the UI thread it is a visible freeze.

The change

The method is undocumented and had no other callers anywhere in the repository (the only two references were the call and the definition), so this removes both. Happy to trim it to just the call site if you would rather keep the method around.

I checked that a document written with the call removed is byte-identical to one written with it, apart from the trailer's random /ID, which differs between any two saves of the same document anyway.

Annot.update() unconditionally calls Annot.update_timing_test(), a
static method that counts to 30,000 in pure Python and returns a sum
the caller discards. It costs about 0.68 ms per annotation.

The method is undocumented and had no other callers, so it is removed
along with the call.
@github-actions

Copy link
Copy Markdown
Contributor


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@brokkoli71

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@brokkoli71

Copy link
Copy Markdown
Author

recheck

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