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

[Aleksey Shipilev]

On Tue, 16 May 2023 04:31:27 GMT, Sergey Kuksenko <skuksenko at openjdk.org> wrote:

> Add test for virtual thread executor

Suggestions:

jmh-core-it/src/test/java/org/openjdk/jmh/it/threads/BenchExecutorTest.java line 46:

> 44: import org.openjdk.jmh.runner.RunnerException;
> 45: import org.openjdk.jmh.runner.options.Options;
> 46: import org.openjdk.jmh.runner.options.OptionsBuilder;

Please check if we need all these imports?

jmh-core-it/src/test/java/org/openjdk/jmh/it/threads/BenchExecutorTest.java line 56:

> 54: 
> 55: /**
> 56:  * Tests if harness executes setup, run, and tearDown in the same workers.

This comment does not relate to the test. Copy-paste error?

jmh-core-it/src/test/java/org/openjdk/jmh/it/threads/BenchExecutorTest.java line 62:

> 60: 
> 61:     @Param("false")
> 62:     boolean expected;

`expected` -> `isVirtual`

jmh-core-it/src/test/java/org/openjdk/jmh/it/threads/BenchExecutorTest.java line 74:

> 72:     @Setup(Level.Trial)
> 73:     public void setupRun() {
> 74:         setupRunThread.add(VirtualAPI.isVirtual(Thread.currentThread()));

There seem to be little point in continuously adding the single `boolean` value here. I'd suggest we collect `Thread` instances instead, and check that there is a single expected thread, whether virtual or not.

jmh-core-it/src/test/java/org/openjdk/jmh/it/threads/BenchExecutorTest.java line 199:

> 197:                 return false;
> 198:             }
> 199:         }

Suggestion:

        public static boolean isVirtual(Thread t) {
            if (!hasVirtualThreads()) {
                return false;
            }
            
            try {
                return (boolean) IS_VIRTUAL.invoke(t);
            } catch (IllegalAccessException | InvocationTargetException e) {
                // Fall-through
            }
            
            return false;
        }

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

Changes requested by shade (Committer).

PR Review: https://git.openjdk.org/jmh/pull/103#pullrequestreview-1428064052
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1196204664
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1194835058
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1196200979
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1196200375
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1196204001