RFR: 8304919: Implementation of Virtual Threads [v2]

[Leonid Mesnik]

On Tue, 28 Mar 2023 19:36:18 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> JEP 444 proposes to make virtual threads a permanent feature in Java 21. The APIs that were preview APIs in Java 19/20 are changed to permanent and their `@since`/equivalent are changed to 21 (as per the guidance in JEP 12). The JNI and JVMTI versions are bumped as this is the first change in 21 to need the new version number. A lot of tests are updated to drop `@enablePreview` and --enable-preview.
>> 
>> There is one API change from Java 19/20, the preview API Thread.Builder.allowSetThreadLocals(boolean) is dropped. This requires an update to the JVMTI GetThreadInfo implementation to read the TCCL consistently.
>> 
>> In addition, there are a small number of implementation changes to sync up from the loom fibers branch:
>> 
>> - A number of stack frames are `@Hidden` to reduce noise in the stack traces. This exposed a few issues with the stack walker code. More specifically, the cases where  end of a continuation falls precisely at the end of the batch, or where the remaining frames are hidden, weren't handled correctly.
>> - The code to emit the JFR jdk.ThreadSleepEvent is refactored so it's in Thread rather than in two classes.
>> - A few robustness improvements for OOME and SOE. There is more to do here, for future PRs.
>> - New system property to print a stack trace when a virtual thread sets its own value of a TL.
>> - ThreadPerTaskExecutor is changed to use FutureTask.
>> 
>> Testing: tier1-6.
>
> 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

Changes requested by lmesnik (Reviewer).

src/java.base/share/classes/java/lang/System.java line 2566:

> 2564: 
> 2565:             public <V> V executeOnCarrierThread(Callable<V> task) throws Exception {
> 2566:                 if (Thread.currentThread() instanceof VirtualThread vthread) {

Any specific reason to don't use Thread.currentThread().isVirtual()?

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.

test/jdk/com/sun/jdi/TestScaffold.java line 980:

> 978: 
> 979:         if (wrapper.equals("Virtual")) {
> 980:             threadFactory = Thread.ofVirtual().factory();

Should be line 
469: argInfo.targetVMArgs += "--enable-preview ";
removed also?

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?

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 ?

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.

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?

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

PR Review: https://git.openjdk.org/jdk/pull/13203#pullrequestreview-1361847681
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151259675
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151175072
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151168150
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151168861
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151112603
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151153758
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1151119165