[issue27441] redundant assignments to ob_size of new ints that _PyLong_New returned

[Oren Milman]

Oren Milman added the comment:

I am sorry, but I can't see why micro-benchmarking is needed here, as my patches only remove code that does nothing, while they don't add any new code.

The assembly the compiler generates (on my PC) for 'Py_SIZE(v) = negative ? -ndigits : ndigits;' in PyLong_FromLongLong is: 
('[edx+8]' is 'Py_SIZE(v)',
 '[esp+10h+var_4]' is 'negative',
  The 'lea ecx, [edx+0Ch]' and 'mov eax, edi' instructions set ecx and eax for later (I haven't removed them in order to be as precise as possible.))
    ...
    cmp     [esp+10h+var_4], 0
    lea     ecx, [edx+0Ch]
    jz      short loc_1E0D48EC
        neg     ebx
loc_1E0D48EC:
    mov     eax, edi
    mov     [edx+8], ebx
    ...
In contrast, the assembly the compiler generates for 'if (negative) Py_SIZE(v) = -ndigits;' is:
    ...
    cmp     [esp+10h+var_4], 0
    lea     ecx, [edx+0Ch]
    jz      short loc_1E0D482F
        neg     ebx
        mov     [edx+8], ebx
loc_1E0D482F:
    mov     eax, edi
    ...
Comparing the assembly generated for the other original '?:' expressions with my corresponding patches looks quite the same. Each patch moves the assignment from code which is executed in both of the flows, to code which is executed in only one of the flows.

Am I missing anything that might cause my patches to introduce a performance penalty?


I searched (all of the cpython repo) for other places in which Py_SIZE() is set to the same value, and indeed found one in Objects/longobject.c in _PyLong_Init:
The loop that initializes the small_ints array goes over every element in the array, and checks whether it was already initialized. For some reason, even when it realizes the current element was already initialized, it still sets 'Py_SIZE(v)' and 'v->ob_digit[0]' (to the values they are already set to).
These redundant assignments were first added in changeset 45072 (https://hg.python.org/cpython/rev/f183f1189824), and remained there to this day.

So I added a patch to move these assignments so they would be executed only in case the current element of small_ints wasn't already initialized.
The updated patches diff file is attached. I also ran the tests again, and got quite the same output (the output is attached).

Have you spotted any other places in which Py_SIZE() is set to the same value?

----------
Added file: http://bugs.python.org/file43661/issue27441_ver2.diff

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue27441>
_______________________________________