Repository navigation
Add _PyObject_FastCallKeywords(): avoid the creation of a temporary dictionary for keyword arguments #72017
Description
| BPO | 27830 |
|---|---|
| Nosy | @scoder, @vstinner, @vadmium, @serhiy-storchaka |
| Files |
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 2016-09-10.03:36:41.951>
created_at = <Date 2016-08-22.22:44:24.153>
labels = ['interpreter-core', 'performance']
title = 'Add _PyObject_FastCallKeywords(): avoid the creation of a temporary dictionary for keyword arguments'
updated_at = <Date 2017-01-18.10:28:05.257>
user = 'https://github.com/vstinner'bugs.python.org fields:
activity = <Date 2017-01-18.10:28:05.257>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2016-09-10.03:36:41.951>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2016-08-22.22:44:24.153>
creator = 'vstinner'
dependencies = []
files = ['44191', '44215']
hgrepos = []
issue_num = 27830
keywords = ['patch']
message_count = 11.0
messages = ['273408', '273412', '273606', '273607', '273611', '273677', '273678', '273744', '274419', '275410', '285714']
nosy_count = 5.0
nosy_names = ['scoder', 'vstinner', 'python-dev', 'martin.panter', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue27830'
versions = ['Python 3.6']
Activity
vstinner commentedon Aug 22, 2016
In the issue bpo-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 (bpo-27213). PyArg_ParseTupleAndKeywords() accepts a tuple and a dict, but private function _PyArg_ParseTupleAndKeywordsFast() (bpo-27574) 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:
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 bpo-27810.
vstinner commentedon Aug 22, 2016
Here is a first patch:
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 ;-)
python-dev commentedon Aug 24, 2016
New changeset 5ec1095612d6 by Victor Stinner in branch 'default':
Add _PyObject_FastCallKeywords()
https://hg.python.org/cpython/rev/5ec1095612d6
vstinner commentedon Aug 24, 2016
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.
python-dev commentedon Aug 24, 2016
New changeset 0ff25676505d by Victor Stinner in branch 'default':
Issue bpo-27830: Fix _PyObject_FastCallKeywords()
https://hg.python.org/cpython/rev/0ff25676505d
python-dev commentedon Aug 25, 2016
New changeset ffcfa4f005a3 by Victor Stinner in branch 'default':
Issue bpo-27830: Revert, remove _PyFunction_FastCallKeywords()
https://hg.python.org/cpython/rev/ffcfa4f005a3
vstinner commentedon Aug 25, 2016
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 bpo-27213.
IMO the issue bpo-27213 can be a major enhancement for Python 3.6. I prefer to wait until the issue bpo-27213 is implemented (that we agreed on the new format), before working again on _PyObject_FastCallKeywords().
[-]Add _PyObject_FastCallKeywords(): pass keyword arguments as (key, value) pairs[/-][+]Add _PyObject_FastCallKeywords(): avoid the creation of a temporary dictionary for keyword arguments[/+]vadmium commentedon Aug 27, 2016
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)
^~~~~~~~~~~~~~~
python-dev commentedon Sep 5, 2016
New changeset 119af59911a0 by Victor Stinner in branch 'default':
Issue bpo-27830: Remove unused _PyStack_AsDict()
https://hg.python.org/cpython/rev/119af59911a0
python-dev commentedon Sep 9, 2016
New changeset 18d2d128cd1e by Victor Stinner in branch 'default':
Add _PyObject_FastCallKeywords()
https://hg.python.org/cpython/rev/18d2d128cd1e
python-dev commentedon Jan 18, 2017
New changeset 1b7a5bdd05ed by Victor Stinner in branch 'default':
_PyObject_FastCallKeywords() now checks the result
https://hg.python.org/cpython/rev/1b7a5bdd05ed