Am 2020-06-02 um 00:09 schrieb Michael Osipov:
Am 2020-06-01 um 15:11 schrieb Oleg Kalnichevski:
On Sun, 2020-05-31 at 23:13 +0200, Michael Osipov wrote:
Am 2020-05-30 um 22:53 schrieb Oleg Kalnichevski:
On Sat, 2020-05-30 at 21:58 +0200, Michael Osipov wrote:
...
Now we need to decide if this is as good as it gets or we want
to
put
more research into it.

I read liked you idea of the chunks. 2048 seems too low for me
because
most well write with a buffer of at least 4 KiB.

So I think we can try to things to miminize overhead:

* Make this feature optional, just like we do with Expectation.
* Try to increase the step chunk size, e.g., 64 KiB or greater.
With
that we don't peek on each and every invocation.


Yes, we may consider making it optional / disabled by default

Via RequestConfig?

We can give step chunking another go but it is less important now
as I
have inserted the check into the low level socket stream whose
write
operations should be already quite optimized by the protocol layer.

But wouldn't that still reduce the check count because we'd hit the
wait
with SSLSocket? I am bit confused by your statement.


It might but what I am trying to say is that even now there would
likely be only one check on every large data chunk eliminating the
worst case scenario of the application layer writing one or two bytes
at a time each triggering a check and incurring a potential hit of 50
ms.

I have tried the modified patch. Works. Picked up the the chunk patch and raised to 64 KiB. Also in consideration that AbstractHttpEntity uses a default buffer size of 4 KiB and TCP max packet size of 64 KiB.
Then tried with to different connections to the server at work:

* VPN connection: Performs great for plain and TLS, after 128 KiB (two probes) in almost all cases 401 is seen. Works also w/o chunks great. * Via ZScaler app which routes the traffic through a HTTP proxy to the corporate network. It takes way more probes to detect the 401. But still performs acceptable. Chunks made also virtually no difference. At some point it is even worse with broken connections (plain HTTP). HTTPS performs much worse.

So at least for the test options I have here, I tend to agree with you. We either drop the step chunks or need to reduce the size to a sane value.

I have now reduced the chunk size which performs nicely. I hope that it will not be necessary and no one will write byte-by-byte. See attachment.

My last question would be: How can we wire the RequestConfig parameter to the DefaultBHttpClientConnection class? Http1Config?

M
diff --git 
a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java
 
b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java
index dffcd39d9..6994ec512 100644
--- 
a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java
+++ 
b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java
@@ -161,26 +161,36 @@ public void sendRequestEntity(final ClassicHttpRequest 
request) throws HttpExcep
                     final InputStream socketInputStream = 
socketHolder.getInputStream();
                     final OutputStream socketOutputStream = 
socketHolder.getOutputStream();
 
+                    long totalBytes = 0;
+                    long chunks = -1;
+
                     void checkForEarlyResponse() throws IOException {
-                        if (ssl ? isDataAvailable(Timeout.ONE_MILLISECOND) : 
(socketInputStream.available() > 0)) {
-                            throw new ResponseOutOfOrderException();
+                        final long n = totalBytes / (8 * 1024);
+                        if (n > chunks) {
+                            chunks = n;
+                            if (ssl ? isDataAvailable(Timeout.ONE_MILLISECOND) 
: (socketInputStream.available() > 0)) {
+                                throw new ResponseOutOfOrderException();
+                            }
                         }
                     }
 
                     @Override
                     public void write(final byte[] b) throws IOException {
+                        totalBytes += b.length;
                         checkForEarlyResponse();
                         socketOutputStream.write(b);
                     }
 
                     @Override
                     public void write(final byte[] b, final int off, final int 
len) throws IOException {
+                        totalBytes += len;
                         checkForEarlyResponse();
                         socketOutputStream.write(b, off, len);
                     }
 
                     @Override
                     public void write(final int b) throws IOException {
+                        totalBytes++;
                         checkForEarlyResponse();
                         socketOutputStream.write(b);
                     }
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to