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

gh-143732: allow dict subclasses to be specialized by kumaraditya303 · Pull Request #148128 · python/cpython · GitHub

/ cpython Public

gh-143732: allow dict subclasses to be specialized - #148128

Merged
markshannon merged 23 commits into
python:mainfrom
kumaraditya303:jit-dict-opt
May 4, 2026
Merged

gh-143732: allow dict subclasses to be specialized #148128
markshannon merged 23 commits into
python:mainfrom
kumaraditya303:jit-dict-opt

Conversation

kumaraditya303 commented Apr 5, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

This is revival of #132383 which allowed specialization for dict subclasses which do not override __getitem__. This PR also allows specialization for dict subclasses which do not override __setitem__.

This is primarily targeted for defaultdict which is the most performance critical subclass of dict.

kumaraditya303 marked this pull request as ready for review April 5, 2026 18:39
kumaraditya303 added the performance Performance or resource usage label Apr 5, 2026

Copy link
Copy Markdown
Contributor Author

@markshannon Can you take a look at this? I'd like get this in time for the beta release.

markshannon 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

Looks good overall.
The old uops need to be removed and some assertions may no longer be valid.

Also, take care with recorded types. Only strengthen type information on proven or guards, not recorded information.

Comment thread Python/bytecodes.c
Comment thread Python/bytecodes.c
Comment thread Python/bytecodes.c Outdated
Comment thread Python/bytecodes.c Outdated
Comment thread Python/bytecodes.c Outdated
Comment thread Python/bytecodes.c Outdated

Copy link
Copy Markdown
Member

GitHub doen't put comments in order, it seems. The "above" in the comments refers to earlier lines, not earlier comments.

Copy link
Copy Markdown
Contributor Author

I still think the assert(PyDict_Check(dict)); asserts should be removed.

I removed the assertions now and added watchers for all cases.

I have made the requested changes; please review again

bedevere-app Bot commented Apr 30, 2026

Copy link
Copy Markdown

Thanks for making the requested changes!

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

bedevere-app Bot requested a review from markshannon April 30, 2026 17:44

read-the-docs-community Bot commented Apr 30, 2026
edited
Loading

Copy link
Copy Markdown

markshannon 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

I still think we need more watchers. Sorry, I should have spotted that in the last review.

bedevere-app Bot commented May 1, 2026

Copy link
Copy Markdown

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

And if you don't make the requested changes, you will be poked with soft cushions!

kumaraditya303 requested a review from markshannon May 1, 2026 11:42

Copy link
Copy Markdown
Contributor Author

Looks like the CI is affected by the Ubuntu outage today.

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

bedevere-app Bot commented May 3, 2026

Copy link
Copy Markdown

Thanks for making the requested changes!

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

markshannon 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

This looks good now. Thanks.

markshannon merged commit 5847931 into python:main May 4, 2026
79 of 82 checks passed

Copy link
Copy Markdown

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

Hi! The buildbot AMD64 Debian root 3.x (tier-1) has failed when building commit 5847931.

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/345/builds/14120) and take a look at the build logs.
  4. Check if the failure is related to this commit (5847931) 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/345/builds/14120

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

==

Click to see traceback logs
Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/__init__.py", line 948, in gc_collect
    gc.collect()
    ~~~~~~~~~~^^
ResourceWarning: unclosed file <_io.FileIO name=11 mode='wb' closefd=True>

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance or resource usage skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL