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