Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
JelleZijlstra merged 10 commits intopython:mainfromJelleZijlstra:lazyproto
Apr 17, 2025

Conversation

JelleZijlstra
Copy link
Member

@JelleZijlstraJelleZijlstra commentedApr 16, 2025•
edited
Loading

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 firstisinstance() 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.

pythongh-132494 made typing.py eagerly import annotationlib again becausetyping contains several protocols. Avoid this by determining annotationslazily. This should also make protocol creation faster:Unpatched:$ ./python.exe -m timeit -s 'from typing import Protocol, runtime_checkable' '''@runtime_checkableclass 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 loopPatched:$ ./python.exe -m timeit -s 'from typing import Protocol, runtime_checkable' '''@runtime_checkableclass 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 loopThis 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 theoptimization too much.A downside of this change is that any errors that happen during the determination of attributes nowhappen only the first time isinstance() is called. This seems OK since these errors happen only infairly exotic circumstances.Another downside is that any attributes added after class initialization now get picked up as protocolmembers. This came up in the typing test suite due to `@final`, but may cause issues elsewhere too.
@AlexWaygood
Copy link
Member

If I understand correctly, this will significantly slow down thefirstisinstance() call against a runtime-checkable protocol, but followingisinstance() 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 anisinstance() call to warm the__protocol_attrs_cache__ cache before the benchmark starts timing things?

@JelleZijlstra
Copy link
MemberAuthor

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

@AlexWaygood
Copy link
Member

AlexWaygood commentedApr 16, 2025•
edited
Loading

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

@JelleZijlstra
Copy link
MemberAuthor

Michael confirmed that it does a warmup run by default.

AlexWaygood reacted with thumbs up emoji

@bedevere-app
Copy link

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

@JelleZijlstra
Copy link
MemberAuthor

I have made the requested changes; please review again

@bedevere-app
Copy link

Thanks for making the requested changes!

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

Copy link
Member

@AlexWaygoodAlexWaygood left a 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

@JelleZijlstra
Copy link
MemberAuthor

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

Copy link
Member

@AlexWaygoodAlexWaygood left a 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 ontoProtocol classes". If users are following that advice, I don'tthink 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
Copy link
MemberAuthor

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.

@AlexWaygood
Copy link
Member

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

@AlexWaygood
Copy link
Member

probably need to change the PR title ;)

@JelleZijlstraJelleZijlstra changed the titlegh-132493: Lazily determine protocol attributesgh-132493: Avoid eager import of annotationlib in typing (again)Apr 16, 2025
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@JelleZijlstraJelleZijlstraenabled auto-merge (squash)April 17, 2025 15:41
@JelleZijlstraJelleZijlstra merged commit5707837 intopython:mainApr 17, 2025
38 checks passed
@bedevere-bot
Copy link

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

Hi! The buildbotaarch64 Fedora Stable Clang 3.x (tier-2) has failed when building commit5707837.

What do you need to do:

  1. Don't panic.
  2. Checkthe 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", line1079, in_bootstrap_innerself._context.run(self.run)~~~~~~~~~~~~~~~~~^^^^^^^^^^  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line1021, inrunself._target(*self._args,**self._kwargs)~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/test_interpreters/test_stress.py", line30, intask    interp= interpreters.create()  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/support/interpreters/__init__.py", line76, increateid= _interpreters.create(reqrefs=True)interpreters.InterpreterError:interpreter creation failedkTraceback (most recent call last):  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line1079, in_bootstrap_innerself._context.run(self.run)~~~~~~~~~~~~~~~~~^^^^^^^^^^  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line1021, inrunself._target(*self._args,**self._kwargs)~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/test_interpreters/test_stress.py", line47, inrun    interp= interpreters.create()  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/support/interpreters/__init__.py", line76, increateid= _interpreters.create(reqrefs=True)interpreters.InterpreterError:interpreter creation failedk

@bedevere-bot
Copy link

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

Hi! The buildbotAMD64 Windows11 Bigmem 3.x (tier-1) has failed when building commit5707837.

What do you need to do:

  1. Don't panic.
  2. Checkthe 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_HEADNote:switching to '5707837049e8eade92e29f20f055112821b75cd0'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by switching back to a branch.If you want to create a new branch to retain commits you create, you maydo 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 falseHEAD 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", line77, in<module>    main()~~~~^^  File"R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Tools\buildbot\..\..\PCbuild\get_external.py", line50, inmain    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", line19, infetch_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", line242, inurlretrievewhile block:= fp.read(bs):~~~~~~~^^^^  File"R:\buildarea\3.x.ambv-bb-win11.bigmem\build\externals\pythonx86\tools\Lib\http\client.py", line479, inread    s=self.fp.read(amt)  File"R:\buildarea\3.x.ambv-bb-win11.bigmem\build\externals\pythonx86\tools\Lib\socket.py", line719, inreadintoreturnself._sock.recv_into(b)~~~~~~~~~~~~~~~~~~~~^^^  File"R:\buildarea\3.x.ambv-bb-win11.bigmem\build\externals\pythonx86\tools\Lib\ssl.py", line1304, inrecv_intoreturnself.read(nbytes, buffer)~~~~~~~~~^^^^^^^^^^^^^^^^  File"R:\buildarea\3.x.ambv-bb-win11.bigmem\build\externals\pythonx86\tools\Lib\ssl.py", line1138, inreadreturnself._sslobj.read(len, buffer)~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^ConnectionResetError:[WinError 10054] An existing connection was forcibly closed by the remote hostCould Not Find R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\*.pycThe system cannot find the file specified.Could Not Find R:\buildarea\3.x.ambv-bb-win11.bigmem\build\PCbuild\python*.zip

@JelleZijlstraJelleZijlstra deleted the lazyproto branchApril 17, 2025 16:42
@JelleZijlstra
Copy link
MemberAuthor

I don't think that's this PR

AlexWaygood reacted with thumbs up emoji

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

@StanFromIrelandStanFromIrelandStanFromIreland left review comments

@AlexWaygoodAlexWaygoodAlexWaygood approved these changes

Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@JelleZijlstra@AlexWaygood@bedevere-bot@StanFromIreland
[8]ページ先頭
©2009-2025 Movatter.jp