[vstinner]

There is no need to set PySetObject explicitly to 0/NULL, since _PyType_AllocNoTrack() already initialize memory to zeros.

• Issue: [C API] Add PyFrozenWriter C API to create a frozenset object #160147

[eendebakpt]

Changes are correct. I checked and the compiler does not optimize it away (because type is not compile time), so this is a minor optimization. Did you do benchmarks victor?

I am wondering if we should do this more (there are dozens of sites in cpython where this can be done, some hot paths are enumerate and BaseException_new).

[vstinner]

Did you do benchmarks victor?

No, I didn't, but I can run a micro-benchmark:

Benchmark: ./python -m pyperf timeit -s 'Set=set' 'Set()' --duplicate=1024

Result: Mean +- std dev: [ref] 34.2 ns +- 1.3 ns -> [optim] 33.6 ns +- 0.6 ns: 1.02x faster.

In fact, if I rerun the benchmark... I get the opposite result: Mean +- std dev: [ref] 32.6 ns +- 0.6 ns -> [optim] 33.8 ns +- 0.8 ns: 1.04x slower

So it's hard to measure the benefits of this change, it's lost in the benchmark noise.

x86-64 assembly code, before: there are multiple instructions to set members to zero, like mov QWORD PTR [rax+0xc0],0x0 or pxor xmm0,xmm0 + movups XMMWORD PTR [rax+0x10],xmm0.

Details

(gdb) disassemble set_new
Dump of assembler code for function set_new:
   0x00000000005257f0 <+0>:	push   rbp
   0x00000000005257f1 <+1>:	xor    esi,esi
   0x00000000005257f3 <+3>:	mov    rbp,rsp
   0x00000000005257f6 <+6>:	call   0x53c0e0 <_PyType_AllocNoTrack>
   0x00000000005257fb <+11>:	test   rax,rax
   0x00000000005257fe <+14>:	je     0x52586a <set_new+122>
   0x0000000000525800 <+16>:	pxor   xmm0,xmm0
   0x0000000000525804 <+20>:	lea    rdx,[rax+0x40]
   0x0000000000525808 <+24>:	mov    QWORD PTR [rax+0x20],0x7
   0x0000000000525810 <+32>:	lea    rdi,[rax-0x10]
   0x0000000000525814 <+36>:	movups XMMWORD PTR [rax+0x10],xmm0
   0x0000000000525818 <+40>:	movdqa xmm0,XMMWORD PTR [rip+0x29ee40]        # 0x7c4660
   0x0000000000525820 <+48>:	mov    QWORD PTR [rax+0x28],rdx
   0x0000000000525824 <+52>:	mov    rdx,0xfffffffffffffff0
   0x000000000052582b <+59>:	movups XMMWORD PTR [rax+0x30],xmm0
   0x000000000052582f <+63>:	mov    QWORD PTR [rax+0xc0],0x0
   0x000000000052583a <+74>:	mov    rcx,QWORD PTR fs:[rdx]
   0x000000000052583e <+78>:	mov    rdx,QWORD PTR [rax-0x8]
   0x0000000000525842 <+82>:	mov    rsi,QWORD PTR [rcx+0x1d90]
   0x0000000000525849 <+89>:	and    edx,0x3
   0x000000000052584c <+92>:	mov    r8,QWORD PTR [rsi+0x8]
   0x0000000000525850 <+96>:	or     rdx,r8
   0x0000000000525853 <+99>:	mov    QWORD PTR [r8],rdi
   0x0000000000525856 <+102>:	mov    QWORD PTR [rax-0x8],rdx
   0x000000000052585a <+106>:	mov    QWORD PTR [rax-0x10],rsi
   0x000000000052585e <+110>:	mov    QWORD PTR [rsi+0x8],rdi
   0x0000000000525862 <+114>:	add    QWORD PTR [rcx+0x1d78],0x1
   0x000000000052586a <+122>:	pop    rbp
   0x000000000052586b <+123>:	ret
End of assembler dump.

x86-64 assembly code, after: the code is 23 bytes smaller (124 => 101 bytes), the instructions to set members to zero are gone.

Details

(gdb) disassemble set_new
Dump of assembler code for function set_new:
   0x00000000005257f0 <+0>:	push   rbp
   0x00000000005257f1 <+1>:	xor    esi,esi
   0x00000000005257f3 <+3>:	mov    rbp,rsp
   0x00000000005257f6 <+6>:	call   0x53c1a0 <_PyType_AllocNoTrack>
   0x00000000005257fb <+11>:	test   rax,rax
   0x00000000005257fe <+14>:	je     0x525853 <set_new+99>
   0x0000000000525800 <+16>:	lea    rdx,[rax+0x40]
   0x0000000000525804 <+20>:	mov    QWORD PTR [rax+0x20],0x7
   0x000000000052580c <+28>:	lea    rdi,[rax-0x10]
   0x0000000000525810 <+32>:	mov    QWORD PTR [rax+0x28],rdx
   0x0000000000525814 <+36>:	mov    rdx,0xfffffffffffffff0
   0x000000000052581b <+43>:	mov    QWORD PTR [rax+0x30],0xffffffffffffffff
   0x0000000000525823 <+51>:	mov    rcx,QWORD PTR fs:[rdx]
   0x0000000000525827 <+55>:	mov    rdx,QWORD PTR [rax-0x8]
   0x000000000052582b <+59>:	mov    rsi,QWORD PTR [rcx+0x1d90]
   0x0000000000525832 <+66>:	and    edx,0x3
   0x0000000000525835 <+69>:	mov    r8,QWORD PTR [rsi+0x8]
   0x0000000000525839 <+73>:	or     rdx,r8
   0x000000000052583c <+76>:	mov    QWORD PTR [r8],rdi
   0x000000000052583f <+79>:	mov    QWORD PTR [rax-0x8],rdx
   0x0000000000525843 <+83>:	mov    QWORD PTR [rax-0x10],rsi
   0x0000000000525847 <+87>:	mov    QWORD PTR [rsi+0x8],rdi
   0x000000000052584b <+91>:	add    QWORD PTR [rcx+0x1d78],0x1
   0x0000000000525853 <+99>:	pop    rbp
   0x0000000000525854 <+100>:	ret
End of assembler dump.

[vstinner]

I am wondering if we should do this more (there are dozens of sites in cpython where this can be done, some hot paths are enumerate and BaseException_new).

I'm not sure that it's worth it to update all constructions setting members to 0/NULL, but I'm not against it neither :-) As I shown in my previous comment, the machine code is shorter and instructions to set members to zero are gone, but there is no significant impact on performance.

It's just that I'm modifying setobject.c code, and I spotted this missed optimization opportunity.