[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: > I'm willing Object/call.c solves such placement issue. I also *hope* that a call.c file would *help* a little bit, but I'm not sure that it will fix *all* code placement issues. I created the issue #29524 with a patch creating Objects/call.c. --

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

INADA Naoki added the comment: Yes, I used --enable-optimization this time. But my patch is not good for branch prediction of my CPU in this time. I'm willing Object/call.c solves such placement issue. BTW, since benefit of GetMethod is small, how about this? * Add _PyMethod_FastCallKeywords *

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: > performance benefit is small. > https://gist.github.com/methane/32fe57cd4aaac1c5c37f75cbbfbe7562 Are you using PGO+LTO compilation? Without PGO, the noise of code placement can be too high. In your "perf stat" comparisons, I see that "insn per cycle" is

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: >> I'm not sure about the change on PyObject_CallMethod*() only for empty >> format string. > > There are many place using _PyObject_CallMethodId() to call method without > args. I'm more interested by an optimization PyObject_CallMethod*() for any number of

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

INADA Naoki added the comment: performance benefit is small. https://gist.github.com/methane/32fe57cd4aaac1c5c37f75cbbfbe7562 -- Added file: http://bugs.python.org/file46619/callmethod3.patch ___ Python tracker

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

INADA Naoki added the comment: > I'm not sure about the change on PyObject_CallMethod*() only for empty format > string. There are many place using _PyObject_CallMethodId() to call method without args. Maybe, we should recommend to use _PyObject_CallMethodIdObjArgs() when no arguments, and

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

Roundup Robot added the comment: New changeset b1dc6b6d5fa20f63f9651df2e7986a066c88ff7d by Victor Stinner in branch 'master': Issue #29507: Fix _PyObject_CallFunctionVa() https://github.com/python/cpython/commit/b1dc6b6d5fa20f63f9651df2e7986a066c88ff7d --

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: callmethod2.patch: I like that change on object_vacall(), I'm not sure about the change on PyObject_CallMethod*() only for empty format string. I suggest to split your patch into two parts, and first focus on object_vacall(). Do you have a benchmark for this

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

Roundup Robot added the comment: New changeset e5cd74868dfc by Victor Stinner in branch 'default': Issue #29507: Fix _PyObject_CallFunctionVa() https://hg.python.org/cpython/rev/e5cd74868dfc -- ___ Python tracker

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

INADA Naoki added the comment: Thanks for finishing my draft patch, Victor. callmetohd2.patch is same trick for PyObject_CallMethod* APIs in abstract.c. It fixes segv in callmethod.patch. And APIs receiving format string can do same trick when format is empty too. As I grepping

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: I checked typeobject.c: there is a single case where we use the result of lookup_maybe_method()/lookup_method() for something else than calling the unbound method: slot_tp_finalize() calls PyErr_WriteUnraisable(del), the case discussed in my previous message

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: Oh, I was too lazy to run the full test suite, I only ran a subset and I was bitten by buildbots :-) test_unraisable() of test_exceptions fails. IHMO the BrokenRepr subtest on this test function is really implementation specific. To fix buildbots, I removed

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

Roundup Robot added the comment: New changeset be663c9a9e24 by Victor Stinner in branch 'default': Issue #29507: Update test_exceptions https://hg.python.org/cpython/rev/be663c9a9e24 -- ___ Python tracker

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

Yury Selivanov added the comment: patch looks good to me. -- ___ Python tracker ___ ___ Python-bugs-list

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: Naoki: "method_fastcall4.patch looks clean enough, and performance benefit seems nice." Ok, I pushed the patch with minor changes: * replace "variants:" with "Variants:" * rename lookup_maybe_unbound() to lookup_maybe_method() * rename lookup_method_unbound()

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: Raymond Hettinger: "+1 Though this is a rather large and impactful patch, I think it is a great idea. It will be one of the highest payoff applications of FASTCALL, broadly benefitting a lot of code." In my experience, avoiding temporary tuple to pass

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

