classification
Title: Add _PyObject_FastCallKeywords(): avoid the creation of a temporary dictionary for keyword arguments
Type: performance Stage:
Components: Interpreter Core Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: haypo, martin.panter, python-dev, scoder, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2016-08-22 22:44 by haypo, last changed 2017-01-18 10:28 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
fastcall_keywords.patch haypo, 2016-08-22 23:30 review
do_call.patch haypo, 2016-08-24 22:44 review
Messages (11)
msg273408 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-08-22 22:44
In the issue #27809, I added support for keyword arguments to the function _PyObject_FastCallDict() (previously called _PyObject_FastCall). Keyword arguments are passed as a Python dictionary (dict).

_PyObject_FastCallDict() is efficient when keyword arguments are not used. But when keyword arguments are used, the creation of a temporary dictionary to pass keyword arguments can defeat the "fastcall" optimization for positional arguments.

See Serhiy Storchaka's comment:
http://bugs.python.org/issue27809#msg273367

Extract: "It may be more efficient to push values and names as continuous arrays (issue27213). PyArg_ParseTupleAndKeywords() accepts a tuple and a dict, but private function _PyArg_ParseTupleAndKeywordsFast() (issue27574) can be changed to accept positional and keyword arguments as continuous arrays: (int nargs, PyObject **args, int nkwargs, PyObject **kwnames, PyObject **kwargs)."

And Stefan Behnel's comment:
http://bugs.python.org/issue27809#msg273391

Serhiy proposed to pass keyword arguments as (key, value) pairs in C array. We can probably use the simple "stack" format used in Python/ceval.c:

   PyObject* _PyObject_FastCallKeywords(PyObject *func, PyObject **stack, int nargs, int nkwargs)

where nargs is number of positional arguments at the beginning of stack, and nkwargs is the number of (key, value) pairs at the end of stack.

This function would be the fundation for a new C calling convention: METH_FASTCALL, see the issue #27810.
msg273412 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-08-22 23:30
Here is a first patch:

* Add _PyObject_FastCallKeywords(): main entry point
* Add _PyCFunction_FastCallKeywords(): implementation for C function, but currently it's just an empty skeleton
* Rename fast_function() to PyFunction_FastCallKeywords() and ajust its API

In this first implementation, _PyObject_FastCallKeywords() has a full implementation for Python function, otherwise it only uses _PyObject_FastCallDict() fast path if the function has no keyword argument. In the last resort, it creates a temporary tuple and maybe also a temporary dict, to call PyObject_Call().

To test the patch, I modified do_call() to call _PyObject_FastCallKeywords(). I'm not sure that it makes sense in term of performance at this stage.

Serhiy: You can use this first implementation to experimental for fast argument parser for keyword arguments passed as (key, value) pairs ;-)
msg273606 - (view) Author: Roundup Robot (python-dev) Date: 2016-08-24 22:41
New changeset 5ec1095612d6 by Victor Stinner in branch 'default':
Add _PyObject_FastCallKeywords()
https://hg.python.org/cpython/rev/5ec1095612d6
msg273607 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-08-24 22:44
I pushed a change adding _PyObject_FastCallKeywords(), but this function is not used yet.

--

I didn't push the do_call() change: I attach a new (shorter) patch which changes it.

do_call.patch includes _PyCFunction_FastCallKeywords() for this line of Python/ceval.c:

+        C_TRACE(result, _PyCFunction_FastCallKeywords(func, stack, nargs, nkwargs));

I don't understand exactly the purpose of C_TRACE(), so I chose to keep it to avoid any risk of regression.
msg273611 - (view) Author: Roundup Robot (python-dev) Date: 2016-08-24 23:14
New changeset 0ff25676505d by Victor Stinner in branch 'default':
Issue #27830: Fix _PyObject_FastCallKeywords()
https://hg.python.org/cpython/rev/0ff25676505d
msg273677 - (view) Author: Roundup Robot (python-dev) Date: 2016-08-25 21:30
New changeset ffcfa4f005a3 by Victor Stinner in branch 'default':
Issue #27830: Revert, remove _PyFunction_FastCallKeywords()
https://hg.python.org/cpython/rev/ffcfa4f005a3
msg273678 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-08-25 21:33
> New changeset ffcfa4f005a3 by Victor Stinner in branch 'default':
> Issue #27830: Revert, remove _PyFunction_FastCallKeywords()
> https://hg.python.org/cpython/rev/ffcfa4f005a3

I'm sorry, I misunderstood Serhiy's comment. I understood that Serhiy proposed to reuse the current format "PyObject **stack, int nargs, int nkwargs", but in fact he proposes something new in the issue #27213.

IMO the issue #27213 can be a major enhancement for Python 3.6. I prefer to wait until the issue #27213 is implemented (that we agreed on the new format), before working again on _PyObject_FastCallKeywords().
msg273744 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-08-27 01:46
FYI revision ffcfa4f005a3 removed the code that uses your new _PyStack_AsDict() function, so now there is a compiler warning:

Objects/abstract.c:2313:1: warning: ‘_PyStack_AsDict’ defined but not used [-Wunused-function]
 _PyStack_AsDict(PyObject **stack, Py_ssize_t nkwargs, PyObject *func)
 ^~~~~~~~~~~~~~~
msg274419 - (view) Author: Roundup Robot (python-dev) Date: 2016-09-05 18:44
New changeset 119af59911a0 by Victor Stinner in branch 'default':
Issue #27830: Remove unused _PyStack_AsDict()
https://hg.python.org/cpython/rev/119af59911a0
msg275410 - (view) Author: Roundup Robot (python-dev) Date: 2016-09-09 19:50
New changeset 18d2d128cd1e by Victor Stinner in branch 'default':
Add _PyObject_FastCallKeywords()
https://hg.python.org/cpython/rev/18d2d128cd1e
msg285714 - (view) Author: Roundup Robot (python-dev) Date: 2017-01-18 10:28
New changeset 1b7a5bdd05ed by Victor Stinner in branch 'default':
_PyObject_FastCallKeywords() now checks the result
https://hg.python.org/cpython/rev/1b7a5bdd05ed
History
Date User Action Args
2017-01-18 10:28:05python-devsetmessages: + msg285714
2016-09-10 03:36:41hayposetstatus: open -> closed
dependencies: - Rework CALL_FUNCTION* opcodes
resolution: fixed
2016-09-09 19:50:29python-devsetmessages: + msg275410
2016-09-05 18:44:49python-devsetmessages: + msg274419
2016-08-27 01:46:49martin.pantersetnosy: + martin.panter
messages: + msg273744
2016-08-25 21:35:07haypolinkissue27810 dependencies
2016-08-25 21:34:54hayposetdependencies: + Rework CALL_FUNCTION* opcodes
2016-08-25 21:34:34hayposettitle: Add _PyObject_FastCallKeywords(): pass keyword arguments as (key, value) pairs -> Add _PyObject_FastCallKeywords(): avoid the creation of a temporary dictionary for keyword arguments
2016-08-25 21:33:29hayposetmessages: + msg273678
2016-08-25 21:30:41python-devsetmessages: + msg273677
2016-08-24 23:14:31python-devsetmessages: + msg273611
2016-08-24 22:44:15hayposetfiles: + do_call.patch

messages: + msg273607
2016-08-24 22:41:18python-devsetnosy: + python-dev
messages: + msg273606
2016-08-22 23:30:22hayposetfiles: + fastcall_keywords.patch
keywords: + patch
messages: + msg273412
2016-08-22 22:44:24haypocreate