Skip to content

Optimize PyFloat_Pack*() functions - #158483

Open
vstinner wants to merge 2 commits into
python:mainfrom
vstinner:float_pack
Open

vstinner wants to merge 2 commits into
python:mainfrom
vstinner:float_pack

Conversation

@vstinner

Copy link
Copy Markdown
Member

PyFloat_Pack4() and PyFloat_Pack8() avoid temporary buffer in the native byte order.

Use _Py_bswapXX() functions to reverse bytes.

PyFloat_Pack4() and PyFloat_Pack8() avoid temporary buffer in the
native byte order.

Use _Py_bswapXX() functions to reverse bytes.
@vstinner

Copy link
Copy Markdown
Member Author

Benchmark:

import pyperf, sys, _testcapi
runner = pyperf.Runner()
le = int(sys.byteorder == 'little')
for d in (1.0, float('nan')):
    runner.bench_func(f'pack2({d}) native', _testcapi.float_pack, 2, d, le)
    runner.bench_func(f'pack4({d}) native', _testcapi.float_pack, 4, d, le)
    runner.bench_func(f'pack8({d}) native', _testcapi.float_pack, 8, d, le)
    be = int(not le)
    runner.bench_func(f'pack2({d}) byteswap', _testcapi.float_pack, 2, d, be)
    runner.bench_func(f'pack4({d}) byteswap', _testcapi.float_pack, 4, d, be)
    runner.bench_func(f'pack8({d}) byteswap', _testcapi.float_pack, 8, d, be)

Result:

Benchmark ref change
pack2(1.0) native 68.1 ns 56.2 ns: 1.21x faster
pack4(1.0) native 58.0 ns 49.2 ns: 1.18x faster
pack8(1.0) native 52.8 ns 49.7 ns: 1.06x faster
pack2(1.0) byteswap 67.2 ns 59.2 ns: 1.13x faster
pack4(1.0) byteswap 60.1 ns 51.4 ns: 1.17x faster
pack8(1.0) byteswap 59.1 ns 50.8 ns: 1.16x faster
pack2(nan) native 54.2 ns 51.8 ns: 1.05x faster
pack4(nan) native 54.7 ns 53.0 ns: 1.03x faster
pack8(nan) native 57.9 ns 52.4 ns: 1.10x faster
pack2(nan) byteswap 60.5 ns 53.5 ns: 1.13x faster
pack4(nan) byteswap 54.1 ns 53.1 ns: 1.02x faster
pack8(nan) byteswap 54.9 ns 53.0 ns: 1.04x faster
Geometric mean (ref) 1.11x faster

For example, PyFloat_Pack8() x86-64 assembly code before:

   0x00000000004a6250 <+0>:	test   esi,esi
   0x00000000004a6252 <+2>:	jne    0x4a62a0 <PyFloat_Pack8+80>

   0x00000000004a6254 <+4>:	movq   rax,xmm0
   0x00000000004a6259 <+9>:	mov    rdx,rax
   0x00000000004a625c <+12>:	mov    BYTE PTR [rdi+0x7],al
   0x00000000004a625f <+15>:	shr    rdx,0x10
   0x00000000004a6263 <+19>:	mov    BYTE PTR [rdi+0x6],ah
   0x00000000004a6266 <+22>:	mov    BYTE PTR [rdi+0x5],dl
   0x00000000004a6269 <+25>:	mov    rdx,rax
   0x00000000004a626c <+28>:	shr    rdx,0x18
   0x00000000004a6270 <+32>:	mov    BYTE PTR [rdi+0x4],dl
   0x00000000004a6273 <+35>:	mov    rdx,rax
   0x00000000004a6276 <+38>:	shr    rdx,0x20
   0x00000000004a627a <+42>:	mov    BYTE PTR [rdi+0x3],dl
   0x00000000004a627d <+45>:	mov    rdx,rax
   0x00000000004a6280 <+48>:	shr    rdx,0x28
   0x00000000004a6284 <+52>:	mov    BYTE PTR [rdi+0x2],dl
   0x00000000004a6287 <+55>:	mov    rdx,rax
   0x00000000004a628a <+58>:	shr    rax,0x38
   0x00000000004a628e <+62>:	shr    rdx,0x30
   0x00000000004a6292 <+66>:	mov    BYTE PTR [rdi],al
   0x00000000004a6294 <+68>:	xor    eax,eax
   0x00000000004a6296 <+70>:	mov    BYTE PTR [rdi+0x1],dl
   0x00000000004a6299 <+73>:	ret

   0x00000000004a62a0 <+80>:	movsd  QWORD PTR [rdi],xmm0
   0x00000000004a62a4 <+84>:	xor    eax,eax
   0x00000000004a62a6 <+86>:	ret

