Repository navigation
Add tp_fastcall to PyTypeObject: support FASTCALL calling convention for all callable objects #73445
Description
Activity
A new FASTCALL calling convention was added to Python 3.6. It allows to avoid the creation of a temporary tuple to pass positional arguments and a temporary dictionary to pass keyword arguments. A new METH_FASTCALL calling convention was added for C functions. Most functions now support fastcall, except objects with a __call__() method which have to go through slot_tp_call() which still requires a tuple and dictionary.
I tried multiple implementations to support fast calls to call the __call__() method, but I had practical and technical issues.
First, I tried to reuse the tp_call field to PyTypeObject: it can be a regular call (tuple/dict for arguments) or a fast call. I added a flag to the tp_flags field. It was tricky to support class inheritance, decide to set or clear the flag. But the real blocker issue is fAthat it is obviously breaks the backward compatibility: existing code calling directly tp_call with the regular calling convention will crash immediatly, and the error is not catched during compilation, even if the code is recompiled.
I propose a different design: add a new tp_fastcall field to PyTypeObject and use a wrapper for tp_call when tp_fastcall is defined. If a type defines tp_fastcall but not, the tp_call wrapper "simply" calls tp_fastcall. Advantages:
- The wrapper is trivial
- Minor changes to PyType_Ready() to support inheritance (simple logic)
- Fully backward compatible
- If tp_call is called directly without keyword arguments, there is no overhead but a speedup!
Inheritance:
- If a type only defines tp_call, tp_fastcall is not inherited from the parent: tp_fastcall is set to NULL.
- If a type only defines tp_fastcall: tp_fastcall is always use (tp_call uses the wrapper)
- If a type defines tp_call and tp_fastcall, PyObject_Call() uses tp_call whereas _PyObject_FastCallDict() uses tp_fastcall.
Functions of the C API will be modified to use tp_fastcall if available.
The plan is then to patch most Python types to replace their tp_call with tp_fastcall. First, most important (common) types like Python and C functions, descriptors, and the various kinds of wrappers should be patched. Later, we should maybe discuss on a case by case basis to decide if it's worth it.
I will try to run benchmark before any kind.
Naoki wants to use FASTCALL for CALL_METHOD:
http://bugs.python.org/issue26110#msg283093tp_fastcall should allow to easily implement it.
I started to work on FASTCALL, because I dislike the "cached tuple" hack used in some performance critical code, and the hack causes various kinds of tricky but severe issues (can lead to segfault).
Thanks to tp_fastcall, it becomes possible to drop the "cached tuple" hack from property_descr_get() *and* keep good performances.
First, a benchmark to show the performance gain of using "cached tuple". I modified property_descr_get() to use Python 3.4 code which doesn't have the optimization:
$ ./python -m perf compare_to py34.json ref.json Median +- std dev: [py34] 75.0 ns +- 1.7 ns -> [ref] 50.0 ns +- 0.9 ns: 1.50x faster (-33%)It's MUCH faster, good job. But it requires complex and fragile code. Ok, let's see with operator.itemgetter() supporting tp_fastcall, Python modified to use tp_fastcall and without the "cached arg" hack:
$ ./python -m perf compare_to ref.json fastcall_wrapper.json Median +- std dev: [ref] 50.0 ns +- 0.9 ns -> [fastcall_wrapper] 48.2 ns +- 1.5 ns: 1.04x faster (-4%)It's a little bit faster, but that's not the point. The point is that it isn't slower and it doesn't require to modify C code to benefit of the optimization! Just to be clear, another benchmark result on property_descr_get() without "cache args", without fastcall (py34) and with fastcall ("fastcall_wrapper"):
$ ./python -m perf compare_to py34.json fastcall_wrapper.json Median +- std dev: [py34] 75.0 ns +- 1.7 ns -> [fastcall_wrapper] 48.2 ns +- 1.5 ns: 1.56x faster (-36%)Summary:
- tp_fastcall avoids to remove the "cached args" hack which will fix severe issue in corner cases
- tp_fastcall makes existing code faster for free. I mean, builtin types should be modified to support tp_fastcall, most all code *calling* these types don't need any change.
- If a type only defines tp_fastcall: tp_fastcall is always use (tp_call uses the wrapper)
Is tp_call set to the wrapper rather then inheriting? What if tp_call is defined in a superclass?
- If a type defines tp_call and tp_fastcall, PyObject_Call() uses tp_call whereas _PyObject_FastCallDict() uses tp_fastcall.
I would consider this as a bug. It would be weird if different ways of calling cause executing different code.
What about dynamically changed Python types? What if you set or delete the __call__ attribute of Python class?
- If a type only defines tp_fastcall: tp_fastcall is always use (tp_call uses the wrapper)
Is tp_call set to the wrapper rather then inheriting?
It is set to the wrapper. Defining tp_fastcall should work as defining tp_call: it should override the tp_call and tp_fastcall slots.
What if tp_call is defined in a superclass?
What is a superclass? Do you have an example?
- If a type defines tp_call and tp_fastcall, PyObject_Call() uses tp_call whereas _PyObject_FastCallDict() uses tp_fastcall.
I would consider this as a bug. It would be weird if different ways of calling cause executing different code.
It's a micro-optimization to avoid arguments unpacking/packing, conversions needed when you switch from the regular calling convention to the fast call convention, or the opposite.
I don't think that types defining tp_call and tp_fastcall will be common.
What about dynamically changed Python types?
What do you mean? Do you have an example?
What if you set or delete the __call__ attribute of Python class?
I don't know how these things work :-) Let me try on a patched Python (using tp_fastcall):
$ ./python >>> class A: ... def __call__(self): print("A") ... >>> a=A() >>> a() A >>> A.__call__=lambda self: print("B!") >>> a() B! >>> del A.__call__ >>> a() Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: 'A' object is not callable
It seems like "it just works", but I don't know how it works internally :-)
a() uses a dynamic lookup of a.__class__.__call__, no?
Ok, here is a first patch: tp_fastcall.patch
-
Add tp_fastcall to PyTypeObject
-
Add fastcall_wrapper() and use it for tp_call in PyType_Ready()
-
Modify _PyObject_FastCallDict() and _PyObject_FastCallKeywords() to use tp_fastcall if defined
-
Modify a lot of types to define tp_fastcall instead of tp_call (to use FASTCALL calling convention)
-
Add a few new helper functions:
- _PyObject_FastCall_Prepend() -- similar to _PyObject_Call_Prepend()
- _Py_FastCall_FromArgs() -- ugly name, sorry about that, it should be renamed :-)
- PyArg_UnpackStack() -- similar to PyArg_UnpackTuple()
- _PyArg_NoStackKeywords() -- similar to _PyArg_NoKeywords
Hum, I should make sure that these new functions are not public. By the way, I should probably rename PyArg_UnpackStack() to _PyArg_UnpackStack().
-
tp_fastcall.patch modifies property_descr_get() to use tp_fastcall if available, or use the cached tuple. Once performances are validated (no regression), the cached tuple as to pass position arguments, should go away.
By the way, msg285390 uses the following micro-benchmark:
---import perf bench = perf.Runner() bench.timeit("namedtuple.attr", "a.a", "from collections import namedtuple as n; a = n('n', 'a b c')(1, 2, 3)", duplicate=20)
It tests property_descr_get() with operator.itemgetter(). It would be nice to benchmark property_descr_get() with other functions.
PyArg_UnpackStack() is declared as in the limited API since 3.3. If you want to add PyArg_UnpackStack() to the limited API, define it as added in 3.7.
For compatibility with extensions built with older Pythons you should define new type flag and read tp_fastcall only if the flag is set. See for example Py_TPFLAGS_HAVE_FINALIZE.
Ok, I pushed a new commit "Add Py_TPFLAGS_HAVE_FASTCALL flag" in my Git branch.
(By the way, I'm using the Git branch as a working copy, it can change anytime to get a nice list of commits. If you want reliable changes, use patches attached to this issue.)
Serhiy Storchaka!:
For compatibility with extensions built with older Pythons you should define new type flag and read tp_fastcall only if the flag is set. See for example Py_TPFLAGS_HAVE_FINALIZE.
Wait, there is maybe an issue with the stable API in my design :-/
Let's say that a builtin type NEW defines tp_fastcall and uses fastcall_wrapper() for tp_call. A type OLD inherits from NEW and don't override tp_call (not tp_fastcall). Type OLD comes from an extension compiled with Python 3.6 and use the stable ABI. Does the type OLD contain the field tp_fastcall in memory? Is it possible to copy the tp_fastcall filed from type NEW into the type OLD?
If not, the fastcall_wrapper() should be smarter to not get tp_fastcall from the type OLD but follow the MRO to find the first type with the Py_TPFLAGS_HAVE_FASTCALL flag. Is it possible that this short loop to find has a cost on performances?
I ran benchmarks on tp_fastcall.patch. Hum, it seems like there is a performance issue somewhere :-( Almost all benchmarks are much slower with the patch.
haypo@speed-python$ python3 -m perf compare_to 2017-01-11_00-07-default-b9404639a18c.json tp_fastcall_ref_b9404639a18c.json -G
Slower (63):- unpickle_pure_python: 671 us +- 16 us -> 952 us +- 9 us: 1.42x slower (+42%)
- pickle_pure_python: 1.02 ms +- 0.02 ms -> 1.40 ms +- 0.02 ms: 1.37x slower (+37%)
- telco: 16.6 ms +- 0.6 ms -> 22.7 ms +- 0.3 ms: 1.37x slower (+37%)
- xml_etree_generate: 215 ms +- 3 ms -> 290 ms +- 7 ms: 1.35x slower (+35%)
- hexiom: 17.7 ms +- 0.2 ms -> 23.6 ms +- 0.1 ms: 1.33x slower (+33%)
- xml_etree_process: 182 ms +- 3 ms -> 240 ms +- 4 ms: 1.32x slower (+32%)
- genshi_text: 69.0 ms +- 0.8 ms -> 90.4 ms +- 0.8 ms: 1.31x slower (+31%)
- logging_simple: 27.5 us +- 0.4 us -> 35.8 us +- 0.4 us: 1.30x slower (+30%)
(...)
Faster (1):
- spectral_norm: 271 ms +- 2 ms -> 257 ms +- 2 ms: 1.06x faster (-5%)
Strange, I'm unable to reproduce the result on a different computer:
haypo@smithers$ ./python -m perf compare_to ref.json tp_fastcall.json -v
Median +- std dev: [ref] 20.0 ms +- 0.5 ms -> [tp_fastcall] 20.0 ms +-
0.6 ms: 1.00x slower (+0%)
Not significant!Strange, I'm unable to reproduce the result on a different computer:
haypo@smithers$ ./python -m perf compare_to ref.json tp_fastcall.json -v
Median +- std dev: [ref] 20.0 ms +- 0.5 ms -> [tp_fastcall] 20.0 ms +-
0.6 ms: 1.00x slower (+0%)
Not significant!Oops, I forgot to mention that I tried the telco benchmark.
Slower (63):
- unpickle_pure_python: 671 us +- 16 us -> 952 us +- 9 us: 1.42x slower (+42%)
...
This benchmark was run with LTO+PGO. It seems like PGO produced less efficient machine code with the patch. Maybe a missed optimization.
Oh, I just realized that I announced that objects with a __call__() are faster, but it's not the case yet! Such object still gets slot_tp_call() as tp_call which isn't a fastcall!
- unpickle_pure_python: 671 us +- 16 us -> 952 us +- 9 us: 1.42x slower (+42%)
24 remaining items
Oh, I had a local change not commited yet, so tp_fastcall-6.patch didn't get the [review] button. I just pushed the commit and then posted again the exact same patch.
performance results on tp_fastcall-6.patch (diff >= 2%):
---
haypo@speed-python$ python3 -m perf compare_to /home/haypo/benchmarks/2017-01-23_14-02-default-cebc9c7ad195.json tp_fastcall-6_ref_cebc9c7ad195.json -G --min-speed=2
Slower (6):- scimark_monte_carlo: 202 ms +- 5 ms -> 222 ms +- 8 ms: 1.10x slower (+10%)
- telco: 13.0 ms +- 0.2 ms -> 14.2 ms +- 0.4 ms: 1.09x slower (+9%)
- call_method: 11.0 ms +- 0.3 ms -> 11.5 ms +- 0.5 ms: 1.05x slower (+5%)
- scimark_sparse_mat_mult: 8.57 ms +- 0.14 ms -> 8.92 ms +- 0.15 ms: 1.04x slower (+4%)
- fannkuch: 938 ms +- 7 ms -> 974 ms +- 11 ms: 1.04x slower (+4%)
- scimark_fft: 667 ms +- 9 ms -> 690 ms +- 10 ms: 1.03x slower (+3%)
Faster (17):
- scimark_lu: 362 ms +- 13 ms -> 322 ms +- 8 ms: 1.12x faster (-11%)
- chameleon: 23.5 ms +- 0.3 ms -> 21.3 ms +- 0.3 ms: 1.10x faster (-9%)
- xml_etree_process: 179 ms +- 3 ms -> 169 ms +- 3 ms: 1.06x faster (-5%)
- xml_etree_generate: 208 ms +- 4 ms -> 199 ms +- 3 ms: 1.05x faster (-4%)
- logging_silent: 586 ns +- 8 ns -> 562 ns +- 7 ns: 1.04x faster (-4%)
- richards: 147 ms +- 2 ms -> 141 ms +- 3 ms: 1.04x faster (-4%)
- pickle_pure_python: 1.03 ms +- 0.02 ms -> 994 us +- 15 us: 1.04x faster (-4%)
- regex_compile: 379 ms +- 2 ms -> 367 ms +- 4 ms: 1.03x faster (-3%)
- raytrace: 1.08 sec +- 0.01 sec -> 1.04 sec +- 0.02 sec: 1.03x faster (-3%)
- unpickle: 31.0 us +- 0.4 us -> 30.0 us +- 0.4 us: 1.03x faster (-3%)
- pathlib: 42.3 ms +- 0.5 ms -> 41.0 ms +- 0.5 ms: 1.03x faster (-3%)
- scimark_sor: 391 ms +- 7 ms -> 379 ms +- 12 ms: 1.03x faster (-3%)
- genshi_xml: 167 ms +- 2 ms -> 162 ms +- 2 ms: 1.03x faster (-3%)
- django_template: 356 ms +- 5 ms -> 347 ms +- 4 ms: 1.03x faster (-3%)
- regex_dna: 283 ms +- 1 ms -> 276 ms +- 4 ms: 1.03x faster (-3%)
- hexiom: 18.0 ms +- 0.1 ms -> 17.6 ms +- 0.2 ms: 1.02x faster (-2%)
- call_simple: 10.7 ms +- 0.4 ms -> 10.5 ms +- 0.2 ms: 1.02x faster (-2%)
Benchmark hidden because not significant (41): (...)
---Or less verbose output, only show difference >= 5%:
---
haypo@speed-python$ python3 -m perf compare_to /home/haypo/benchmarks/2017-01-23_14-02-default-cebc9c7ad195.json tp_fastcall-6_ref_cebc9c7ad195.json -G --min-speed=5
Slower (2):- scimark_monte_carlo: 202 ms +- 5 ms -> 222 ms +- 8 ms: 1.10x slower (+10%)
- telco: 13.0 ms +- 0.2 ms -> 14.2 ms +- 0.4 ms: 1.09x slower (+9%)
Faster (3):
- scimark_lu: 362 ms +- 13 ms -> 322 ms +- 8 ms: 1.12x faster (-11%)
- chameleon: 23.5 ms +- 0.3 ms -> 21.3 ms +- 0.3 ms: 1.10x faster (-9%)
- xml_etree_process: 179 ms +- 3 ms -> 169 ms +- 3 ms: 1.06x faster (-5%)
Benchmark hidden because not significant (59): (...)
---To be honest, I'm disappointed, I expected more faster benchmarks. Maybe the cost of the temporary tuple/dict to pass arguments to function is not as high as I expected.
telco is also my favorite benchmark, I'm sad that it's 1.09x slower. I should investigate that.
scimark_monte_carlo and scimark_lu benchmarks are still unstable for an unknown reasons, so I don't care too much of these two benchmarks.
tp_fastcall-6.patch: "call_method: 11.0 ms +- 0.3 ms -> 11.5 ms +- 0.5 ms: 1.05x slower (+5%)"
I looked at the C code: the patch doesn't change any C function used by this benchmark. IHMO it's only a better of code locality. Maybe the PGO compilation produced less efficient code with the patch.
Another run at my machine. (tp_fastcall-6.patch)
+ ../python.default -m perf compare_to default.json patched.json -G --min-speed=2 Slower (8): - logging_silent: 736 ns +- 7 ns -> 783 ns +- 12 ns: 1.06x slower (+6%) - unpickle_pure_python: 829 us +- 13 us -> 875 us +- 12 us: 1.06x slower (+6%) - scimark_sor: 482 ms +- 8 ms -> 507 ms +- 14 ms: 1.05x slower (+5%) - xml_etree_iterparse: 223 ms +- 6 ms -> 228 ms +- 7 ms: 1.02x slower (+2%) - scimark_fft: 728 ms +- 16 ms -> 745 ms +- 19 ms: 1.02x slower (+2%) - scimark_sparse_mat_mult: 8.71 ms +- 0.32 ms -> 8.90 ms +- 0.28 ms: 1.02x slower (+2%) - json_dumps: 29.3 ms +- 0.3 ms -> 29.9 ms +- 0.3 ms: 1.02x slower (+2%) - pathlib: 51.2 ms +- 0.6 ms -> 52.3 ms +- 0.8 ms: 1.02x slower (+2%)
Faster (8):
- unpack_sequence: 132 ns +- 2 ns -> 123 ns +- 2 ns: 1.08x faster (-7%)
- xml_etree_parse: 319 ms +- 11 ms -> 303 ms +- 11 ms: 1.05x faster (-5%)
- call_simple: 14.3 ms +- 0.5 ms -> 13.7 ms +- 0.5 ms: 1.04x faster (-4%)
- call_method_unknown: 16.6 ms +- 1.1 ms -> 16.0 ms +- 0.2 ms: 1.04x faster (-4%)
- call_method: 15.1 ms +- 0.1 ms -> 14.5 ms +- 0.5 ms: 1.04x faster (-4%)
- xml_etree_process: 241 ms +- 4 ms -> 235 ms +- 4 ms: 1.03x faster (-3%)
- call_method_slots: 14.7 ms +- 0.2 ms -> 14.3 ms +- 0.2 ms: 1.02x faster (-2%)
- genshi_xml: 197 ms +- 3 ms -> 193 ms +- 2 ms: 1.02x faster (-2%)
"While I feel your work is great, performance benefit seems very small,
compared complexity of this patch."I have to agree. I spent a lot of times on benhchmarking these tp_fast* changes. While one or two benchmarks are faster, it's not really the case for the others.
I also agree with the complexity. In Python 3.6, most FASTCALL changes were internals. For example, using PyObject_CallFunctionObjArgs() now uses FASTCALL internally, without having to modify callers of the API. I tried to only use _PyObject_FastCallDict/Keywords() in a few places where the speedup was significant.
The main visible change of Python 3.6 FASTCALL is the new METH_CALL calling convention for C function. Your change modifying print() to use METH_CALL has a significant impact on the telco benchmark, without no drawback. I tested further changes to use METH_FASTCALL in struct and decimal modules, and they optimize telco even more.
To continue the optimization work, I guess that using METH_CALL in more cases, using Argument Clinic whenever possible, would have a more concrete and measurable impact on performances, than this big tp_fastcall patch.
But I'm not ready to abandon the whole approach yet, so I change the status to Pending. I may come back in one or two months, to check if I didn't miss anything obvious to unlock even more optimizations ;-)
python-dev commented
on Feb 9, 2017 python-devmannequinMannequinNew changeset 31342913fb1e by Victor Stinner in branch 'default':
Fix PyCFunction_Call() performance issue
https://hg.python.org/cpython/rev/31342913fb1epython-dev commented
on Feb 9, 2017 python-devmannequinMannequinNew changeset e9807e307f1f561a2dfe3e9f97a38444528dba86 by Victor Stinner in branch 'master':
Fix PyCFunction_Call() performance issue
e9807e3I don't completely reject the issue. I may come back later if I find a way to make it even more efficient. But I prefer to close the issue as REJECTED to make it clear that right now with the current implementation and benchmark results, it's not worth it.
For compatibility with extensions built with older Pythons you should define new type flag and read tp_fastcall only if the flag is set.
Can you comment on #4944 why you think that such compatibility should be guaranteed? According to PEP-384, the layout of PyTypeObject is not part of the stable ABI.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: