New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add _PyObject_FastCallKeywords(): avoid the creation of a temporary dictionary for keyword arguments #72017
Comments
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: 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: 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 bpo-27810. |
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 ;-) |
New changeset 5ec1095612d6 by Victor Stinner in branch 'default': |
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. |
New changeset 0ff25676505d by Victor Stinner in branch 'default': |
New changeset ffcfa4f005a3 by Victor Stinner in branch 'default': |
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(). |
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] |
New changeset 119af59911a0 by Victor Stinner in branch 'default': |
New changeset 18d2d128cd1e by Victor Stinner in branch 'default': |
New changeset 1b7a5bdd05ed by Victor Stinner in branch 'default': |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: