RFR: CODETOOLS-7903471: JMH: Add test for virtual threads executor [v3]

Aleksey Shipilev shade at openjdk.org
Thu May 25 14:34:36 UTC 2023


On Wed, 24 May 2023 03:51:49 GMT, Sergey Kuksenko <skuksenko at openjdk.org> wrote:

>> Add test for virtual thread executor
>
> Sergey Kuksenko has updated the pull request incrementally with one additional commit since the last revision:
> 
>   extend tests

Looks fine with minor nits:

Actually, no, wait. See GHA failures, there are plenty of them.

jmh-core-it/src/test/java/org/openjdk/jmh/it/threads/BenchmarkBenchSameThreadTest.java line 123:

> 121:     public void teardownZZZ() { // should perform last
> 122:         Assert.assertFalse("Test sanity", testInvocationThread.isEmpty());
> 123:         if(benchmarkExecutorType != ExecutorType.CACHED_TPE) { // CachedThreadPool doesn't guarantee same thread rule

Suggestion:

        if (benchmarkExecutorType != ExecutorType.CACHED_TPE) { // CachedThreadPool doesn't guarantee same thread rule

jmh-core-it/src/test/java/org/openjdk/jmh/it/threads/BenchmarkBenchSameThreadTest.java line 131:

> 129:             Assert.assertTrue("test <: teardownInvocation", testInvocationThread.containsAll(teardownInvocationThread));
> 130:         }
> 131:         if(benchmarkExecutorType == ExecutorType.VIRTUAL_TPE) {

Suggestion:

        if (benchmarkExecutorType == ExecutorType.VIRTUAL_TPE) {

jmh-core-it/src/test/java/org/openjdk/jmh/it/threads/BenchmarkBenchSameThreadTest.java line 148:

> 146:             Assert.assertTrue("testInvocation thread kind", testInvocationThread.stream().noneMatch(VirtualAPI::isVirtual));
> 147:         }
> 148:         if(benchmarkExecutorType == ExecutorType.FJP) {

Suggestion:

        if (benchmarkExecutorType == ExecutorType.FJP) {

jmh-core-it/src/test/java/org/openjdk/jmh/it/threads/BenchmarkBenchSameThreadTest.java line 226:

> 224:                     .include(Fixtures.getTestMask(this.getClass()))
> 225:                     .jvmArgsAppend("-Djmh.executor=CUSTOM")
> 226:                     .jvmArgsAppend("-Djmh.executor.class="+CustomExecutor.class.getName())

Suggestion:

                    .jvmArgsAppend("-Djmh.executor.class=" + CustomExecutor.class.getName())

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

Marked as reviewed by shade (Committer).

PR Review: https://git.openjdk.org/jmh/pull/103#pullrequestreview-1444033945
Changes requested by shade (Committer).

PR Review: https://git.openjdk.org/jmh/pull/103#pullrequestreview-1444038464
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1205607853
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1205608047
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1205608365
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1205608648


More information about the jmh-dev mailing list