◐ Shell
reader mode source ↗
Skip to content

GH-105848: Replace KW_NAMES + CALL with LOAD_CONST + CALL_KW#109300

Merged
brandtbucher merged 14 commits into
python:mainfrom
brandtbucher:call-kw
Sep 13, 2023
Merged

GH-105848: Replace KW_NAMES + CALL with LOAD_CONST + CALL_KW#109300
brandtbucher merged 14 commits into
python:mainfrom
brandtbucher:call-kw

Conversation

@brandtbucher

@brandtbucher brandtbucher commented Sep 12, 2023 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

Get rid of the magic kwnames side-channel and split out calls with a tuple of keyword names on the stack (CALL_KW) from those with no keyword arguments (the existing CALL family). Stats on an earlier version of this branch suggested that specializing CALL_KW just isn't worth it.

Other cleanup:

The new CALL_KW instruction is mostly copied and lightly modified from the existing CALL instruction. They can probably share code using uops, but that's a project for another day.

📚 Documentation preview 📚: https://cpython-previews--109300.org.readthedocs.build/

@brandtbucher brandtbucher added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Sep 12, 2023
@brandtbucher brandtbucher self-assigned this Sep 12, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Sep 12, 2023
Closed
@brandtbucher brandtbucher added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section labels Sep 12, 2023
@bedevere-bot

bedevere-bot commented Sep 12, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @brandtbucher for commit 53917a5 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot

bedevere-bot commented Sep 12, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @brandtbucher for commit 53917a5 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the Test PR w/ refleak buildbots; report in status section label Sep 12, 2023
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 12, 2023
@brandtbucher

brandtbucher commented Sep 12, 2023

Copy link
Copy Markdown
Member Author

1% faster.

@markshannon

markshannon commented Sep 12, 2023

Copy link
Copy Markdown
Member

Can you add CALL_KW to the list of specializable instructions that we don't specialize for the stats. https://github.com/python/cpython/blob/main/Python/specialize.c#L135

@brettcannon brettcannon removed their request for review September 12, 2023 17:25
gvanrossum
gvanrossum reviewed Sep 12, 2023
View reviewed changes

@gvanrossum gvanrossum left a comment •
edited
Loading

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.

Hide comment

I just love seeing this! Mostly some docs nits.

@brandtbucher

brandtbucher commented Sep 13, 2023

Copy link
Copy Markdown
Member Author

Ignoring the buildbot failures since they're all either failing on main or on other PRs also. Not a very informative run... :(

@brandtbucher

brandtbucher commented Sep 13, 2023

Copy link
Copy Markdown
Member Author

Can you add CALL_KW to the list of specializable instructions that we don't specialize for the stats. https://github.com/python/cpython/blob/main/Python/specialize.c#L135

That's only useful if we're recording failure kinds, right? We're not in this case, so I don't think it would actually add anything other than an empty section.

@brandtbucher brandtbucher merged commit 22e65ee into python:main Sep 13, 2023
@markshannon markshannon mentioned this pull request Oct 24, 2023
Closed
@coderabbitai coderabbitai Bot mentioned this pull request Apr 3, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericsnowcurrently ericsnowcurrently ericsnowcurrently left review comments

@gvanrossum gvanrossum gvanrossum left review comments

@ncoghlan ncoghlan Awaiting requested review from ncoghlan

@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

@markshannon markshannon Awaiting requested review from markshannon

@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner

Assignees

@brandtbucher brandtbucher

Labels

interpreter-core (Objects, Python, Grammar, and Parser dirs)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@brandtbucher @bedevere-bot @markshannon @ericsnowcurrently @gvanrossum