Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]
On Wed, 1 Jun 2022 06:04:14 GMT, Alan Bateman wrote: >> This patch adds an alternative virtual thread implementation where each >> virtual thread is backed by an OS thread. It doesn't scale but it can be >> used by ports that don't have continuations support in the VM. Aside from >> scalability, the lack of continuations support means: >> >> 1. JVM TI is not supported when running with --enable-preview (the JVM TI >> spec allows for this) >> 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs >> and so needs JVM TI) >> >> The VM option "VMContinuations" is added as an experimental option so it can >> be used by tests. A number of tests are changed to re-run with >> -XX:-VMContinuations. A new jtreg property is added so that tests that need >> the underlying VM support to be present can use "@requires vm.continuations" >> in the test description. A follow-up change would be to add "@requires >> vm.continuations" to the ~70 serviceability/jvmti/vthread that run with >> preview features enabled. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 11 commits: > > - Fixed another typo in comment > - Merge > - Fix typos in comments > - Allowing linking without intrinsic stub, contributed-by: rehn > - Continuation clinit should fail if no continuations support > - Fix merge issue with test > - Change VMContinuations to be experimental option, ensure JNI GetEnv > returns EVERSION > - Update > - Expend test coverage > - Merge > - ... and 1 more: > https://git.openjdk.java.net/jdk/compare/3deb58a8...a5c06cb6 > A new jtreg property is added so that tests that need the underlying VM > support to be present can use "@requires vm.continuations" in the test > description. A follow-up change would be to add "@requires vm.continuations" > to the ~70 serviceability/jvmti/vthread that run with preview features > enabled. See #8990. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]
On Wed, 1 Jun 2022 06:26:23 GMT, David Holmes wrote: >> Alan Bateman has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 11 commits: >> >> - Fixed another typo in comment >> - Merge >> - Fix typos in comments >> - Allowing linking without intrinsic stub, contributed-by: rehn >> - Continuation clinit should fail if no continuations support >> - Fix merge issue with test >> - Change VMContinuations to be experimental option, ensure JNI GetEnv >> returns EVERSION >> - Update >> - Expend test coverage >> - Merge >> - ... and 1 more: >> https://git.openjdk.java.net/jdk/compare/3deb58a8...a5c06cb6 > > src/java.base/share/classes/java/lang/Thread.java line 742: > >> 740: * @param name thread name, can be null >> 741: * @param characteristics thread characteristics >> 742: * @param bound true when bound to an OS thread > > Nit: s/OS/VM/ ? I think it would be confusing to use "VM" here. The intention is that "true" means the virtual thread will be bound to an OS thread once it has started. Yes, technically it's whatever the VM implements but I think that is too much to try to explain in the parameter description. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]
On Wed, 1 Jun 2022 06:04:14 GMT, Alan Bateman wrote: >> This patch adds an alternative virtual thread implementation where each >> virtual thread is backed by an OS thread. It doesn't scale but it can be >> used by ports that don't have continuations support in the VM. Aside from >> scalability, the lack of continuations support means: >> >> 1. JVM TI is not supported when running with --enable-preview (the JVM TI >> spec allows for this) >> 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs >> and so needs JVM TI) >> >> The VM option "VMContinuations" is added as an experimental option so it can >> be used by tests. A number of tests are changed to re-run with >> -XX:-VMContinuations. A new jtreg property is added so that tests that need >> the underlying VM support to be present can use "@requires vm.continuations" >> in the test description. A follow-up change would be to add "@requires >> vm.continuations" to the ~70 serviceability/jvmti/vthread that run with >> preview features enabled. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 11 commits: > > - Fixed another typo in comment > - Merge > - Fix typos in comments > - Allowing linking without intrinsic stub, contributed-by: rehn > - Continuation clinit should fail if no continuations support > - Fix merge issue with test > - Change VMContinuations to be experimental option, ensure JNI GetEnv > returns EVERSION > - Update > - Expend test coverage > - Merge > - ... and 1 more: > https://git.openjdk.java.net/jdk/compare/3deb58a8...a5c06cb6 src/java.base/share/classes/java/lang/Thread.java line 742: > 740: * @param name thread name, can be null > 741: * @param characteristics thread characteristics > 742: * @param bound true when bound to an OS thread Nit: s/OS/VM/ ? - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]
> This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits: - Fixed another typo in comment - Merge - Fix typos in comments - Allowing linking without intrinsic stub, contributed-by: rehn - Continuation clinit should fail if no continuations support - Fix merge issue with test - Change VMContinuations to be experimental option, ensure JNI GetEnv returns EVERSION - Update - Expend test coverage - Merge - ... and 1 more: https://git.openjdk.java.net/jdk/compare/3deb58a8...a5c06cb6 - Changes: https://git.openjdk.java.net/jdk/pull/8939/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8939&range=02 Stats: 747 lines in 72 files changed: 574 ins; 53 del; 120 mod Patch: https://git.openjdk.java.net/jdk/pull/8939.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8939/head:pull/8939 PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 15:39:39 GMT, Alan Bateman wrote: >> This patch adds an alternative virtual thread implementation where each >> virtual thread is backed by an OS thread. It doesn't scale but it can be >> used by ports that don't have continuations support in the VM. Aside from >> scalability, the lack of continuations support means: >> >> 1. JVM TI is not supported when running with --enable-preview (the JVM TI >> spec allows for this) >> 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs >> and so needs JVM TI) >> >> The VM option "VMContinuations" is added as an experimental option so it can >> be used by tests. A number of tests are changed to re-run with >> -XX:-VMContinuations. A new jtreg property is added so that tests that need >> the underlying VM support to be present can use "@requires vm.continuations" >> in the test description. A follow-up change would be to add "@requires >> vm.continuations" to the ~70 serviceability/jvmti/vthread that run with >> preview features enabled. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Allowing linking without intrinsic stub, contributed-by: rehn Looks okay to me. src/java.management/share/classes/sun/management/ThreadImpl.java line 586: > 584: > 585: /** > 586: * Returns the ThreadInfo objects from the given array that > coresspond to platform typo: coresspond -> correspond src/java.management/share/classes/sun/management/Util.java line 92: > 90: > 91: /** > 92: * Returns true if the given Thread is a virutal thread. typo: virutal -> virtual this typo appears in other comments too. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 15:39:39 GMT, Alan Bateman wrote: >> This patch adds an alternative virtual thread implementation where each >> virtual thread is backed by an OS thread. It doesn't scale but it can be >> used by ports that don't have continuations support in the VM. Aside from >> scalability, the lack of continuations support means: >> >> 1. JVM TI is not supported when running with --enable-preview (the JVM TI >> spec allows for this) >> 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs >> and so needs JVM TI) >> >> The VM option "VMContinuations" is added as an experimental option so it can >> be used by tests. A number of tests are changed to re-run with >> -XX:-VMContinuations. A new jtreg property is added so that tests that need >> the underlying VM support to be present can use "@requires vm.continuations" >> in the test description. A follow-up change would be to add "@requires >> vm.continuations" to the ~70 serviceability/jvmti/vthread that run with >> preview features enabled. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Allowing linking without intrinsic stub, contributed-by: rehn Looks good, tested zero, thanks! - Marked as reviewed by rehn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 17:03:54 GMT, Aleksey Shipilev wrote: > I expected this change to fix the broken ARM32 port, but it doesn't work. There is work required to get the arm32 port working again, currently tracked as JDK-828636 but there may be further issues beyond that. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 16:54:41 GMT, Boris Ulasevich wrote: > I expected this change to fix the broken ARM32 port, but it doesn't work. It would not fix ARM32, because the interpreter stubs need to be predicated on `Continuations::enabled()`. Also, as my ARM32 experiments show (https://github.com/openjdk/jdk/pull/8634/files#diff-027490ce3f4a92be9b489d9d2e54c7baaea87b7489399b198543c79f1ce1e2e3R4208-R4216) -- there is a breakage somewhere in C2 as well, which this patch would not seem to resolve as well. So, this is a stepping stone for *some* support, but it does not resolve porting situation fully. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 15:39:39 GMT, Alan Bateman wrote: >> This patch adds an alternative virtual thread implementation where each >> virtual thread is backed by an OS thread. It doesn't scale but it can be >> used by ports that don't have continuations support in the VM. Aside from >> scalability, the lack of continuations support means: >> >> 1. JVM TI is not supported when running with --enable-preview (the JVM TI >> spec allows for this) >> 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs >> and so needs JVM TI) >> >> The VM option "VMContinuations" is added as an experimental option so it can >> be used by tests. A number of tests are changed to re-run with >> -XX:-VMContinuations. A new jtreg property is added so that tests that need >> the underlying VM support to be present can use "@requires vm.continuations" >> in the test description. A follow-up change would be to add "@requires >> vm.continuations" to the ~70 serviceability/jvmti/vthread that run with >> preview features enabled. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Allowing linking without intrinsic stub, contributed-by: rehn I expected this change to fix the broken ARM32 port, but it doesn't work. # A fatal error has been detected by the Java Runtime Environment: # # Internal Error (templateInterpreterGenerator_arm.cpp:732), pid=27345, tid=27346 # Error: Unimplemented() # # JRE version: (19.0) (build ) # Java VM: OpenJDK Server VM (19-commit6f6486e9-adhoc.re.jdk, mixed mode, g1 gc, linux-arm) # Problematic frame: # V [libjvm.so+0xa14fe0] TemplateInterpreterGenerator::generate_Continuation_doYield_entry()+0x2c - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
> This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Allowing linking without intrinsic stub, contributed-by: rehn - Changes: - all: https://git.openjdk.java.net/jdk/pull/8939/files - new: https://git.openjdk.java.net/jdk/pull/8939/files/7989708b..126e38b6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8939&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8939&range=00-01 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8939.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8939/head:pull/8939 PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread
On Sun, 29 May 2022 14:46:39 GMT, Alan Bateman wrote: > This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. > Since the package `jdk.internal.access` is > exported[1](#user-content-fn-1-97aea7d7960164849e591e42b91fb5c4) to the > `java.management` module, this can use `MethodHandle`s obtained using the > trusted lookup. That export is for another reason, and probably should be re-examined so that java.base doesn't need to export this package to java.management. In any case, we expect there will be compiler support soon to allow java.management be compiled with code that makes use of preview APIs in java.base. That will at least us reference Thread::isVirtual from code. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread
On Sun, 29 May 2022 14:46:39 GMT, Alan Bateman wrote: > This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. Since the package `jdk.internal.access` is exported[^1] to the `java.management` module, this can use `MethodHandle`s obtained using the trusted lookup. [^1]: https://github.com/openjdk/jdk/blob/73ba7fdce838ba8a2c227a972c176311e6cc0b41/src/java.base/share/classes/module-info.java#L151-L154 src/java.management/share/classes/java/lang/management/ThreadInfo.java line 971: > 969: throw new InternalError(e); > 970: } > 971: } Suggestion: private static boolean isVirtual(Thread thread) { try { return (boolean) IS_VIRTUAL.invokeExact(thread); } catch (Error | RuntimeException e) { throw e; } catch (Throwable t) { throw new InternalError(t); } } private static final MethodHandle IS_VIRTUAL; static { final JavaLangInvokeAccess JLIA = SharedSecrets.getJavaLangInvokeAccess(); try { MethodHandle m = JLIA.findVirtual(Thread.class, "isVirtual", MethodType.methodType(boolean.class)); assert m != null; IS_VIRTUAL = m; } catch (Exception e) { throw new InternalError(e); } } src/java.management/share/classes/sun/management/ThreadImpl.java line 661: > 659: > 660: private static final Method THREAD_IS_VIRTUAL = threadIsVirtual(); > 661: private static final Field THREADINFO_VIRTUAL = threadInfoVirtual(); Suggestion: private static boolean isVirtual(Thread thread) { try { return (boolean) THREAD_IS_VIRTUAL.invokeExact(thread); } catch (Error | RuntimeException e) { throw e; } catch (Throwable t) { throw new InternalError(t); } } /** * Returns true if the given ThreadInfo is for a virutal thread. */ private static boolean isVirtual(ThreadInfo threadInfo) { try { return (boolean) THREADINFO_VIRTUAL.invokeExact(threadInfo); } catch (Error | RuntimeException e) { throw e; } catch (Throwable t) { throw new InternalError(t); } } private static final JavaLangInvokeAccess JLIA = SharedSecrets.getJavaLangInvokeAccess(); private static MethodHandle threadIsVirtual() { try { MethodHandle m = JLIA.findVirtual(Thread.class, "isVirtual", MethodType.methodType(boolean.class)); assert m != null; return m; } catch (Exception e) { throw new InternalError(e); } } @SuppressWarnings("removal") private static MethodHandle threadInfoVirtual() { PrivilegedExceptionAction pa = () -> ThreadInfo.class.getDeclaredField("virtual"); try { return JLIA.unreflectField(AccessController.doPrivileged(pa), false); } catch (Exception e) { throw new InternalError(e); } } private static final MethodHandle THREAD_IS_VIRTUAL = threadIsVirtual(); private static final MethodHandle THREADINFO_VIRTUAL = threadInfoVirtual(); - PR: https://git.openjdk.java.net/jdk/pull/8939