Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-132493: Avoid eager import of annotationlib in typing (again)#132596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Conversation
JelleZijlstra commented Apr 16, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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.
AlexWaygood commented Apr 16, 2025
If I understand correctly, this will significantly slow down the first Since it's a micro-benchmark anyway, we could possibly even adjust that benchmark so that it does an |
JelleZijlstra commented Apr 16, 2025
I think it might already do some warmup runs? That makes sense for the specializing interpreter anyway. I'll post on Discord. |
AlexWaygood commented Apr 16, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
It's a shame we can't use |
JelleZijlstra commented Apr 16, 2025
Michael confirmed that it does a warmup run by default. |
Uh oh!
There was an error while loading. Please reload this page.
When you're done making the requested changes, leave the comment: |
JelleZijlstra commented Apr 16, 2025
I have made the requested changes; please review again |
Thanks for making the requested changes! @AlexWaygood: please review the changes made to this pull request. |
AlexWaygood left a comment
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
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
JelleZijlstra commented Apr 16, 2025
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 left a comment
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.
JelleZijlstra commented Apr 16, 2025
An alternative that keeps us from importing annotationlib too early is to do something like So we only import annotationlib in the case where we actually need deferred annotations. I think I prefer that for compatibility. |
AlexWaygood commented Apr 16, 2025
Ah that sounds like an appealing alternative. Let's keep the new regression test you added as well, though! |
Misc/NEWS.d/next/Library/2025-04-16-06-50-13.gh-issue-132493.XvnL7t.rst Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
AlexWaygood commented Apr 16, 2025
probably need to change the PR title ;) |
Co-authored-by: Alex Waygood <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.
5707837 into python:mainUh oh!
There was an error while loading. Please reload this page.
bedevere-bot commented Apr 17, 2025
|
bedevere-bot commented Apr 17, 2025
|
JelleZijlstra commented Apr 17, 2025
I don't think that's this PR |
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.