This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author rhettinger
Recipients corona10, pablogsal, rhettinger, serhiy.storchaka, vstinner
Date 2021-03-18.04:42:41
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1616042561.99.0.641965967018.issue42161@roundup.psfhosted.org>
In-reply-to
Content
> Thanks Raymond, I fixed the code.

Can you fix all the other cases where this is used in inner-loop; otherwise, it is undoing everyone else's optimizations and fast paths.

Also, it seems the that primary motivation for this is support subinterpreters.  That PEP hasn't been approved, so we should not be making code worse until we know there is going to some offsetting benefit.

For example, the inner-loop code in math_lcm() used to have "res == _PyLong_Zero" which was fast a register-to-register comparison (1 cycle at worst and typically 0 cycles with branch prediction).  Also there used to be zero memory accesses.   The new code has five sequentially dependent operations and four memory accesses (not what we want in an inner-loop):

    movq    __PyRuntime@GOTPCREL(%rip), %rax
    movq    616(%rax), %rax
    movq    16(%rax), %rax
    cmpq    %r12, 3560(%rax)
    jne L326

Ideally, the whole PR should be reverted until the subinterpreter PEP is approved.  If not, then at least the changes should be made more carefully, hoisting the new call out of the hot loop:

+   zero = _PyLong_GetZero();
    for (i = 1; i < nargs; i++) {
        x = PyNumber_Index(args[i]);
        if (x == NULL) {
            Py_DECREF(res);
            return NULL;
        }
-       if (res == _PyLong_GetZero()) {
+       if (res == zero) {
            /* Fast path: just check arguments.
               It is okay to use identity comparison here. */
            Py_DECREF(x);
            continue;
        }
        Py_SETREF(res, long_lcm(res, x));
        Py_DECREF(x);
        if (res == NULL) {
            return NULL;
        }
    }
    return res;
History
Date User Action Args
2021-03-18 04:42:42rhettingersetrecipients: + rhettinger, vstinner, serhiy.storchaka, corona10, pablogsal
2021-03-18 04:42:41rhettingersetmessageid: <1616042561.99.0.641965967018.issue42161@roundup.psfhosted.org>
2021-03-18 04:42:41rhettingerlinkissue42161 messages
2021-03-18 04:42:41rhettingercreate