Raymond Hettinger <raymond.hettin...@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@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 <rep...@bugs.python.org>
<https://bugs.python.org/issue42161>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
