RFR: 8304919: Implementation of Virtual Threads [v2]

[Alan Bateman]

On Tue, 28 Mar 2023 21:36:04 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> Alan Bateman 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:
>> 
>>  - ThreadSleepEvent refactoring
>>  - Merge
>>  - Merge
>>  - Initial sync from fibers branch
>
> test/hotspot/jtreg/runtime/vthread/JNIMonitor/JNIMonitor.java line 31:
> 
>> 29:  * @summary Tests that JNI monitors work correctly with virtual threads
>> 30:  * @library /test/lib
>> 31:  * @compile JNIMonitor.java
> 
> I think that test file is compiled implicitly. So this line could be just removed. 
> The same is for all other similar tests.

Most of the test changes were just a few sed commands remove lines with `@enablePreview`, remove  `--enable-preview -source ${jdk.version}` from `@compile` tags. We can remove the `@compile` tags that don't specify any other options as they aren't needed now, up to you.

> test/jdk/com/sun/management/ThreadMXBean/VirtualThreads.java line 143:
> 
>> 141:             long[] tids = new long[] { tid0, tid1 };
>> 142:             long[] cpuTimes = bean.getThreadCpuTime(tids);
>> 143:             if (Thread.currentThread().isVirtual()) {
> 
> How it worked before?

tid0 is the thread ID of a platform therad. tid1 is the threadID of a virtual thread. The only change here is allow this test run with the main wrapper plugin ([CODETOOLS-7903373](https://bugs.openjdk.org/browse/CODETOOLS-7903373)), it would otherwise have to be excluded for those runs.

> test/jdk/java/lang/Thread/virtual/GetStackTrace.java line 30:
> 
>> 28:  * @requires vm.continuations
>> 29:  * @modules java.base/java.lang:+open
>> 30:  * @run testng/othervm -XX:+UnlockDiagnosticVMOptions -XX:+ShowHiddenFrames GetStackTrace
> 
> shouldn't be main/othervm ?

You're right, this came over from the loom repo and I didn't know that it wasn't running (no @Test).  I think it would be better to run this test without ShowHiddenFrames, it's just need to known the expected bottom most frame.

> test/jdk/java/lang/Thread/virtual/VirtualThreadPinnedEventThrows.java line 29:
> 
>> 27:  * @modules java.base/jdk.internal.event
>> 28:  * @compile/module=java.base jdk/internal/event/VirtualThreadPinnedEvent.java
>> 29:  * @run junit VirtualThreadPinnedEventThrows
> 
> Shouldn't be 'junit/othervm' used here to ensure that updated VirtualThreadPinnedEvent is used? I don't know these details of jtreg.

This is using the jtreg support for patching system modules. jtreg runs the test in a new VM with --patch-module.

> test/jdk/jdk/incubator/concurrent/StructuredTaskScope/PreviewFeaturesNotEnabled.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
> 
> Not sure I understand what happens. The file is 
> test/jdk/jdk/incubator/concurrent/StructuredTaskScope/PreviewFeaturesNotEnabled.java
> while it contains is public class VirtualThreadPinnedEvent. Copied by mistake?

Thank, I'm not sure what happened there as this test is deleted.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151507288
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151502964
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151500272
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151495733
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151496227