Decompress/compress output buffer growth via _PyBytes_Resize causes avoidable copies and heap-layout-dependent performance

[Dreamsorcerer]

Summary

isal_zlib grows its output in a single PyBytes object that is doubled with _PyBytes_Resize() (i.e. realloc) as output accumulates:

• arrange_output_buffer_with_maximum() — src/isal/isal_shared.h#L230, starting from DEF_BUF_SIZE = 16 KiB (isal_shared.h#L40)
• used by e.g. Decompress.decompress — src/isal/isal_zlibmodule.c#L915 and the other compress/decompress/flush paths.

Decompressing a 512 KiB payload reallocs the buffer 6 times (16K→32K→…→1M when the buffer fills exactly, plus the final shrink resize). Each step is a glibc realloc that either extends in place or moves+copies depending on what happens to neighbour the chunk, so:

1. Avoidable copy traffic — up to ~1.5 MiB memcpy per 512 KiB decompressed in the worst case, and
2. Nondeterministic performance — the copy volume depends on heap layout, i.e. on everything the process allocated before the call.

Testing against aiohttp's benchmark:

backend	cycle spread
stdlib zlib	0.03%
isal.isal_zlib	37.4% (22.9M–33.1M cycles)

The entire delta between runs localizes to the memmove blob behind glibc realloc (ISA-L's own internal memmoves are bit-identical across runs, so this is purely the Python-side buffer management, not ISA-L).

Suggested fix

CPython removed exactly this pattern from zlibmodule.c/bz2/lzma in python/cpython#85658 (bpo-41486), merged as python/cpython#21740 for 3.10: _BlocksOutputBuffer keeps a list of bytes blocks and joins once at the end — "no overhead of resizing the buffer" — with measured speedups on top of the determinism. The header is self-contained and copyable: Include/internal/pycore_blocks_output_buffer.h, current usage in Modules/zlibmodule.c.

[rhpvorderman]

I think they use the new pybyteswriter API now. I did not use the blocks buffer back then because I felt that it was slower in the most common case which is smaller amounts of data. This was based on benchmarking on my PC though.

The problem is that the blocksoutputbuffer always needs a memcpy, whilst writing to a bytes object directly does not need a memcpy when the data is not resized. Even when resizing the existing allocated memory might have been big enough already.

You decompress a 512 KiB payload here, but the most common case is decompressing really big files into streams and these get output in 128 KiB buffers. So whatever works best for that particular use case is the optimal solution.

Also spread is bad, but if the worst performing case is still faster than the alternative with no spread, I take the spread over the predictability of performing poorly.

I do think the current way things are done (the old CPython way before 3.10) is indeed rather not elegant, and I'd like to replace it with something easier to maintain that is closer to the CPython source (PyBytesWriter) or something that performs better. The one advantage it does have is that it is rather simple to implement and does not take too much code.

[Dreamsorcerer]

Our main concern is benchmark stability. We can see variations upto 30% at random, which makes it difficult to know when an actual regression happened.

That 30% is over the entire benchmark too, it's not isolating isal's code. The actual benchmark causing issues is sending large (512 KiB) websocket messages: https://github.com/aio-libs/aiohttp/blob/9e08ba02abe573ce9e4cc541d4e3c3646adb43a7/tests/test_benchmarks_client_ws.py#L177

I had an agent test out the changes, and these are the performance changes it reported:

For your most common case (big file decompressed in 128 KiB output buffers):

metric	stock 1.8.0	blocks-buffer (hybrid)
native wall-clock, min of 12 interleaved rounds	598 ms	578 ms (×1.036)
callgrind cycles (CodSpeed instrument mode)	504–527M (4.4% heap-layout spread)	452M (0.06% spread, 10–14% fewer)
allocator ops per 128 KiB call	1 alloc + 3 grow-reallocs + shrink	1 exact-size alloc, 0 reallocs, 0 copies

Full tests:

case	native min (stock → patched)	callgrind cycles (stock → patched)	callgrind spread (stock → patched)
stream-128K (256 MiB, 128 KiB caps)	598 ms → 578 ms (+3.6%)	519M → 452M (−13%)	4.43% → 0.06%
websocket-like (200 × 512 KiB msgs, max_length=4 MiB+1)	43.5 ms → 42.5 ms (+2.3%)	21.1–41.7M → 23.2M	59.2% → 0.01%
one-shot decompress, 256 MiB	689 ms → 849 ms (−19%)	460M → 464M (−1%)	0.36% → 0.00%
compressobj, 128 MiB incompressible	239 ms → 313 ms (−23%)	169M → 169M (parity)	0.88% → 0.00%

So, it suggests a performance improvement and massive stability improvement on your most common case and our benchmark. A fairly significant reduction in performance for large objects being handled in one go.

I can submit the patch it produced as a PR, if you want to review, but I've not looked at it myself (and probably am not familiar enough to review it).