PyObject_CallFunction(func, "O", arg) special case
Hi, The PyObject_CallFunction() function has a special case when the format string is "O", to pass exactly one Python object: * If the argument is a tuple, the tuple is unpacked: it behaves like func(*arg) * Otherwise, it behaves like func(arg) This case is not documented in the C API ! https://docs.python.org/dev/c-api/object.html#c.PyObject_CallFunction The following C functions have the special case: * PyObject_CallFunction(), _PyObject_CallFunction_SizeT() * PyObject_CallMethod(), _PyObject_CallMethod_SizeT() * _PyObject_CallMethodId(), _PyObject_CallMethodId_SizeT() I guess that it's a side effect of the implementation: the code uses Py_BuildValue() and then checks if the value is a tuple or not. Py_BuildValue() is a little bit surprising: * "i" creates an integer object * "ii" creates a tuple * "(i)" and "(ii)" create a tuple. Getting a tuple or not depends on the length of the format string. It is not obvious when you have nested tuples like "O(OO)". Because of the special case, passing a tuple as the only argument requires to write "((...))" instead of just "(...)". In the past, this special behaviour caused a bug in generator.send(arg), probably because the author of the C code implementing generator.send() wasn't aware of the special case. See the issue: http://bugs.python.org/issue21209 I found code using "O" format in the new _asyncio module, and I'm quite sure that unpacking special case is not expected. So I opened an issue: http://bugs.python.org/issue28920 Last days, I patched functions of PyObject_CallFunction() family to use internally fast calls. I implemented the special case to keep backward compatibility. I replaced a lot of code using PyObject_CallFunction() with PyObject_CallFunctionObjArgs() when the format string was only made of "O", PyObject* arguments. I made this change to optimize the code, but indirectly, it avoids also the special case for code which used exactly "O" format. See: http://bugs.python.org/issue28915 When I made these changes, I found some functions which rely the unpacking feature! * time_strptime() (change 49a7fdc0d40a) * unpickle() of _ctypes (change ceb22b8f6d32) I don't know well what we are supposed to do. I don't think that changing the behaviour of PyObject_CallFunction() to remove the special case is a good idea. It would be an obvious backward incompatible change which can break applications. I guess that the minimum is to document the special case? Victor
2016-12-09 18:46 GMT+01:00 Victor Stinner <victor.stinner@gmail.com>:
Last days, I patched functions of PyObject_CallFunction() family to use internally fast calls. (...) http://bugs.python.org/issue28915
Oh, I forgot to mention the performance results of these changes. Python slots are now a little bit faster. Extract of the issue: http://bugs.python.org/issue28915#msg282748 Microbenchmark on a simple class with an __int__() method, call int(o): int(o): Median +- std dev: [ref] 239 ns +- 13 ns -> [patch] 219 ns +- 14 ns: 1.10x faster (-9%) Microbenchmark on a simple class with an __getitem__() method, call o[100]: o[100]: Median +- std dev: [ref] 211 ns +- 11 ns -> [patch] 172 ns +- 11 ns: 1.23x faster (-19%) Comparison between Python 2.7, 3.5, 3.7 and 3.7+patch, 3.5 is used as the reference: int(o) ====== Median +- std dev: [3.5] 271 ns +- 15 ns -> [3.7] 239 ns +- 13 ns: 1.13x faster (-12%) Median +- std dev: [3.5] 271 ns +- 15 ns -> [patch] 219 ns +- 14 ns: 1.24x faster (-19%) Median +- std dev: [3.5] 271 ns +- 15 ns -> [2.7] 401 ns +- 21 ns: 1.48x slower (+48%) o[100] ====== Median +- std dev: [3.5] 206 ns +- 5 ns -> [3.7] 211 ns +- 11 ns: 1.02x slower (+2%) Not significant! Median +- std dev: [3.5] 206 ns +- 5 ns -> [patch] 172 ns +- 11 ns: 1.20x faster (-17%) Median +- std dev: [3.5] 206 ns +- 5 ns -> [2.7] 254 ns +- 15 ns: 1.23x slower (+23%) Victor
I am not sure, but soon, I will be a great fan of your work, once I get to work on this! Thank your for inspiring me to work on these stuff! Best regards, Annapoornima On Fri, Dec 9, 2016 at 11:33 PM, Victor Stinner <victor.stinner@gmail.com> wrote:
2016-12-09 18:46 GMT+01:00 Victor Stinner <victor.stinner@gmail.com>:
Last days, I patched functions of PyObject_CallFunction() family to use internally fast calls. (...) http://bugs.python.org/issue28915
Oh, I forgot to mention the performance results of these changes. Python slots are now a little bit faster. Extract of the issue: http://bugs.python.org/issue28915#msg282748
Microbenchmark on a simple class with an __int__() method, call int(o): int(o): Median +- std dev: [ref] 239 ns +- 13 ns -> [patch] 219 ns +- 14 ns: 1.10x faster (-9%)
Microbenchmark on a simple class with an __getitem__() method, call o[100]: o[100]: Median +- std dev: [ref] 211 ns +- 11 ns -> [patch] 172 ns +- 11 ns: 1.23x faster (-19%)
Comparison between Python 2.7, 3.5, 3.7 and 3.7+patch, 3.5 is used as the reference:
int(o) ======
Median +- std dev: [3.5] 271 ns +- 15 ns -> [3.7] 239 ns +- 13 ns: 1.13x faster (-12%) Median +- std dev: [3.5] 271 ns +- 15 ns -> [patch] 219 ns +- 14 ns: 1.24x faster (-19%) Median +- std dev: [3.5] 271 ns +- 15 ns -> [2.7] 401 ns +- 21 ns: 1.48x slower (+48%)
o[100] ======
Median +- std dev: [3.5] 206 ns +- 5 ns -> [3.7] 211 ns +- 11 ns: 1.02x slower (+2%) Not significant! Median +- std dev: [3.5] 206 ns +- 5 ns -> [patch] 172 ns +- 11 ns: 1.20x faster (-17%) Median +- std dev: [3.5] 206 ns +- 5 ns -> [2.7] 254 ns +- 15 ns: 1.23x slower (+23%)
Victor _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/ annakoppad%40gmail.com
On 09.12.16 19:46, Victor Stinner wrote:
The PyObject_CallFunction() function has a special case when the format string is "O", to pass exactly one Python object:
* If the argument is a tuple, the tuple is unpacked: it behaves like func(*arg) * Otherwise, it behaves like func(arg)
This case is not documented in the C API ! https://docs.python.org/dev/c-api/object.html#c.PyObject_CallFunction
It is documented for Py_BuildValue(), and the documentation of PyObject_CallFunction() refers to Py_BuildValue(). I just found that in spite of your changes of parameter names, the documentation still have old names: PyObject* PyObject_CallMethod(PyObject *o, const char *method, const char *format, ...)
Hi, 2016-12-10 7:43 GMT+01:00 Serhiy Storchaka <storchaka@gmail.com>:
It is documented for Py_BuildValue(), and the documentation of PyObject_CallFunction() refers to Py_BuildValue().
You're right, but even if I read the documentation of Py_BuildValue() every week, I never noticed that PyObject_CallFunction() behaves differently if the result of Py_BuildValue() is a tuple or not. It is not explicit in the PyObject_CallFunction() documentation. Anyway, as I wrote, it would be a bad idea to change the behaviour, it would break too much C code in the wild, so I just proposed a patch to enhance the documentation: http://bugs.python.org/issue28977
I just found that in spite of your changes of parameter names, the documentation still have old names:
PyObject* PyObject_CallMethod(PyObject *o, const char *method, const char *format, ...)
Ah? It's possible that I forgot to update the documentation of some functions. But sorry, I don't see where. Can you point me which file is outdated please? Since I finished to "cleanup abstract.h" (issue #28838), I will now work on updating the documentation (doc in .rst files and comments in .h files) of functions of the PyObject_Call() family. At least, make sure that .rst and .h almost contain the same doc ;-) Victor
On 15.12.16 10:45, Victor Stinner wrote:
Ah? It's possible that I forgot to update the documentation of some functions. But sorry, I don't see where. Can you point me which file is outdated please?
https://docs.python.org/3/c-api/object.html#c.PyObject_CallMethod Seems online documentation is not updated.
2016-12-15 10:46 GMT+01:00 Serhiy Storchaka <storchaka@gmail.com>:
https://docs.python.org/3/c-api/object.html#c.PyObject_CallMethod
Seems online documentation is not updated.
"Last updated on Dec 08, 2016." Oh, right. No idea what/who compiles the documentation. I expected at least one build per day? Someone also proposed to have a mirror of the CPython documentation on readthedocs. Victor
2016-12-15 10:54 GMT+01:00 Victor Stinner <victor.stinner@gmail.com>:
Oh, right. No idea what/who compiles the documentation. I expected at least one build per day?
I am told that it can be related to https://github.com/python/docsbuild-scripts/pull/7 Victor
On 09.12.16 19:46, Victor Stinner wrote:
Last days, I patched functions of PyObject_CallFunction() family to use internally fast calls. I implemented the special case to keep backward compatibility.
I replaced a lot of code using PyObject_CallFunction() with PyObject_CallFunctionObjArgs() when the format string was only made of "O", PyObject* arguments. I made this change to optimize the code, but indirectly, it avoids also the special case for code which used exactly "O" format. See: http://bugs.python.org/issue28915
I didn't have a change to make a review of your patches, because they were pushed 7 minutes after publishing a patch. I'm still in the process of post-commit reviewing. Could you please give more time for pre-commit review? I thought about similar changes, but since I didn't have evidences that they cause performance gain, I dropped my patches. Do you have benchmark results that proof the speed up for all your changes? Did you checked that your changes do not increase C stack consumption?
Oh, I found another recent bug fixed because of the "O" weird behaviour: http://bugs.python.org/issue26478 "dict views don't implement subtraction correctly" Victor 2016-12-09 18:46 GMT+01:00 Victor Stinner <victor.stinner@gmail.com>:
Hi,
The PyObject_CallFunction() function has a special case when the format string is "O", to pass exactly one Python object:
* If the argument is a tuple, the tuple is unpacked: it behaves like func(*arg) * Otherwise, it behaves like func(arg)
This case is not documented in the C API ! https://docs.python.org/dev/c-api/object.html#c.PyObject_CallFunction
The following C functions have the special case:
* PyObject_CallFunction(), _PyObject_CallFunction_SizeT() * PyObject_CallMethod(), _PyObject_CallMethod_SizeT() * _PyObject_CallMethodId(), _PyObject_CallMethodId_SizeT()
I guess that it's a side effect of the implementation: the code uses Py_BuildValue() and then checks if the value is a tuple or not. Py_BuildValue() is a little bit surprising:
* "i" creates an integer object * "ii" creates a tuple * "(i)" and "(ii)" create a tuple.
Getting a tuple or not depends on the length of the format string. It is not obvious when you have nested tuples like "O(OO)".
Because of the special case, passing a tuple as the only argument requires to write "((...))" instead of just "(...)".
In the past, this special behaviour caused a bug in generator.send(arg), probably because the author of the C code implementing generator.send() wasn't aware of the special case. See the issue: http://bugs.python.org/issue21209
I found code using "O" format in the new _asyncio module, and I'm quite sure that unpacking special case is not expected. So I opened an issue: http://bugs.python.org/issue28920
Last days, I patched functions of PyObject_CallFunction() family to use internally fast calls. I implemented the special case to keep backward compatibility.
I replaced a lot of code using PyObject_CallFunction() with PyObject_CallFunctionObjArgs() when the format string was only made of "O", PyObject* arguments. I made this change to optimize the code, but indirectly, it avoids also the special case for code which used exactly "O" format. See: http://bugs.python.org/issue28915
When I made these changes, I found some functions which rely the unpacking feature!
* time_strptime() (change 49a7fdc0d40a) * unpickle() of _ctypes (change ceb22b8f6d32)
I don't know well what we are supposed to do. I don't think that changing the behaviour of PyObject_CallFunction() to remove the special case is a good idea. It would be an obvious backward incompatible change which can break applications.
I guess that the minimum is to document the special case?
Victor
participants (3)
-
Annapoornima Koppad -
Serhiy Storchaka -
Victor Stinner