Fix issues gh-86199 gh-86795 by MojoVampire · Pull Request #92192 · python/cpython

MojoVampire · 2022-05-02T20:49:22Z

Fix issues #86199 and #86795 by centralizing copying of keyword arguments in `PyObject_Call` only when needed

gh-86199: `**kwargs` are not copied by the compiler when they are the only source of keyword arguments for a call. -

gh-86795: `PyObject_Call` makes copies of the keyword arguments dictionary when calling non-vectorcall functions to consistently ensure the caller's `dict` is not modified, whether the `dict` comes from the eval loop or a direct C extension call to `PyObject_Call`, matching the documented equivalence with `callable(*args, **kwargs)`. Reduces overhead of `callable(**kwargs)` ~30%.

MojoVampire · 2022-05-02T20:55:54Z

Hmm... I've actually signed the CLA before under the old system, but it looks like the old private GitHub e-mail I was using doesn't work anymore (didn't realize this when I committed with it still configured in my .gitconfig). Not sure how to fix this retroactively. Not sure which of the several valid e-mails I now have should be the one attached to the contributor agreement for that matter.

Figured it out, managed to reset the authorship and repush so it's now credited to my new private GitHub e-mail that CLA-bot recognizes, so everything should be good now.

Preventing compiler from inserting unnecessary dict build and merge when only **kwargs passed Make PyObject_Call copy any incoming keyword argument dicts when the callable in question is not vectorcall to avoid possibility of callee modifying caller's dict Add tests that verify documented equivalence between callable(**kwargs) and PyObject_Call(callable, (), kwargs)

erlend-aasland

Looks good, apart from one more reST nit.

Also: @mentioning people in commit messages has historically led to an awful lot of unneeded pinging from forks, other PR's, etc. I don't know if GitHub addressed this problem¹, but I find it best to stay on the cautious side, and just not do that; reducing the number of pings is a welcome consideration 😉

UPDATE: confirming that I actually got an extra ping because of the commit message ↩

…e-86199.IZbF0m.rst Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>

MojoVampire · 2022-05-04T03:40:11Z

Looks good, apart from one more reST nit.

Accepted as given.

Also: @mentioning people in commit messages has historically led to an awful lot of unneeded pinging from forks, other PR's, etc. I don't know if GitHub addressed this problem1, but I find it best to stay on the cautious side, and just not do that; reducing the number of pings is a welcome consideration 😉

Oops, did not know that would happen. Will avoid in the future.

…ut dict is plain dict with no history of deletions (the common case)

methane

LGTM

erlend-aasland · 2022-05-08T20:58:47Z

Please resolve conflicts.

Also, please fix the PR title to gh-86795: <insert short summary of the actual change here>. You can describe the change in more detail (and mention gh-86199) in the PR body; this will make it easier for the core dev who merges this to create a sensible merge commit message.

markshannon · 2022-06-10T15:37:57Z

@MojoVampire?

erlend-aasland · 2022-09-04T22:05:57Z

@MojoVampire, are you planning to follow up this PR? If not, I suggest closing it.

MojoVampire · 2022-11-09T22:54:28Z

Sorry, I've had a hell of a year. I'm going to try to rebase this soon.

python-cla-bot · 2025-04-18T09:45:48Z

The following commit authors need to sign the Contributor License Agreement:

26495692+MojoVampire@users.noreply.github.com

$CLA signed$

github-actions · 2026-04-10T18:16:25Z

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

methane · 2026-04-11T08:44:15Z

is this issue still exist?

StanFromIreland · 2026-05-03T17:05:37Z

Hello! I'm sorry but I'm closing this PR as it is stale and has requested changes that have not been addressed in quite some time.

Instead of a new _MAKE_KWARGS_A_DICT uop and an interpreter-side no-alias helper, follow the structure of the original pythonGH-92192: - The mapping-to-exact-dict conversion is folded into the existing _MAKE_CALLARGS_A_TUPLE op (no new uop, no macro changes). - The callee-cannot-mutate-caller-kwargs guarantee moves into _PyObject_Call: the tp_call branch now receives a copy of the kwargs dict (empty dicts are passed as NULL), making the documented equivalence with callable(*args, **kwargs) hold for all C API callers as well (pythongh-86795). - On free-threaded builds the interpreter simply always copies the kwargs dict (status quo semantics); the zero-copy optimization only applies to default builds for now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

$@MojoVampire$

MojoVampire requested a review from markshannon as a code owner May 2, 2022 20:49

$@bedevere-bot$ bedevere-bot added the awaiting review label May 2, 2022

MojoVampire added 2 commits May 2, 2022 20:59

$@MojoVampire$

MojoVampire force-pushed the fix-issues-86199-86795 branch from 4a8517f to b205d6b Compare May 2, 2022 21:01

$@MojoVampire$ MojoVampire changed the title ~~Fix issues 86199 86795~~ Fix issues #86199 #86795 May 2, 2022

$@MojoVampire$ MojoVampire changed the title ~~Fix issues #86199 #86795~~ Fix issues gh-86199 gh-86795 May 2, 2022

$markshannon$

markshannon reviewed May 2, 2022

View reviewed changes

Comment thread Objects/call.c

$erlend-aasland$

erlend-aasland reviewed May 3, 2022

View reviewed changes

Comment thread Objects/call.c Outdated

Comment thread Misc/NEWS.d/next/Core and Builtins/2022-05-02-20-19-06.gh-issue-86199.IZbF0m.rst Outdated

Comment thread Objects/call.c Outdated

Comment thread Objects/call.c Outdated

MojoVampire added 2 commits May 3, 2022 16:16

$@MojoVampire$

Address feedback from @markshannon and @erlend-aasland

565912a

$@MojoVampire$

Use unprefixed name for non-exported functions

cd150b8

$@MojoVampire$

MojoVampire requested review from erlend-aasland and markshannon May 3, 2022 16:20

$erlend-aasland$

erlend-aasland reviewed May 3, 2022

View reviewed changes

Comment thread Misc/NEWS.d/next/Core and Builtins/2022-05-02-20-19-06.gh-issue-86199.IZbF0m.rst Outdated

$methane$

methane reviewed May 4, 2022

View reviewed changes

Comment thread Objects/call.c

$@MojoVampire$ $@erlend-aasland$

Update Misc/NEWS.d/next/Core and Builtins/2022-05-02-20-19-06.gh-issu…

4b20dae

…e-86199.IZbF0m.rst Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>

$@MojoVampire$

MojoVampire requested review from erlend-aasland and methane May 4, 2022 03:40

$@MojoVampire$

Fast path for completely empty dict is faster than presizing when inp…

19db1da

…ut dict is plain dict with no history of deletions (the common case)

$methane$

methane approved these changes May 4, 2022

View reviewed changes

$@bedevere-bot$ bedevere-bot added awaiting merge and removed awaiting review labels May 4, 2022

$@markshannon$ markshannon added awaiting changes and removed awaiting merge labels May 23, 2022

$@erlend-aasland$ erlend-aasland added the pending The issue will be closed if no feedback is provided label Sep 4, 2022

$@erlend-aasland$ erlend-aasland removed the pending The issue will be closed if no feedback is provided label Nov 11, 2022

$@python$ python deleted a comment Apr 7, 2025

$@github-actions$ github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 10, 2026

$@StanFromIreland$ StanFromIreland closed this May 3, 2026

$@eendebakpt$ eendebakpt mentioned this pull request Aug 25, 2026

gh-86199: Avoid an unnecessary dict copy for f(**kwargs) calls #156384

Open

Uh oh!

Uh oh!

Conversation

MojoVampire commented May 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix issues #86199 and #86795 by centralizing copying of keyword arguments in PyObject_Call only when needed

Uh oh!

MojoVampire commented May 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erlend-aasland left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

Uh oh!

Uh oh!

MojoVampire commented May 4, 2022

Uh oh!

methane left a comment

Choose a reason for hiding this comment

Uh oh!

erlend-aasland commented May 8, 2022

Uh oh!

markshannon commented Jun 10, 2022

Uh oh!

erlend-aasland commented Sep 4, 2022

Uh oh!

MojoVampire commented Nov 9, 2022

Uh oh!

python-cla-bot Bot commented Apr 18, 2025

Uh oh!

github-actions Bot commented Apr 10, 2026

Uh oh!

methane commented Apr 11, 2026

Uh oh!

StanFromIreland commented May 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

$@MojoVampire$ MojoVampire commented May 2, 2022 •

edited

Loading

Fix issues #86199 and #86795 by centralizing copying of keyword arguments in `PyObject_Call` only when needed

MojoVampire commented May 2, 2022 •

edited

Loading

$@erlend-aasland$ erlend-aasland left a comment •

edited

Loading

$@methane$ methane left a comment