Repository navigation
Recommend PyObject_Call* APIs over PyEval_Call*() APIs #73734
Description
Activity
As reading call.c, PyEval_Call* APIs are mostly duplicate of PyObject_Call* APIs.
While they are not documented, they seems part of limited APIs.
So we can't remove them easily. Let's deprecate them for now.Please note that the two sets of APIs are not identical, e.g. you cannot simply replace PyEval_CallObject() with PyObject_Call(), since the former applies a few extra checks and defaults, which the latter doesn't.
Thanks, Lemburg to pointing it out.
Here is detail of the difference.## PyEval_CallFunction(), PyEval_CallMethod()
They are very similar to PyObject_CallFunction() and PyObject_CallMethod(). difference are:
- PyEval_Call...() doesn't respect Py_SSIZE_T_CLEAN
- PyObject_Call... has following special case. PyEval_CallFunction(callable, "i", (int)i) will raise TypeError("keyword list must be a tuple") and PyObject_CallFunction(callable, "i", (int)i) calls
callable(i)
if (nargs == 1 && PyTuple_Check(stack[0])) { /* Special cases for backward compatibility: - PyObject_CallFunction(func, "O", tuple) calls func(*tuple) - PyObject_CallFunction(func, "(OOO)", arg1, arg2, arg3) calls func(*(arg1, arg2, arg3)): func(arg1, arg2, arg3) */ PyObject *args = stack[0]; result = _PyObject_FastCall(callable, &PyTuple_GET_ITEM(args, 0), PyTuple_GET_SIZE(args)); }
PyEval_CallFunction is not called from Python source tree.
PyEval_CallMethod has only one caller in tree and format string is "(Oi)". It can be replaced with PyObject_CallMethod safely.## PyEval_CallObject(), PyEval_CallObjectWithKeywords()
PyEval_CallObject() is a just macro calling PyEval_CallObjectWithKeywords() (call it COWK later).
PyEval_CallObject() is identical to PyObject_CallObject(). Only difference is it's a macro or function.COWK is similar to PyObject_Call(), but COWK raise TypeError when args is not a tuple or kwds is not a dictionary and PyObject_Call() uses assert.
There are only two caller of PyEval_CallObjectWithKeywords() other than PyEval_CallObject and PyObject_CallObject.
One is tp_call of weakcallableproxy. Type of kwargs is checked before calling tp_call.
Another is in threading (boot->keyw). The threading module checks it's a dict.
So replacing them to PyObject_CallObject() is safe.----
While they are complex API, there are few (or no) callers in Python tree.
It's too hard to maintain. Actually, I found regression of COWK in Python 3.6.It calls _PyObject_FastCallDict() when args is NULL. If kwargs is not dict, it can crash instead of raising TypeError.
See also bpo-11165
See also bpo-485165, bpo-8276 and bpo-11173.
Thanks, but you missed the main difference:
The argument tuple can be NULL for PyEval_CallObject() (it then gets replaced with an empty tuple), but will raise a segfault for PyObject_Call().
Now, apart from looking at the use cases in the core, you also have to check whether you are removing useful functionality when deprecating APIs. Code in third party extensions may not be that easy to adapt to the PyObject_Call*() APIs, since they lack several checks which the PyEval_Call*() APIs apply.
Deprecation of existing published APIs is only an option in case there are other APIs which can reasonably replace them and those other APIs would have to have been around for a while, since otherwise third party code would have to provide wrappers for Python versions which don't supply these APIs or only ones which do not implement the extra functionality.
E.g. if you now add the extra support for args being NULL to PyObject_Call() in Python 3.7, third party code would have to either switch being using PyEval_CallObject() and PyObject_Call() depending on Python version or provide its own wrappers around the two APIs depending on Python version.
Since calling objects is rather common in Python extensions, special care has to be taken.
It would have been better to add the special case for args == NULL and the extra types checks to PyObject_Call() a long long time ago, since it's the only API that allows calling an object with both args and kwargs. Alas, didn't happen, so we have to live with it.
It may actually be better to add a new API PyObject_CallObjectWithKeywords() which works like the PyEval_CallObjectWithKeywords() API and then deprecate the PyEval_COWK() API. Third party code can then use a simple macro to provide a backwards compatibility shim for older Python versions.
The argument tuple can be NULL for PyEval_CallObject() (it then gets replaced with an empty tuple), but will raise a segfault for PyObject_Call().
PyObject_CallObject() accepts NULL as args. Macro vs function is only difference of them.
On the other hand, PyObject_CallObjectWithKeyword() doesn't have identical function.
So I agree your suggestion to add NULL support to PyObject_Call().We have more fast _PyObject_FastCall* APIs for performance critical code.
Additional cost to check NULL in PyObject_Call() would be negligible.Any opinion from others?
Looking through Python's history, it's interesting that PyObject_Call() did apply the args == NULL checks up until Python 2.1.
In Python 2.2 this was replaced by a direct call to tp_call, without the checks. However, the tp_call slots don't do this check as you can see in function_call() of function objects. And indeed, the documentation of PyObject_Call() was changed in that version as well to disallow args == NULL.
"you now add the extra support for args being NULL to PyObject_Call()"
I dislike this idea. I don't want to change the API of this function.
If it is likely that NULL is the result of a previous error:
args = Py_BuildValue(...); res = PyObject_Call(func, args, NULL);
There are already enough ways to call a function with no argument:
res = PyObject_CallFunction(func, NULL); res = PyObject_CallFunctionObjArgs(func, NULL); res = _PyObject_CallNoArg(func) # private
If you want to call a function only with keyword arguments, well, create an empty tuple ... which is a singleton in CPython, no risk of memory allocation failure ... and use PyObject_Call(), no?
Please note that the two sets of APIs are not identical, e.g. you cannot simply replace PyEval_CallObject() with PyObject_Call(), since the former applies a few extra checks and defaults, which the latter doesn't.
IMHO these checks are too expensive at runtime for little benefit. If you pass non-tuple to PyObject_Call(), Python immediately crash. You are immediately noticied of the bug :-) I don't think that such bugs are common enough to justify the overhead.
Any idea of the popularity of the undocumented PyEval_xxx() functions? Are they used by Cython for example? By a single random extension module in the world?
I'm more in favor of modifying PyEval_xxx() to call PyObject_xxx() and deprecate them.
On 14.02.2017 11:16, STINNER Victor wrote:
> Please note that the two sets of APIs are not identical, e.g. you cannot simply replace PyEval_CallObject() with PyObject_Call(), since the former applies a few extra checks and defaults, which the latter doesn't.
IMHO these checks are too expensive at runtime for little benefit. If you pass non-tuple to PyObject_Call(), Python immediately crash. You are immediately noticied of the bug :-) I don't think that such bugs are common enough to justify the overhead.
From the design of the abstract API layer, it is rather
uncommon to have these not do extra checks to prevent segfaults.
They were originally designed to be safe and developer friendly.OTOH, the PyEval_* APIs were designed to be fast, only for people
who know what they are doing and for interpreter internals.Historically, this design approach appears to have been swapped
somewhere between Python 2.1 and 2.2 for the call APIs,
which is unfortunate.So from a design perspective, it would be better to have the
abstract APIs again do proper checks and leave the low level,
"segfault protected" :-) call APIs around as PyEval_Call*()
or better yet: not make them public at all.Any idea of the popularity of the undocumented PyEval_xxx() functions? Are they used by Cython for example? By a single random extension module in the world?
Well, I know that our eGenix extensions are using them
and there are quite a few hits on github as well:https://github.com/search?utf8=%E2%9C%93&q=PyEval_CallObjectWithKeywords&type=Code&ref=searchresults
I'm more in favor of modifying PyEval_xxx() to call PyObject_xxx() and deprecate them.
7 remaining items
- changed the title
[-]deprecate PyEval_Call*() functions.[/-][+]Recommend PyObject_Call* APIs over PyEval_Call*() APIs[/+]on Feb 15, 2017 I think first at all PyEval_Call* functions should be documented (bpo-11165). The documentation should recommend to use corresponding PyObject_Call* functions and explicitly describe the difference between PyEval_Call* and PyObject_Call* APIs.
Few releases after deprecating PyEval_Call APIs in documentation we can add the Py_DEPRECATED attribute for emitting compiler warnings. Few releases after deprecating in code we can remove PyEval_Call* declarations from headers, but keep exporting them in binary library. In Python 4 (or other major release that will break binary compatibility) they can be removed at all.
- changed the title
[-]Recommend PyObject_Call* APIs over PyEval_Call*() APIs[/-][+]Recommend PyObject_Call* APIs over PyEval_Call*() APIs[/+]on Feb 15, 2017 builtin min and max doesn't support FASTCALL
(...): 1.16x slower (+16%)
Hum, the slowdown is not negligible, even if functools.reduce() is rarely used.
Do you think that it would be insane to have two code paths instead?
- New FASTCALL path if func suports FASTCALL calling _PyObject_FastCall()
- Current code path using cached args tuple otherwise
For example, you can use args==NULL marker for the FASTCALL path. Maybe we need a _PyObject_SupportFastCall() function which would return 1 for Python functions and C functions, except of C functions with METH_VARARGS flag set.
My expectation is a speedup for functions supporting FASTCALL, but *no slowdown* for functions not supporting FASTCALL.
--
property_descr_get() also caches args tuple. I started my work on FASTCALL because this optimization caused bugs (but also because I wanted to make Python faster, but that's a different topic ;-)).
In the past, the _pickle module also used cached args tuple, but the cache was removed because it was vulnerable to race conditions.
For this issue, I suggest to leave functools.reduce() unchanged, but open a new issue to discuss what to do with code still using a cached args tuple.
My long term goal with FASTCALL was to remove completely cached tuple used to call functions. I wrote tp_fastcall for that, but tp_fastcall (issue bpo-29259) was rejected. The rejected tp_fastcall blocked my long term plan, we have to find a different approach.
Maybe we should add support for FASTCALL for a little bit more functions, and later simply remove the optimization (hack, cached tuple)?
I understand the arguments for not removing these functions. However, I still think that we should deprecate them but without planning in advance when they should be removed. Victor said that we should document these functions as "please don't use them", and that is exactly what a deprecation message accomplishes.
Most other projects that I know have only a minimum deprecation period (i.e. feature X can only be removed if it was deprecated for Y time). I don't understand why CPython insists that the minimum of 2 releases should also be a maximum (i.e. feature X MUST be removed after it was deprecated for Y time). I don't see the problem with long-term deprecations for stuff that we don't plan to remove soon.
FYI, PyEval_CallFunction and PyEval_CallMethod doesn't respect Py_SSIZE_T_CLEAN.
So runtime warning is raised when they are used with "#" format.I just spent 2 hours trying to find what should I replace PyEval_CallObjectWithKeywords (used in some obscure maintained code) with, then debugging coredumps which started to appear once I replaced it with PyObject_Call (which doesn't handle NULL as args).
Please, next time you decide to deprecate API, include writing detailed instruction of what to replace it with.
PS My code before change:
PyObject* ret = PyEval_CallObjectWithKeywords(func, arg, kw);my code after change:
if (arg != NULL && ! PyTuple_Check(arg)) { PyErr_SetString(PyExc_TypeError, "argument list must be a tuple"); return NULL; } if (kw != NULL && !PyDict_Check(kw)) { PyErr_SetString(PyExc_TypeError, "keyword list must be a dictionary"); return NULL; } PyObject* ret = (arg == NULL) ? _PyObject_FastCallDict(func, NULL, 0, kw) : PyObject_Call(func, arg, kw);I am by far unconvinced that this is an improvement.
I proposed PR #105108 to remove these functions in Python 3.13.
Please, next time you decide to deprecate API, include writing detailed instruction of what to replace it with.
I'm sorry about that and you're right. I took your remark in account in my PR #105108 which removes these functions. I proposed detailed instructions on how to replace removed functions. Your recipe uses private functions which must be avoided (they can disappear or change anytime). I proposed recipes only with public functions.
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: