Signal STOP on the finishing multipart chunk, not an extra empty read by pavel-ptashyts · Pull Request #2232 · AsyncHttpClient/async-http-client · GitHub
Skip to content

Signal STOP on the finishing multipart chunk, not an extra empty read#2232

Open
pavel-ptashyts wants to merge 1 commit into
AsyncHttpClient:mainfrom
maygemdev:perf/multipart-stop-on-last-chunk
Open

Signal STOP on the finishing multipart chunk, not an extra empty read#2232
pavel-ptashyts wants to merge 1 commit into
AsyncHttpClient:mainfrom
maygemdev:perf/multipart-stop-on-last-chunk

Conversation

@pavel-ptashyts

Copy link
Copy Markdown
Contributor

MultipartBody.transferTo(ByteBuf) returned CONTINUE even on the call that wrote the last part and set done=true, so the terminal STOP was only discovered on the NEXT call — which allocated a fresh pooled buffer, found done==true, and returned STOP with an empty buffer. That extra readChunk/nextChunk cycle happened once per multipart request on the HTTPS / disabled-zero-copy path (BodyChunkedInput).

Return 'done ? STOP : CONTINUE' so the finishing call itself reports STOP. Both ByteBuf consumers send the bytes written on that call before honouring STOP — BodyChunkedInput returns the buffer and sets endOfInput (eliminating the extra empty readChunk); the HTTP/2 pump returns a readable buffer before checking state — so the terminal chunk is never dropped. The zero-copy (WritableByteChannel) and plain-HTTP FileRegion paths are unaffected.

Because STOP can now carry the body's last bytes, MultipartBodyTest's transferWithCopy helper (which exited on STOP without counting that call) is updated to count bytes on every call, mirroring the real consumers. Adds a test that the finishing call reports STOP and still carries all of the body's bytes; existing multipart body/part/upload tests pass unchanged. No public API change.

MultipartBody.transferTo(ByteBuf) returned CONTINUE even on the call that wrote the last part and set done=true, so the terminal STOP was only discovered on the NEXT call — which allocated a fresh pooled buffer, found done==true, and returned STOP with an empty buffer. That extra readChunk/nextChunk cycle happened once per multipart request on the HTTPS / disabled-zero-copy path (BodyChunkedInput).

Return 'done ? STOP : CONTINUE' so the finishing call itself reports STOP. Both ByteBuf consumers send the bytes written on that call before honouring STOP — BodyChunkedInput returns the buffer and sets endOfInput (eliminating the extra empty readChunk); the HTTP/2 pump returns a readable buffer before checking state — so the terminal chunk is never dropped. The zero-copy (WritableByteChannel) and plain-HTTP FileRegion paths are unaffected.

Because STOP can now carry the body's last bytes, MultipartBodyTest's transferWithCopy helper (which exited on STOP without counting that call) is updated to count bytes on every call, mirroring the real consumers. Adds a test that the finishing call reports STOP and still carries all of the body's bytes; existing multipart body/part/upload tests pass unchanged. No public API change.

Fixes finding AsyncHttpClient#9.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant