You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[WIP] Add a new _PyObject_FastCall() function which avoids the creation of a tuple or dict for arguments #71001
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=Noneclosed_at=<Date2016-09-01.13:15:25.801>created_at=<Date2016-04-21.08:57:21.562>labels= ['performance']
title='[WIP] Add a new _PyObject_FastCall() function which avoids the creation of a tuple or dict for arguments'updated_at=<Date2016-09-01.13:15:25.800>user='https://github.com/vstinner'
The difference with PyObject_Call() is that its API avoids the creation of a tuple and a dictionary to pass parameters to functions when possible. Currently, the temporary tuple and dict can be avoided to call Python functions (nice, isn't it?) and C function declared with METH_O (not the most common API, but many functions are declared like that).
The patch only modifies property_descr_set() to test the feature, but I'm sure that *a lot* of C code can be modified to use this new function to beneift from its optimization.
Should we make this new _PyObject_CallStack() function official: call it PyObject_CallStack() (without the understand prefix) or experiment it in CPython 3.6 and decide later to make it public? If it's made private, it will require a large replacement patch later to replace all calls to _PyObject_CallStack() with PyObject_CallStack() (strip the underscore prefix).
The next step is to add a new METH_STACK flag to pass parameters to C functions using a similar API (PyObject **stack, int na, int nk) and modify the argument clinic to use this new API.
Thanks to Larry Hasting who gave me the idea in a previous edition of Pycon US ;-)
This issue was created after the discussion on issue bpo-26811 which is an issue in a micro-optimization in property_descr_set() to avoid the creation of a tuple: it caches a private tuple inside property_descr_set().
"Stack" in the function name looks a little confusing. I understand that this is related to the stack of bytecode interpreter, but this looks as raising pretty deep implementation detail. The way of packing positional and keyword arguments in the continuous array is not clear. Wouldn't be better to provide separate arguments for positional and keyword arguments?
What is the performance effect of using this function? For example compare the performance of namedtuple's attribute access of current code, the code with with this patch, and unoptimized code in 3.4:
./python -m timeit -r 11 -s "from collections import namedtuple as n; a = n('n', 'a b c')(1, 2, 3)" -- "a.a"
Is there any use of this function with keyword arguments?
./python -m timeit -r 11 -s "from collections import namedtuple as n; a = n('n', 'a b c')(1, 2, 3)" -- "a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a"
Python 3.6 unpatched: 0.968 usec
call_stack.patch: 1.27 usec
Python 3.6 with property_descr_get() of Python 3.4: 1.32 usec
"Python 3.6 with property_descr_get() of Python 3.4": replace the current optimization with "return PyObject_CallFunctionObjArgs(gs->prop_get, obj, NULL);".
Oh, in fact the tested code calls a property where the final function is operator.itemgetter(0). _PyObject_CallStack() creates a temporary tuple to call PyObject_Call() which calls func->ob_type->tp_call, itemgetter_call().
Problem: tp_call API uses (PyObject *args, PyObject *kwargs). It doesn't accept directly a stack (a C array of PyObject*). And it may be more difficult to modify tp_call.
In short, my patch disables the optimization on property with my current incomplete implementation.
call_stack-2.patch: A little bit more complete patch, it adds a tp_call_stack field to PyTypeObject an use it in _PyObject_CallStack().
Updated microbenchmark on Python 3.6, best of 3 runs:
./python -m timeit -r 11 -s "from collections import namedtuple as n; a = n('n', 'a b c')(1, 2, 3)" -- "a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a"
Python 3.6 unpatched: 0.968 usec
call_stack.patch: 1.27 usec
Python 3.6 with property_descr_get() of Python 3.4: 1.32 usec
call_stack-2.patch: 0.664 usec
call_stack-2.patch makes this micro-benchmark 31% faster, not bad! It also makes calls to C functions almost 2x as fast if you replace current unoptimized calls with _PyObject_CallStack()!!
IHMO we should continue to experiment, making function calls 2x faster is worth it ;-)
Serhiy: "See also bpo-23507. May be your function help to optimize filter(), map(), sorted()?"
IMHO the API is generic enough to be usable in a lot of cases.
Serhiy: "Is there any use of this function with keyword arguments?"
Calling functions with keywords is probably the least common case for function calls in C code. But I would like to provide a fast function to call with keywords. Maybe we need two functions just to make the API cleaner? The difference would just be that "int k" would be omitted?
I proposed an API (PyObject **stack, int na, int nk) based on the current code in Python/ceval.c. I'm not sure that it's the best API ever :-)
In fact, there is already PyObject_CallFunctionObjArgs() which can be modified to reuse internally _PyObject_CallStack(), and its API is maybe more convenient than my proposed API.
With call_stack-2.patch attribute access in namedtuple is only 25% slower than attribute access in ordinary Python object! Definitely this this worth to continue to experiment!
But adding new slot to PyTypeObject sets the bar too high. Try to use your function to speed up all cases mentioned in bpo-23507: sorted()/list.sort(), min() and max() with the key argument, filter(), map(), some iterators from itertools (groupby(), dropwhile(), takewhile(), accumulate(), filterfalse()), thin wrappers around special method (round(), math.floor(), etc). Use it in wrappers around PyObject_Call() like PyObject_CallFunctionObjArgs(). May be this will cause an effect even on some macrobenchmarks.
Yes, I've been working on a patch to do this as well. I called the calling convention METH_RAW, to go alongside METH_ZERO METH_O etc. My calling convention was exactly the same as yours: PyObject *(PyObject *o, PyObject **stack, int na, int nk). I only had to modify two functions in ceval.c to support it: ext_do_call() and call_function().
And yes, the overarching goal was to have Argument Clinic generate custom argument parsing code for every function. Supporting the calling convention was the easy part; generating code was quite complicated. I believe I got a very simple version of it working at one point, supporting positional parameters only, with some optional arguments. Parsing arguments by hand gets very complicated indeed when you introduce keyword arguments.
I haven't touched this patch in most of a year. I hope to return to it someday. In the meantime it's fine by me if you add support for this and rewrite some functions by hand to use it.
p.s. My last name has two S's. If you continue to leave off one of them, I shall remove one from yours, Mr. TINNER.
Since early microbenchmarks are promising, I wrote a more complete implementations which tries to use the fast-path (avoid temporary tuple/dict) in all PyObject_Call*() functions.
The next step would be to add a METH_FASTCALL flag. IMHO adding such new flag requires to enhance Argument Clinic to be able to use it, at least when a function doesn't accept keyword parameters.
PyObject_CallFunction() & friends have a weird API: if call with the format string "O", the behaviour depends if the object parameter is a tuple or not. If it's a tuple, the tuple is unpacked. It's a little bit weird. I recall that it led to a bug in the implementation in generators in Python: issue bpo-21209! Moreover, if the format string is "(...)", parenthesis are ignored. If you want to call a function with one argument which is a tuple, you have to write "((...))". It's a little bit weird, but we cannot change that without breaking the (Python) world :-)
call_stack-3.patch:
I renamed the main function to _PyObject_FastCall()
I added PyObject_CallNoArg(): call a function with no parameter
I added Py_VaBuildStack() and _Py_VaBuildStack_SizeT() helpers for PyObject_Call*() functions using a format string
I renamed the new slot to tp_fastcall
Nice change in the WITH_CLEANUP_START opcode (ceval.c):
/* XXX Not the fastest way to call it... \*/
res = PyObject_CallFunctionObjArgs(exit_func, exc, val, tb, NULL);
changed the title [-]Add a new _PyObject_CallStack() function which avoids the creation of a tuple or dict for arguments[/-][+]Add a new _PyObject_FastCall() function which avoids the creation of a tuple or dict for arguments[/+]on Apr 21, 2016
PyObject_Call*() implementations with _PyObject_FastCall() look much more complex than with PyObject_Call() (even not counting additional complex functions in modsupport.c). And I'm not sure there is a benefit. May be for first stage we can do without this.
I created a repository. I will work there and make some experiment. It would help to have a better idea of the concrete performance. When I will have a better view of all requires changes to get best performances everywhere, I will start a discussion to see which parts are worth it or not. In my latest microbenchmarks, functions calls (C/Python, mixed) are between 8% and 40% faster. I'm now running the CPython benchmark suite.
changed the title [-]Add a new _PyObject_FastCall() function which avoids the creation of a tuple or dict for arguments[/-][+][WIP] Add a new _PyObject_FastCall() function which avoids the creation of a tuple or dict for arguments[/+]on Apr 22, 2016
Changes of my current implementation, ad4a53ed1fbf.diff.
The good thing is that all changes are internals (really?). Even if you don't modify your C extensions (nor your Python code), you should benefit of the new fast call is *a lot* of cases.
IMHO the best tricky part are changes on the PyTypeObject. Is it ok to add a new tp_fastcall slot? Should we add even more slots using the fast call convention like tp_fastnew and tp_fastinit? How should we handle the inheritance of types with that?
_PyObject_FastCall() is the root of the new feature.
(*) type: add a new "tp_fastcall" field to the PyTypeObject structure.
It's unclear to me how inheritance is handled here. Maybe it's simply broken, but it's strange because it looks like it works :-) Maybe it's very rare that tp_call is overidden in a child class?
TODO: maybe reuse the "tp_call" field? (risk of major backward incompatibility...)
(*) slots: add a new "fastwrapper" field to the wrappercase structure. Add a fast wrapper to all slots (really all? i should check).
I don't think that consumers of the C API are of this change, or maybe only a few projects.
TODO: maybe remove "fastwrapper" and reuse the "wrapper" field? (low risk of backward compatibility?)
(*) Implement fast call for Python function (_PyFunction_FastCall) and C functions (PyCFunction_FastCall)
(*) Add a new METH_FASTCALL calling convention for C functions. Right now, it is used for 4 builtin functions: sorted(), getattr(), iter(), next().
Argument Clinic should be modified to emit C code using this new fast calling convention.
(*) Implement fast call in the following functions (types):
method()
method_descriptor()
wrapper_descriptor()
method_wrapper()
operator.itemgetter => used by collections.namedtuple to get an item by its name
() Modify PyObject_Call() functins to reuse internally the fast call. "tp_fastcall" is preferred over "tp_call" (FIXME: is it really useful to do that?).
The following functions are able to avoid temporary tuple/dict without having to modify the code calling them:
It's not required to modify code using these functions to use the 3 new shiny functions (PyObject_CallNoArg, PyObject_CallArg1, _PyObject_FastCall). For example, replacing PyObject_CallFunctionObjArgs(func, NULL) with PyObject_CallNoArg(func) is just a micro-optimization, the tuple is already avoided. But PyObject_CallNoArg() should use less memory of the C stack and be a "little bit" faster.
(*) Add new helpers: new Include/pystack.h file, Py_VaBuildStack(), etc.
Results of the CPython benchmark suite. Reference = default branch at rev 496e094f4734, patched: fastcall fork at rev 2b4b7def2949.
Oh, I forgot to mention that I modified perf.py to run each benchmark using 10 fresh processes to test multiple random seeds for the randomized hash function, instead of testing a fixed seed (PYTHONHASHSEED=1). This change should reduce the noise in the benchmark results.
I ran the benchmark suite using --rigorous.
I will open a new issue later for my perf.py change.
Could you repeat benchmarks on different computer? Better with different CPU or compiler.
Sorry, I don't really have the bandwith to repeat the benchmarks. PGO+LTO compilation is slow and running the benchmark suite in rigorous mode is very slow.
What do you expect from running the benchmark on a different computer?
Results look as a noise. Some tests become slower, others become faster. If results on different machine will show the same sets of slowing down and speeding up tests, this likely is not a noise.
I made progress on my FASTCALL branch. I removed tp_fastnew, tp_fastinit and
tp_fastnew fields from PyTypeObject to replace them with new type flags (ex:
Py_TPFLAGS_FASTNEW) to avoid code duplication and reduce the memory footprint.
Before, each function was simply duplicated.
This change introduces a backward incompatibility change: it's not more
possible to call directly tp_new, tp_init and tp_call. I don't know yet if such
change would be acceptable in Python 3.6, nor if it is worth it.
I spent a lot of ot time on the CPython benchmark suite to check for
performance regression. In fact, I spent most of my time to try to understand
why most benchmarks looked completly unstable. I now tuned correctly my
system and patched perf.py to get reliable benchmarks.
On the latest run of the benchmark suite, most benchmarks are faster! I have to investigate why 3 benchmarks are still slower. In the run, normal_startup was not significant, etree_parse was faster (instead of slower), but raytrace was already slower (but only 1.13x slower). It may be the "noise" of the PGO compilation. I already noticed that once: see the issue bpo-27056 "pickle: constant propagation in _Unpickler_Read()".
Result of the benchmark suite:
slower (3):
raytrace: 1.06x slower
etree_parse: 1.03x slower
normal_startup: 1.02x slower
faster (18):
unpickle_list: 1.11x faster
chameleon_v2: 1.09x faster
etree_generate: 1.08x faster
etree_process: 1.08x faster
mako_v2: 1.06x faster
call_method_unknown: 1.06x faster
django_v3: 1.05x faster
regex_compile: 1.05x faster
etree_iterparse: 1.05x faster
fastunpickle: 1.05x faster
meteor_contest: 1.05x faster
pickle_dict: 1.05x faster
float: 1.04x faster
pathlib: 1.04x faster
silent_logging: 1.04x faster
call_method: 1.03x faster
json_dump_v2: 1.03x faster
call_simple: 1.03x faster
not significant (21):
2to3
call_method_slots
chaos
fannkuch
fastpickle
formatted_logging
go
json_load
nbody
nqueens
pickle_list
pidigits
regex_effbot
regex_v8
richards
simple_logging
spectral_norm
startup_nosite
telco
tornado_http
unpack_sequence
I know that my patch is simply giant and cannot be merged like that.
Since the performance is still promising, I plan to split my giant
patch into smaller patches, easier to review. I will try to check that
individual patches don't make Python slower. This work will take time.
Status of the my FASTCALL implementation (34456cce64bb.patch):
Add METH_FASTCALL calling convention to C functions, similar
to METH_VARARGS|METH_KEYWORDS
Clinic uses METH_FASTCALL when possible (it may use METH_FASTCALL
for all cases in the future)
Add new C functions:
_PyObject_FastCall(func, stack, nargs, kwds): root of the FASTCALL branch
PyObject_CallNoArg(func)
PyObject_CallArg1(func, arg)
Add new type flags changing the calling conventions of tp_new, tp_init and
tp_call:
Py_TPFLAGS_FASTNEW
Py_TPFLAGS_FASTINIT
Py_TPFLAGS_FASTCALL
Backward incompatible change of Py_TPFLAGS_FASTNEW and Py_TPFLAGS_FASTINIT
flags: calling explicitly type->tp_new() and type->tp_init() is now a bug
and is likely to crash, since the calling convention can now be FASTCALL.
New _PyType_CallNew() and _PyType_CallInit() functions to call tp_new and
tp_init of a type. Functions which called tp_new and tp_init directly were
patched.
New helpers function to parse functions functions:
Many changes were done in the typeobject.c file to handle FASTCALL, new
type flags, handle correctly flags when a new type is created, etc.
ceval.c: add _PyFunction_FastCall() function (somehow, I only exposed
existing code)
A large part of the patch changes existing code to use the new calling
convention in many functions of many modules. Some changes were generated
by the Argument Clinic. IMHO the best would be to use Argument Clinic in more
places, rather than patching manually the code.
Hum, I recompiled the patched Python, again with PGO+LTO, and ran the same benchmark with the same command. In short, I replayed exaclty the same scenario. And... Only raytrace remains slower, etree_parse and normal_startup moved to the "not significant" list.
The difference in the benchmark result doesn't come from the benchmark. For example, I ran gain the normal_startup benchmark 3 times: I got the same result 3 times.
In short, I replayed exaclty the same scenario. And... Only raytrace remains slower, (...)
Oh, it looks like the reference binary calls the garbage collector less frequently than the patched python. In the patched Python, collections of the generation 2 are needed, whereas no collection of the generation 2 is needed on the reference binary.
This result was unfair: my fastcall branch contained the optimization of the issue bpo-27056. I just pushed this optimization into the default branch.
I ran again the benchmark: the result is now "not significant", as expected, since the benchmark is a microbenchmark testing C functions of Modules/_pickle.c, it doesn't really rely on the performance of (C or Python) functions calls.
I fixed even more issues with my setup to run benchmark. Results should be even more reliable. Moreover, I fixed multiple reference leaks in the code which introduced performance regressions. I started to write articles to explain how to run stable benchmarks:
I splitted the giant patch into smaller patches easier to review. The first part (_PyObject_FastCall, _PyObject_FastCallDict) is already merged. Other issues were opened to implement the full feature. I now close this issue.
Activity
vstinner commentedon Apr 21, 2016
Attached patch adds the following new function:
where na is the number of positional arguments and nk is the number of (key, pair) arguments stored in the stack.
Example of C code to call a function with one positional argument:
Simple, isn't it?
The difference with PyObject_Call() is that its API avoids the creation of a tuple and a dictionary to pass parameters to functions when possible. Currently, the temporary tuple and dict can be avoided to call Python functions (nice, isn't it?) and C function declared with METH_O (not the most common API, but many functions are declared like that).
The patch only modifies property_descr_set() to test the feature, but I'm sure that *a lot* of C code can be modified to use this new function to beneift from its optimization.
Should we make this new _PyObject_CallStack() function official: call it PyObject_CallStack() (without the understand prefix) or experiment it in CPython 3.6 and decide later to make it public? If it's made private, it will require a large replacement patch later to replace all calls to _PyObject_CallStack() with PyObject_CallStack() (strip the underscore prefix).
The next step is to add a new METH_STACK flag to pass parameters to C functions using a similar API (PyObject **stack, int na, int nk) and modify the argument clinic to use this new API.
Thanks to Larry Hasting who gave me the idea in a previous edition of Pycon US ;-)
This issue was created after the discussion on issue bpo-26811 which is an issue in a micro-optimization in property_descr_set() to avoid the creation of a tuple: it caches a private tuple inside property_descr_set().
serhiy-storchaka commentedon Apr 21, 2016
"Stack" in the function name looks a little confusing. I understand that this is related to the stack of bytecode interpreter, but this looks as raising pretty deep implementation detail. The way of packing positional and keyword arguments in the continuous array is not clear. Wouldn't be better to provide separate arguments for positional and keyword arguments?
What is the performance effect of using this function? For example compare the performance of namedtuple's attribute access of current code, the code with with this patch, and unoptimized code in 3.4:
Is there any use of this function with keyword arguments?
vstinner commentedon Apr 21, 2016
Microbenchmark on Python 3.6, best of 3 runs:
./python -m timeit -r 11 -s "from collections import namedtuple as n; a = n('n', 'a b c')(1, 2, 3)" -- "a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a"
"Python 3.6 with property_descr_get() of Python 3.4": replace the current optimization with "return PyObject_CallFunctionObjArgs(gs->prop_get, obj, NULL);".
Oh, in fact the tested code calls a property where the final function is operator.itemgetter(0). _PyObject_CallStack() creates a temporary tuple to call PyObject_Call() which calls func->ob_type->tp_call, itemgetter_call().
Problem: tp_call API uses (PyObject *args, PyObject *kwargs). It doesn't accept directly a stack (a C array of PyObject*). And it may be more difficult to modify tp_call.
In short, my patch disables the optimization on property with my current incomplete implementation.
serhiy-storchaka commentedon Apr 21, 2016
See also bpo-23507. May be your function help to optimize filter(), map(), sorted()?
vstinner commentedon Apr 21, 2016
call_stack-2.patch: A little bit more complete patch, it adds a tp_call_stack field to PyTypeObject an use it in _PyObject_CallStack().
Updated microbenchmark on Python 3.6, best of 3 runs:
./python -m timeit -r 11 -s "from collections import namedtuple as n; a = n('n', 'a b c')(1, 2, 3)" -- "a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a; a.a"
call_stack-2.patch makes this micro-benchmark 31% faster, not bad! It also makes calls to C functions almost 2x as fast if you replace current unoptimized calls with _PyObject_CallStack()!!
IHMO we should continue to experiment, making function calls 2x faster is worth it ;-)
Serhiy: "See also bpo-23507. May be your function help to optimize filter(), map(), sorted()?"
IMHO the API is generic enough to be usable in a lot of cases.
Serhiy: "Is there any use of this function with keyword arguments?"
Calling functions with keywords is probably the least common case for function calls in C code. But I would like to provide a fast function to call with keywords. Maybe we need two functions just to make the API cleaner? The difference would just be that "int k" would be omitted?
I proposed an API (PyObject **stack, int na, int nk) based on the current code in Python/ceval.c. I'm not sure that it's the best API ever :-)
In fact, there is already PyObject_CallFunctionObjArgs() which can be modified to reuse internally _PyObject_CallStack(), and its API is maybe more convenient than my proposed API.
serhiy-storchaka commentedon Apr 21, 2016
With call_stack-2.patch attribute access in namedtuple is only 25% slower than attribute access in ordinary Python object! Definitely this this worth to continue to experiment!
But adding new slot to PyTypeObject sets the bar too high. Try to use your function to speed up all cases mentioned in bpo-23507: sorted()/list.sort(), min() and max() with the key argument, filter(), map(), some iterators from itertools (groupby(), dropwhile(), takewhile(), accumulate(), filterfalse()), thin wrappers around special method (round(), math.floor(), etc). Use it in wrappers around PyObject_Call() like PyObject_CallFunctionObjArgs(). May be this will cause an effect even on some macrobenchmarks.
larryhastings commentedon Apr 21, 2016
Yes, I've been working on a patch to do this as well. I called the calling convention METH_RAW, to go alongside METH_ZERO METH_O etc. My calling convention was exactly the same as yours: PyObject *(PyObject *o, PyObject **stack, int na, int nk). I only had to modify two functions in ceval.c to support it: ext_do_call() and call_function().
And yes, the overarching goal was to have Argument Clinic generate custom argument parsing code for every function. Supporting the calling convention was the easy part; generating code was quite complicated. I believe I got a very simple version of it working at one point, supporting positional parameters only, with some optional arguments. Parsing arguments by hand gets very complicated indeed when you introduce keyword arguments.
I haven't touched this patch in most of a year. I hope to return to it someday. In the meantime it's fine by me if you add support for this and rewrite some functions by hand to use it.
p.s. My last name has two S's. If you continue to leave off one of them, I shall remove one from yours, Mr. TINNER.
vstinner commentedon Apr 21, 2016
Since early microbenchmarks are promising, I wrote a more complete implementations which tries to use the fast-path (avoid temporary tuple/dict) in all PyObject_Call*() functions.
The next step would be to add a METH_FASTCALL flag. IMHO adding such new flag requires to enhance Argument Clinic to be able to use it, at least when a function doesn't accept keyword parameters.
PyObject_CallFunction() & friends have a weird API: if call with the format string "O", the behaviour depends if the object parameter is a tuple or not. If it's a tuple, the tuple is unpacked. It's a little bit weird. I recall that it led to a bug in the implementation in generators in Python: issue bpo-21209! Moreover, if the format string is "(...)", parenthesis are ignored. If you want to call a function with one argument which is a tuple, you have to write "((...))". It's a little bit weird, but we cannot change that without breaking the (Python) world :-)
call_stack-3.patch:
Nice change in the WITH_CLEANUP_START opcode (ceval.c):
+ arg_stack[0] = exc;
+ arg_stack[1] = val;
+ arg_stack[2] = tb;
+ res = _PyObject_FastCall(exit_func, arg_stack, 3, 0);
I don't know if it's a common byetcode, nor if the change is really faster.
[-]Add a new _PyObject_CallStack() function which avoids the creation of a tuple or dict for arguments[/-][+]Add a new _PyObject_FastCall() function which avoids the creation of a tuple or dict for arguments[/+]vstinner commentedon Apr 21, 2016
Yeah, that would be a nice first step.
Ooops, I'm sorry Guido Hastings :-(
serhiy-storchaka commentedon Apr 21, 2016
PyObject_Call*() implementations with _PyObject_FastCall() look much more complex than with PyObject_Call() (even not counting additional complex functions in modsupport.c). And I'm not sure there is a benefit. May be for first stage we can do without this.
vstinner commentedon Apr 22, 2016
I created a repository. I will work there and make some experiment. It would help to have a better idea of the concrete performance. When I will have a better view of all requires changes to get best performances everywhere, I will start a discussion to see which parts are worth it or not. In my latest microbenchmarks, functions calls (C/Python, mixed) are between 8% and 40% faster. I'm now running the CPython benchmark suite.
[-]Add a new _PyObject_FastCall() function which avoids the creation of a tuple or dict for arguments[/-][+][WIP] Add a new _PyObject_FastCall() function which avoids the creation of a tuple or dict for arguments[/+]vstinner commentedon Apr 22, 2016
Changes of my current implementation, ad4a53ed1fbf.diff.
The good thing is that all changes are internals (really?). Even if you don't modify your C extensions (nor your Python code), you should benefit of the new fast call is *a lot* of cases.
IMHO the best tricky part are changes on the PyTypeObject. Is it ok to add a new tp_fastcall slot? Should we add even more slots using the fast call convention like tp_fastnew and tp_fastinit? How should we handle the inheritance of types with that?
(*) Add 2 new public functions:
(*) Add 1 new private function:
_PyObject_FastCall() is the root of the new feature.
(*) type: add a new "tp_fastcall" field to the PyTypeObject structure.
It's unclear to me how inheritance is handled here. Maybe it's simply broken, but it's strange because it looks like it works :-) Maybe it's very rare that tp_call is overidden in a child class?
TODO: maybe reuse the "tp_call" field? (risk of major backward incompatibility...)
(*) slots: add a new "fastwrapper" field to the wrappercase structure. Add a fast wrapper to all slots (really all? i should check).
I don't think that consumers of the C API are of this change, or maybe only a few projects.
TODO: maybe remove "fastwrapper" and reuse the "wrapper" field? (low risk of backward compatibility?)
(*) Implement fast call for Python function (_PyFunction_FastCall) and C functions (PyCFunction_FastCall)
(*) Add a new METH_FASTCALL calling convention for C functions. Right now, it is used for 4 builtin functions: sorted(), getattr(), iter(), next().
Argument Clinic should be modified to emit C code using this new fast calling convention.
(*) Implement fast call in the following functions (types):
() Modify PyObject_Call() functins to reuse internally the fast call. "tp_fastcall" is preferred over "tp_call" (FIXME: is it really useful to do that?).
The following functions are able to avoid temporary tuple/dict without having to modify the code calling them:
It's not required to modify code using these functions to use the 3 new shiny functions (PyObject_CallNoArg, PyObject_CallArg1, _PyObject_FastCall). For example, replacing PyObject_CallFunctionObjArgs(func, NULL) with PyObject_CallNoArg(func) is just a micro-optimization, the tuple is already avoided. But PyObject_CallNoArg() should use less memory of the C stack and be a "little bit" faster.
(*) Add new helpers: new Include/pystack.h file, Py_VaBuildStack(), etc.
Please ignore unrelated changes.
9 remaining items
vstinner commentedon Apr 29, 2016
Oh, I forgot to mention that I modified perf.py to run each benchmark using 10 fresh processes to test multiple random seeds for the randomized hash function, instead of testing a fixed seed (PYTHONHASHSEED=1). This change should reduce the noise in the benchmark results.
I ran the benchmark suite using --rigorous.
I will open a new issue later for my perf.py change.
serhiy-storchaka commentedon Apr 29, 2016
Could you repeat benchmarks on different computer? Better with different CPU or compiler.
vstinner commentedon Apr 29, 2016
Sorry, I don't really have the bandwith to repeat the benchmarks. PGO+LTO compilation is slow and running the benchmark suite in rigorous mode is very slow.
What do you expect from running the benchmark on a different computer?
serhiy-storchaka commentedon Apr 29, 2016
Results look as a noise. Some tests become slower, others become faster. If results on different machine will show the same sets of slowing down and speeding up tests, this likely is not a noise.
vstinner commentedon Apr 29, 2016
As I wrote, it's really hard to get a reliable benchmark result. I did my best.
See also discussions about the CPython benchmark suite on the speed list:
https://mail.python.org/pipermail/speed/
I'm not sure that you will get less noise on other computers. IMHO many benchmarks are simply "broken" (not reliable).
vstinner commentedon May 19, 2016
Hi,
I made progress on my FASTCALL branch. I removed tp_fastnew, tp_fastinit and
tp_fastnew fields from PyTypeObject to replace them with new type flags (ex:
Py_TPFLAGS_FASTNEW) to avoid code duplication and reduce the memory footprint.
Before, each function was simply duplicated.
This change introduces a backward incompatibility change: it's not more
possible to call directly tp_new, tp_init and tp_call. I don't know yet if such
change would be acceptable in Python 3.6, nor if it is worth it.
I spent a lot of ot time on the CPython benchmark suite to check for
performance regression. In fact, I spent most of my time to try to understand
why most benchmarks looked completly unstable. I now tuned correctly my
system and patched perf.py to get reliable benchmarks.
On the latest run of the benchmark suite, most benchmarks are faster! I have to investigate why 3 benchmarks are still slower. In the run, normal_startup was not significant, etree_parse was faster (instead of slower), but raytrace was already slower (but only 1.13x slower). It may be the "noise" of the PGO compilation. I already noticed that once: see the issue bpo-27056 "pickle: constant propagation in _Unpickler_Read()".
Result of the benchmark suite:
slower (3):
faster (18):
not significant (21):
I know that my patch is simply giant and cannot be merged like that.
Since the performance is still promising, I plan to split my giant
patch into smaller patches, easier to review. I will try to check that
individual patches don't make Python slower. This work will take time.
vstinner commentedon May 19, 2016
New patch: 34456cce64bb.patch
b/Modules/clinic/pyexpat.c.h | 32
b/Modules/clinic/sha1module.c.h | 8
b/Modules/clinic/sha256module.c.h | 14
b/Modules/clinic/sha512module.c.h | 14
b/Modules/clinic/signalmodule.c.h | 50
b/Modules/clinic/unicodedata.c.h | 42
b/Modules/clinic/zlibmodule.c.h | 68 -
b/Modules/itertoolsmodule.c | 20
b/Modules/main.c | 2
b/Modules/pyexpat.c | 3
b/Modules/signalmodule.c | 9
b/Modules/xxsubtype.c | 4
b/Objects/abstract.c | 403 ++++---
b/Objects/bytesobject.c | 2
b/Objects/classobject.c | 36
b/Objects/clinic/bytearrayobject.c.h | 90 -
b/Objects/clinic/bytesobject.c.h | 66 -
b/Objects/clinic/dictobject.c.h | 10
b/Objects/clinic/unicodeobject.c.h | 10
b/Objects/descrobject.c | 162 +-
b/Objects/dictobject.c | 26
b/Objects/enumobject.c | 8
b/Objects/exceptions.c | 91 +
b/Objects/fileobject.c | 29
b/Objects/floatobject.c | 25
b/Objects/funcobject.c | 77 -
b/Objects/genobject.c | 2
b/Objects/iterobject.c | 6
b/Objects/listobject.c | 20
b/Objects/longobject.c | 40
b/Objects/methodobject.c | 139 ++
b/Objects/object.c | 4
b/Objects/odictobject.c | 2
b/Objects/rangeobject.c | 12
b/Objects/tupleobject.c | 21
b/Objects/typeobject.c | 1463 +++++++++++++++++++-------
b/Objects/unicodeobject.c | 58 -
b/Objects/weakrefobject.c | 22
b/PC/clinic/msvcrtmodule.c.h | 42
b/PC/clinic/winreg.c.h | 128 +-
b/PC/clinic/winsound.c.h | 26
b/PCbuild/pythoncore.vcxproj | 4
b/Parser/tokenizer.c | 7
b/Python/ast.c | 31
b/Python/bltinmodule.c | 173 +--
b/Python/ceval.c | 591 +++++++++-
b/Python/clinic/bltinmodule.c.h | 104 +
b/Python/clinic/import.c.h | 18
b/Python/codecs.c | 17
b/Python/errors.c | 105 -
b/Python/getargs.c | 284 ++++-
b/Python/import.c | 27
b/Python/modsupport.c | 244 +++-
b/Python/pythonrun.c | 10
b/Python/sysmodule.c | 32
b/Tools/clinic/clinic.py | 115 +-
pystack.c | 288 +++++
pystack.h | 64 +
121 files changed, 5420 insertions(+), 2802 deletions(-)
vstinner commentedon May 19, 2016
Status of the my FASTCALL implementation (34456cce64bb.patch):
Add METH_FASTCALL calling convention to C functions, similar
to METH_VARARGS|METH_KEYWORDS
Clinic uses METH_FASTCALL when possible (it may use METH_FASTCALL
for all cases in the future)
Add new C functions:
Add new type flags changing the calling conventions of tp_new, tp_init and
tp_call:
Backward incompatible change of Py_TPFLAGS_FASTNEW and Py_TPFLAGS_FASTINIT
flags: calling explicitly type->tp_new() and type->tp_init() is now a bug
and is likely to crash, since the calling convention can now be FASTCALL.
New _PyType_CallNew() and _PyType_CallInit() functions to call tp_new and
tp_init of a type. Functions which called tp_new and tp_init directly were
patched.
New helpers function to parse functions functions:
New Py_Build functons:
New _PyStack API to handle a stack:
Many changes were done in the typeobject.c file to handle FASTCALL, new
type flags, handle correctly flags when a new type is created, etc.
ceval.c: add _PyFunction_FastCall() function (somehow, I only exposed
existing code)
A large part of the patch changes existing code to use the new calling
convention in many functions of many modules. Some changes were generated
by the Argument Clinic. IMHO the best would be to use Argument Clinic in more
places, rather than patching manually the code.
vstinner commentedon May 19, 2016
Hum, I recompiled the patched Python, again with PGO+LTO, and ran the same benchmark with the same command. In short, I replayed exaclty the same scenario. And... Only raytrace remains slower, etree_parse and normal_startup moved to the "not significant" list.
The difference in the benchmark result doesn't come from the benchmark. For example, I ran gain the normal_startup benchmark 3 times: I got the same result 3 times.
### normal_startup ###
Avg: 0.295168 +/- 0.000991 -> 0.294926 +/- 0.00048: 1.00x faster
Not significant
### normal_startup ###
Avg: 0.294871 +/- 0.000606 -> 0.294883 +/- 0.00072: 1.00x slower
Not significant
### normal_startup ###
Avg: 0.295096 +/- 0.000706 -> 0.294967 +/- 0.00068: 1.00x faster
Not significant
IMHO the difference comes from the data collected by PGO.
vstinner commentedon May 19, 2016
Oh, it looks like the reference binary calls the garbage collector less frequently than the patched python. In the patched Python, collections of the generation 2 are needed, whereas no collection of the generation 2 is needed on the reference binary.
vstinner commentedon May 20, 2016
This result was unfair: my fastcall branch contained the optimization of the issue bpo-27056. I just pushed this optimization into the default branch.
I ran again the benchmark: the result is now "not significant", as expected, since the benchmark is a microbenchmark testing C functions of Modules/_pickle.c, it doesn't really rely on the performance of (C or Python) functions calls.
vstinner commentedon May 25, 2016
I fixed even more issues with my setup to run benchmark. Results should be even more reliable. Moreover, I fixed multiple reference leaks in the code which introduced performance regressions. I started to write articles to explain how to run stable benchmarks:
Summary of benchmarks at the revision e6f3bf996c01:
Faster (25):
Slower (4):
Not significat (13):
I'm now investigating why 4 benchmarks are slower.
Note: I'm still using my patched CPython benchmark suite to get more stable benchmark. I will send patches upstream later.
vstinner commentedon Sep 1, 2016
I splitted the giant patch into smaller patches easier to review. The first part (_PyObject_FastCall, _PyObject_FastCallDict) is already merged. Other issues were opened to implement the full feature. I now close this issue.