On Fri, 30 Sep 2022 09:10:05 GMT, Jaikiran Pai <[email protected]> wrote:

> Can I please get a review for this test-only change which proposes to improve 
> the test run duration of `java/net/vthread/HttpALot.java` test, on Linux? 
> This relates to https://bugs.openjdk.org/browse/JDK-8294610
> 
> Experiments have shown that on Linux, due to delayed TCP ACKs from the client 
> (for some previous sent data/packet), the server sent response waits for the 
> ACKs before sending the response data (in a TCP packet). Each delayed ACK is 
> 40 milli seconds and given the number of requests this test (intentionally) 
> issues, the delay accumulates causing the test to take longer duration.
> 
> With the change in this PR, the test completes in around 6 to 8 seconds on a 
> Linux system as compared to 60 odd seconds previously on the same setup. This 
> change hasn't shown any negative impact on macos (which continues to run this 
> test in around 5 to 6 seconds like previously). I will trigger some runs in 
> our CI setup to make sure this works as expected in all other OS too.
> 
> As noted in the linked JBS issue, additional experiments to see if anything 
> can be improved in the JDK network layer will be carried out separately, 
> outside  the context of this test.

test/jdk/java/net/vthread/HttpALot.java line 32:

> 30:  * @library /test/lib
> 31:  * @compile --enable-preview -source ${jdk.version} HttpALot.java
> 32:  * @comment The test launch command intentionally uses 
> -Dsun.net.httpserver.nodelay=true

Maybe replace this "The test runs with -Dsun.net.httpserver.nodelay=true" as 
the words "launch command intentionally uses" aren't really needed.

test/jdk/java/net/vthread/HttpALot.java line 36:

> 34:  *          to avoid (occasional 40 milli seconds) delays in receiving 
> responses from the server.
> 35:  *          Given the number of requests being fired in this test, the 
> delayed responses can
> 36:  *          accumulate into longer test run duration

It might be clearer to say that "to avoid occasional 40ms delays receiving 
responses from the server on Linux.". I think you can drop "Given the number 
..." sentence.

test/jdk/java/net/vthread/HttpALot.java line 39:

> 37:  * @run main/othervm/timeout=600
> 38:  *     --enable-preview
> 39:  *     -Dsun.net.httpserver.nodelay=true

good, I'm happy this issue was found and the HTTP server already has a way to 
change this option.

-------------

PR: https://git.openjdk.org/jdk/pull/10504

Reply via email to