RFR: 8330523: Reduce runtime and improve efficiency of KeepAliveTest [v3]

[Daniel Jeliński]

On Sat, 20 Apr 2024 23:13:48 GMT, Christoph Langer <clanger at openjdk.org> wrote:

>> The test case sun/net/www/http/HttpClient/KeepAliveTest.java could be more effective.
>> 
>> It tests a matrix of HTTP client settings and server behavior, resulting in 160 individual test scenarios. Each is tested in an own freshly spawned JVM via the `@run main/othervm` directive. The need for new VMs is due to the fact that the behavior of the HTTP client is determined at VM initialization and can not be changed later on. However, for each distinct type of client settings, one VM can be reused. This would lead us from 160 JVM instantiations down to 16 which has a factor 10 influence on test runtime.
>> 
>> E.g. on my developer laptop runtime went down from ~100s to ~10s.
>> 
>> I also made additional cleanups/refactoring in the test.
>
> Christoph Langer has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Simplify the test further
>  - Merge branch 'master' into JDK-8330523
>  - Small further cleanup
>  - JDK-8330523

This looks reasonable.

test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 112:

> 110:     private volatile boolean isProxySet;
> 111: 
> 112:     private CountDownLatch serverLatch = new CountDownLatch(1);

CountDownLatch is not reusable; consider using a Semaphore here

test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 113:

> 111: 
> 112:     private CountDownLatch serverLatch = new CountDownLatch(1);
> 113:     private Thread server;

this can be converted to a local variable easily

test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 315:

> 313:         keepAliveKeyClassconstructor.setAccessible(true);
> 314: 
> 315:         Logger logger = Logger.getLogger("sun.net.www.protocol.http.HttpURLConnection");

the logger was a static field for a reason; we don't want it to be GCed in the middle of the test

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

PR Review: https://git.openjdk.org/jdk/pull/18817#pullrequestreview-2019394795
PR Review Comment: https://git.openjdk.org/jdk/pull/18817#discussion_r1577641489
PR Review Comment: https://git.openjdk.org/jdk/pull/18817#discussion_r1577643840
PR Review Comment: https://git.openjdk.org/jdk/pull/18817#discussion_r1577600648