Issue 32571: Speed up and clean up getting optional attributes in C code (original) (raw)

Created on 2018-01-16 18:18 by serhiy.storchaka, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 5205 closed serhiy.storchaka,2018-01-16 18:21
PR 5222 merged serhiy.storchaka,2018-01-17 17:30
PR 5332 merged methane,2018-01-26 03:29
Messages (8)
msg310101 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-16 18:18
In there was introduced a new private C API function _PyObject_GetAttrWithoutError() which never raises an AttributeError, but returns NULL without error set if an attribute is absent. This allowed to speed up Python builtins hasattr() and getattr() with the default argument. But C code also could gain a benefit from this. It is common to look up an attribute and silence an AttributeError. Actually it is more common than the opposite case. The proposed patch adds yet one function, _PyObject_GetAttrIdWithoutError(), and uses these two functions if it is possible to avoid checking an AttributeError after PyObject_GetAttr() and _PyObject_GetAttrId(). This could speed up some code, and also makes it cleaner.
msg310153 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-01-17 11:32
Can I ask why this is a private API? It's extremely useful for extension modules too. For example, numpy has been carrying around a reimplementation of PyObject_GetAttr for years, exactly to avoid the cost of creating an AttributeError exception in common cases: https://github.com/numpy/numpy/blob/master/numpy/core/src/private/get_attr_string.h (To emphasize the similarity, note that the code above used to be called PyArray_GetAttrString_SuppressException, until a refactoring last year: https://github.com/numpy/numpy/commit/69a423b23492ecf8471efc4e59ab8a93b03d8454. It's always been specifically used for looking up numpy-specific special methods like __array__, __array_interface__, etc., though, which is why it can get away with those shortcuts -- we know they usually aren't defined.)
msg310199 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-17 17:35
PR 5222 provides functions _PyObject_LookupAttr() and _PyObject_LookupAttrId() with alternate interface. Some code becomes even more simpler. The possible drawback of this approach is that the compiler should allocate the variable for the result on the stack instead of passing the result in a register (unless compilers are smart enough for optimizing out this case).
msg310200 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-17 17:41
Because this API is experimental. It was changed in recent 24 hours and may be changed more before merging or even after releasing 3.7.
msg310204 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-01-17 18:41
Well, mostly I just want to draw your attention to that use case to encourage you to make it public when it's ready :-)
msg310665 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-01-25 08:49
New changeset f320be77ffb73e3b9e7fc98c37b8df3975d84b40 by INADA Naoki (Serhiy Storchaka) in branch 'master': bpo-32571: Avoid raising unneeded AttributeError and silencing it in C code (GH-5222) https://github.com/python/cpython/commit/f320be77ffb73e3b9e7fc98c37b8df3975d84b40
msg310746 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-01-26 07:22
New changeset e76daebc0c8afa3981a4c5a8b54537f756e805de by INADA Naoki in branch 'master': bpo-32571: Fix reading uninitialized memory (GH-5332) https://github.com/python/cpython/commit/e76daebc0c8afa3981a4c5a8b54537f756e805de
msg310787 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-26 20:08
Closing this one now. Thanks Serhiy and Inada-san!
History
Date User Action Args
2022-04-11 14:58:56 admin set github: 76752
2018-01-26 20:08:05 yselivanov set status: open -> closednosy: + yselivanovmessages: + resolution: fixedstage: patch review -> resolved
2018-01-26 07:22:53 methane set messages: +
2018-01-26 03:29:38 methane set pull_requests: + <pull%5Frequest5178>
2018-01-25 08:49:45 methane set messages: +
2018-01-17 18:41:56 njs set messages: +
2018-01-17 17:41:36 serhiy.storchaka set messages: +
2018-01-17 17:35:42 serhiy.storchaka set messages: +
2018-01-17 17:30:16 serhiy.storchaka set pull_requests: + <pull%5Frequest5073>
2018-01-17 11:32:31 njs set nosy: + njsmessages: +
2018-01-16 18:21:45 serhiy.storchaka set keywords: + patchstage: patch reviewpull_requests: + <pull%5Frequest5059>
2018-01-16 18🔞05 serhiy.storchaka create