[
https://issues.apache.org/jira/browse/HTTPCORE-726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17619021#comment-17619021
]
Oleg Kalnichevski commented on HTTPCORE-726:
--------------------------------------------
> For a fix I was thinking of updating the capacity tracking to be similar to
> {{ReactiveDataConsumer}} in tracking the number of bytes read and then
> reporting that delta to {{{}CapacityChannel{}}}.
[~johnlcox] I tried out several different things but ended up choosing the fix
you have proposed with some fairly small tweaks. Please review the change-set
[1] and let me know if my modifications to your original patch look acceptable.
Please also feel free to let me know your GitHub account name or email address.
I will amend the commit with your name as the author.
> This does seem to prevent the buffer from expanding to huge sizes, but it
> still expands more than I would expect because after each read from the
> {{{}SharedInputBuffer{}}}.
In my local tests the input buffer gets expanded a little more beyond the
initial window size (64K) which looks acceptable to me given that the test case
uses a producer on the server that sends data in a tight stream, while the
consumer on the client side reads 16 byte at a time and generally is quite a
bit slower.
{noformat}
Expanding capacity: 63488
Expanding capacity: 64512
Expanding capacity: 65536
{noformat}
What we can do though is make {{SharedInputBuffer}} expand more aggressively
until its internal buffer is smaller than the initial window size (64K), thus
reducing the number of allocations, data copy operations and intermediate
garbage.
> Separately from that, the piece I'm still not understanding is how
> CapacityWindow in AbstractHttp1StreamDuplexer is intended to control when the
> more bytes are read off the socket. Is it odd that it's setting OP_READ even
> though the window is still negative after being incremented?
It took me some time to recall what the rationale was but here is it. Unlike
HTTP/2 data frame counting with HTTP/1.1 is just an approximation that has no
bearing on what the opposite endpoint will end up sending. The assumption is
that if the consumer pushes a capacity update greater than 0 it is capable of
receiving at least _some_ data. Even if the total value of the window is
negative it is just an approximation so it is okay to ask the server for more
data even if it can be more than the declared capacity.
I am thinking about changing this behavior in the 5.2 if you are willing to
work with me and help me with testing.
Oleg
[1]
[https://github.com/apache/httpcomponents-core/commit/dc179e1416c65edc36de49cf8edf42ab1e941833]
> SharedInputBuffer expands unbounded on large responses
> ------------------------------------------------------
>
> Key: HTTPCORE-726
> URL: https://issues.apache.org/jira/browse/HTTPCORE-726
> Project: HttpComponents HttpCore
> Issue Type: Bug
> Components: HttpCore
> Affects Versions: 5.1.3
> Environment: MacOS 12, Debian 9, Debian 10
> Reporter: John Leacox
> Priority: Major
> Fix For: 5.1.5, 5.2-beta3
>
> Attachments: SharedInputBuffer_Unbounded_Growth_Example.patch,
> SharedInputBuffer_Unbounded_Growth_Possible_Fix.patch
>
>
> Large responses can result in the {{SharedInputBuffer}} expanding unbounded,
> but with small intervals. Each expansion is the minimum 1024 byte aligned
> size bigger than the current buffer that is filling it beyond its current
> capacity. This leads to a ton of heap allocations and garbage collection each
> time is expands.
>
> It looks like the capacity updates to {{CapacityChannel}} are reporting
> current capacity rather than deltas/increments, so that even when small reads
> occur, the whole capacity is reported again to the channel and the capacity
> window also grows unbounded.
>
> I've attached a patch that includes some {{println}} statements for the
> capacity window size and for buffer being expanded along with the integration
> test {{testLargeResponseConsumer}} that shows the buffer and window growing
> quite large (the test passes, but the println statements show how large they
> are growing).
>
> For a fix I was thinking of updating the capacity tracking to be similar to
> {{ReactiveDataConsumer}} in tracking the number of bytes read and then
> reporting that delta to {{CapacityChannel}}. This does seem to prevent the
> buffer from expanding to huge sizes, but it still expands more than I would
> expect because after each read from the {{SharedInputBuffer}}. This remaining
> expansion appears to be from {{CapacityWindow}} always setting the IO
> sessions with {{OP_READ}} even when the window is still negative after the
> update.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
