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 METH_FASTCALL: new calling convention for C functions #71997
Comments
The issue bpo-27128 added _PyObject_FastCall() to avoid the creation of temporary tuples when calling functions. I propose to add a new METH_FASTCALL calling convention. The example using METH_VARARGS | METH_KEYWORDS:
PyObject* func(DirEntry *self, PyObject *args, PyObject *kwargs)
becomes:
PyObject* func(DirEntry *self, PyObject **args, int nargs, PyObject *kwargs) Using METH_VARARGS, args is a Python tuple. Using METH_FASTCALL, args is a C array of PyObject*, and there is a second nargs parameter. Later, Argument Clinic will be modified to *generate* code using the new METH_FASTCALL calling convention. Code written with Argument Clinic will only need to be updated by Argument Clinic to get the new faster calling convention (avoid the creation of a temporary tuple for positional arguments). This issue depends on the issue bpo-27809 "_PyObject_FastCall(): add support for keyword arguments". I will wait until this dependency is implemented, before working on the implementation of this part. For a full implementation, see my first attempt in the issue bpo-26814. I will extract the code from this branch to write a new patch. |
I agree that this would be cool. There is a tiny bit of a backwards compatibility concern as the new function signature would be incompatible with anything we had before, but I would guess that any code that chooses to bypass PyObject_Call() & friends would at least fall back to using it if it finds flags that it cannot handle. Cython code definitely does and always did, but there might be bugs. This change is the perfect way to "find" those. ;-) Anyway, Victor, I definitely appreciate your efforts in this direction. |
Right. If you call directly PyCFunction functions, you will likely get quickly a crash. But... who call directly PyCFunction functions? Why not using the 30+ functions to call functions? Hopefully, it's easy to support METH_FASTCALL in an existing function getting a tuple: int nargs = (int)PyTuple_GET_SIZE(args);
PyObject **stack = &PyTuple_GETITEM(args, 0);
result = func(self, stack, nargs, kwargs); I guess that Cython calls directly PyCFunction. cpyext module of PyPy probably too. Ok, except of them, are you aware of other projects doing that? |
Extensive callback interfaces like map() come to mind, where a large number of calls becomes excessively time critical and might thus have made people implement their own special purpose calling code. However, I don't know any such code (outside of Cython) and I agree that this is such a special optimisation that there can't be many cases. If there are others, they'll likely be using the obvious fallback already, or otherwise just adapt when Py3.6 comes out and move on. The fix isn't complex at all. I do not consider this a road block. |
I didn't patch map_next() of Python/bltinmodule.c nor list.sort() for use fast call yet. With my full patch, map() and list.sort() are *faster*, but even more changes are needed ;-) https://bugs.python.org/issue26814#msg263999
I'm now aware of map() or list.sort()/sorted() function rewritten from scratch for better performance, but I can imagine that someone did that. But do these specialized implementation use PyObject_Call() or "inline" PyCFunction_Call()? If someone "inlines" PyCFunction_Call(), be prepared to "backward incompatible changes", since it's common that CPython internals change in a subtle way.
I used GitHub to search for such code using "case METH_VARARGS". I found:
In short, I found nothing. But I didn't search far, maybe there are a few other code bases using "case METH_VARARGS:"? |
New changeset a25c39873d93 by Victor Stinner in branch 'default': |
fastcall.patch combines two changes: changeset: 103513:74abb8ddf7f2 Issue bpo-27810:
changeset: 103512:d55abcddd194 Issue bpo-27810: Add a new calling convention for C functions: PyObject* func(PyObject *self, PyObject **args,
Py_ssize_t nargs, PyObject *kwnames); Where args is a C array of positional arguments followed by values of keyword |
New changeset 86b0f5a900c7 by Victor Stinner in branch 'default': New changeset 633f850038c3 by Victor Stinner in branch 'default': New changeset 97a68adbe826 by Victor Stinner in branch 'default': |
New changeset 3934e070c9db by Victor Stinner in branch 'default': |
New changeset 603bef7bb744 by Serhiy Storchaka in branch 'default': |
Stefan Behnel: "There is a tiny bit of a backwards compatibility concern as the new function signature would be incompatible with anything we had before," Python 3.6 will probably have two "fast call" calling convention:
_PyObject_FastCallKeywods() is not really written to be called directly: Python/ceval.c calls you, but you may call _PyObject_FastCallKeywods() again "wrapper" functions, like functools.partial(). Currently, tp_call (and tp_init and tp_new) still expects a (tuple, dict) for positional and keyword arguments, but later I will add something to also support METH_FASTCALL for callable objects. I just don't know yet what is the best option to make this change. -- The main idea is implemented (implement METH_FASTCALL), I close the issue. I will open new issues for more specific changes, and maybe extend the API (especially tp_call). |
Could you please wrap "#define METH_FASTCALL 0x0080" with "#ifndef Py_LIMITED_API"/"#endif"? I think this method still is not stable, and we shouldn't encourage using it in third-party extensions. I would use two methods: just METH_FASTCALL and METH_FASTCALL|METH_KEYWORDS. The former is simpler and more stable, it takes just an array of positional parameters. The latter takes also keyword arguments, and the format of passing keyword arguments may be changed. |
New changeset 08a500e8b482 by Victor Stinner in branch 'default': |
Sorry, this is a mistake. I tried to exclude all new symbols related to fast call from the stable API. It should now be fixed. |
New changeset ecd218c41cd4 by Victor Stinner in branch 'default': |
Breakage due to the usage of borrowed references in _PyStack_UnpackDict(): bpo-36907 |
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: