Title: suboptimal code in Py_ReprEnter()
Type: performance Stage: resolved
Components: Interpreter Core Versions: Python 3.7
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: Oren Milman, rhettinger, serhiy.storchaka
Priority: normal Keywords:

Created on 2017-08-12 10:37 by Oren Milman, last changed 2017-08-17 13:16 by serhiy.storchaka. This issue is now closed.

Messages (2)
msg300193 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-08-12 10:37
in Objects/object.c, Py_ReprEnter() does the following:
    - try to retrieve the Py_Repr list from the thread-state dict.
    - in case the list is not in the dict, add it to the dict as an empty list.
    - check whether the received object is in the Py_Repr list, even in case the
      list was just created, and guaranteed to be empty.

I propose to put this check inside an else clause, so that it wouldn't take
place in case the list is guaranteed to be empty, i.e.:
    list = _PyDict_GetItemId(dict, &PyId_Py_Repr);
    if (list == NULL) {
        list = PyList_New(0);
    else {
        i = PyList_GET_SIZE(list);
        while (--i >= 0) {
            if (PyList_GET_ITEM(list, i) == obj)
                return 1;

I ran the test suite, and it seems that this change doesn't break anything, so
I would be happy to open a PR for it.
msg300217 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-13 10:36
PyList_GET_SIZE(list) is cheap (especially in comparison of _PyDict_GetItemId(), _PyDict_SetItemId() and PyList_New()), and its tiny overhead can be avoided at most once per thread's lifetime. I'm sure you can't measure the effect of this change.

If surrounding code be modified for other reasons, may be your change could be applied. But it alone just makes a code churn.
Date User Action Args
2017-08-17 13:16:37serhiy.storchakasetstatus: open -> closed
resolution: rejected
stage: resolved
2017-08-13 10:36:23serhiy.storchakasetnosy: + rhettinger, serhiy.storchaka
messages: + msg300217
2017-08-12 10:37:02Oren Milmancreate