Assembly code after:

   0x00000000004a6220 <+0>:	movq   rax,xmm0
   0x00000000004a6225 <+5>:	test   esi,esi
   0x00000000004a6227 <+7>:	mov    rdx,rax
   0x00000000004a622a <+10>:	bswap  rdx
   0x00000000004a622d <+13>:	cmove  rax,rdx
   0x00000000004a6231 <+17>:	mov    QWORD PTR [rdi],rax
   0x00000000004a6234 <+20>:	xor    eax,eax
   0x00000000004a6236 <+22>:	ret

The conditional jump is replaced with more efficient cmove, and bytes are swapped by bswap rdx instruction.

@vstinner

Copy link
Copy Markdown
Member Author

cc @skirpichev

@skirpichev skirpichev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Though, not sure if the second case in Pack2 does make sense, see comment.

Comment thread Objects/floatobject.c Outdated
@vstinner

Copy link
Copy Markdown
Member Author

@skirpichev: I pushed a change to also use _Py_bswap16()+memcpy() at the end of PyFloat_Pack2().

Update benchmark results (CPU isolated, after running sudo python3 -m pyperf system tune):

Benchmark ref bswap
pack4(1.0) native 95.2 ns 94.4 ns: 1.01x faster
pack8(1.0) native 93.1 ns 91.2 ns: 1.02x faster
pack8(1.0) byteswap 94.2 ns 89.4 ns: 1.05x faster
pack2(nan) native 95.7 ns 92.6 ns: 1.03x faster
pack4(nan) native 94.9 ns 94.3 ns: 1.01x faster
pack8(nan) native 94.0 ns 89.7 ns: 1.05x faster
pack4(nan) byteswap 94.6 ns 94.0 ns: 1.01x faster
pack8(nan) byteswap 94.3 ns 90.4 ns: 1.04x faster
Geometric mean (ref) 1.02x faster

Benchmark hidden because not significant (4): pack2(1.0) native, pack2(1.0) byteswap, pack4(1.0) byteswap, pack2(nan) byteswap

This time it's no longer "1.11x faster", but at least, it's not slower on any benchmark: it's either faster or as fast :-)

Esp. for le case performance gain is 1.05x, maybe a noise.

Oh sure. It's really hard to measure the speedup, the difference is really tiny and can be lost in noise. I'm using CPU isolation on Linux to reduce the noise.

Details
$ python3 -m pyperf show ref.json bswap.json -q
ref
===

pack2(1.0) native: Mean +- std dev: 110 ns +- 11 ns
pack4(1.0) native: Mean +- std dev: 95.2 ns +- 1.5 ns
pack8(1.0) native: Mean +- std dev: 93.1 ns +- 0.9 ns
pack2(1.0) byteswap: Mean +- std dev: 109 ns +- 7 ns
pack4(1.0) byteswap: Mean +- std dev: 94.5 ns +- 0.6 ns
pack8(1.0) byteswap: Mean +- std dev: 94.2 ns +- 1.6 ns
pack2(nan) native: Mean +- std dev: 95.7 ns +- 4.7 ns
pack4(nan) native: Mean +- std dev: 94.9 ns +- 1.1 ns
pack8(nan) native: Mean +- std dev: 94.0 ns +- 6.2 ns
pack2(nan) byteswap: Mean +- std dev: 94.8 ns +- 5.5 ns
pack4(nan) byteswap: Mean +- std dev: 94.6 ns +- 0.7 ns
pack8(nan) byteswap: Mean +- std dev: 94.3 ns +- 1.4 ns

bswap
=====

pack2(1.0) native: Mean +- std dev: 108 ns +- 1 ns
pack4(1.0) native: Mean +- std dev: 94.4 ns +- 0.7 ns
pack8(1.0) native: Mean +- std dev: 91.2 ns +- 6.4 ns
pack2(1.0) byteswap: Mean +- std dev: 108 ns +- 1 ns
pack4(1.0) byteswap: Mean +- std dev: 95.3 ns +- 5.7 ns
pack8(1.0) byteswap: Mean +- std dev: 89.4 ns +- 0.9 ns
pack2(nan) native: Mean +- std dev: 92.6 ns +- 0.8 ns
pack4(nan) native: Mean +- std dev: 94.3 ns +- 0.5 ns
pack8(nan) native: Mean +- std dev: 89.7 ns +- 1.1 ns
pack2(nan) byteswap: Mean +- std dev: 93.5 ns +- 2.9 ns
pack4(nan) byteswap: Mean +- std dev: 94.0 ns +- 0.8 ns
pack8(nan) byteswap: Mean +- std dev: 90.4 ns +- 2.5 ns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants