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

Sergey Kuksenko skuksenko at openjdk.org
Wed May 17 15:02:43 UTC 2023


On Wed, 17 May 2023 09:22:27 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Add test for virtual thread executor
>
> 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?

yes

> 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?

Yeah. When code speaks more than comments, comments become invisible. :)

> 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.

There are other tests that check a single expected thread.

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

PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1196635304
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1196626975
PR Review Comment: https://git.openjdk.org/jmh/pull/103#discussion_r1196630410


More information about the jmh-dev mailing list