Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]

2022-06-02 Thread Aleksey Shipilev
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]

2022-06-02 Thread Alan Bateman
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]

2022-06-01 Thread David Holmes
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]

2022-06-01 Thread Alan Bateman
> 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=8939=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]

2022-05-31 Thread Mandy Chung
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]

2022-05-31 Thread Robbin Ehn
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]

2022-05-31 Thread Alan Bateman
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]

2022-05-31 Thread Aleksey Shipilev
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]

2022-05-31 Thread Boris Ulasevich
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]

2022-05-31 Thread Alan Bateman
> 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=8939=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8939=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

2022-05-31 Thread Alan Bateman
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

2022-05-31 Thread ExE Boss
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