Decode a lone response body part in place - #2303
Merged
hyperxpro merged 3 commits intoAug 5, 2026
Merged
Conversation
Response#getResponseBody went through getResponseBodyAsBytes, which concatenates every body part into a freshly allocated array. A body that arrived in a single read was therefore copied twice: once to concatenate a single part with nothing, and once to decode. That is the common case for responses small enough to land in one socket read, and it is the path the default AsyncCompletionHandlerBase puts every caller of executeRequest(request) on. Decode straight from the part when there is exactly one. A rough probe on JDK 17 over a single-part ASCII body measured 496 -> 47 ns at 512 B, 2277 -> 373 ns at 4 KB, 2963 -> 1450 ns at 16 KB and 24913 -> 12248 ns at 128 KB, alongside one fewer whole-body allocation. Several parts are still concatenated before decoding, never decoded one at a time, because a multi-byte character can straddle a part boundary. The new test pins that by splitting a two-byte UTF-8 character across two parts and asserting both shapes decode alike. getResponseBodyAsBytes and getResponseBodyAsByteBuffer are deliberately left alone: they hand the array to the caller, so they keep copying rather than expose a part's own array. A second test pins that too. Claude Code on behalf of Pavel Ptashyts Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hyperxpro
requested changes
Aug 3, 2026
Review feedback on AsyncHttpClient#2303. getResponseBodyAsStream wraps the bytes in a ByteArrayInputStream, which never exposes the array it holds, so it can read a lone part's own array for the same reason getResponseBody can. Both now go through one private sharedBodyBytes(), which keeps the guard and the constraint on it in a single place. Cut the comment down. Two of its five lines described what the previous version did, which belongs in a commit message, and the file carries no other comments. Add a case for a Lazy part over a slice. Lazy is where the shortcut is least obvious, because its getBodyPartBytes returns only the readable region rather than the whole backing array, so a later change reaching for getBodyByteBuf().array() would read the surrounding bytes instead. The case covers both the decode and the stream path. Claude Code on behalf of Pavel Ptashyts Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hyperxpro
requested changes
Aug 5, 2026
Co-authored-by: Aayush Atharva <24762260+hyperxpro@users.noreply.github.com>
hyperxpro
approved these changes
Aug 5, 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
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.
Problem
For the default handler (
executeRequest(request)->AsyncCompletionHandlerBase->
Response) the body is copied three times:HttpHandler.handleChunk->EagerResponseBodyPartcopies each chunk out ofthe network buffer into a heap
byte[](needed:channelReadreleases themessage afterwards);
NettyResponse.getResponseBodyAsByteBufferconcatenates every part into afreshly allocated array;
getResponseBody(charset)decodes that array.Step 2 is pure waste when there is only one part, which is the case for any body
that lands in a single socket read: it copies a single part into a new array with
nothing to concatenate it with.
Change
getResponseBody(Charset)decodes straight from the part when there is exactlyone of them. The array does not escape the method, so the part's own array can be
decoded in place.
Several parts are still concatenated before decoding, never decoded one at a
time, because a multi-byte character can straddle a part boundary.
getResponseBodyAsBytesandgetResponseBodyAsByteBufferare deliberately leftuntouched: they hand the array to the caller, so they keep making a defensive
copy rather than expose a part's own array. There is no aliasing change anywhere
in this PR.
Measurements
Rough probe on JDK 17 over a single-part ASCII body, concatenate-then-decode
versus decode-in-place. Not JMH, so read the shape rather than the digits:
Plus one fewer whole-body allocation per response. The percentages look large
partly because a pure-ASCII body decodes through a JDK intrinsic, which makes the
removed copy a big share of what is left.
Tests
Two added to
NettyAsyncResponseTest:testGetResponseBodyDecodesOnePartAndSplitPartsIdenticallysplits the two-byteUTF-8 encoding of U+00E9 across two parts and asserts one-part and split-part
bodies decode alike. This pins the constraint the comment states: it fails if
anyone later makes the multi-part path decode part by part.
testGetResponseBodyAsBytesDoesNotShareTheBodyPartArraypins thatgetResponseBodyAsBytesstill returns a fresh array and never the part's own.The body bytes are built as an explicit
byte[]rather than a string literal tokeep the source ASCII per
AGENTS.md.Verification
mvnw clean verify- BUILD SUCCESS, 1373 tests (1371 before, plus these two),0 failures, 0 errors, 19 skipped. Error Prone, NullAway and Revapi clean.
LargeResponseTest,NoNullResponseTest,BodyDeferringAsyncHandlerTestandRedirectBodyTest, which exercise the multi-part path, are green.Caveat on the testing gate:
AGENTS.mdrequires the build to run on JDK 11 andno JDK 11 is installed on this machine, so it was run on JDK 17 (also in the
CI matrix). The JDK 11 leg of CI on this PR is the real gate.
Not in scope
The multi-part case still concatenates. A
CompositeByteBuf.toString(charset)variant measured faster there (Netty decodes a multi-component buffer through a
recycled, un-zeroed thread-local array instead of a fresh
byte[]), but itregressed at high part counts in the same probe, so it needs proper benchmarking
before it becomes a change. Removing copy 1 would mean retaining network buffers
and giving
Responsea lifecycle, which is public API and wants a designdiscussion first.
No public API change here. Same review pass as #2300, #2301 and #2302.
Claude Code on behalf of @pavel-ptashyts
🤖 Generated with Claude Code