Repository navigation
gh-87613: Pass NULL for an empty **kwargs in Argument Clinic - #161382
Conversation
The parser allocated an empty dict for **kwargs when there were no keyword arguments. Pass NULL instead, like the caller does, and handle NULL in the impls that used the dict unconditionally. All **kwargs impls were audited: _csv and zoneinfo already accept NULL, and the rest (structseq and SimpleNamespace `__replace__`, _interpreters.new_config, atexit.register, SubElement) now check for it. atexit stores None, which the run loop already maps back to NULL. pyperf, PGO+LTO, `t = time.gmtime(0)`, `ns = SimpleNamespace(a=1)`: | Benchmark | main | this change | |---------------------------------|:------:|:---------------------:| | struct_time.`__replace__`() | 229 ns | 47.0 ns: 4.87x faster | | SimpleNamespace.`__replace__`() | 72.2 ns | 65.5 ns: 1.10x faster | Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bedevere-app
Bot
added
awaiting core review
type-feature
| # NULL when there are no keywords: no call allocates an empty dict. | ||
| return f"{self.parser_name} = Py_XNewRef(kwargs);\n" |
There was a problem hiding this comment.
Is there anywhere to document the new behavior, to make sure this isn't a footgun?
There was a problem hiding this comment.
https://devguide.python.org/development-tools/clinic/ is all the docs and doesn't go into that much detail. For https://docs.python.org/3.13/c-api/call.html#c.vectorcallfunc it is documented as able to be NULL, so this to me is bringing AC parsers into that same state.
|
@eendebakpt could you take a look at this one? This + the "support **kwargs in AC" that comes after enable using AC vectorcall for a speedup on most objects, in particular |
eendebakpt
left a comment
eendebakpt
left a comment
There was a problem hiding this comment.
Looks good to me. The new behavior of AC is fine (it is a behavior change for external users of AC, but AC is supported to be only an internal tool).
The generated parser allocated an empty dict for **kwargs when there were no keyword arguments. Pass NULL instead, like the caller does, and handle NULL in the impls that used the dict unconditionally.
This is preparation work for supporting **kwargs in Argument Clinic Vectorcall where allocating a dict instead of passing NULL makes a significant performance impact.
All **kwargs impls were audited: _csv and zoneinfo already accept NULL, and the rest (structseq and SimpleNamespace
__replace__, _interpreters.new_config, atexit.register, SubElement) now check for it. atexit stores None, which the run loop already maps back to NULL.pyperf, PGO+LTO,
t = time.gmtime(0),ns = SimpleNamespace(a=1):__replace__()__replace__()