FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-141510: Update specializer to support frozendict with BINARY_SUBSCR by corona10 · Pull Request #144911 · python/cpython · GitHub

/ cpython Public

gh-141510: Update specializer to support frozendict with BINARY_SUBSCR - #144911

Closed
corona10 wants to merge 13 commits into
python:mainfrom
corona10:gh-141510-specializer
ConversationCommits (13)ChecksFiles changed
Closed

gh-141510: Update specializer to support frozendict with BINARY_SUBSCR#144911
corona10 wants to merge 13 commits into
python:mainfrom
corona10:gh-141510-specializer

Conversation

corona10 commented Feb 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

We need to register frozendict for support in the specializer.

Since frozendict is only introduced in Python 3.15, it is not yet widely used. However, it is thread safe for many operations, including CALL_LEN and BINARY_OP_SUBSCR. It could also be optimized more aggressively in the JIT using KNOWN_CLASS optimizations in the pipeline.

This is just part of a draft PR. I am also happy to make _BINARY_OP_SUBSCR_FROZEN_DICT part of _BINARY_OP_SUBSCR_DICT, depending on what the JIT team prefers.

Without this PR, frozendict would be at a performance disadvantage, even though it is intended to be a first-class citizen alongside dict.

bedevere-app Bot mentioned this pull request Feb 17, 2026
Closed
corona10 changed the title gh-141510: Update specialzier to support frozendict with BINARY_SUBSCR gh-141510: Update specializer to support frozendict with BINARY_SUBSCR Feb 17, 2026

Sacul0457 commented Feb 17, 2026

Copy link
Copy Markdown
Contributor

Would it be good if _GUARD_NOS_FROZEN_DICT was added to optimizer_bytecodes.c so that it could be removed by the optimizer if the type is known?

Fidget-Spinner commented Feb 17, 2026

Copy link
Copy Markdown
Member

I think it's better to focus on getting the specializer correct in this PR for now. We can leave the JIT for another PR.

corona10 commented Feb 17, 2026

Copy link
Copy Markdown
Member Author

@Sacul0457 I think that we have to, I will update the PR or handle it at the seperate PR, but it needs feedback from JIT team first :)

vstinner reviewed Feb 17, 2026
View reviewed changes
Comment thread Objects/dictobject.c Outdated
Comment thread Include/internal/pycore_dict.h

Fidget-Spinner commented Feb 17, 2026

Copy link
Copy Markdown
Member

This needs tests in test_opcache for the specialization success and failure case.

corona10 commented Feb 17, 2026

Copy link
Copy Markdown
Member Author

Once the PR is merged, I am also going to work on CONTAINS_OP_FROZENDICT too.

corona10 requested a review from vstinner February 17, 2026 14:53
vstinner reviewed Feb 17, 2026
View reviewed changes
Comment thread Include/internal/pycore_dict.h Outdated
Comment thread Include/internal/pycore_dict.h
Update Include/internal/pycore_dict.h
4c7b59a 
Co-authored-by: Victor Stinner <vstinner@python.org>
corona10 requested a review from vstinner February 17, 2026 15:31

corona10 commented Feb 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

@Fidget-Spinner Now whole unit tests are passed.

vstinner reviewed Feb 17, 2026
View reviewed changes
Comment thread Objects/dictobject.c Outdated
Comment thread Python/bytecodes.c
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2026-02-17-21-46-55.gh-issue-141510.8SqYuG.rst Outdated
corona10 requested a review from vstinner February 17, 2026 21:28
vstinner approved these changes Feb 17, 2026
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

markshannon commented Feb 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Do you have any evidence that this will improve performance?

Opcodes are a limited resource, and since no one is using frozen dicts yet, this is just going to slow down the interpreter by adding bulk.

Relaxing the guard in BINARY_OP_SUBSCR_DICT to check for any dict, and removing the redundant check in PyDict_GetItemRef would achieve your goal without lots of extra code or any slow down.

corona10 commented Feb 18, 2026

Copy link
Copy Markdown
Member Author

Relaxing the guard in BINARY_OP_SUBSCR_DICT to check for any dict, and removing the redundant check in PyDict_GetItemRef would achieve your goal without lots of extra code or any slow down.

I like that option too.

corona10 mentioned this pull request Feb 18, 2026
Merged

vstinner commented Feb 18, 2026

Copy link
Copy Markdown
Member

@corona10 wrote #144949 to modify existing CONTAINS_OP_DICT and BINARY_OP_SUBSCR_DICT opcodes for frozendict. I close this PR.

vstinner closed this Feb 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

vstinner vstinner approved these changes
markshannon Awaiting requested review from markshannon markshannon is a code owner
methane Awaiting requested review from methane methane is a code owner
Fidget-Spinner Awaiting requested review from Fidget-Spinner

Assignees

No one assigned

Labels

awaiting merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL