Repository navigation
Decompress/compress output buffer growth via _PyBytes_Resize causes avoidable copies and heap-layout-dependent performance #256
Description
Activity
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.
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).
- added a commit that references this issue
on Sep 14, 2026 spent some time sparing with claude on a few options in bdraco#1
Landed on one that doesn't show any regressions from the aiohttp cases and some speed ups. Mostly uses what is already there as
IgzipDecompressor.decompressalready was optimized. The new check is for small payloads to not allocate too much up front when we know the max compression sizeEdit: first experiment, do not use as can over-allocate to max expansion size 1032x
This codes trys to allocate at least 1032x the input size up front capped at 16 MiB. That is the kind of allocation needed for adverserial gzip bomb type inputs. The synthetic benchmarks may be faster, but if you allocate and deallocate 16 MiB what will happen is that the same piece of memory will probably be reused over and over again. So of course it will be faster. But most of that memory will be unused and this is very bad behaviour of a program.
We have this Dutch saying: "De bel wel hoorden luiden maar niet weten waar de klepel hangt", which I find hard to translate but it means that somebody heard the ringing of the bell but not entirely gets how the bell is ringing. That is the case with this Claude solution.
The solution is clearly to allocate enough memory up front without over allocating. That IS the problem. It is hard to solve. Just taking the input times 1032 so you always have enough is not the solution.
To further elaborate on the usefulness of AI in these situation. This 1032 times allocation is a gotcha that I did spot. It always leaves me wondering how many gotchas are there that I did not spot.
So to be clear I do agree that this is a problem. I am just thinking of a good way to solve this. Currently it underallocates in your specific use case.
A solution might be to give a parameter that allows setting the initial allocation. That gives the most control to the user but requires expanding stuff.
Another solution is setting the initial allocation at a more reasonable and predictable guesstimate based on the input size (4x or something).
What is also possible is to have the Decompress object have an internal buffer that is not continuously allocated and deallocated but persists (and amortized resizing is not so much a problem over its longer lifespan). A bytes object can be simply copied from that buffer in one go by one memcpy. That would be the more sensible way to do things in my opinion. But since I copied it from CPYthon I did not implement it. (It requires some thought with regards to multithreading as well so it is not a quick solution). This works well for reasonably big compressed things, but not so much for small things as the internal buffer hardly lives longer than the buffer solution that is there now.
Yet another solution is to use something like the blocks buffer as suggested. That would be a move to the PyBytesWriter API. That would require quite the rewrite but I think it would be worth it if it fixes this issue.
A solution might be to give a parameter that allows setting the initial allocation. That gives the most control to the user but requires expanding stuff.
We're currently using it as a drop-in replacement for zlib. So ideally any solution wouldn't change the API, so we can continue to use zlib/zlib_ng/isal interchangeably.
This codes trys to allocate at least 1032x the input size up front capped at 16 MiB. That is the kind of allocation needed for adverserial gzip bomb type inputs. The synthetic benchmarks may be faster, but if you allocate and deallocate 16 MiB what will happen is that the same piece of memory will probably be reused over and over again. So of course it will be faster. But most of that memory will be unused and this is very bad behaviour of a program.
What it came up with first was much worse 16MiB always. I tried to reduce that by capping to the max size (1032x), but couldn't come up with something that was better. I was hoping someone had a idea on that.
To further elaborate on the usefulness of AI in these situation. This 1032 times allocation is a gotcha that I did spot. It always leaves me wondering how many gotchas are there that I did not spot.
Sadly I can't blame AI here as that's my fault for not providing detail above (and why I didn't move it to PR to bring it forward as a ready solution) as I am still experimenting with it. Every solution I played around with got much more complex because we simplify don't know how much the data is going to expand to in advance, all we can do is guess based on common data formats so those end up better.
- added a commit that references this issue
on Sep 14, 2026 After playing around with it some more, I've landed on having Decompress.decompress start its output buffer between 16 KiB and 1 MiB (never above max_length), sized at 8x the input, then doubling as before.
To be clear, since I wasn't before: I'm not proposing this as a PR yet, I'm still experimenting. It significantly reduces reallocs on the
orjsontest data (the four sample files go from 23 buffer grows to 4, and small messages stay at zero) and on a live Home Assistant instance only the entity and device registry dumps realloc now.side note: When I was looking at the data being sent from HA much of WebSocket messages are in the range of 150 bytes to 700 bytes for state updates (most common - 98% of traffic), 2-8KB for automation events, and 1MB-4MB for registries so I looked at how it could perform better by avoiding the heap allocation and copying out of a 4KiB stack buffer + not releasing the gil for small payloads in bdraco#2
Summary
isal_zlib grows its output in a single PyBytes object that is doubled with _PyBytes_Resize() (i.e. realloc) as output accumulates:
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:
Testing against aiohttp's benchmark:
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.