Add tp_fastcall to PyTypeObject: support FASTCALL calling convention for all callable objects

[vstinner]

BPO	29259
Nosy	@vstinner, @methane, @serhiy-storchaka, @jdemeyer
PRs	bpo-29534: _decimal difference with _pydecimal #65 bpo-30524: Fix _PyStack_UnpackDict() #1886
Files	tp_fastcall.patch tp_fastcall-2.patch tp_fastcall-3.patch tp_fastcall-4.patch tp_fastcall-5.patch tp_fastcall-6.patch

^{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:

assignee = None
closed_at = <Date 2017-02-23.16:00:47.565>
created_at = <Date 2017-01-13.12:24:34.942>
labels = ['3.7', 'performance']
title = 'Add tp_fastcall to PyTypeObject: support FASTCALL calling convention for all callable objects'
updated_at = <Date 2019-04-03.15:40:34.099>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2019-04-03.15:40:34.099>
actor = 'jdemeyer'
assignee = 'none'
closed = True
closed_date = <Date 2017-02-23.16:00:47.565>
closer = 'vstinner'
components = []
creation = <Date 2017-01-13.12:24:34.942>
creator = 'vstinner'
dependencies = []
files = ['46281', '46309', '46323', '46326', '46347', '46419']
hgrepos = []
issue_num = 29259
keywords = ['patch']
message_count = 50.0
messages = ['285388', '285389', '285390', '285400', '285404', '285405', '285411', '285416', '285419', '285420', '285425', '285426', '285451', '285463', '285484', '285568', '285570', '285571', '285602', '285603', '285666', '285667', '285674', '285693', '285704', '285705', '285708', '285710', '285712', '285715', '285724', '285726', '285731', '285741', '285776', '285863', '286289', '286291', '286299', '286303', '286307', '286313', '286314', '287374', '287376', '288458', '293583', '295517', '322747', '339397']
nosy_count = 5.0
nosy_names = ['vstinner', 'methane', 'python-dev', 'serhiy.storchaka', 'jdemeyer']
pr_nums = ['65', '1886']
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue29259'
versions = ['Python 3.7']

[vstinner]

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.

[vstinner]

Naoki wants to use FASTCALL for CALL_METHOD:
http://bugs.python.org/issue26110#msg283093

tp_fastcall should allow to easily implement it.

[vstinner]

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.

[serhiy-storchaka]

• 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?

[vstinner]

• 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?

[vstinner]

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().

[vstinner]

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.

[serhiy-storchaka]

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.

[vstinner]

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?

[vstinner]

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%)

[vstinner]

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!

[vstinner]

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.

[vstinner]

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!