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-100239: specialize dict subclasses with no getitem override by iritkatriel · Pull Request #132383 · python/cpython

Conversation

@iritkatriel

@iritkatriel iritkatriel commented Apr 11, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Perf stats show the specialisation numbers improve, but perf is overall the same.

https://github.com/faster-cpython/benchmarking-public/blob/main/results/bm-20250410-3.14.0a6%2B-bf2d1dd/README.md

  • Issue: Specialize long tail of binary operations using a table. #100239

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
iritkatriel added 3 commits April 10, 2025 19:21
@iritkatriel
pythongh-100239: spcialize dict subclasses with no getitem override
bf2d1dd
@iritkatriel
revert typeobject.c
a4997f8
@iritkatriel
Merge remote-tracking branch 'upstream/main' into dicts
7ea58ab
@iritkatriel
iritkatriel requested a review from markshannon as a code owner April 11, 2025 07:42
@bedevere-app bedevere-app Bot added the awaiting core review label Apr 11, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Apr 11, 2025
Specialize long tail of binary operations using a table. #100239
Open
@iritkatriel iritkatriel added the skip news label Apr 11, 2025
@iritkatriel iritkatriel changed the title gh-100239: spcialize dict subclasses with no getitem override gh-100239: specialize dict subclasses with no getitem override Apr 11, 2025
@iritkatriel

iritkatriel commented Apr 11, 2025

Copy link
Copy Markdown
Member Author

The tests will be great again once we merge the fix for #132284.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303
kumaraditya303 reviewed Apr 12, 2025
View reviewed changes
Comment thread Python/bytecodes.c Outdated
PyObject *dict = PyStackRef_AsPyObjectBorrow(dict_st);

assert(PyDict_CheckExact(dict));
assert(PyDict_Check(dict));

@kumaraditya303 kumaraditya303 Apr 12, 2025

Copy link
Copy Markdown
Contributor

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.

I think you also need to change _GUARD_NOS_DICT to PyDict_Check instead of PyDict_CheckExact for this to work but I am not sure.

Sorry, something went wrong.

Uh oh!

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

@iritkatriel iritkatriel Apr 12, 2025

Copy link
Copy Markdown
Member Author

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.

It does look like it from the code, but I don't see more misses.

image

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@iritkatriel
relax the guard
182858e
@iritkatriel

iritkatriel commented Apr 12, 2025

Copy link
Copy Markdown
Member Author

Perf with the fix to the guard: https://github.com/faster-cpython/benchmarking-public/blob/main/results/bm-20250412-3.14.0a7%2B-3d9cf36/README.md
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
markshannon
markshannon reviewed Apr 14, 2025
View reviewed changes

@markshannon markshannon 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.

Is it worth making dict_subscript non-static to avoid the indirection PyDict_Type.tp_as_mapping->mp_subscript?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel
apply review comments
d5dc19b
@iritkatriel
iritkatriel requested a review from methane as a code owner April 14, 2025 15:32
methane
methane approved these changes Apr 15, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Apr 15, 2025
markshannon
markshannon reviewed Apr 15, 2025
View reviewed changes

@markshannon markshannon 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.

It looks like the test_opcode test needs updating.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel
apply review
417e761
@kumaraditya303 kumaraditya303 mentioned this pull request Apr 5, 2026
gh-143732: allow dict subclasses to be specialized #148128
Merged
@github-actions

github-actions Bot commented Apr 23, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 23, 2026
@kumaraditya303

kumaraditya303 commented May 29, 2026

Copy link
Copy Markdown
Contributor

Superseded by #148128
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kumaraditya303 kumaraditya303 closed this May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon markshannon left review comments
@kumaraditya303 kumaraditya303 kumaraditya303 left review comments
@methane methane methane approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

awaiting merge skip news stale Stale PR or inactive for long period of time.

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

@iritkatriel @kumaraditya303 @methane @markshannon
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.