GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-132493: Avoid eager import of annotationlib in typing (again) by JelleZijlstra · Pull Request #132596 · python/cpython

Conversation

@JelleZijlstra

@JelleZijlstra JelleZijlstra commented Apr 16, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

gh-132494 made typing.py eagerly import annotationlib again because
typing contains several protocols. Avoid this by using annotationlib only if .__annotations__ fails.

An earlier version of this PR instead made it so we compute the protocol attrs only when needed (generally on the first isinstance() call). That would make startup faster, but has some compatibility implications in cases where attributes get added to the class later, so I think this version is safer.

  • Issue: Avoid eagerly evaluating annotations #132493

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@JelleZijlstra
pythongh-132493: Lazily determine protocol attributes
1a970b3
pythongh-132494 made typing.py eagerly import annotationlib again because
typing contains several protocols. Avoid this by determining annotations
lazily. This should also make protocol creation faster:

Unpatched:

$ ./python.exe -m timeit -s 'from typing import Protocol, runtime_checkable' '''@runtime_checkable
class MyProtocol(Protocol):
    def meth(self): pass
'''
50000 loops, best of 5: 9.28 usec per loop
$ ./python.exe -m timeit -s 'from typing import Protocol, runtime_checkable' '''class MyProtocol(Protocol):
    def meth(self): pass
'''
50000 loops, best of 5: 9.05 usec per loop

Patched:

$ ./python.exe -m timeit -s 'from typing import Protocol, runtime_checkable' '''@runtime_checkable
class MyProtocol(Protocol):
    def meth(self): pass
'''
50000 loops, best of 5: 7.69 usec per loop
$ ./python.exe -m timeit -s 'from typing import Protocol, runtime_checkable' '''class MyProtocol(Protocol):
    def meth(self): pass
'''
50000 loops, best of 5: 7.78 usec per loop

This was on a debug build though and I haven't compared it with versions where Protocol just accessed
`.__annotations__` directly, and it's not a huge difference, so I don't think it's worth calling out the
optimization too much.

A downside of this change is that any errors that happen during the determination of attributes now
happen only the first time isinstance() is called. This seems OK since these errors happen only in
fairly exotic circumstances.

Another downside is that any attributes added after class initialization now get picked up as protocol
members. This came up in the typing test suite due to `@final`, but may cause issues elsewhere too.
@JelleZijlstra
JelleZijlstra requested a review from AlexWaygood as a code owner April 16, 2025 13:50
@bedevere-app bedevere-app Bot added the awaiting core review label Apr 16, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Apr 16, 2025
Avoid eagerly evaluating annotations #132493
Open
@JelleZijlstra
news
781084d
@AlexWaygood

AlexWaygood commented Apr 16, 2025

Copy link
Copy Markdown
Member

If I understand correctly, this will significantly slow down the first isinstance() call against a runtime-checkable protocol, but following isinstance() calls will be unaffected. I think that's a good tradeoff, but it might be worth giving the folks who closely watch the benchmarks a heads-up. It might do "interesting" things to this benchmark: https://github.com/python/pyperformance/blob/main/pyperformance/data-files/benchmarks/bm_typing_runtime_protocols/run_benchmark.py

Since it's a micro-benchmark anyway, we could possibly even adjust that benchmark so that it does an isinstance() call to warm the __protocol_attrs_cache__ cache before the benchmark starts timing things?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra

JelleZijlstra commented Apr 16, 2025

Copy link
Copy Markdown
Member Author

I think it might already do some warmup runs? That makes sense for the specializing interpreter anyway. I'll post on Discord.

AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood

AlexWaygood commented Apr 16, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

It's a shame we can't use functools.cached_property for properties on metaclasses :-( it would simplify the caching quite nicely

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra

JelleZijlstra commented Apr 16, 2025

Copy link
Copy Markdown
Member Author

Michael confirmed that it does a warmup run by default.

AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood
AlexWaygood requested changes Apr 16, 2025
View reviewed changes
Comment thread Lib/typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added awaiting changes and removed awaiting core review labels Apr 16, 2025
@bedevere-app

bedevere-app Bot commented Apr 16, 2025

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra
fix with child classes
da69f9d
@JelleZijlstra

JelleZijlstra commented Apr 16, 2025

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added awaiting change review and removed awaiting changes labels Apr 16, 2025
@bedevere-app

bedevere-app Bot commented Apr 16, 2025

Copy link
Copy Markdown

Thanks for making the requested changes!

@AlexWaygood: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app
bedevere-app Bot requested a review from AlexWaygood April 16, 2025 15:27
AlexWaygood
AlexWaygood reviewed Apr 16, 2025
View reviewed changes

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this looks pretty good

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/test/test_typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra
review
f10143f
@JelleZijlstra JelleZijlstra mentioned this pull request Apr 16, 2025
Non-runtime_checkable subclass of a runtime_checkable Protocol allows isinstance() #132604
Closed
@JelleZijlstra

JelleZijlstra commented Apr 16, 2025

Copy link
Copy Markdown
Member Author

I realize this also impacts the note in the docs (https://docs.python.org/3.14/library/typing.html#typing.runtime_checkable) "The members of a runtime-checkable protocol are now considered “frozen” at runtime as soon as the class has been created."

This no longer quite holds; they are now frozen as of the first isinstance() check. Not a great compatibility story; I may want to think more about how to handle this.

AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood
AlexWaygood approved these changes Apr 16, 2025
View reviewed changes

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change does make sense to me. I agree that the "Protocol classes are now frozen at class-creation time" note in the docs no longer holds true, but I think the message to users from that note is "don't monkeypatch attributes onto Protocol classes". If users are following that advice, I don't think they would be broken by this change...?

We might be able to just edit the existing "Changed in Python 3.12" note to make it a bit more vague about when protocols are frozen, rather than adding a "Changed in Python 3.14" note? Not sure.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting change review labels Apr 16, 2025
@JelleZijlstra

JelleZijlstra commented Apr 16, 2025

Copy link
Copy Markdown
Member Author

An alternative that keeps us from importing annotationlib too early is to do something like

try:
    annotations = cls.__annotations__
except Exception:
    annotations = get_annotations(cls, format=Format.FORWARDREF)

So we only import annotationlib in the case where we actually need deferred annotations. I think I prefer that for compatibility.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood

AlexWaygood commented Apr 16, 2025

Copy link
Copy Markdown
Member

Ah that sounds like an appealing alternative. Let's keep the new regression test you added as well, though!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

StanFromIreland
StanFromIreland reviewed Apr 16, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2025-04-16-06-50-13.gh-issue-132493.XvnL7t.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra JelleZijlstra mentioned this pull request Apr 16, 2025
gh-118761: Add helper to ensure that lazy imports are actually lazy #132614
Merged
JelleZijlstra added 2 commits April 16, 2025 13:55
@JelleZijlstra
Merge remote-tracking branch 'upstream/main' into lazyproto
30c2a3c
@JelleZijlstra
Alternative approach
61cf996
@JelleZijlstra JelleZijlstra added the skip news label Apr 16, 2025
@JelleZijlstra
JelleZijlstra requested a review from AlexWaygood April 16, 2025 21:11
AlexWaygood
AlexWaygood reviewed Apr 16, 2025
View reviewed changes
Comment thread Lib/test/test_typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood

AlexWaygood commented Apr 16, 2025

Copy link
Copy Markdown
Member

probably need to change the PR title ;)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra JelleZijlstra changed the title gh-132493: Lazily determine protocol attributes gh-132493: Avoid eager import of annotationlib in typing (again) Apr 16, 2025
@JelleZijlstra @AlexWaygood
Apply suggestions from code review
c4f2709
Co-authored-by: Alex Waygood <[email protected]>
JelleZijlstra
JelleZijlstra commented Apr 16, 2025
View reviewed changes
Comment thread Lib/typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra
Update Lib/typing.py
493c815
AlexWaygood
AlexWaygood approved these changes Apr 17, 2025
View reviewed changes
JelleZijlstra added 2 commits April 17, 2025 08:33
@JelleZijlstra
Merge branch 'main' into lazyproto
22ba75a
@JelleZijlstra
annotationlib is now lazily imported again
f3ed4fb
@JelleZijlstra
JelleZijlstra enabled auto-merge (squash) April 17, 2025 15:41
@JelleZijlstra
JelleZijlstra merged commit 5707837 into python:main Apr 17, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Apr 17, 2025
@bedevere-bot

bedevere-bot commented Apr 17, 2025

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot aarch64 Fedora Stable Clang 3.x (tier-2) has failed when building commit 5707837.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/234/builds/7415) and take a look at the build logs.
  4. Check if the failure is related to this commit (5707837) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/234/builds/7415

Failed tests:

  • test_interpreters

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line 1079, in _bootstrap_inner
    self._context.run(self.run)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line 1021, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/test_interpreters/test_stress.py", line 30, in task
    interp = interpreters.create()
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/support/interpreters/__init__.py", line 76, in create
    id = _interpreters.create(reqrefs=True)
interpreters.InterpreterError: interpreter creation failed
k


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line 1079, in _bootstrap_inner
    self._context.run(self.run)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line 1021, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/test_interpreters/test_stress.py", line 47, in run
    interp = interpreters.create()
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/support/interpreters/__init__.py", line 76, in create
    id = _interpreters.create(reqrefs=True)
interpreters.InterpreterError: interpreter creation failed
k
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot

bedevere-bot commented Apr 17, 2025

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Windows11 Bigmem 3.x (tier-1) has failed when building commit 5707837.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1079/builds/6274) and take a look at the build logs.
  4. Check if the failure is related to this commit (5707837) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1079/builds/6274

Summary of the results of the build (if available):

Click to see traceback logs
remote: Enumerating objects: 6, done.        
remote: Counting objects:  20% (1/5)        
remote: Counting objects:  40% (2/5)        
remote: Counting objects:  60% (3/5)        
remote: Counting objects:  80% (4/5)        
remote: Counting objects: 100% (5/5)        
remote: Counting objects: 100% (5/5), done.        
remote: Compressing objects:  20% (1/5)        
remote: Compressing objects:  40% (2/5)        
remote: Compressing objects:  60% (3/5)        
remote: Compressing objects:  80% (4/5)        
remote: Compressing objects: 100% (5/5)        
remote: Compressing objects: 100% (5/5), done.        
remote: Total 6 (delta 0), reused 0 (delta 0), pack-reused 1 (from 1)        
From https://github.com/python/cpython
 * branch                    main       -> FETCH_HEAD
Note: switching to '5707837049e8eade92e29f20f055112821b75cd0'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 5707837049e gh-132493: Avoid eager import of annotationlib in typing (again) (#132596)
Switched to and reset branch 'main'

Traceback (most recent call last):
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Tools\buildbot\..\..\PCbuild\get_external.py", line 77, in <module>
    main()
    ~~~~^^
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Tools\buildbot\..\..\PCbuild\get_external.py", line 50, in main
    zip_path = fetch_zip(
        args.tag,
    ...<3 lines>...
        verbose=args.verbose,
    )
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Tools\buildbot\..\..\PCbuild\get_external.py", line 19, in fetch_zip
    filename, headers = urlretrieve(
                        ~~~~~~~~~~~^
        url,
        ^^^^
        zip_dir / f'{commit_hash}.zip',
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        reporthook=reporthook,
        ^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\externals\pythonx86\tools\Lib\urllib\request.py", line 242, in urlretrieve
    while block := fp.read(bs):
                   ~~~~~~~^^^^
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\externals\pythonx86\tools\Lib\http\client.py", line 479, in read
    s = self.fp.read(amt)
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\externals\pythonx86\tools\Lib\socket.py", line 719, in readinto
    return self._sock.recv_into(b)
           ~~~~~~~~~~~~~~~~~~~~^^^
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\externals\pythonx86\tools\Lib\ssl.py", line 1304, in recv_into
    return self.read(nbytes, buffer)
           ~~~~~~~~~^^^^^^^^^^^^^^^^
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\externals\pythonx86\tools\Lib\ssl.py", line 1138, in read
    return self._sslobj.read(len, buffer)
           ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
ConnectionResetError: [WinError 10054] An existing connection was forcibly closed by the remote host

Could Not Find R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\*.pyc
The system cannot find the file specified.
Could Not Find R:\buildarea\3.x.ambv-bb-win11.bigmem\build\PCbuild\python*.zip
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra
JelleZijlstra deleted the lazyproto branch April 17, 2025 16:42
@JelleZijlstra

JelleZijlstra commented Apr 17, 2025

Copy link
Copy Markdown
Member Author

I don't think that's this PR

AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StanFromIreland StanFromIreland StanFromIreland left review comments
@AlexWaygood AlexWaygood AlexWaygood approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@JelleZijlstra @AlexWaygood @bedevere-bot @StanFromIreland
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.