carterkozak commented on pull request #206: URL: https://github.com/apache/httpcomponents-core/pull/206#issuecomment-665710369
> This started out like a very straightforward and cheap improvement based on InputStream#available and now it has ballooned into something with a strategy, three JIRA tickets and a UN Security Council type of discussion. Sorry, I hope I haven't created too much noise, I've tried to isolate findings into separate tickets to keep each discussion somewhat isolated, but I fear I could have done better! > Can we tackle the issue in iterative steps? I completely agree that this should be done in small steps, my intent for this PR was to avoid changing any functionality, and to simply provide a mechanism that can be used to configure non-default functionality. I've tried to brainstorm potential implementations here in order to collect feedback and ensure the strategy interface can support potential future requirements and ideas, I don't think it's a good idea to make functional changes in this PR. > If the out of order response check indeed decreases connection throughput by a factor of 6 we have to consider removing #isDataAvailable call from 5.0.x My use case wouldn't be impacted if the isDataAvailable check is removed, my goal for this PR is to provide a way for me to disable the check without adversely impacting other HC users. Very supportive of removing the check from 5.0.x, I defer to your judgement on whether or not the more complex form should be kept in 5.1, or removed entirely. > Could you please confirm your findings? Of course, I have some analysis from an internal production service. These graphs are limited to requests with size between 40MiB and 50MiB. The graphs are based on data from our request logs, the charts are based user-agents before and after bypassing out of order response checks. Note that this includes some amount of time processing the request on the server in addition to pure transfer rate. Clients using out of order response checks (default 5.0.1 behavior):  Clients with the check disabled by subclassing DefaultBHttpClientConnection perform substantially better:  The best case drops from six seconds to below a half second when the check is disabled, and the 50th percentile drops from just over six seconds to just under one second. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
