[issue42161] Remove private _PyLong_Zero and _PyLong_One variables

[Raymond Hettinger]

Raymond Hettinger <raymond.hettinger at gmail.com> added the comment:

> 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 at 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;

----------
nosy: +pablogsal, serhiy.storchaka
status: closed -> open

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue42161>
_______________________________________