GeezerVison is showing github.com — Slow functions calls for C-coded functions, accepting positional-or-kwarg argument vs positional-only · Issue #136681 · python/cpython
Slow functions calls for C-coded functions, accepting positional-or-kwarg argument vs positional-only #136681DescriptionBug report
Bug description:
I don't see measurable difference on pure-Python level, e.g. for
# a.py
import pyperf
def f1(x):
return 1
def f2(x, /):
return 1
runner = pyperf.Runner()
runner.bench_func("f1(1)", f1, 1)
runner.bench_func("f2(1)", f2, 1)I got on 3.13:
$ python a.py -q
f1(1): Mean +- std dev: 233 ns +- 2 ns
f2(1): Mean +- std dev: 232 ns +- 2 ns
But for functions, implemented in C, using the Argument Clinic - the difference is bigger:
$ python bench.py -q -o patch.json
isfinite(1.0): Mean +- std dev: 152 ns +- 1 ns
$ git checkout master && make -s
Checked 114 modules (35 built-in, 78 shared, 1 n/a on linux-x86_64, 0 disabled, 0 missing, 0 failed on import)
$ python bench.py -q -o ref.json
isfinite(1.0): Mean +- std dev: 136 ns +- 1 ns
| Benchmark |
ref |
patch |
| isfinite(1.0) |
136 ns |
152 ns: 1.12x slower |
(In the second version - the math.isfinite() accept also keyword argument x)
a patch
(run ./python Tools/clinic/clinic.py Modules/mathmodule.c !)
diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c
index 7c2a421dd6..e33281ac8b 100644
--- a/Modules/mathmodule.c
+++ b/Modules/mathmodule.c
@@ -3156,14 +3156,13 @@ math_radians_impl(PyObject *module, double x)
math.isfinite
x: double
- /
Return True if x is neither an infinity nor a NaN, and False otherwise.
[clinic start generated code]*/
static PyObject *
math_isfinite_impl(PyObject *module, double x)
-/*[clinic end generated code: output=8ba1f396440c9901 input=46967d254812e54a]*/
+/*[clinic end generated code: output=8ba1f396440c9901 input=a47ad6e72998b81d]*/
{
return PyBool_FromLong((long)isfinite(x));
}
This performance penalty affects API decisions (e.g. #131886) and that looks as a bug.
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Linked PRs
Reacted by Pieter Eendebak Activityserhiy-storchaka commented on Jul 16, 2025 Look at the generated code. _PyArg_UnpackKeywords has a fast path. I would check if kwnames is NULL here. Reacted by Sergey B Kirpichev skirpichev commented on Jul 16, 2025
Look at the generated code. _PyArg_UnpackKeywords has a fast path. I would check if kwnames is NULL here.
No. It's already checked. Twice. Both in _PyArg_UnpackKeywords() function and in the _PyArg_UnpackKeywords() macro. And this path - hits.
The problem is code for initialization of _kwtuple in clinic's generated output:
#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)
#define NUM_KEYWORDS 1
static struct {
PyGC_Head _this_is_not_used;
PyObject_VAR_HEAD
Py_hash_t ob_hash;
PyObject *ob_item[NUM_KEYWORDS];
} _kwtuple = {
.ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS)
.ob_hash = -1,
.ob_item = { _Py_LATIN1_CHR('x'), },
};
#undef NUM_KEYWORDS
#define KWTUPLE (&_kwtuple.ob_base.ob_base)
#else // !Py_BUILD_CORE
# define KWTUPLE NULL
#endif // !Py_BUILD_CORE
static const char * const _keywords[] = {"x", NULL};
static _PyArg_Parser _parser = {
.keywords = _keywords,
.fname = "isfinite",
.kwtuple = KWTUPLE,
};
#undef KWTUPLE
PyObject *argsbuf[1];
double x;
args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser,
/*minpos*/ 1, /*maxpos*/ 1, /*minkw*/ 0, /*varpos*/ 0, argsbuf);If I move that to if (kwnames) { ... } block - speed loss is lost. serhiy-storchaka commented on Jul 16, 2025 Static initializer should only be executed once. skirpichev commented on Jul 16, 2025 Hmm, with following in the clinic output:
if (kwnames) {
PyObject *argsbuf[1];
args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser,
/*minpos*/ 1, /*maxpos*/ 1, /*minkw*/ 0, /*varpos*/ 0, argsbuf);
}I got same numbers as in the master (i.e. with positional-only isfinite) for a simple benchmark:
# a.py
import pyperf
from math import isfinite
runner = pyperf.Runner()
runner.bench_func('isfinite(1.0)', isfinite, 1.0)(CPython built with default configure flags.)
patch for the AC
diff --git a/Tools/clinic/libclinic/parse_args.py b/Tools/clinic/libclinic/parse_args.py
index 0e15d2f163..bb25576481 100644
--- a/Tools/clinic/libclinic/parse_args.py
+++ b/Tools/clinic/libclinic/parse_args.py
@@ -622,7 +622,6 @@ def parse_general(self, clang: CLanguage) -> None:
self.flags = "METH_FASTCALL|METH_KEYWORDS"
self.parser_prototype = PARSER_PROTOTYPE_FASTCALL_KEYWORDS
self.declarations = declare_parser(self.func, codegen=self.codegen)
- self.declarations += "\nPyObject *argsbuf[%s];" % (len(self.converters) or 1)
if self.varpos:
self.declarations += "\nPyObject * const *fastargs;"
argsname = 'fastargs'
@@ -640,15 +639,18 @@ def parse_general(self, clang: CLanguage) -> None:
argsname = 'fastargs'
argname_fmt = 'fastargs[%d]'
self.declarations = declare_parser(self.func, codegen=self.codegen)
- self.declarations += "\nPyObject *argsbuf[%s];" % (len(self.converters) or 1)
self.declarations += "\nPyObject * const *fastargs;"
self.declarations += "\nPy_ssize_t nargs = PyTuple_GET_SIZE(args);"
if has_optional_kw:
self.declarations += "\nPy_ssize_t noptargs = %s + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - %d;" % (nargs, self.min_pos + self.min_kw_only)
unpack_args = '_PyTuple_CAST(args)->ob_item, nargs, kwargs, NULL'
parser_code = [libclinic.normalize_snippet(f"""
- {argsname} = _PyArg_UnpackKeywords({unpack_args}, &_parser,
- /*minpos*/ {self.min_pos}, /*maxpos*/ {self.max_pos}, /*minkw*/ {self.min_kw_only}, /*varpos*/ {1 if self.varpos else 0}, argsbuf);
+ if ({"kwnames" if self.fastcall else "kwargs"}) {{{{
+ PyObject *argsbuf[{len(self.converters) or 1}];
+
+ {argsname} = _PyArg_UnpackKeywords({unpack_args}, &_parser,
+ /*minpos*/ {self.min_pos}, /*maxpos*/ {self.max_pos}, /*minkw*/ {self.min_kw_only}, /*varpos*/ {1 if self.varpos else 0}, argsbuf);
+ }}}}
if (!{argsname}) {{{{
goto exit;
}}}}
Though, if I benchmark a loop with:
# b.py
import pyperf
from math import isfinite
def f(n):
for _ in range(n):
isfinite(1.0)
runner = pyperf.Runner()
for n in [1, 2, 10, 100]:
s = f'isfinite(1.0) x {n:3} times'
runner.bench_func(s, f, n)it's still worse!. I got
| Benchmark |
ref |
patch |
patch0 |
| isfinite(1.0) x 1 times |
661 ns |
699 ns: 1.06x slower |
717 ns: 1.08x slower |
| isfinite(1.0) x 2 times |
757 ns |
802 ns: 1.06x slower |
842 ns: 1.11x slower |
| isfinite(1.0) x 10 times |
1.38 us |
1.56 us: 1.13x slower |
1.70 us: 1.23x slower |
| isfinite(1.0) x 100 times |
8.40 us |
10.1 us: 1.21x slower |
11.3 us: 1.34x slower |
| Geometric mean |
(ref) |
1.11x slower |
1.19x slower |
where patch0 - version without if (kwnames) condition.
I suspect this might be related to specialization for c-methods calls in Python/specialize.c. (Edit: indeed, it's because specialize_c_call().)
@serhiy-storchaka, the AC patch alone does make sense for you? serhiy-storchaka commented on Jul 16, 2025 No, this change is not correct. _PyArg_UnpackKeywords cannot be omitted if kwnames is NULL. For example, it checks that the correct number of positional arguments was passed.
There are two implementations of _PyArg_UnpackKeywords -- as a macro and as a C function. The macro version handles the fast path, it should add very little overhead. The function implementation is only used for non-fast path. So my suspect is that for some reasons the fast path may be not correctly detected, so the code goes the general slow path. Please check if the _PyArg_UnpackKeywords function was called in that case. skirpichev commented on Jul 16, 2025
No, this change is not correct. _PyArg_UnpackKeywords cannot be omitted if kwnames is NULL. For example, it checks that the correct number of positional arguments was passed.
That was just a draft:)
So my suspect is that for some reasons the fast path may be not correctly detected, so the code goes the general slow path. Please check if the _PyArg_UnpackKeywords function was called in that case.
I already did this first. No - fast path was taken.
I think I've found a simpler solution: just add a static decorator for argsbuf. Here benchmark for math.fmin (ref - current main, patch-v0 - support for kwargs, patch-v1 - make also argsbuf static). Up to a noise - the v1 patch match timings in the main branch:
| Benchmark |
ref |
patch-v0 |
patch-v1 |
| fmin(1.0, 2.0) |
168 ns |
185 ns: 1.10x slower |
not significant |
| fmin(1.0, 2.0) x 2 times |
908 ns |
956 ns: 1.05x slower |
926 ns: 1.02x slower |
| fmin(1.0, 2.0) x 10 times |
2.04 us |
2.23 us: 1.09x slower |
2.06 us: 1.01x slower |
| fmin(1.0, 2.0) x 100 times |
14.8 us |
16.5 us: 1.11x slower |
not significant |
| Geometric mean |
(ref) |
1.09x slower |
1.01x slower |
# bench.py
import pyperf
from math import fmin
def f(n):
for _ in range(n):
fmin(1.0, 2.0)
runner = pyperf.Runner()
runner.bench_func('fmin(1.0, 2.0)', fmin, 1.0, 2.0)
for n in [2, 10, 100]:
s = f'fmin(1.0, 2.0) x {n:3} times'
runner.bench_func(s, f, n)For one-arg functions (like isfinite()) - the difference holds also in the v1 version for loops. I hope this could be fixed too, using different specialized instructions. kumaraditya303 commented on Jul 17, 2025 There are redundant checks in the implementation of these functions:
PyObject * const *
_PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
PyObject *kwargs, PyObject *kwnames,
struct _PyArg_Parser *parser,
int minpos, int maxpos, int minkw, int varpos,
PyObject **buf)
{
PyObject *kwtuple;
PyObject *keyword;
int i, posonly, minposonly, maxargs;
int reqlimit = minkw ? maxpos + minkw : minpos;
Py_ssize_t nkwargs;
PyObject * const *kwstack = NULL;
assert(kwargs == NULL || PyDict_Check(kwargs));
assert(kwargs == NULL || kwnames == NULL);
if (parser == NULL) {
PyErr_BadInternalCall();
return NULL;
}
if (kwnames != NULL && !PyTuple_Check(kwnames)) {
PyErr_BadInternalCall();
return NULL;
}Here the check for NULL check for parser and type check for kwnames can be turned into asserts, they should not be runtime checks, similarly for other functions as well. skirpichev commented on Jul 17, 2025 The function is not the reason for a slowdown, as the fast path is taken in the macro. skirpichev commented on Jul 30, 2025 @serhiy-storchaka, @kumaraditya303 - #136732 implements a variation of the storage class specifier. So far, I kept this PR as a draft, as it's huge. Though, actual changes in AC are small. Let me know if it worth to make this review-ready.
This doesn't fix problem completely. Still, there are some differences, especially for 1-arg case (as noted in #136681 (comment)).
In the main:
| Benchmark |
posonly-ref |
posorkw-ref |
| fmin(1.0, 2.0) |
171 ns |
190 ns: 1.11x slower |
| fmin(1.0, 2.0) x 2 times |
967 ns |
992 ns: 1.03x slower |
| fmin(1.0, 2.0) x 10 times |
2.10 us |
2.26 us: 1.08x slower |
| fmin(1.0, 2.0) x 100 times |
14.8 us |
16.5 us: 1.12x slower |
| Geometric mean |
(ref) |
1.08x slower |
| Benchmark |
posonly-ref |
posorkw-ref |
| isfinite(1.0) |
136 ns |
154 ns: 1.13x slower |
| isfinite(1.0) x 2 times |
768 ns |
838 ns: 1.09x slower |
| isfinite(1.0) x 10 times |
1.39 us |
1.69 us: 1.21x slower |
| isfinite(1.0) x 100 times |
8.38 us |
11.3 us: 1.35x slower |
| Geometric mean |
(ref) |
1.19x slower |
With the patch:
| Benchmark |
posonly-patch |
posorkw-patch |
| fmin(1.0, 2.0) |
170 ns |
180 ns: 1.06x slower |
| fmin(1.0, 2.0) x 2 times |
929 ns |
946 ns: 1.02x slower |
| fmin(1.0, 2.0) x 10 times |
2.06 us |
2.09 us: 1.01x slower |
| fmin(1.0, 2.0) x 100 times |
14.8 us |
15.0 us: 1.02x slower |
| Geometric mean |
(ref) |
1.03x slower |
| Benchmark |
posonly-patch |
posorkw-patch |
| isfinite(1.0) |
136 ns |
141 ns: 1.04x slower |
| isfinite(1.0) x 2 times |
750 ns |
804 ns: 1.07x slower |
| isfinite(1.0) x 10 times |
1.37 us |
1.59 us: 1.16x slower |
| isfinite(1.0) x 100 times |
8.38 us |
10.4 us: 1.25x slower |
| Geometric mean |
(ref) |
1.13x slower |
Details
import pyperf
from math import fmin
def f(n):
for _ in range(n):
fmin(1.0, 2.0)
runner = pyperf.Runner()
runner.bench_func('fmin(1.0, 2.0)', fmin, 1.0, 2.0)
for n in [2, 10, 100]:
s = f'fmin(1.0, 2.0) x {n:3} times'
runner.bench_func(s, f, n)# bench2.py
import pyperf
from math import isfinite
def f(n):
for _ in range(n):
isfinite(1.0)
runner = pyperf.Runner()
runner.bench_func('isfinite(1.0)', isfinite, 1.0)
for n in [2, 10, 100]:
s = f'isfinite(1.0) x {n:3} times'
runner.bench_func(s, f, n)skirpichev commented on Sep 10, 2025 added a commit that references this issue on Sep 27, 2026gh-157407: Argument Clinic: parse positional-only `defining_class` me…
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|