Repository navigation
Conversation
Decompress.decompress started its output buffer at 16 KiB, clamped that down to max_length, and then doubled it with _PyBytes_Resize until the output fit or max_length was reached. Every doubling is a realloc that may or may not copy the whole buffer depending on heap layout, which makes the run time depend on what else the process allocated (pycompression#256). IgzipDecompressor.decompress already allocated min(max_length, 16 MiB) up front (aa79253). Move that policy into a shared helper in isal_shared.h and use it from both decompressors. Callers that pass max_length almost always fill it (fixed-size block reads, websocket message size limits), so this is one allocation that is shrunk at most once instead of a realloc chain: - decompress(chunk, 128 KiB): one 128 KiB allocation, no realloc at all. - decompress(msg, 4 MiB + 1) with a 512 KiB message (aiohttp): one allocation and one in-place shrink instead of six doublings. - no max_length, or sys.maxsize: unchanged, so large one-shot calls keep the current growth behaviour. The concatenated output is byte-identical. Because ISA-L reads ahead into its internal output buffer each time output space runs out, the split between the returned bytes and unconsumed_tail for a single capped call can differ from before. Add tests that lock in exact-size chunks for a range of max_length values, and benchmark_scripts/benchmark_output_buffer.py covering the cases from the issue and the call shapes aiohttp uses. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R5pkuwR7q3BJ3gj5hAwGRt
Compress.compress, isal_zlib.compress and igzip_lib.compress started their output buffer at 16 KiB regardless of the input size, so compressing a 512 KiB message of mostly incompressible data grew the buffer five times by doubling, and a 128 KiB chunk twice. Start from a level-aware estimate of the output instead: level 0 uses static Huffman tables and expands incompressible input by about 23%, levels 1-3 fall back to stored blocks so the output stays close to the input size. A 32 KiB slack covers the wrapper and output that ISA-L held back from an earlier call (about 22 KiB for levels 1-2). Inputs smaller than 16 KiB keep the 16 KiB buffer, and the estimate is capped at 16 MiB, above which the buffer grows as before. This is only a hint: the growth path is unchanged for outputs that exceed it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R5pkuwR7q3BJ3gj5hAwGRt
bdraco
force-pushed
the
preallocate-max-length-output-buffer
branch
from
September 14, 2026 01:45
da0c162 to
971f329
Compare
Preallocating max_length is wasteful when the input is tiny: a 100 byte websocket message with aiohttp's default 4 MiB limit allocated 4 MiB and shrank it to 100 bytes on every message. Deflate expands by at most 1032:1, so cap the preallocation at pending output ISA-L still holds plus (input + 8 bytes of bit buffer) * 1032 plus 16 KiB of slack. A 100 byte message now gets a ~132 KiB buffer, a 128 KiB input slice or a 300 KiB websocket payload still gets the full max_length, and unbounded calls are unchanged. The bound is an upper bound, so the buffer still never has to grow on these paths. Also add a tiny-message websocket send case to the benchmark. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R5pkuwR7q3BJ3gj5hAwGRt
- Merge initial_output_buffer_size and inflate_output_bound into one inflate_initial_buffer_size used by both decompressors. The input bound only applies when the input cannot fill the capped size, which also removes the overflow guard. Use ISAL_DEF_MAX_MATCH and the bit buffer size instead of magic numbers. - Compute the Decompress.decompress buffer size after taking the lock, since it now reads the inflate state. - Collapse IgzipDecompressor's hard_limit if/else and simplify compress_initial_buffer_size. - Tests: size data by max_length and compress it once, drop cases that existing tests already cover. The new tests took about 140 s, mostly max_length=1 over 3.5 MB, and now take under 2 s. - Benchmark: build each case's data only when it runs, so --cases skips unrelated setup and one case's data does not stay alive during the next; use timeit.repeat. - Shorten the changelog entries and the isal_shared.h header note. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R5pkuwR7q3BJ3gj5hAwGRt
The previous approach preallocated max_length, bounded by the maximum deflate expansion of 1032:1. As pointed out in pycompression#256, that sizes every buffer for a decompression bomb: a 20 KiB JSON websocket message with a 4 MiB limit allocated 4 MiB. Start from a guess instead: 8 times the input, at least 16 KiB and at most 1 MiB, never above max_length. On the orjson sample payloads 8 times covers typical JSON without growing. Small messages often compress far better because of earlier context, so the 16 KiB floor keeps them growth-free. Larger or more compressible outputs grow as before. The 1 MiB cap bounds the transient over-allocation for incompressible input. IgzipDecompressor is left as upstream has it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R5pkuwR7q3BJ3gj5hAwGRt
bdraco
changed the title
|
|
aiolibsbot
approved these changes
Sep 14, 2026
aiolibsbot
left a comment
aiolibsbot
left a comment
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
- Restore the original isal_shared.h header rationale for the single output buffer instead of claiming it grows in place without copies. - Rename the decompress sizing constants to DEF_DECOMP_GUESS_RATIO and DEF_MAX_DECOMP_GUESS_SIZE and define them next to DEF_BUF_SIZE, so they are not mistaken for the exported DECOMP_* flags. - Fix the MAX_LENGTHS comment, which still mentioned the 4 MiB limit. - Benchmark: every case now returns the uncompressed bytes it processed and is checked against the expected size before timing, and the stream and HTTP body cases fail if decompression does not reach the end of the stream, so truncated output cannot look like a speed-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R5pkuwR7q3BJ3gj5hAwGRt
PR Review — Size the initial decompress buffer from the inputMerge-ready: all five findings from the last review are fixed at What's done well:
Prior findings, now fixed:
Not verified: Python is not available in this review shell, so I did not build the extension or run the tests or benchmarks. The realloc counts and timings are the author's. I also could not confirm that the uncompressed ✅ Resolved since last review (3)Previously-flagged issues verified fixed
Checklist
Silent Failure Analysis
🟡 **1. MEDIUM** — discarded result / vacuous validation
|
aiolibsbot
approved these changes
Sep 14, 2026
aiolibsbot
left a comment
aiolibsbot
left a comment
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Experiment for pycompression#256, on this fork to test before proposing anything upstream.
Rule
Decompress.decompresssizes its initial output buffer from the input:DEF_BUF_SIZE, the current starting size).max_lengthwhen one is given.The compress side uses a separate hint:
compress(),igzip_lib.compress()andcompressobj().compress()start at the input size plus 25% at level 0 or plus 6.25% at levels 1 to 3, plus 32 KiB.IgzipDecompressor,Decompress.flush, and the one-shotdecompress()functions are unchanged.Why these numbers
max_length, bounded by the maximum deflate expansion of 1032:1. As the maintainer pointed out in the issue, that sizes every buffer for a decompression bomb: a 20 KiB JSON message with a 4 MiB limit allocated 4 MiB.Benchmarks
Everything below compares this PR against
develop, run back to back on the same machine: macOS 26.6 on arm64 with Python 3.14.5, and Linux aarch64 in Docker with glibc 2.36 and Python 3.14.In the comparison columns, "faster" and "slower" compare speed: "25% faster" means this PR does the same work 1.25 times as fast. Differences under 2% are shown as "same", and differences of 5% or more are in bold.
Real traffic: Home Assistant websocket frames
Receiving two frames captured from a live Home Assistant instance, the way aiohttp does it: one decompressor per connection,
decompress(unconsumed_tail + payload + b"\x00\x00\xff\xff", max_msg_size + 1). The varied streams change timestamps, ids and states in every message. Time per message, median of 5 interleaved runs. Lower is better.These frames already fit the 16 KiB starting buffer on
develop, so no change is expected here. The table confirms the new sizing adds no cost to the common small-message path.macOS:
Linux:
orjson payloads received as one websocket message
The four orjson sample files, each received as a single message, compressed either by aiohttp (isal level 0) or by a zlib level 6 peer. The last row sends every JSON object from three of the files as its own message. Time per message, median of 6 interleaved runs. Lower is better.
macOS:
Linux:
The one consistent slowdown on macOS, citm_catalog from a zlib level 6 peer, is where each build's doubling lands, not extra work. That message compresses 45 to 1.
developgrows 7 times from 16 KiB and copies about 1.95 MiB. This PR starts at about 300 KiB, grows 3 times and copies about 2.16 MiB, because its last doubling overshoots the 1.65 MiB output further. Payloads that land the other way, such as github.json from the same sender, are faster. On macOS every one of those grows moved the buffer, as did the final shrink, in both builds. On Linux the same citm_catalog message is 3% faster with this PR, and no orjson payload is more than 2% slower.Allocation behaviour
Counted with a
reallocinterposer, one warm run per case. "grows" is the number ofrealloccalls to a larger size, the ones that may copy depending on heap layout.orjson samples received the way aiohttp does for websockets:
macOS:
Linux:
benchmark_scripts/benchmark_output_buffer.pycases,--size-mib 64:macOS:
Linux:
aiohttp-ws-recv-512K-xis aiohttp's own benchmark payload,b"x" * 512 KiB, which compresses 100 to 250 to 1. No input-proportional guess reaches that without over-allocating for everything else, so it still grows 4 times per message instead of 6.Synthetic cases
python benchmark_scripts/benchmark_output_buffer.py --rounds 7, best of 3 interleaved runs. Total time per case. Lower is better.macOS:
Linux:
Several threads decompressing at once
Each thread has its own decompressor. Three workloads run through it: 26 MiB of JSON in 128 KiB blocks, 4 KiB websocket messages, and 180 B state change frames. Numbers are MiB of output per second across all threads, best of 3 within a run and median of 3 interleaved runs. Higher is better. This checks that the larger initial buffers do not add contention.
macOS:
Linux:
For both builds, four threads receiving small messages are slower than one.
Decompress.decompressreleases the GIL on every call, and for messages this small the hand-off costs more than the work. That behaviour predates this PR, and #2 addresses it on the compression side.Instruction counts
Executed instructions counted with callgrind on Linux, including interpreter start-up. These counts do not depend on heap layout or CPU contention. The small-message receive does the same work on both builds, and the block-streaming case, where
developreallocated 1506 times, executes 0.7% fewer instructions.Tests
tests/test_compat.py: exact-size chunks formax_lengthfrom 1 to 512 KiB + 1,max_lengthlarger than the output, exact fill, repeated empty input, a websocket-like sync-flush stream, and compress roundtrips of zeros and random data at all levels for sizes around 16 KiB, 128 KiB, 1 MiB and 17 MiB.tests/test_igzip_lib.py: an exact-size chunk loop forIgzipDecompressor, as a regression test for the unchanged code.🤖 Generated with Claude Code
https://claude.ai/code/session_01R5pkuwR7q3BJ3gj5hAwGRt