Repository navigation
Conversation
te_chunked/2 ignored its state and ran read_size/1 and read_chunk/2
from the start of the buffer on every socket read until the whole chunk
was there. That match stopped the next append in execute/2 from
extending the buffer in place, so every read copied the whole buffer.
owner_recv/1 reads with length 0, 1460 bytes at a time by default, so
one 8 MB chunk took about 5700 reads and 2.3 s, a 16 MB one over 10 s.
The state now carries {need, Bytes}, the size of the whole chunk from
the start of the buffer, and te_chunked/2 returns more after a
byte_size/1 check until that much is buffered. 1.x fixed the same case
for benoitc#690 by reading the rest of the chunk in one recv from
hackney_response (benoitc#710), which 2.0.0 removed. A single recv would also
put the whole chunk under recv_timeout, and gen_tcp returns
{error, enomem} for a length above 64 MB.
Fixes benoitc#378
read_full_body/2 appended each chunk with <<Acc/binary, Chunk/binary>>, which copies the body read so far every time. With the parser fixed, a 16 MB body in 16 KB chunks still took about 800 ms, 93% of it in read_full_body/2 by eprof. The chunks now go into a list with a running size for the max_body_size check and are joined once at the end; the same body takes 30 ms.
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.
Fixes #378.
Reading a chunked body was quadratic in the chunk size.
te_chunked/2ignored its state and ranread_size/1andread_chunk/2from the start of the buffer after every socket read, and that match made each append inexecute/2copy the whole buffer.owner_recv/1reads with length 0, so one 8 MB chunk arrives in about 5700 reads of 1460 bytes.read_full_body/2appended each chunk to the body the same way. 1.x fixed this for #690 by reading the rest of the chunk in one recv fromhackney_response(#710), which 2.0.0 removed.te_chunked/2now keeps{need, Bytes}for an incomplete chunk, the extra state #378 proposed, and returnsmoreafter abyte_size/1check until the whole chunk is buffered.read_full_body/2collects the chunks in a list and joins them once. I did not bring back the #710 recv: it puts the whole chunk underrecv_timeout, andgen_tcp:recv/3returns{error, enomem}above 64 MB.max_body_sizeis still checked per complete chunk, and the HTTP/2 and HTTP/3 accumulators are untouched.16 MB body on loopback, OTP 27, 4.9.1 -> this branch: 2 x 8 MB chunks 4756 -> 22 ms, one 16 MB chunk 10675 -> 23 ms, 1 MB chunks 527 -> 27 ms, 16 KB chunks 901 -> 30 ms. 1.25.0 takes about 12 ms for the 2 x 8 MB case.
Five new tests in
hackney_http_testsfeed the parser directly: a 512 KB chunk in 1460-byte reads, a body with extensions and trailers split at every byte, the{need, N}state across reads, and a bad chunk terminator at every split. Full eunit passes, 1235 tests, xref and dialyzer clean.