Give the memory-limited response tests room for allocation noise - #5234
Merged
Merged
Conversation
The test reads a 10 MiB body and allowed a peak of 10.5 MiB, so half a mebibyte of headroom. It exists to catch the body being buffered a second time, which takes the peak to 20 MiB, and 5% is far tighter than that needs. On CI the test intermittently peaked at 11.0 MiB and failed. Allow 13 MiB instead. A retained second copy still trips the limit by a 7 MiB margin, and the noise that was failing the job no longer does.
test_memory_usage_single_chunk and test_memory_usage_splitting_chunk allowed 10 KiB over their measured peak, and test_memory_usage allowed 0.37 MiB. Margins that thin do not measure a memory bound, they measure whether the allocator behaves exactly as it did when the number was written, and CI has now failed on two of these tests in a day. Each of these guards against the body being held a second time, which takes the peak to about 20 MiB, so there is a lot of room between the real peak and the thing being caught. Use it: every limit here keeps at least a 5 MiB margin below a doubled body, and an added copy still trips all six parametrizations.
illia-v
reviewed
Sep 8, 2026
illia-v
approved these changes
Sep 9, 2026
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.
Several
limit_memorymarks intest/test_response.pysit within a rounding error of their measured peak, so ordinary allocation noise fails them. Two have failed on CI in the past day:Measured with
memrayon Python 3.13:test_memory_usagetest_memory_usage_single_chunktest_memory_usage_splitting_chunktest_buffer_memory_usage_no_decodingThree of them allow 10 KiB of slack on a 10 MiB allocation. That is not a memory bound, it is an assertion that the allocator behaves exactly as it did when the number was written.
Each of these tests guards against the body being retained a second time, which takes the peak to roughly 20 MiB, so there is a wide gap between the real peak and the regression being caught. The new limits keep at least a 5 MiB margin below a doubled body while giving noise 2-3 MiB instead of 0.01 MiB.
test_memory_usage_decode_with_max_length(10 MB) andtest_buffer_memory_usage_decode_one_chunk(25 MB) are left alone: neither has failed, and I did not measure them.Verification
test/test_response.pypasses on 3.12 and 3.13: 169 passed, 45 skipped.memory_usagefamily undercoverage run --parallel-mode, as the CI session runs them: clean.parametrizations fail at the new limits, so they still catch what they
are for.
One caveat stated plainly: I could not reproduce the CI overshoot locally on either 3.12 or 3.13, with or without coverage. The case rests on the measured margins and the observed failures rather than on a local reproduction. If you would rather see the extra megabyte accounted for before widening anything, I am happy to dig into that instead.