Skip to content

Commit fc0af8f

Browse files
eendebakptJosh Rosenbergclaude
committed
gh-86199: Avoid an unnecessary dict copy for f(**kwargs) calls
Since Python 3.9 a call with a lone ** unpacking compiled to BUILD_MAP 0 + DICT_MERGE 1 + CALL_FUNCTION_EX, copying the kwargs dict on every call. For vectorcall callees (all Python functions and most builtins) that copy is wasted work: the dict is immediately unpacked onto a flat argument vector. The compiler now pushes the ** operand as-is. CALL_FUNCTION_EX converts a non-exact mapping to a dict itself (reusing the DICT_MERGE machinery, so error messages are unchanged), and PyObject_Call copies the dict only on the tp_call path, where the callee would otherwise receive the caller's dict directly -- so a callee still can never mutate the caller's kwargs, and the documented PyObject_Call/callable(*args, **kwargs) equivalence now holds for the C API too (gh-86795). _PyStack_UnpackDict takes a critical section while copying the dict's items out, retrying if the dict was resized in between, which makes unpacking a shared dict safe on free-threaded builds. f(**d) with a small dict is ~1.4x faster; calls without ** unpacking are unaffected. Co-authored-by: Josh Rosenberg <1178095+MojoVampire@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ee521e8 commit fc0af8f

9 files changed

Lines changed: 538 additions & 23 deletions

File tree

Lib/test/test_call.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,30 @@ def test_kwargs_order_preserved_in_c_functions(self):
9595
self.assertEqual(list(collections.OrderedDict(b=1, a=2, c=3)),
9696
['b', 'a', 'c'])
9797

98+
def test_kwargs_unpacking_mutation_isolated(self):
99+
# gh-86199: a callee mutating its **kwargs must never affect the
100+
# caller's dict.
101+
def fn(**kw):
102+
kw['injected'] = None
103+
d = {'a': 1}
104+
fn(**d)
105+
self.assertEqual(d, {'a': 1})
106+
e = {}
107+
fn(**e)
108+
self.assertEqual(e, {})
109+
110+
@cpython_only
111+
@unittest.skipIf(_testcapi is None, "requires _testcapi")
112+
def test_kwargs_unpacking_not_aliased_in_tp_call(self):
113+
# gh-86199: a tp_call callee receives a private copy of the kwargs
114+
# dict, never the caller's dict.
115+
get_kwargs = _testcapi.get_kwargs # METH_VARARGS | METH_KEYWORDS
116+
d = {'a': 1}
117+
self.assertIsNot(get_kwargs(**d), d)
118+
self.assertEqual(get_kwargs(**d), d)
119+
e = {}
120+
self.assertIsNot(get_kwargs(**e), e)
121+
98122
def test_frames_are_popped_after_failed_calls(self):
99123
# GH-93252: stuff blows up if we don't pop the new frame after
100124
# recovering from failed calls:
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import unittest
2+
from unittest import TestCase
3+
4+
from test.support import threading_helper, import_helper
5+
6+
_testcapi = import_helper.import_module("_testcapi")
7+
8+
threading_helper.requires_working_threading(module=True)
9+
10+
11+
class TestKwargsUnpackRace(TestCase):
12+
def test_mutate_kwargs_during_unpack(self):
13+
# gh-86199: unpacking a shared kwargs dict must tolerate another
14+
# thread resizing it.
15+
num_mutators, num_callers = 2, 6
16+
iters = 1000
17+
min_keys, max_keys = 4, 3000
18+
19+
fastcalldict = _testcapi.pyobject_fastcalldict
20+
21+
def target(**kwargs):
22+
return len(kwargs)
23+
24+
shared = {f"k{i}": i for i in range(min_keys)}
25+
26+
def resize_kwargs():
27+
for _ in range(iters):
28+
for i in range(min_keys, max_keys):
29+
shared[f"k{i}"] = i
30+
for i in range(max_keys - 1, min_keys - 1, -1):
31+
shared.pop(f"k{i}", None)
32+
33+
def call_target():
34+
for _ in range(iters):
35+
try:
36+
fastcalldict(target, (), shared)
37+
except Exception:
38+
pass
39+
40+
threading_helper.run_concurrently(
41+
[resize_kwargs] * num_mutators + [call_target] * num_callers)
42+
43+
44+
if __name__ == "__main__":
45+
unittest.main()
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Speed up calls of the form ``f(**kwargs)`` by no longer always copying the
2+
keyword arguments before the call; such calls are now up to 1.4x faster.
3+
This also fixes :c:func:`PyObject_Call` to copy the keyword arguments before
4+
passing them to a callee that would otherwise receive the caller's dict, so a
5+
called object can no longer mutate the caller's dict (:gh:`86795`), and makes
6+
unpacking a shared keyword-arguments dict safe on the free-threaded build.

Modules/_testinternalcapi/test_cases.c.h

Lines changed: 172 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)