Roundup Robot added the comment: New changeset 7b8df4a5d81d by Victor Stinner in branch 'default': Optimize slots: avoid temporary PyMethodObject https://hg.python.org/cpython/rev/7b8df4a5d81d -- nosy: +python-dev ___ Python tracker

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

Raymond Hettinger added the comment: +1 Though this is a rather large and impactful patch, I think it is a great idea. It will be one of the highest payoff applications of FASTCALL, broadly benefitting a lot of code. Let's be sure to be extra careful with this one because it is touching

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: method_fastcall4.patch benchmark results. It's not the first time that I notice that fannkuch and nbody benchmarks become slower. I guess that it's effect of changing code placement because of unrelated change in the C code. Results don't seem significant on

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

INADA Naoki added the comment: method_fastcall4.patch looks clean enough, and performance benefit seems nice. I don't know current test suite covers unusual special methods. Maybe, we can extend test_class to cover !unbound (e.g. @classmethod) case. --

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

Changes by STINNER Victor : Added file: http://bugs.python.org/file46612/bench.py ___ Python tracker ___

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: method_fastcall4.patch: Based on method_fastcall3.patch, I just added call_unbound() and call_unbound_noarg() helper functions to factorize code. I also modified mro_invoke() to be able to remove lookup_method(). I confirm the speedup with attached bench.py:

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

INADA Naoki added the comment: method_fastcall3.patch implement the trick in more general way. (I haven't ran test yet since it's midnight. I'll post result later.) -- Added file: http://bugs.python.org/file46610/method_fastcall3.patch ___ Python

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

Changes by STINNER Victor : -- nosy: +yselivanov ___ Python tracker ___ ___

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: > method_fastcall2.patch is tuning same function (call_method() in > typeobject.c), and uses trick to bypass temporary method object (same to > _PyObject_GetMethod()). Oh, great idea! That's why I put you in the nosy list ;-) You know better than me this

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

INADA Naoki added the comment: I'm sorry, callmethod.patch is tuned other place, and causing SEGV. method_fastcall2.patch is tuning same function (call_method() in typeobject.c), and uses trick to bypass temporary method object (same to _PyObject_GetMethod()). $ ./python -m perf timeit

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

INADA Naoki added the comment: callmethod.patch: + ../python.default -m perf compare_to default.json patched2.json -G --min-speed=1 Slower (5): - logging_silent: 717 ns +- 9 ns -> 737 ns +- 8 ns: 1.03x slower (+3%) - fannkuch: 1.04 sec +- 0.01 sec -> 1.06 sec +- 0.02 sec: 1.02x slower (+2%) -

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

INADA Naoki added the comment: > But PyObject_CallMethod(), _PyObject_CallMethodId(), > PyObject_CallMethodObjArgs(), _PyObject_CallMethodIdObjArgs() can use it too. CallMethod[Id]ObjArgs() can use it easily. But format support is not so easy. -- Added file:

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

INADA Naoki added the comment: Maybe, we can skip Method object entirely using _PyObject_GetMethod(). Currently it is used only in LOAD_METHOD. But PyObject_CallMethod(), _PyObject_CallMethodId(), PyObject_CallMethodObjArgs(), _PyObject_CallMethodIdObjArgs() can use it too. --

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

STINNER Victor added the comment: Maybe PyObject_Call(), _PyObject_FastCallDict(), etc. can also be modified to get the following fast-path: +if (Py_TYPE(func) == _Type) { +result = _PyMethod_FastCall(func, args, nargs); +} But I don't know how common it is to get a

[issue29507] Use FASTCALL in call_method() to avoid temporary tuple

New submission from STINNER Victor: Subset of the (almost) rejected issue #29259 (tp_fastcall), attached patch adds _PyMethod_FastCall() and uses it in call_method() of typeobject.c. The change avoids the creation of a temporary tuple for Python functions and METH_FASTCALL C functions.