Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread

[David Holmes]

On Tue, 6 Jun 2023 00:50:34 GMT, Serguei Spitsyn  wrote:

> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
> VirtualThread blocked on a monitor when called for more than one thread. When 
> called for a single VirtualThread it correctly returns a state that includes 
> the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
> The `VM_GetThreadListStackTraces::doit` should call the 
> `get_threadOop_and_JavaThread` instead of `cv_external_thread_to_JavaThread`. 
> But the `get_threadOop_and_JavaThread` has a check for the current thread by 
> comparing with the JavaThread::current() which does not work for a `VM_op`. 
> Some refactoring of the `GetSingleStackTraceClosure` and 
> `get_threadOop_and_JavaThread` was made to make it working for a `VM_op`.
> 
> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
> discovered during testing.
> A minor refactoring of the `GetSingleStackTraceClosure` was made to fix the 
> issue.
> 
> Also, a new test was added to provide coverage:
>  - `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  
> Testing:
>  - ran new test: 
> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  - TBD: tiers 1-6

Just a passing comment but I happened to notice today that when a virtual 
thread blocks on a legacy synchronization mechanism, it delegates to its 
carrier thread to report its state. It is not at all clear to me how this is 
handled at the JVMTI level.

-

PR Review: https://git.openjdk.org/jdk/pull/14326#pullrequestreview-1464472318

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread

[Alan Bateman]

On Tue, 6 Jun 2023 07:30:26 GMT, David Holmes  wrote:

> Just a passing comment but I happened to notice today that when a virtual 
> thread blocks on a legacy synchronization mechanism, it delegates to its 
> carrier thread to report its state. It is not at all clear to me how this is 
> handled at the JVMTI level.

When mounted, the virtual thread state comes from its carrier. JVM TI 
GetThreadState and other functions that return state do the same. Somehow the 
bulk function GetThreadListStackTraces was missed.

-

PR Comment: https://git.openjdk.org/jdk/pull/14326#issuecomment-1578097719

RFR: JDK-8309462: vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/TestDescription.java crashing due to empty while loop

[JoKern65]

The sys_thread_3() function contains an empty while loop, which by the standard 
can be optimized away. Please refer to discussion in 
https://github.com/llvm/llvm-project/issues/60622
The xlc17 compiler is doing so, and IBM claims that they are following the 
standard and will not fix this on compiler side.
So we have (at least) 3 ways to circumvent this behavior.

1. we can introduce the call of a nop library function, which will hinder the 
optimizer to throw away the loop (This is our proposed solution, but instead of 
a heavy looping thread, the result is a more or less idle thread):
`#include `
`static void`
`sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
`{`
`while (1) {`
`  sleep(1);`
`}`
`}`

2. We can make use of a volatile variable in the loop body which also hinders 
the optimizer to throw away the loop:
`static void`
`sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
`{`
`volatile int i = 1;`
`while (i) {`
`  i += 2;`
`}`
`}`

3. we can use the __attribute__ ((optnone)) modifier in the function 
declaration to suppress the optimization at all:
`static void`
`sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) __attribute__ ((optnone))`
`{`
`while (1) {`
`}`
`}`

To make the third approach platform independent, we can implement it in the 
following way:
In globalDefinitions.hpp
`#ifndef OPTNONE`
`#define OPTNONE`
`#endif`

In globalDefinitions_xlc.hpp
`// optnone support`
`//`
`// To use if a function should not be optimized`
`// Usage:`
`// void* func(size_t size) OPTNONE {...}`
`#define OPTNONE __attribute__(( optnone))``

With this we can change libagentthr001.cpp in a platform independent way to
`static void`
`sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) OPTNONE`
`{`
` while (1) {`
` }`
`}`

-

Commit messages:
 - JDK-8309462

Changes: https://git.openjdk.org/jdk/pull/14330/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14330&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8309462
  Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/14330.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14330/head:pull/14330

PR: https://git.openjdk.org/jdk/pull/14330

Re: RFR: 8309408: Thread.sleep cleanup

[Alan Bateman]

On Mon, 5 Jun 2023 20:05:24 GMT, Chris Plummer  wrote:

> The following commit in loom heavily modified this file with a lot of added 
> expected methods. There are other related tests with similar changes. I'm not 
> so sure I understand the need for so many additions, and also why 
> expectedLength is so out of sync with the number of added method. I don't 
> believe this commit was reviewed individually, but was just part of the 
> overall loom review when merge into jdk. Perhaps it should be revisited.

These tests aren't easy to read or maintain, it would be good to re-visit them. 
In some cases, the tests capture the stack trace asynchronously so the test 
needs to know about all code paths.

As regards ThreadController, used by the nsk/monitoring/stress/thread/straceXXX 
tests, the main thread waits at a barrier (a CountDownLatch) until all sleeping 
threads are ready to sleep. Once the main thread is released, it checks all the 
sleepers are in TIMED_WAITING state and samples their stack traces with the 
ThreadMXBean and related APIs. The test fails if there are frames corresponding 
to methods that the test doesn't know about. If a thread is sleeping then we 
shouldn't see frames for beforeSleep/afterSlee. My reading of these tests is 
that the main thread could poll a SleepingThread after it counts down and 
before it parks in sleep. It's doing an expensive ThreadMXBean::getAllThreadIds 
once released and that may explain why it hasn't been seen.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14303#discussion_r1219348675

Re: RFR: JDK-8309462: vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/TestDescription.java crashing due to empty while loop

[Matthias Baesken]

On Tue, 6 Jun 2023 09:51:09 GMT, JoKern65  wrote:

> The sys_thread_3() function contains an empty while loop, which by the 
> standard can be optimized away. Please refer to discussion in 
> https://github.com/llvm/llvm-project/issues/60622
> The xlc17 compiler is doing so, and IBM claims that they are following the 
> standard and will not fix this on compiler side.
> So we have (at least) 3 ways to circumvent this behavior.
> 
> 1. we can introduce the call of a nop library function, which will hinder the 
> optimizer to throw away the loop (This is our proposed solution, but instead 
> of a heavy looping thread, the result is a more or less idle thread):
> `#include `
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
> `{`
> `while (1) {`
> `  sleep(1);`
> `}`
> `}`
> 
> 2. We can make use of a volatile variable in the loop body which also hinders 
> the optimizer to throw away the loop:
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
> `{`
> `volatile int i = 1;`
> `while (i) {`
> `  i += 2;`
> `}`
> `}`
> 
> 3. we can use the __attribute__ ((optnone)) modifier in the function 
> declaration to suppress the optimization at all:
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) __attribute__ 
> ((optnone))`
> `{`
> `while (1) {`
> `}`
> `}`
> 
> To make the third approach platform independent, we can implement it in the 
> following way:
> In globalDefinitions.hpp
> `#ifndef OPTNONE`
> `#define OPTNONE`
> `#endif`
> 
> In globalDefinitions_xlc.hpp
> `// optnone support`
> `//`
> `// To use if a function should not be optimized`
> `// Usage:`
> `// void* func(size_t size) OPTNONE {...}`
> `#define OPTNONE __attribute__(( optnone))``
> 
> With this we can change libagentthr001.cpp in a platform independent way to
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) OPTNONE`
> `{`
> ` while (1) {`
> ` }`
> `}`

looks okay to me

-

Marked as reviewed by mbaesken (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14330#pullrequestreview-1464841555

Re: RFR: 8294977: Convert test/jdk/java tests from ASM library to Classfile API [v8]

[Chen Liang]

On Tue, 9 May 2023 04:17:28 GMT, Chen Liang  wrote:

>> Summaries:
>> 1. A few recommendations about updating the constant API is made at 
>> https://mail.openjdk.org/pipermail/classfile-api-dev/2023-March/000233.html 
>> and I may update this patch shall the API changes be integrated before
>> 2. One ASM library-specific test, `LambdaAsm` is removed. Others have their 
>> code generation infrastructure upgraded from ASM to Classfile API.
>> 3. Most tests are included in tier1, but some are not:
>> In `:jdk_io`: (tier2, part 2)
>> 
>> test/jdk/java/io/Serializable/records/SerialPersistentFieldsTest.java
>> test/jdk/java/io/Serializable/records/ProhibitedMethods.java
>> test/jdk/java/io/Serializable/records/BadCanonicalCtrTest.java
>> 
>> In `:jdk_instrument`: (tier 3)
>> 
>> test/jdk/java/lang/instrument/RetransformAgent.java
>> test/jdk/java/lang/instrument/NativeMethodPrefixAgent.java
>> test/jdk/java/lang/instrument/asmlib/Instrumentor.java
>> 
>> 
>> @asotona Would you mind reviewing?
>
> Chen Liang has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 12 commits:
> 
>  - Merge branch 'master' into invoke-test-classfile
>  - Switch to ConstantDescs for   and void constants
>  - Merge AnnotationsTest, remove ModuleTargetAttribute call
>  - Merge branch 'invoke-test-classfile' of https://github.com/liachmodded/jdk 
> into invoke-test-classfile
>  - Update test/jdk/java/lang/invoke/8022701/MHIllegalAccess.java
>
>Co-authored-by: Andrey Turbanov 
>  - Merge branch 'master' into invoke-test-classfile
>  - Fix LambdaStackTrace after running
>  - formatting
>  - Fix failed LambdaStackTrace test, use more convenient APIs
>  - Merge branch 'master' of https://git.openjdk.java.net/jdk into 
> invoke-test-classfile
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/d9052b94...5db1e957

keep-alive; this will be updated for the classfile object update.

-

PR Comment: https://git.openjdk.org/jdk/pull/13009#issuecomment-1578507931

Re: RFR: JDK-8309462: vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/TestDescription.java crashing due to empty while loop

[Martin Doerr]

On Tue, 6 Jun 2023 09:51:09 GMT, JoKern65  wrote:

> The sys_thread_3() function contains an empty while loop, which by the 
> standard can be optimized away. Please refer to discussion in 
> https://github.com/llvm/llvm-project/issues/60622
> The xlc17 compiler is doing so, and IBM claims that they are following the 
> standard and will not fix this on compiler side.
> So we have (at least) 3 ways to circumvent this behavior.
> 
> 1. we can introduce the call of a nop library function, which will hinder the 
> optimizer to throw away the loop (This is our proposed solution, but instead 
> of a heavy looping thread, the result is a more or less idle thread):
> `#include `
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
> `{`
> `while (1) {`
> `  sleep(1);`
> `}`
> `}`
> 
> 2. We can make use of a volatile variable in the loop body which also hinders 
> the optimizer to throw away the loop:
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
> `{`
> `volatile int i = 1;`
> `while (i) {`
> `  i += 2;`
> `}`
> `}`
> 
> 3. we can use the __attribute__ ((optnone)) modifier in the function 
> declaration to suppress the optimization at all:
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) __attribute__ 
> ((optnone))`
> `{`
> `while (1) {`
> `}`
> `}`
> 
> To make the third approach platform independent, we can implement it in the 
> following way:
> In globalDefinitions.hpp
> `#ifndef OPTNONE`
> `#define OPTNONE`
> `#endif`
> 
> In globalDefinitions_xlc.hpp
> `// optnone support`
> `//`
> `// To use if a function should not be optimized`
> `// Usage:`
> `// void* func(size_t size) OPTNONE {...}`
> `#define OPTNONE __attribute__(( optnone))``
> 
> With this we can change libagentthr001.cpp in a platform independent way to
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) OPTNONE`
> `{`
> ` while (1) {`
> ` }`
> `}`

LGTM. `sleep(1)` is my preferred solution. Thanks for fixing it!

-

Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14330#pullrequestreview-1465026778

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[Thomas Stuefe]

On Sun, 4 Jun 2023 21:39:58 GMT, Kelvin Nilsen  wrote:

>> OpenJDK Colleagues:
>> 
>> Please review this proposed integration of Generational mode for Shenandoah 
>> GC under https://bugs.openjdk.org/browse/JDK-8307314.
>> 
>> Generational mode of Shenandoah is enabled by adding 
>> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
>> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
>> implementation automatically adjusts the sizes of old generation and young 
>> generation to efficiently utilize the entire heap capacity.  Generational 
>> mode of Shenandoah resembles G1 in the following regards:
>> 
>> 1. Old-generation marking runs concurrently during the time that multiple 
>> young generation collections run to completion.
>> 2. After old-generation marking completes, we perform a sequence of mixed 
>> collections.  Each mixed collection combines collection of young generation 
>> with evacuation of a portion of the old-generation regions identified for 
>> collection based on old-generation marking information.
>> 3. Unlike G1, young-generation collections and evacuations are entirely 
>> concurrent, as with single-generation Shenandoah.
>> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
>> and survivor space within the young generation.  In practice, regions that 
>> were most recently allocated tend to have large amounts of garbage and these 
>> regions tend to be collected with very little effort.  Young-generation 
>> objects that survive garbage collection tend to accumulate in regions that 
>> hold survivor objects.  These regions tend to have smaller amounts of 
>> garbage, and are less likely to be collected.  If they survive a sufficient 
>> number of young-generation collections, the “survivor” regions are promoted 
>> into the old generation.
>> 
>> We expect to refine heuristics as we gain experience with more production 
>> workloads.  In the future, we plan to remove the “experimental” qualifier 
>> from generational mode, at which time we expect that generational mode will 
>> become the default mode for Shenandoah.
>> 
>> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
>> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
>> HyperAlloc, and multiple AWS production workload simulators. We test on 
>> Linux x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and 
>> Windows x64.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove three asserts making comparisons between atomic volatile variables
>   
>   Though changes to the volatile variables are individually protected by
>   Atomic load and store operations, these asserts were not assuring
>   atomic access to multiple volatile variables, each of which could be
>   modified independently of the others.  The asserts were therefore not
>   trustworthy, as has been confirmed by more extensive testing.

Hi @kdnilsen,

I see that following settings changed default values for all of Shenandoah:

- ShenandoahLearningSteps (was 10, now 5)
- ShenandoahImmediateThreshold (was 90, now 70)
- ShenandoahAdaptiveDecayFactor (was 0.5, now 0.1)
- ShenandoahFullGCThreshold (was 3, now 64)

Assuming that the behavior of legacy Shenandoah remains unchanged, I assume the 
switches are now handled differently to arrive at the same behavior.

I see that we now have ShenandoahOOMGCRetries. Does the changed default for 
ShenandoahFullGCThreshold and this new ShenandoahOOMGCRetries switch mean the 
degeneration behavior of legacy Shenandoah did change?

I think the general thrust of my questions is, you assured us that legacy 
Shenandoah will show the same behavior post-patch, but since the settings 
changed, I assume that the meaning of these settings did change. We will need 
to document these effects for users of legacy Shenandoah, in case they need to 
translate existing settings in their environment. A release note would be 
really helpful.

Cheers, Thomas

-

PR Comment: https://git.openjdk.org/jdk/pull/14185#issuecomment-1578693858

Re: RFR: JDK-8309225: Fix xlc17 clang 15 warnings in security and servicability

[Martin Doerr]

On Fri, 2 Jun 2023 10:19:53 GMT, JoKern65  wrote:

> This pr is a split off from JDK-8308288: Fix xlc17 clang warnings in shared 
> code https://github.com/openjdk/jdk/pull/14146
> It handles the part in security and servicability.
> 
> Compiling on AIX with xlc17 which contains the new clang 15 frontend shows 
> the following warnings:
> 
> src/java.security.jgss/share/native/libj2gss/NativeUtil.h:30:
> src/java.security.jgss/share/native/libj2gss/gssapi.h:48:5: error: 
> 'TARGET_OS_MAC' is not defined, evaluates to 0 [-Werror,-Wundef]
> #if TARGET_OS_MAC && (defined(ppc) || defined(ppc64) || defined(i386) || 
> defined(x86_64))
> ^
> TARGET_OS_MAC is not defined. Instead of disabling the warning, I could
> ` #ifndef TARGET_OS_MAC`
>  `#define TARGET_OS_MAC=0`
>  `#endif`
> But this is already handled by disabling the warning for gcc.
> 
> src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c:718:33: error: 
> suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
> struct in6_addr mappedAny = IN6ADDR_ANY_INIT;
> ^~~~
> /usr/include/netinet/in.h:454:32: note: expanded from macro 'IN6ADDR_ANY_INIT'
> #define IN6ADDR_ANY_INIT {0, 0, 0, 0}

LGTM.

-

Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14282#pullrequestreview-1465140605

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v2]

[Serguei Spitsyn]

> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
> VirtualThread blocked on a monitor when called for more than one thread. When 
> called for a single VirtualThread it correctly returns a state that includes 
> the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
> The `VM_GetThreadListStackTraces::doit` should call the 
> `get_threadOop_and_JavaThread` instead of `cv_external_thread_to_JavaThread`. 
> But the `get_threadOop_and_JavaThread` has a check for the current thread by 
> comparing with the JavaThread::current() which does not work for a `VM_op`. 
> Some refactoring of the `GetSingleStackTraceClosure` and 
> `get_threadOop_and_JavaThread` was made to make it working for a `VM_op`.
> 
> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
> discovered during testing.
> A minor refactoring of the `GetSingleStackTraceClosure` was made to fix the 
> issue.
> 
> Also, a new test was added to provide coverage:
>  - `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  
> Testing:
>  - ran new test: 
> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  - TBD: tiers 1-6

Serguei Spitsyn has updated the pull request incrementally with one additional 
commit since the last revision:

  simplify GetSingleStackTraceClosure, fix issue in 
VM_GetThreadListStackTraces::doit, improve test

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14326/files
  - new: https://git.openjdk.org/jdk/pull/14326/files/77718470..4e794bd5

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14326&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14326&range=00-01

  Stats: 27 lines in 4 files changed: 11 ins; 5 del; 11 mod
  Patch: https://git.openjdk.org/jdk/pull/14326.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14326/head:pull/14326

PR: https://git.openjdk.org/jdk/pull/14326

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v3]

[Serguei Spitsyn]

> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
> VirtualThread blocked on a monitor when called for more than one thread. When 
> called for a single VirtualThread it correctly returns a state that includes 
> the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
> The `VM_GetThreadListStackTraces::doit` should call the 
> `get_threadOop_and_JavaThread` instead of `cv_external_thread_to_JavaThread`. 
> But the `get_threadOop_and_JavaThread` has a check for the current thread by 
> comparing with the JavaThread::current() which does not work for a `VM_op`. 
> Some refactoring of the `GetSingleStackTraceClosure` and 
> `get_threadOop_and_JavaThread` was made to make it working for a `VM_op`.
> 
> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
> discovered during testing.
> A minor refactoring of the `GetSingleStackTraceClosure` was made to fix the 
> issue.
> 
> Also, a new test was added to provide coverage:
>  - `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  
> Testing:
>  - ran new test: 
> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  - TBD: tiers 1-6

Serguei Spitsyn has updated the pull request incrementally with two additional 
commits since the last revision:

 - fixed typo in a comment in jvmtiEnvBase.cpp
 - nit: restored one comment as was before

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14326/files
  - new: https://git.openjdk.org/jdk/pull/14326/files/4e794bd5..d20e1221

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14326&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14326&range=01-02

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/14326.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14326/head:pull/14326

PR: https://git.openjdk.org/jdk/pull/14326

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[Kelvin Nilsen]

On Sun, 4 Jun 2023 21:39:58 GMT, Kelvin Nilsen  wrote:

>> OpenJDK Colleagues:
>> 
>> Please review this proposed integration of Generational mode for Shenandoah 
>> GC under https://bugs.openjdk.org/browse/JDK-8307314.
>> 
>> Generational mode of Shenandoah is enabled by adding 
>> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
>> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
>> implementation automatically adjusts the sizes of old generation and young 
>> generation to efficiently utilize the entire heap capacity.  Generational 
>> mode of Shenandoah resembles G1 in the following regards:
>> 
>> 1. Old-generation marking runs concurrently during the time that multiple 
>> young generation collections run to completion.
>> 2. After old-generation marking completes, we perform a sequence of mixed 
>> collections.  Each mixed collection combines collection of young generation 
>> with evacuation of a portion of the old-generation regions identified for 
>> collection based on old-generation marking information.
>> 3. Unlike G1, young-generation collections and evacuations are entirely 
>> concurrent, as with single-generation Shenandoah.
>> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
>> and survivor space within the young generation.  In practice, regions that 
>> were most recently allocated tend to have large amounts of garbage and these 
>> regions tend to be collected with very little effort.  Young-generation 
>> objects that survive garbage collection tend to accumulate in regions that 
>> hold survivor objects.  These regions tend to have smaller amounts of 
>> garbage, and are less likely to be collected.  If they survive a sufficient 
>> number of young-generation collections, the “survivor” regions are promoted 
>> into the old generation.
>> 
>> We expect to refine heuristics as we gain experience with more production 
>> workloads.  In the future, we plan to remove the “experimental” qualifier 
>> from generational mode, at which time we expect that generational mode will 
>> become the default mode for Shenandoah.
>> 
>> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
>> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
>> HyperAlloc, and multiple AWS production workload simulators. We test on 
>> Linux x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and 
>> Windows x64.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove three asserts making comparisons between atomic volatile variables
>   
>   Though changes to the volatile variables are individually protected by
>   Atomic load and store operations, these asserts were not assuring
>   atomic access to multiple volatile variables, each of which could be
>   modified independently of the others.  The asserts were therefore not
>   trustworthy, as has been confirmed by more extensive testing.

Thanks Thomas for the feedback:

These proposed changes represent improvements to both Generational and 
Non-generational modes of operation.  We can revert if that is desired, or we 
can specialize Generational versions of these parameters so that they can have 
different values in different modes, but here is a bit of background.  We've 
done considerable testing on a variety of synthetic workloads and some limited 
testing on production workloads.  As we move towards upstream integration, we 
expect this will help us gain exposure to more production workloads.  The 
following changes were based on results of this testing:

* Decrease ShenandoahLearningSteps to 5 (from 10): For some workloads, we 
observed that there were "way too many" learning cycles being triggered.  We 
also observed that the learning achieved during learning cycles was not as 
trustworthy as the learning achieved during actual operation, because these 
learning cycles typically trigger during initialization phases which are not 
representative of real-world operation and because they usually trigger so 
prematurely that there has not been enough time for allocated objects to die 
before we garbage collect.
* Change ShenandoahImmediateThreshold to 70 from 90: We discovered during 
experiments with settings on certain real production workloads that reducing 
the threshold for abbreviated cycles significantly improved throughput, reduced 
degenerated cycles, and reduced high percentile end-to-end latency on the 
relevant services.  These experiments were based on single-generation 
Shenandoah.  We saw no negative impact of making this change on our various 
workloads.
 * I'll let @earthling-amzn comment on the change to 
ShenandoahAdaptiveDecayFactor.  My recollection is that this change was also 
motivated by experience with single-generation Shenandoah on a real production 
workload.
 * The change of ShenandoahFullGCThreshold from 3 to 64 was motivated by som

Re: RFR: 8305341: Alignment should be enforced by alignas instead of compiler specific attributes [v4]

[Julian Waters]

On Wed, 12 Apr 2023 07:12:10 GMT, Julian Waters  wrote:

>> C11 has been stable for a long time on all platforms, so native code can use 
>> the standard alignas operator for alignment requirements
>
> Julian Waters has updated the pull request incrementally with four additional 
> commits since the last revision:
> 
>  - Restore visCPP
>  - Restore gcc attribute
>  - Revert gcc
>  - Revert

Bumping

-

PR Comment: https://git.openjdk.org/jdk/pull/13258#issuecomment-1578994674

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[William Kemper]

On Sun, 4 Jun 2023 21:39:58 GMT, Kelvin Nilsen  wrote:

>> OpenJDK Colleagues:
>> 
>> Please review this proposed integration of Generational mode for Shenandoah 
>> GC under https://bugs.openjdk.org/browse/JDK-8307314.
>> 
>> Generational mode of Shenandoah is enabled by adding 
>> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
>> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
>> implementation automatically adjusts the sizes of old generation and young 
>> generation to efficiently utilize the entire heap capacity.  Generational 
>> mode of Shenandoah resembles G1 in the following regards:
>> 
>> 1. Old-generation marking runs concurrently during the time that multiple 
>> young generation collections run to completion.
>> 2. After old-generation marking completes, we perform a sequence of mixed 
>> collections.  Each mixed collection combines collection of young generation 
>> with evacuation of a portion of the old-generation regions identified for 
>> collection based on old-generation marking information.
>> 3. Unlike G1, young-generation collections and evacuations are entirely 
>> concurrent, as with single-generation Shenandoah.
>> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
>> and survivor space within the young generation.  In practice, regions that 
>> were most recently allocated tend to have large amounts of garbage and these 
>> regions tend to be collected with very little effort.  Young-generation 
>> objects that survive garbage collection tend to accumulate in regions that 
>> hold survivor objects.  These regions tend to have smaller amounts of 
>> garbage, and are less likely to be collected.  If they survive a sufficient 
>> number of young-generation collections, the “survivor” regions are promoted 
>> into the old generation.
>> 
>> We expect to refine heuristics as we gain experience with more production 
>> workloads.  In the future, we plan to remove the “experimental” qualifier 
>> from generational mode, at which time we expect that generational mode will 
>> become the default mode for Shenandoah.
>> 
>> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
>> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
>> HyperAlloc, and multiple AWS production workload simulators. We test on 
>> Linux x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and 
>> Windows x64.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove three asserts making comparisons between atomic volatile variables
>   
>   Though changes to the volatile variables are individually protected by
>   Atomic load and store operations, these asserts were not assuring
>   atomic access to multiple volatile variables, each of which could be
>   modified independently of the others.  The asserts were therefore not
>   trustworthy, as has been confirmed by more extensive testing.

Lowering `ShenandoahAdaptiveDecayFactor` allows the heuristic to give more 
weight to older samples of the allocation rate and cycle times. We found that 
with the original value (0.5), the heuristics would "forget" history too soon. 
With the original value, the heuristics were more likely to mistime their 
trigger because of a few recent, short cycles. This was particularly true after 
we lowered `ShenandoahImmediateThreshold`, which resulted in more cycles which 
could skip evacuation and updating refs.

-

PR Comment: https://git.openjdk.org/jdk/pull/14185#issuecomment-1579008730

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v2]

[Alan Bateman]

On Mon, 5 Jun 2023 21:26:39 GMT, Serguei Spitsyn  wrote:

> Okay, I see you point. Unfortunately, I've always referred the platform 
> thread with an executed FJP schedular as a carrier thread. The term 'carrier' 
> with this meaning is everywhere in the JVMTI code. It looks very confusing to 
> call a thread to be a carrier thread only during some phases of its execution.

Okay, I'm just pointing out that is_passive_carrier_thread is confusing looks a 
bit strange here as the is testing if a JavaThread is carrying a virtual thread 
oop -  it's not testing if the thread is owned by the virtual thread scheduler.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1219888219

Re: RFR: 8308286 Fix clang warnings in linux code [v3]

[Artem Semenov]

On Thu, 1 Jun 2023 15:04:09 GMT, Weijun Wang  wrote:

>> done
>
> I didn't ask to revert the change. It's `s/TARGET_OS_MAC/defined(__APPLE__)/`.

This is rarely used in the code and is not the essence of the current changes.
If you introduce such changes, then throughout the code.
Moreover, this can lead to problems, such as, for example, here: 
https://bugs.openjdk.org/browse/JDK-8309225

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1220056527

Re: RFR: 8308286 Fix clang warnings in linux code [v3]

[Weijun Wang]

On Tue, 6 Jun 2023 17:32:35 GMT, Artem Semenov  wrote:

>> I didn't ask to revert the change. It's 
>> `s/TARGET_OS_MAC/defined(__APPLE__)/`.
>
> This is rarely used in the code and is not the essence of the current changes.
> If you introduce such changes, then throughout the code.
> Moreover, this can lead to problems, such as, for example, here: 
> https://bugs.openjdk.org/browse/JDK-8309225

I'm not a clang expect, I was just asking if modifying the current `#if 
TARGET_OS_MAC` check into `#if defined(__APPLE__)` is also a solution. The 
comment on lines 46-47 says the condition was copied from a macOS SDK file and 
that's what the file is using now.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1220071779

Re: RFR: 8309506: com/sun/jdi/MultiBreakpointsTest.java fails with virtual test thread factory

[Chris Plummer]

On Mon, 5 Jun 2023 22:13:29 GMT, Chris Plummer  wrote:

> The test fails when the main debuggee thread is a virtual thread, because 
> virtual threads are always daemon threads. Because of this all the test 
> threads that the debuggee creates are also daemon threads. The main deuggee 
> thread immediately exits after creating the test threads, and at that point 
> there are no non-daemon threads left running to keep the debuggee process 
> alive, so the test threads never get a chance to do all the work that is 
> expected of them. 
> 
> The fix is to use Thread.join() to prevent the main test thread from exiting 
> until all the created tests threads have exited. I also updated the test so 
> when run using the virtual test thread factory, the debuggee test threads 
> will now be virtual threads. Previously just the main debuggee thread would 
> be a virtual thread. This is not a bug fix, but does provide better virtual 
> thread testing.

Thanks for the reviews Alex and Serguei!

-

PR Comment: https://git.openjdk.org/jdk/pull/14323#issuecomment-1579243690

Integrated: 8309506: com/sun/jdi/MultiBreakpointsTest.java fails with virtual test thread factory

[Chris Plummer]

On Mon, 5 Jun 2023 22:13:29 GMT, Chris Plummer  wrote:

> The test fails when the main debuggee thread is a virtual thread, because 
> virtual threads are always daemon threads. Because of this all the test 
> threads that the debuggee creates are also daemon threads. The main deuggee 
> thread immediately exits after creating the test threads, and at that point 
> there are no non-daemon threads left running to keep the debuggee process 
> alive, so the test threads never get a chance to do all the work that is 
> expected of them. 
> 
> The fix is to use Thread.join() to prevent the main test thread from exiting 
> until all the created tests threads have exited. I also updated the test so 
> when run using the virtual test thread factory, the debuggee test threads 
> will now be virtual threads. Previously just the main debuggee thread would 
> be a virtual thread. This is not a bug fix, but does provide better virtual 
> thread testing.

This pull request has now been integrated.

Changeset: 571fbdc3
Author:Chris Plummer 
URL:   
https://git.openjdk.org/jdk/commit/571fbdc3110440ec3a36bb6005dc5a0358696df5
Stats: 16 lines in 2 files changed: 10 ins; 2 del; 4 mod

8309506: com/sun/jdi/MultiBreakpointsTest.java fails with virtual test thread 
factory

Reviewed-by: amenkov, sspitsyn

-

PR: https://git.openjdk.org/jdk/pull/14323

Re: RFR: 8309505: com/sun/jdi/MethodEntryExitEvents.java due to finding wrong main thread

[Chris Plummer]

On Mon, 5 Jun 2023 21:55:02 GMT, Chris Plummer  wrote:

> The test fails because it tries to determine the main debuggee thread by 
> allowing it run until the debuggee class is loaded (it waits for the 
> ClassPrepareEvent). Normally this would be done on the main debuggee thread. 
> However, when using virtual threads, the main thread has yet to spawn the 
> virtual thread to run the test on. The test is fixed by instead just waiting 
> until the debuggee main method is entered.
> 
> Tested by running the test locally with and without the virtual test thread 
> wrapper, and also all of tier1 plus tier5 svc tests.

Thanks for the reviews Alex and Serguei!

-

PR Comment: https://git.openjdk.org/jdk/pull/14322#issuecomment-1579258734

Integrated: 8309505: com/sun/jdi/MethodEntryExitEvents.java due to finding wrong main thread

[Chris Plummer]

On Mon, 5 Jun 2023 21:55:02 GMT, Chris Plummer  wrote:

> The test fails because it tries to determine the main debuggee thread by 
> allowing it run until the debuggee class is loaded (it waits for the 
> ClassPrepareEvent). Normally this would be done on the main debuggee thread. 
> However, when using virtual threads, the main thread has yet to spawn the 
> virtual thread to run the test on. The test is fixed by instead just waiting 
> until the debuggee main method is entered.
> 
> Tested by running the test locally with and without the virtual test thread 
> wrapper, and also all of tier1 plus tier5 svc tests.

This pull request has now been integrated.

Changeset: 16ab7bfe
Author:Chris Plummer 
URL:   
https://git.openjdk.org/jdk/commit/16ab7bfe22b96ec8c4b7b7197d55fa41d36e3875
Stats: 7 lines in 2 files changed: 1 ins; 1 del; 5 mod

8309505: com/sun/jdi/MethodEntryExitEvents.java due to finding wrong main thread

Reviewed-by: amenkov, sspitsyn

-

PR: https://git.openjdk.org/jdk/pull/14322

Re: RFR: 8309505: com/sun/jdi/MethodEntryExitEvents.java due to finding wrong main thread [v2]

[Chris Plummer]

> The test fails because it tries to determine the main debuggee thread by 
> allowing it run until the debuggee class is loaded (it waits for the 
> ClassPrepareEvent). Normally this would be done on the main debuggee thread. 
> However, when using virtual threads, the main thread has yet to spawn the 
> virtual thread to run the test on. The test is fixed by instead just waiting 
> until the debuggee main method is entered.
> 
> Tested by running the test locally with and without the virtual test thread 
> wrapper, and also all of tier1 plus tier5 svc tests.

Chris Plummer has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains two commits:

 - Merge
 - Fix virtual thread testing issue by waiting until debuggee main method is 
entered before gleaning the main thread.

-

Changes: https://git.openjdk.org/jdk/pull/14322/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14322&range=01
  Stats: 7 lines in 2 files changed: 1 ins; 1 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/14322.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14322/head:pull/14322

PR: https://git.openjdk.org/jdk/pull/14322

Re: RFR: 8309396: com/sun/jdi/JdbMethodExitTest.java fails with virtual threads due to a bug in determining the main thread id [v2]

[Serguei Spitsyn]

On Sun, 4 Jun 2023 18:47:13 GMT, Chris Plummer  wrote:

>> JdbMethodExitTest.java tries to determine the jdb threadID for the "main" 
>> thread, and then later use it in jdb commands that require a threadID. It 
>> does this by first having the debuggee execute the following:
>> 
>> `System.out.println("threadid="+Thread.currentThread().getId());`
>> 
>> And then later on the test parses the threadID from this output. The problem 
>> is that the id returned by getId() has no relation to threadIDs used by jdb, 
>> which are actually JDWP ObjectIDs. In the past this has worked due to some 
>> dumb luck. getID() always returns 1 for the main thread, which is always the 
>> thread we are executing in. Coincidentally the JDWP ObjectID for the main 
>> Thread object is also always 1 because this is the first ObjectID that the 
>> debug agent ever needs to create. However, when the debuggee main thread is 
>> a virtual thread, neither getId() nor JDWP assign 1 to the threadID, and in 
>> fact both will end up with very different values for the threadID. The end 
>> result is errors from jdb for using an invalid threadID.
>> 
>> The correct threadID can be obtained by executing the jdb "threads" command 
>> and parsing it from a line that looks like the following:
>> 
>> `   (java.lang.VirtualThread)694 main running (at breakpoint)`
>> 
>> Note this test will also fail due to 
>> [JDK-8309334](https://bugs.openjdk.org/browse/JDK-8309334), which should be 
>> fixed first. 
>> 
>> I've tested with mach5 tier5 in a workspace that has integrated the various 
>> CRs mentioned. Once JDK-8309334 is fixed, before integrating this PR I'll 
>> first merge and verify that the test being removed from the problem list by 
>> this PR also passes.
>
> Chris Plummer has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains four commits:
> 
>  - fix minor merge issue
>  - merge
>  - remove test from problem list
>  - properly determine the main threadId

Looks good.
Thanks,
Serguei

-

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14294#pullrequestreview-1465846971

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[Christine Flood]

On Sun, 4 Jun 2023 21:39:58 GMT, Kelvin Nilsen  wrote:

>> OpenJDK Colleagues:
>> 
>> Please review this proposed integration of Generational mode for Shenandoah 
>> GC under https://bugs.openjdk.org/browse/JDK-8307314.
>> 
>> Generational mode of Shenandoah is enabled by adding 
>> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
>> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
>> implementation automatically adjusts the sizes of old generation and young 
>> generation to efficiently utilize the entire heap capacity.  Generational 
>> mode of Shenandoah resembles G1 in the following regards:
>> 
>> 1. Old-generation marking runs concurrently during the time that multiple 
>> young generation collections run to completion.
>> 2. After old-generation marking completes, we perform a sequence of mixed 
>> collections.  Each mixed collection combines collection of young generation 
>> with evacuation of a portion of the old-generation regions identified for 
>> collection based on old-generation marking information.
>> 3. Unlike G1, young-generation collections and evacuations are entirely 
>> concurrent, as with single-generation Shenandoah.
>> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
>> and survivor space within the young generation.  In practice, regions that 
>> were most recently allocated tend to have large amounts of garbage and these 
>> regions tend to be collected with very little effort.  Young-generation 
>> objects that survive garbage collection tend to accumulate in regions that 
>> hold survivor objects.  These regions tend to have smaller amounts of 
>> garbage, and are less likely to be collected.  If they survive a sufficient 
>> number of young-generation collections, the “survivor” regions are promoted 
>> into the old generation.
>> 
>> We expect to refine heuristics as we gain experience with more production 
>> workloads.  In the future, we plan to remove the “experimental” qualifier 
>> from generational mode, at which time we expect that generational mode will 
>> become the default mode for Shenandoah.
>> 
>> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
>> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
>> HyperAlloc, and multiple AWS production workload simulators. We test on 
>> Linux x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and 
>> Windows x64.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove three asserts making comparisons between atomic volatile variables
>   
>   Though changes to the volatile variables are individually protected by
>   Atomic load and store operations, these asserts were not assuring
>   atomic access to multiple volatile variables, each of which could be
>   modified independently of the others.  The asserts were therefore not
>   trustworthy, as has been confirmed by more extensive testing.

I wrote an LRU program back in 2017 which allocates trees and stores them in an 
array in a round robin fashion, freeing the last allocated.  At the time this 
was written it's purpose was to show how generational GCs can hit the wall and 
start performing very badly.  I ran this on a clean openjdk build, a genshen 
build in generational mode and a genshen  build in non-generational mode.   
These results are repeatable for me. 

I would like to understand where the degradation is coming from before moving 
forward with this patch since it appears to penalize those who wish to just run 
traditional Shenandoah.

Clean
cflood@fedora java_programs]$ 
~/genshen/cleanjdk/build/linux-x86_64-server-release/images/jdk/bin/java  
-XX:+UnlockExperimentalVMOptions -XX:+UseShenandoahGC LRU 1000 1000
Took 341892ms to allocate 100 trees in a cache of 1000

Genshen generational (we expect this to be bad)
[cflood@fedora java_programs]$ 
~/genshen/jdk/build/linux-x86_64-server-release/images/jdk/bin/java  
-XX:+UnlockExperimentalVMOptions -XX:+UseShenandoahGC 
-XX:ShenandoahGCMode=generational LRU 1000 1000
Took 442012ms to allocate 100 trees in a cache of 1000

Genshen non-generational (shows what I feel is a significant degradation from 
the clean build)
[cflood@fedora java_programs]$ 
~/genshen/jdk/build/linux-x86_64-server-release/images/jdk/bin/java  
-XX:+UnlockExperimentalVMOptions -XX:+UseShenandoahGC LRU 1000 1000
Took 395679ms to allocate 100 trees in a cache of 1000

I think that generational Shenandoah can be a big win for some applications, 
but I want to fully understand the cost for all applications.

I can't attach a .java file so here it is inline in the post.

class TreeNode {
public TreeNode left, right;
public int val;
}


public class LRU {
static int cache_size;
static int reps;
static int tree_height=16;

private static TreeNode[] trees;

private static int getIndex(int i) {return i % c

Re: RFR: 8309420: con/sun/jdi/StepTest.java fails with virtual thread wrapper

[Serguei Spitsyn]

On Mon, 5 Jun 2023 04:03:57 GMT, Chris Plummer  wrote:

> The test has two issues. The first is that it assume that once the VMStart 
> event has arrived and one "step into" is done, it will be in the main method 
> of the debuggee. Once there, it determines the debuggee class name by looking 
> at the classtype of topmost frame. The problems is when using virtual 
> threads, it is actually in TestScaffold.main() at this point, so the wrong 
> class name is gleaned from the frame. To fix this the test just saves away 
> the debuggee class name, which is passed to the test as the 4th argument.
> 
> The other issue is that the test assumes once it gets to the debuggee go() 
> method, there are only two frames on the stack. It's more like 16 when using 
> virtual threads. The test needs to account for this by counting the number of 
> frames when go() is entered rather than assuming it will be 2.
> 
> Tested locally with and without the wrapper and by running tier5 svc tests.

Looks good.
Thanks,
Serguei

-

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14307#pullrequestreview-1465860157

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[Paul Hohensee]

On Sun, 4 Jun 2023 21:39:58 GMT, Kelvin Nilsen  wrote:

>> OpenJDK Colleagues:
>> 
>> Please review this proposed integration of Generational mode for Shenandoah 
>> GC under https://bugs.openjdk.org/browse/JDK-8307314.
>> 
>> Generational mode of Shenandoah is enabled by adding 
>> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
>> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
>> implementation automatically adjusts the sizes of old generation and young 
>> generation to efficiently utilize the entire heap capacity.  Generational 
>> mode of Shenandoah resembles G1 in the following regards:
>> 
>> 1. Old-generation marking runs concurrently during the time that multiple 
>> young generation collections run to completion.
>> 2. After old-generation marking completes, we perform a sequence of mixed 
>> collections.  Each mixed collection combines collection of young generation 
>> with evacuation of a portion of the old-generation regions identified for 
>> collection based on old-generation marking information.
>> 3. Unlike G1, young-generation collections and evacuations are entirely 
>> concurrent, as with single-generation Shenandoah.
>> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
>> and survivor space within the young generation.  In practice, regions that 
>> were most recently allocated tend to have large amounts of garbage and these 
>> regions tend to be collected with very little effort.  Young-generation 
>> objects that survive garbage collection tend to accumulate in regions that 
>> hold survivor objects.  These regions tend to have smaller amounts of 
>> garbage, and are less likely to be collected.  If they survive a sufficient 
>> number of young-generation collections, the “survivor” regions are promoted 
>> into the old generation.
>> 
>> We expect to refine heuristics as we gain experience with more production 
>> workloads.  In the future, we plan to remove the “experimental” qualifier 
>> from generational mode, at which time we expect that generational mode will 
>> become the default mode for Shenandoah.
>> 
>> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
>> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
>> HyperAlloc, and multiple AWS production workload simulators. We test on 
>> Linux x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and 
>> Windows x64.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove three asserts making comparisons between atomic volatile variables
>   
>   Though changes to the volatile variables are individually protected by
>   Atomic load and store operations, these asserts were not assuring
>   atomic access to multiple volatile variables, each of which could be
>   modified independently of the others.  The asserts were therefore not
>   trustworthy, as has been confirmed by more extensive testing.

Thanks for finding the single-gen regression, we're very happy you took the 
time to run it and write up your results. We're very concerned about single-gen 
regressions too because we have single-gen Shen in production for several 
critical services.

We'd like to propose to push now, and tackle/fix the single-gen issue you 
identified during RDP1, as well as any other significant single-gen regressions 
that may come up. We have four Shen experts on board, Roman, Aleksey, Kelvin, 
and William, so believe it's doable before RDP2 in July. In the worst case that 
we fail, we'd emulate ZGC and move GenShen to it's own directory as an entirely 
separate collector before RDP2. Make sense?

-

PR Comment: https://git.openjdk.org/jdk/pull/14185#issuecomment-1579351993

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[Andrew Haley]

On Sun, 4 Jun 2023 21:39:58 GMT, Kelvin Nilsen  wrote:

>> OpenJDK Colleagues:
>> 
>> Please review this proposed integration of Generational mode for Shenandoah 
>> GC under https://bugs.openjdk.org/browse/JDK-8307314.
>> 
>> Generational mode of Shenandoah is enabled by adding 
>> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
>> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
>> implementation automatically adjusts the sizes of old generation and young 
>> generation to efficiently utilize the entire heap capacity.  Generational 
>> mode of Shenandoah resembles G1 in the following regards:
>> 
>> 1. Old-generation marking runs concurrently during the time that multiple 
>> young generation collections run to completion.
>> 2. After old-generation marking completes, we perform a sequence of mixed 
>> collections.  Each mixed collection combines collection of young generation 
>> with evacuation of a portion of the old-generation regions identified for 
>> collection based on old-generation marking information.
>> 3. Unlike G1, young-generation collections and evacuations are entirely 
>> concurrent, as with single-generation Shenandoah.
>> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
>> and survivor space within the young generation.  In practice, regions that 
>> were most recently allocated tend to have large amounts of garbage and these 
>> regions tend to be collected with very little effort.  Young-generation 
>> objects that survive garbage collection tend to accumulate in regions that 
>> hold survivor objects.  These regions tend to have smaller amounts of 
>> garbage, and are less likely to be collected.  If they survive a sufficient 
>> number of young-generation collections, the “survivor” regions are promoted 
>> into the old generation.
>> 
>> We expect to refine heuristics as we gain experience with more production 
>> workloads.  In the future, we plan to remove the “experimental” qualifier 
>> from generational mode, at which time we expect that generational mode will 
>> become the default mode for Shenandoah.
>> 
>> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
>> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
>> HyperAlloc, and multiple AWS production workload simulators. We test on 
>> Linux x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and 
>> Windows x64.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove three asserts making comparisons between atomic volatile variables
>   
>   Though changes to the volatile variables are individually protected by
>   Atomic load and store operations, these asserts were not assuring
>   atomic access to multiple volatile variables, each of which could be
>   modified independently of the others.  The asserts were therefore not
>   trustworthy, as has been confirmed by more extensive testing.

Marked as reviewed by aph (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14185#pullrequestreview-1465971078

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[Andrew Haley]

On Tue, 6 Jun 2023 19:48:05 GMT, Paul Hohensee  wrote:

> We'd like to propose to push now, and tackle/fix the single-gen issue you 
> identified during RDP1, as well as any other significant single-gen 
> regressions that may come up. We have four Shen experts on board, Roman, 
> Aleksey, Kelvin, and William, so believe it's doable before RDP2 in July. In 
> the worst case that we fail, we'd emulate ZGC and move GenShen to it's own 
> directory as an entirely separate collector before RDP2. Make sense?

That sounds great to me. I'll approve this PR now, but please wait for 
Christine's ack.

-

PR Comment: https://git.openjdk.org/jdk/pull/14185#issuecomment-1579360356

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[Christine Flood]

On Sun, 4 Jun 2023 21:39:58 GMT, Kelvin Nilsen  wrote:

>> OpenJDK Colleagues:
>> 
>> Please review this proposed integration of Generational mode for Shenandoah 
>> GC under https://bugs.openjdk.org/browse/JDK-8307314.
>> 
>> Generational mode of Shenandoah is enabled by adding 
>> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
>> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
>> implementation automatically adjusts the sizes of old generation and young 
>> generation to efficiently utilize the entire heap capacity.  Generational 
>> mode of Shenandoah resembles G1 in the following regards:
>> 
>> 1. Old-generation marking runs concurrently during the time that multiple 
>> young generation collections run to completion.
>> 2. After old-generation marking completes, we perform a sequence of mixed 
>> collections.  Each mixed collection combines collection of young generation 
>> with evacuation of a portion of the old-generation regions identified for 
>> collection based on old-generation marking information.
>> 3. Unlike G1, young-generation collections and evacuations are entirely 
>> concurrent, as with single-generation Shenandoah.
>> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
>> and survivor space within the young generation.  In practice, regions that 
>> were most recently allocated tend to have large amounts of garbage and these 
>> regions tend to be collected with very little effort.  Young-generation 
>> objects that survive garbage collection tend to accumulate in regions that 
>> hold survivor objects.  These regions tend to have smaller amounts of 
>> garbage, and are less likely to be collected.  If they survive a sufficient 
>> number of young-generation collections, the “survivor” regions are promoted 
>> into the old generation.
>> 
>> We expect to refine heuristics as we gain experience with more production 
>> workloads.  In the future, we plan to remove the “experimental” qualifier 
>> from generational mode, at which time we expect that generational mode will 
>> become the default mode for Shenandoah.
>> 
>> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
>> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
>> HyperAlloc, and multiple AWS production workload simulators. We test on 
>> Linux x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and 
>> Windows x64.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove three asserts making comparisons between atomic volatile variables
>   
>   Though changes to the volatile variables are individually protected by
>   Atomic load and store operations, these asserts were not assuring
>   atomic access to multiple volatile variables, each of which could be
>   modified independently of the others.  The asserts were therefore not
>   trustworthy, as has been confirmed by more extensive testing.

This sounds good to me.

On Tue, Jun 6, 2023 at 3:55 PM Andrew Haley ***@***.***>
wrote:

> We'd like to propose to push now, and tackle/fix the single-gen issue you
> identified during RDP1, as well as any other significant single-gen
> regressions that may come up. We have four Shen experts on board, Roman,
> Aleksey, Kelvin, and William, so believe it's doable before RDP2 in July.
> In the worst case that we fail, we'd emulate ZGC and move GenShen to it's
> own directory as an entirely separate collector before RDP2. Make sense?
>
> That sounds great to me. I'll approve this PR now, but please wait for
> Christine's ack.
>
> —
> Reply to this email directly, view it on GitHub
> , or
> unsubscribe
> 
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

-

PR Comment: https://git.openjdk.org/jdk/pull/14185#issuecomment-1579367147

Re: RFR: 8309396: com/sun/jdi/JdbMethodExitTest.java fails with virtual threads due to a bug in determining the main thread id [v3]

[Chris Plummer]

> JdbMethodExitTest.java tries to determine the jdb threadID for the "main" 
> thread, and then later use it in jdb commands that require a threadID. It 
> does this by first having the debuggee execute the following:
> 
> `System.out.println("threadid="+Thread.currentThread().getId());`
> 
> And then later on the test parses the threadID from this output. The problem 
> is that the id returned by getId() has no relation to threadIDs used by jdb, 
> which are actually JDWP ObjectIDs. In the past this has worked due to some 
> dumb luck. getID() always returns 1 for the main thread, which is always the 
> thread we are executing in. Coincidentally the JDWP ObjectID for the main 
> Thread object is also always 1 because this is the first ObjectID that the 
> debug agent ever needs to create. However, when the debuggee main thread is a 
> virtual thread, neither getId() nor JDWP assign 1 to the threadID, and in 
> fact both will end up with very different values for the threadID. The end 
> result is errors from jdb for using an invalid threadID.
> 
> The correct threadID can be obtained by executing the jdb "threads" command 
> and parsing it from a line that looks like the following:
> 
> `   (java.lang.VirtualThread)694 main running (at breakpoint)`
> 
> Note this test will also fail due to 
> [JDK-8309334](https://bugs.openjdk.org/browse/JDK-8309334), which should be 
> fixed first. 
> 
> I've tested with mach5 tier5 in a workspace that has integrated the various 
> CRs mentioned. Once JDK-8309334 is fixed, before integrating this PR I'll 
> first merge and verify that the test being removed from the problem list by 
> this PR also passes.

Chris Plummer has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains five commits:

 - Merge
 - fix minor merge issue
 - merge
 - remove test from problem list
 - properly determine the main threadId

-

Changes: https://git.openjdk.org/jdk/pull/14294/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14294&range=02
  Stats: 11 lines in 2 files changed: 5 ins; 2 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/14294.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14294/head:pull/14294

PR: https://git.openjdk.org/jdk/pull/14294

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[Mark Reinhold]

On Tue, 6 Jun 2023 20:01:02 GMT, Christine Flood  wrote:

> We'd like to propose to push now, and tackle/fix the single-gen issue you 
> identified during RDP1, as well as any other significant single-gen 
> regressions that may come up. We have four Shen experts on board, Roman, 
> Aleksey, Kelvin, and William, so believe it's doable before RDP2 in July. In 
> the worst case that we fail, we'd emulate ZGC and move GenShen to it's own 
> directory as an entirely separate collector before RDP2. Make sense?

Unsolicited advice: If you’re planning for this amount of change during RDP 1 
then I’d say that you’re not ready for RDP 1.

If this patch were less isolated from the rest of HotSpot then I’d be extremely 
nervous.

-

PR Comment: https://git.openjdk.org/jdk/pull/14185#issuecomment-1579385819

Re: RFR: JDK-8309462: vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/TestDescription.java crashing due to empty while loop

[Martin Doerr]

On Tue, 6 Jun 2023 09:51:09 GMT, JoKern65  wrote:

> The sys_thread_3() function contains an empty while loop, which by the 
> standard can be optimized away. Please refer to discussion in 
> https://github.com/llvm/llvm-project/issues/60622
> The xlc17 compiler is doing so, and IBM claims that they are following the 
> standard and will not fix this on compiler side.
> So we have (at least) 3 ways to circumvent this behavior.
> 
> 1. we can introduce the call of a nop library function, which will hinder the 
> optimizer to throw away the loop (This is our proposed solution, but instead 
> of a heavy looping thread, the result is a more or less idle thread):
> `#include `
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
> `{`
> `while (1) {`
> `  sleep(1);`
> `}`
> `}`
> 
> 2. We can make use of a volatile variable in the loop body which also hinders 
> the optimizer to throw away the loop:
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
> `{`
> `volatile int i = 1;`
> `while (i) {`
> `  i += 2;`
> `}`
> `}`
> 
> 3. we can use the __attribute__ ((optnone)) modifier in the function 
> declaration to suppress the optimization at all:
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) __attribute__ 
> ((optnone))`
> `{`
> `while (1) {`
> `}`
> `}`
> 
> To make the third approach platform independent, we can implement it in the 
> following way:
> In globalDefinitions.hpp
> `#ifndef OPTNONE`
> `#define OPTNONE`
> `#endif`
> 
> In globalDefinitions_xlc.hpp
> `// optnone support`
> `//`
> `// To use if a function should not be optimized`
> `// Usage:`
> `// void* func(size_t size) OPTNONE {...}`
> `#define OPTNONE __attribute__(( optnone))`
> 
> With this we can change libagentthr001.cpp in a platform independent way to
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) OPTNONE`
> `{`
> ` while (1) {`
> ` }`
> `}`

Changes requested by mdoerr (Reviewer).

test/hotspot/jtreg/vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/agentthr001.cpp
 line 26:

> 24: #include 
> 25: #include 
> 26: #include 

Breaks Windows build: agentthr001.cpp(26): fatal error C1083: Cannot open 
include file: 'unistd.h': No such file or directory

-

PR Review: https://git.openjdk.org/jdk/pull/14330#pullrequestreview-1466074354
PR Review Comment: https://git.openjdk.org/jdk/pull/14330#discussion_r1220327772

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[Paul Hohensee]

On Sun, 4 Jun 2023 21:39:58 GMT, Kelvin Nilsen  wrote:

>> OpenJDK Colleagues:
>> 
>> Please review this proposed integration of Generational mode for Shenandoah 
>> GC under https://bugs.openjdk.org/browse/JDK-8307314.
>> 
>> Generational mode of Shenandoah is enabled by adding 
>> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
>> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
>> implementation automatically adjusts the sizes of old generation and young 
>> generation to efficiently utilize the entire heap capacity.  Generational 
>> mode of Shenandoah resembles G1 in the following regards:
>> 
>> 1. Old-generation marking runs concurrently during the time that multiple 
>> young generation collections run to completion.
>> 2. After old-generation marking completes, we perform a sequence of mixed 
>> collections.  Each mixed collection combines collection of young generation 
>> with evacuation of a portion of the old-generation regions identified for 
>> collection based on old-generation marking information.
>> 3. Unlike G1, young-generation collections and evacuations are entirely 
>> concurrent, as with single-generation Shenandoah.
>> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
>> and survivor space within the young generation.  In practice, regions that 
>> were most recently allocated tend to have large amounts of garbage and these 
>> regions tend to be collected with very little effort.  Young-generation 
>> objects that survive garbage collection tend to accumulate in regions that 
>> hold survivor objects.  These regions tend to have smaller amounts of 
>> garbage, and are less likely to be collected.  If they survive a sufficient 
>> number of young-generation collections, the “survivor” regions are promoted 
>> into the old generation.
>> 
>> We expect to refine heuristics as we gain experience with more production 
>> workloads.  In the future, we plan to remove the “experimental” qualifier 
>> from generational mode, at which time we expect that generational mode will 
>> become the default mode for Shenandoah.
>> 
>> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
>> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
>> HyperAlloc, and multiple AWS production workload simulators. We test on 
>> Linux x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and 
>> Windows x64.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove three asserts making comparisons between atomic volatile variables
>   
>   Though changes to the volatile variables are individually protected by
>   Atomic load and store operations, these asserts were not assuring
>   atomic access to multiple volatile variables, each of which could be
>   modified independently of the others.  The asserts were therefore not
>   trustworthy, as has been confirmed by more extensive testing.

We understand, and would not have proposed the last chance split-directory 
alternative without the level of isolation Hotspot's GC interface enables. 
We've added single-gen performance enhancements on the way and would like to 
keep them!

-

PR Comment: https://git.openjdk.org/jdk/pull/14185#issuecomment-1579439750

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v6]

[Kelvin Nilsen]

> OpenJDK Colleagues:
> 
> Please review this proposed integration of Generational mode for Shenandoah 
> GC under https://bugs.openjdk.org/browse/JDK-8307314.
> 
> Generational mode of Shenandoah is enabled by adding 
> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
> implementation automatically adjusts the sizes of old generation and young 
> generation to efficiently utilize the entire heap capacity.  Generational 
> mode of Shenandoah resembles G1 in the following regards:
> 
> 1. Old-generation marking runs concurrently during the time that multiple 
> young generation collections run to completion.
> 2. After old-generation marking completes, we perform a sequence of mixed 
> collections.  Each mixed collection combines collection of young generation 
> with evacuation of a portion of the old-generation regions identified for 
> collection based on old-generation marking information.
> 3. Unlike G1, young-generation collections and evacuations are entirely 
> concurrent, as with single-generation Shenandoah.
> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
> and survivor space within the young generation.  In practice, regions that 
> were most recently allocated tend to have large amounts of garbage and these 
> regions tend to be collected with very little effort.  Young-generation 
> objects that survive garbage collection tend to accumulate in regions that 
> hold survivor objects.  These regions tend to have smaller amounts of 
> garbage, and are less likely to be collected.  If they survive a sufficient 
> number of young-generation collections, the “survivor” regions are promoted 
> into the old generation.
> 
> We expect to refine heuristics as we gain experience with more production 
> workloads.  In the future, we plan to remove the “experimental” qualifier 
> from generational mode, at which time we expect that generational mode will 
> become the default mode for Shenandoah.
> 
> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
> HyperAlloc, and multiple AWS production workload simulators. We test on Linux 
> x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and Windows 
> x64.

Kelvin Nilsen has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove an inappropriate copyright notice

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14185/files
  - new: https://git.openjdk.org/jdk/pull/14185/files/8d80780a..9811d2aa

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14185&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14185&range=04-05

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/14185.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14185/head:pull/14185

PR: https://git.openjdk.org/jdk/pull/14185

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v3]

[Chris Plummer]

On Tue, 6 Jun 2023 13:37:03 GMT, Serguei Spitsyn  wrote:

>> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
>> VirtualThread blocked on a monitor when called for more than one thread. 
>> When called for a single VirtualThread it correctly returns a state that 
>> includes the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
>> The `VM_GetThreadListStackTraces::doit` should call the 
>> `get_threadOop_and_JavaThread` instead of 
>> `cv_external_thread_to_JavaThread`. But the `get_threadOop_and_JavaThread` 
>> has a check for the current thread by comparing with the 
>> JavaThread::current() which does not work for a `VM_op`. Some refactoring of 
>> the `get_threadOop_and_JavaThread` was made to make it working for a `VM_op`.
>> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
>> discovered during testing.
>> 
>> The list of changes is:
>> - minor refactoring of the function`get_threadOop_and_JavaThread`: added an 
>> overloaded version of this function with the extra parameter `JavaThread* 
>> cur_thread`. It is called instead of 
>> `JvmtiExport::cv_external_thread_to_JavaThread` from the 
>> `VM_GetThreadListStackTraces::doit`.
>> - `GetSingleStackTraceClosure::do_thread()`: The use of `jt->threadObj()` is 
>> replaced with the `JNIHandles::resolve_external_guard(_jthread)`.
>>  - added new test to provide needed coverage: 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  
>> Testing:
>>  - ran new test: 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  - TBD: tiers 1-6 (all are good)
>
> Serguei Spitsyn has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - fixed typo in a comment in jvmtiEnvBase.cpp
>  - nit: restored one comment as was before

Changes requested by cjplummer (Reviewer).

test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
 line 63:

> 61: public void run() {
> 62: log("TestTask.run()");
> 63: }

I think this should be an abstract method.

test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
 line 106:

> 104: final Thread.State expState = Thread.State.WAITING;
> 105: reentrantLock.lock();
> 106: String name = "ObjectMonitorTestTask";

Should be "ReentrantLockTestTask"

test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/libThreadListStackTracesTest.cpp
 line 35:

> 33: extern "C" {
> 34: 
> 35: JNIEXPORT jint JNICALL 
> Java_ThreadListStackTracesTest_getStateSingle(JNIEnv* jni, jclass clazz, 
> jthread vthread) {

I'd suggest splitting into 2 lines just like 
Java_ThreadListStackTracesTest_getStateMultiple() for the sake of consistency 
and being able to more easily compare the two.

-

PR Review: https://git.openjdk.org/jdk/pull/14326#pullrequestreview-1466112714
PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220365810
PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220367970
PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220361274

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v2]

[Alex Menkov]

On Mon, 5 Jun 2023 19:00:49 GMT, Serguei Spitsyn  wrote:

>> When a virtual thread is mounted, the carrier thread should be reported as 
>> "waiting" until the virtual thread unmounts. Right now, GetThreadState 
>> reports a state based the JavaThread status when it should return 
>> JVMTI_THREAD_STATE_WAITING | JVMTI_THREAD_STATE_WAITING_INDEFINITELY.
>> The fix adds:
>>  - a special case for passive carrier threads
>>  - necessary test coverage to the existing JVMTI test: 
>> `serviceability/jvmti/vthread/ThreadStateTest`.
>> 
>> Testing:
>>- tested with the updated test: 
>> `serviceability/jvmti/vthread/ThreadStateTest`
>>- submitted mach5 tiers 1-5
>>- TBD: to submit mach5 tier 6
>
> Serguei Spitsyn 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 three additional 
> commits since the last revision:
> 
>  - Merge
>  - minor tweaks in libThreadStateTest.cpp
>  - 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING

src/hotspot/share/prims/jvmtiEnvBase.cpp line 764:

> 762: 
> 763:   if (is_passive_carrier_thread(jt, thread_oop)) {
> 764: state |= (JVMTI_THREAD_STATE_WAITING | 
> JVMTI_THREAD_STATE_WAITING_INDEFINITELY);

Not sure I understand this.
I'd expect 
`JVMTI_THREAD_STATE_ALIVE | JVMTI_THREAD_STATE_WAITING | 
JVMTI_THREAD_STATE_WAITING_INDEFINITELY` to be returned in the case.
How can a thread be JVMTI_THREAD_STATE_RUNNABLE and JVMTI_THREAD_STATE_WAITING 
at the same time?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220384120

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v3]

[Serguei Spitsyn]

On Tue, 6 Jun 2023 21:07:46 GMT, Chris Plummer  wrote:

>> Serguei Spitsyn has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - fixed typo in a comment in jvmtiEnvBase.cpp
>>  - nit: restored one comment as was before
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
>  line 63:
> 
>> 61: public void run() {
>> 62: log("TestTask.run()");
>> 63: }
> 
> I think this should be an abstract method.

Thanks. Fixed now.

> test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
>  line 106:
> 
>> 104: final Thread.State expState = Thread.State.WAITING;
>> 105: reentrantLock.lock();
>> 106: String name = "ObjectMonitorTestTask";
> 
> Should be "ReentrantLockTestTask"

Thanks. Fixed now.

> test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/libThreadListStackTracesTest.cpp
>  line 35:
> 
>> 33: extern "C" {
>> 34: 
>> 35: JNIEXPORT jint JNICALL 
>> Java_ThreadListStackTracesTest_getStateSingle(JNIEnv* jni, jclass clazz, 
>> jthread vthread) {
> 
> I'd suggest splitting into 2 lines just like 
> Java_ThreadListStackTracesTest_getStateMultiple() for the sake of consistency 
> and being able to more easily compare the two.

Thanks. I've overlooked this. Fixed now.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220390676
PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220392423
PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220387806

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v7]

[Kelvin Nilsen]

> OpenJDK Colleagues:
> 
> Please review this proposed integration of Generational mode for Shenandoah 
> GC under https://bugs.openjdk.org/browse/JDK-8307314.
> 
> Generational mode of Shenandoah is enabled by adding 
> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
> implementation automatically adjusts the sizes of old generation and young 
> generation to efficiently utilize the entire heap capacity.  Generational 
> mode of Shenandoah resembles G1 in the following regards:
> 
> 1. Old-generation marking runs concurrently during the time that multiple 
> young generation collections run to completion.
> 2. After old-generation marking completes, we perform a sequence of mixed 
> collections.  Each mixed collection combines collection of young generation 
> with evacuation of a portion of the old-generation regions identified for 
> collection based on old-generation marking information.
> 3. Unlike G1, young-generation collections and evacuations are entirely 
> concurrent, as with single-generation Shenandoah.
> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
> and survivor space within the young generation.  In practice, regions that 
> were most recently allocated tend to have large amounts of garbage and these 
> regions tend to be collected with very little effort.  Young-generation 
> objects that survive garbage collection tend to accumulate in regions that 
> hold survivor objects.  These regions tend to have smaller amounts of 
> garbage, and are less likely to be collected.  If they survive a sufficient 
> number of young-generation collections, the “survivor” regions are promoted 
> into the old generation.
> 
> We expect to refine heuristics as we gain experience with more production 
> workloads.  In the future, we plan to remove the “experimental” qualifier 
> from generational mode, at which time we expect that generational mode will 
> become the default mode for Shenandoah.
> 
> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
> HyperAlloc, and multiple AWS production workload simulators. We test on Linux 
> x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and Windows 
> x64.

Kelvin Nilsen has updated the pull request incrementally with one additional 
commit since the last revision:

  Exit during initialization on unsupported platforms

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14185/files
  - new: https://git.openjdk.org/jdk/pull/14185/files/9811d2aa..cc149904

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14185&range=06
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14185&range=05-06

  Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/14185.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14185/head:pull/14185

PR: https://git.openjdk.org/jdk/pull/14185

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v4]

[Serguei Spitsyn]

> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
> VirtualThread blocked on a monitor when called for more than one thread. When 
> called for a single VirtualThread it correctly returns a state that includes 
> the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
> The `VM_GetThreadListStackTraces::doit` should call the 
> `get_threadOop_and_JavaThread` instead of `cv_external_thread_to_JavaThread`. 
> But the `get_threadOop_and_JavaThread` has a check for the current thread by 
> comparing with the JavaThread::current() which does not work for a `VM_op`. 
> Some refactoring of the `get_threadOop_and_JavaThread` was made to make it 
> working for a `VM_op`.
> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
> discovered during testing.
> 
> The list of changes is:
> - minor refactoring of the function`get_threadOop_and_JavaThread`: added an 
> overloaded version of this function with the extra parameter `JavaThread* 
> cur_thread`. It is called instead of 
> `JvmtiExport::cv_external_thread_to_JavaThread` from the 
> `VM_GetThreadListStackTraces::doit`.
> - `GetSingleStackTraceClosure::do_thread()`: The use of `jt->threadObj()` is 
> replaced with the `JNIHandles::resolve_external_guard(_jthread)`.
>  - added new test to provide needed coverage: 
> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  
> Testing:
>  - ran new test: 
> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  - TBD: tiers 1-6 (all are good)

Serguei Spitsyn has updated the pull request incrementally with one additional 
commit since the last revision:

  addressed new test related review comments

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14326/files
  - new: https://git.openjdk.org/jdk/pull/14326/files/d20e1221..e982f97e

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14326&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14326&range=02-03

  Stats: 6 lines in 2 files changed: 1 ins; 2 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/14326.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14326/head:pull/14326

PR: https://git.openjdk.org/jdk/pull/14326

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v7]

[Kelvin Nilsen]

On Thu, 1 Jun 2023 13:32:49 GMT, Thomas Stuefe  wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Exit during initialization on unsupported platforms
>
> test/hotspot/jtreg/gc/shenandoah/TestEvilSyncBug.java line 33:
> 
>> 31:  * @modules java.base/jdk.internal.misc
>> 32:  *  java.management
>> 33:  * @run driver/timeout=480 TestEvilSyncBug 
>> -XX:ShenandoahGCHeuristics=aggressive
> 
> Probably fine, but why this change to non-generational testing? Will 
> aggressive heuristic sharpen the test?

We moved this argument from the source code (original line 64) to here so that 
we can invoke the test differently in generational mode.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14185#discussion_r1220410522

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v4]

[Chris Plummer]

On Tue, 6 Jun 2023 21:37:25 GMT, Serguei Spitsyn  wrote:

>> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
>> VirtualThread blocked on a monitor when called for more than one thread. 
>> When called for a single VirtualThread it correctly returns a state that 
>> includes the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
>> The `VM_GetThreadListStackTraces::doit` should call the 
>> `get_threadOop_and_JavaThread` instead of 
>> `cv_external_thread_to_JavaThread`. But the `get_threadOop_and_JavaThread` 
>> has a check for the current thread by comparing with the 
>> JavaThread::current() which does not work for a `VM_op`. Some refactoring of 
>> the `get_threadOop_and_JavaThread` was made to make it working for a `VM_op`.
>> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
>> discovered during testing.
>> 
>> The list of changes is:
>> - minor refactoring of the function`get_threadOop_and_JavaThread`: added an 
>> overloaded version of this function with the extra parameter `JavaThread* 
>> cur_thread`. It is called instead of 
>> `JvmtiExport::cv_external_thread_to_JavaThread` from the 
>> `VM_GetThreadListStackTraces::doit`.
>> - `GetSingleStackTraceClosure::do_thread()`: The use of `jt->threadObj()` is 
>> replaced with the `JNIHandles::resolve_external_guard(_jthread)`.
>>  - added new test to provide needed coverage: 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  
>> Testing:
>>  - ran new test: 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  - TBD: tiers 1-6 (all are good)
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   addressed new test related review comments

Looks good.

-

Marked as reviewed by cjplummer (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14326#pullrequestreview-1466172053

Re: RFR: JDK-8309462: vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/TestDescription.java crashing due to empty while loop

[Christoph Langer]

On Tue, 6 Jun 2023 09:51:09 GMT, JoKern65  wrote:

> The sys_thread_3() function contains an empty while loop, which by the 
> standard can be optimized away. Please refer to discussion in 
> https://github.com/llvm/llvm-project/issues/60622
> The xlc17 compiler is doing so, and IBM claims that they are following the 
> standard and will not fix this on compiler side.
> So we have (at least) 3 ways to circumvent this behavior.
> 
> 1. we can introduce the call of a nop library function, which will hinder the 
> optimizer to throw away the loop (This is our proposed solution, but instead 
> of a heavy looping thread, the result is a more or less idle thread):
> `#include `
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
> `{`
> `while (1) {`
> `  sleep(1);`
> `}`
> `}`
> 
> 2. We can make use of a volatile variable in the loop body which also hinders 
> the optimizer to throw away the loop:
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
> `{`
> `volatile int i = 1;`
> `while (i) {`
> `  i += 2;`
> `}`
> `}`
> 
> 3. we can use the __attribute__ ((optnone)) modifier in the function 
> declaration to suppress the optimization at all:
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) __attribute__ 
> ((optnone))`
> `{`
> `while (1) {`
> `}`
> `}`
> 
> To make the third approach platform independent, we can implement it in the 
> following way:
> In globalDefinitions.hpp
> `#ifndef OPTNONE`
> `#define OPTNONE`
> `#endif`
> 
> In globalDefinitions_xlc.hpp
> `// optnone support`
> `//`
> `// To use if a function should not be optimized`
> `// Usage:`
> `// void* func(size_t size) OPTNONE {...}`
> `#define OPTNONE __attribute__(( optnone))`
> 
> With this we can change libagentthr001.cpp in a platform independent way to
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) OPTNONE`
> `{`
> ` while (1) {`
> ` }`
> `}`

The solution to use sleep looks fine. Please address the comments and also 
update the copyright year.

test/hotspot/jtreg/vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/agentthr001.cpp
 line 141:

> 139: sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) {
> 140: while (1) {
> 141:   sleep(1);

Maybe you could add a comment here that the call to sleep is necessary to avoid 
the compiler optimization to elide the loop.

-

Changes requested by clanger (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14330#pullrequestreview-1466167108
PR Review Comment: https://git.openjdk.org/jdk/pull/14330#discussion_r1220417459

Re: RFR: JDK-8309462: vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/TestDescription.java crashing due to empty while loop

[Christoph Langer]

On Tue, 6 Jun 2023 20:43:36 GMT, Martin Doerr  wrote:

>> The sys_thread_3() function contains an empty while loop, which by the 
>> standard can be optimized away. Please refer to discussion in 
>> https://github.com/llvm/llvm-project/issues/60622
>> The xlc17 compiler is doing so, and IBM claims that they are following the 
>> standard and will not fix this on compiler side.
>> So we have (at least) 3 ways to circumvent this behavior.
>> 
>> 1. we can introduce the call of a nop library function, which will hinder 
>> the optimizer to throw away the loop (This is our proposed solution, but 
>> instead of a heavy looping thread, the result is a more or less idle thread):
>> `#include `
>> `static void`
>> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
>> `{`
>> `while (1) {`
>> `  sleep(1);`
>> `}`
>> `}`
>> 
>> 2. We can make use of a volatile variable in the loop body which also 
>> hinders the optimizer to throw away the loop:
>> `static void`
>> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
>> `{`
>> `volatile int i = 1;`
>> `while (i) {`
>> `  i += 2;`
>> `}`
>> `}`
>> 
>> 3. we can use the __attribute__ ((optnone)) modifier in the function 
>> declaration to suppress the optimization at all:
>> `static void`
>> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) __attribute__ 
>> ((optnone))`
>> `{`
>> `while (1) {`
>> `}`
>> `}`
>> 
>> To make the third approach platform independent, we can implement it in the 
>> following way:
>> In globalDefinitions.hpp
>> `#ifndef OPTNONE`
>> `#define OPTNONE`
>> `#endif`
>> 
>> In globalDefinitions_xlc.hpp
>> `// optnone support`
>> `//`
>> `// To use if a function should not be optimized`
>> `// Usage:`
>> `// void* func(size_t size) OPTNONE {...}`
>> `#define OPTNONE __attribute__(( optnone))`
>> 
>> With this we can change libagentthr001.cpp in a platform independent way to
>> `static void`
>> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) OPTNONE`
>> `{`
>> ` while (1) {`
>> ` }`
>> `}`
>
> test/hotspot/jtreg/vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/agentthr001.cpp
>  line 26:
> 
>> 24: #include 
>> 25: #include 
>> 26: #include 
> 
> Breaks Windows build: agentthr001.cpp(26): fatal error C1083: Cannot open 
> include file: 'unistd.h': No such file or directory

On Windows I think you need windows.h. I guess you can do it like here: 
https://github.com/openjdk/jdk/blob/4a75fd462c002a209201d8bfc8d6c9eb286a7444/src/java.base/share/native/libjli/wildcard.c#L99

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14330#discussion_r1220410431

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v7]

[Kelvin Nilsen]

On Tue, 6 Jun 2023 21:43:36 GMT, Kelvin Nilsen  wrote:

>> test/hotspot/jtreg/gc/shenandoah/TestEvilSyncBug.java line 33:
>> 
>>> 31:  * @modules java.base/jdk.internal.misc
>>> 32:  *  java.management
>>> 33:  * @run driver/timeout=480 TestEvilSyncBug 
>>> -XX:ShenandoahGCHeuristics=aggressive
>> 
>> Probably fine, but why this change to non-generational testing? Will 
>> aggressive heuristic sharpen the test?
>
> We moved this argument from the source code (original line 64) to here so 
> that we can invoke the test differently in generational mode.

See line 64 of the original source code.  We did not change the behavior.  We 
only changed how the behavior is realized.  This enables generalization of the 
test to generational mode.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14185#discussion_r1220418431

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v7]

[Kelvin Nilsen]

On Thu, 1 Jun 2023 14:25:19 GMT, Thomas Stuefe  wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Exit during initialization on unsupported platforms
>
> test/hotspot/jtreg/gc/shenandoah/oom/TestAllocOutOfMemory.java line 92:
> 
>> 90: expectFailure("-Xmx16m",
>> 91:   "-XX:+UnlockExperimentalVMOptions",
>> 92:   "-XX:+UseShenandoahGC",
> 
> Nit: should not need UnlockExperimentalVMOptions anymore.

We actually do need to UnlockExperimentalVMOptions because generational mode is 
currently an experimental feature.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14185#discussion_r1220417069

Re: RFR: JDK-8309225: Fix xlc17 clang 15 warnings in security and servicability

[Christoph Langer]

On Fri, 2 Jun 2023 10:19:53 GMT, JoKern65  wrote:

> This pr is a split off from JDK-8308288: Fix xlc17 clang warnings in shared 
> code https://github.com/openjdk/jdk/pull/14146
> It handles the part in security and servicability.
> 
> Compiling on AIX with xlc17 which contains the new clang 15 frontend shows 
> the following warnings:
> 
> src/java.security.jgss/share/native/libj2gss/NativeUtil.h:30:
> src/java.security.jgss/share/native/libj2gss/gssapi.h:48:5: error: 
> 'TARGET_OS_MAC' is not defined, evaluates to 0 [-Werror,-Wundef]
> #if TARGET_OS_MAC && (defined(ppc) || defined(ppc64) || defined(i386) || 
> defined(x86_64))
> ^
> TARGET_OS_MAC is not defined. Instead of disabling the warning, I could
> ` #ifndef TARGET_OS_MAC`
>  `#define TARGET_OS_MAC=0`
>  `#endif`
> But this is already handled by disabling the warning for gcc.
> 
> src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c:718:33: error: 
> suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
> struct in6_addr mappedAny = IN6ADDR_ANY_INIT;
> ^~~~
> /usr/include/netinet/in.h:454:32: note: expanded from macro 'IN6ADDR_ANY_INIT'
> #define IN6ADDR_ANY_INIT {0, 0, 0, 0}

Marked as reviewed by clanger (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14282#pullrequestreview-1466180987

Re: RFR: 8309396: com/sun/jdi/JdbMethodExitTest.java fails with virtual threads due to a bug in determining the main thread id [v3]

[Chris Plummer]

On Tue, 6 Jun 2023 20:07:33 GMT, Chris Plummer  wrote:

>> JdbMethodExitTest.java tries to determine the jdb threadID for the "main" 
>> thread, and then later use it in jdb commands that require a threadID. It 
>> does this by first having the debuggee execute the following:
>> 
>> `System.out.println("threadid="+Thread.currentThread().getId());`
>> 
>> And then later on the test parses the threadID from this output. The problem 
>> is that the id returned by getId() has no relation to threadIDs used by jdb, 
>> which are actually JDWP ObjectIDs. In the past this has worked due to some 
>> dumb luck. getID() always returns 1 for the main thread, which is always the 
>> thread we are executing in. Coincidentally the JDWP ObjectID for the main 
>> Thread object is also always 1 because this is the first ObjectID that the 
>> debug agent ever needs to create. However, when the debuggee main thread is 
>> a virtual thread, neither getId() nor JDWP assign 1 to the threadID, and in 
>> fact both will end up with very different values for the threadID. The end 
>> result is errors from jdb for using an invalid threadID.
>> 
>> The correct threadID can be obtained by executing the jdb "threads" command 
>> and parsing it from a line that looks like the following:
>> 
>> `   (java.lang.VirtualThread)694 main running (at breakpoint)`
>> 
>> Note this test will also fail due to 
>> [JDK-8309334](https://bugs.openjdk.org/browse/JDK-8309334), which should be 
>> fixed first. 
>> 
>> I've tested with mach5 tier5 in a workspace that has integrated the various 
>> CRs mentioned. Once JDK-8309334 is fixed, before integrating this PR I'll 
>> first merge and verify that the test being removed from the problem list by 
>> this PR also passes.
>
> Chris Plummer has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains five commits:
> 
>  - Merge
>  - fix minor merge issue
>  - merge
>  - remove test from problem list
>  - properly determine the main threadId

Thanks for the reviews Alex and Serguei!

-

PR Comment: https://git.openjdk.org/jdk/pull/14294#issuecomment-1579501697

Integrated: 8309396: com/sun/jdi/JdbMethodExitTest.java fails with virtual threads due to a bug in determining the main thread id

[Chris Plummer]

On Fri, 2 Jun 2023 21:47:47 GMT, Chris Plummer  wrote:

> JdbMethodExitTest.java tries to determine the jdb threadID for the "main" 
> thread, and then later use it in jdb commands that require a threadID. It 
> does this by first having the debuggee execute the following:
> 
> `System.out.println("threadid="+Thread.currentThread().getId());`
> 
> And then later on the test parses the threadID from this output. The problem 
> is that the id returned by getId() has no relation to threadIDs used by jdb, 
> which are actually JDWP ObjectIDs. In the past this has worked due to some 
> dumb luck. getID() always returns 1 for the main thread, which is always the 
> thread we are executing in. Coincidentally the JDWP ObjectID for the main 
> Thread object is also always 1 because this is the first ObjectID that the 
> debug agent ever needs to create. However, when the debuggee main thread is a 
> virtual thread, neither getId() nor JDWP assign 1 to the threadID, and in 
> fact both will end up with very different values for the threadID. The end 
> result is errors from jdb for using an invalid threadID.
> 
> The correct threadID can be obtained by executing the jdb "threads" command 
> and parsing it from a line that looks like the following:
> 
> `   (java.lang.VirtualThread)694 main running (at breakpoint)`
> 
> Note this test will also fail due to 
> [JDK-8309334](https://bugs.openjdk.org/browse/JDK-8309334), which should be 
> fixed first. 
> 
> I've tested with mach5 tier5 in a workspace that has integrated the various 
> CRs mentioned. Once JDK-8309334 is fixed, before integrating this PR I'll 
> first merge and verify that the test being removed from the problem list by 
> this PR also passes.

This pull request has now been integrated.

Changeset: 65bdbc7a
Author:Chris Plummer 
URL:   
https://git.openjdk.org/jdk/commit/65bdbc7a8c06e5f119c9db832734664780933e01
Stats: 11 lines in 2 files changed: 5 ins; 2 del; 4 mod

8309396: com/sun/jdi/JdbMethodExitTest.java fails with virtual threads due to a 
bug in determining the main thread id

Reviewed-by: amenkov, sspitsyn

-

PR: https://git.openjdk.org/jdk/pull/14294

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v3]

[Serguei Spitsyn]

> When a virtual thread is mounted, the carrier thread should be reported as 
> "waiting" until the virtual thread unmounts. Right now, GetThreadState 
> reports a state based the JavaThread status when it should return 
> JVMTI_THREAD_STATE_WAITING | JVMTI_THREAD_STATE_WAITING_INDEFINITELY.
> The fix adds:
>  - a special case for passive carrier threads
>  - necessary test coverage to the existing JVMTI test: 
> `serviceability/jvmti/vthread/ThreadStateTest`.
> 
> Testing:
>- tested with the updated test: 
> `serviceability/jvmti/vthread/ThreadStateTest`
>- submitted mach5 tiers 1-5
>- TBD: to submit mach5 tier 6

Serguei Spitsyn has updated the pull request incrementally with one additional 
commit since the last revision:

  review: removed JVMTI_THREAD_STATE_RUNNABLE from a carrier thread state

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14298/files
  - new: https://git.openjdk.org/jdk/pull/14298/files/e60da02e..b5eb3835

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14298&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14298&range=01-02

  Stats: 4 lines in 2 files changed: 2 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/14298.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14298/head:pull/14298

PR: https://git.openjdk.org/jdk/pull/14298

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v2]

[Serguei Spitsyn]

On Tue, 6 Jun 2023 21:22:40 GMT, Alex Menkov  wrote:

>> Serguei Spitsyn 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 three additional 
>> commits since the last revision:
>> 
>>  - Merge
>>  - minor tweaks in libThreadStateTest.cpp
>>  - 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 764:
> 
>> 762: 
>> 763:   if (is_passive_carrier_thread(jt, thread_oop)) {
>> 764: state |= (JVMTI_THREAD_STATE_WAITING | 
>> JVMTI_THREAD_STATE_WAITING_INDEFINITELY);
> 
> Not sure I understand this.
> I'd expect 
> `JVMTI_THREAD_STATE_ALIVE | JVMTI_THREAD_STATE_WAITING | 
> JVMTI_THREAD_STATE_WAITING_INDEFINITELY` to be returned in the case.
> How can a thread be JVMTI_THREAD_STATE_RUNNABLE and 
> JVMTI_THREAD_STATE_WAITING at the same time?

Good catch, thanks. Fixed now.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220428448

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v3]

[Serguei Spitsyn]

On Tue, 6 Jun 2023 15:43:02 GMT, Alan Bateman  wrote:

>> Okay, I see you point. Unfortunately, I've always referred the platform 
>> thread with an executed FJP schedular as a carrier thread. The term 
>> 'carrier' with this meaning is everywhere in the JVMTI code. It looks very 
>> confusing to call a thread to be a carrier thread only during some phases of 
>> its execution.
>
>> Okay, I see you point. Unfortunately, I've always referred the platform 
>> thread with an executed FJP schedular as a carrier thread. The term 
>> 'carrier' with this meaning is everywhere in the JVMTI code. It looks very 
>> confusing to call a thread to be a carrier thread only during some phases of 
>> its execution.
> 
> Okay, I'm just pointing out that is_passive_carrier_thread looks a bit 
> strange here as it is testing if a JavaThread is carrying a virtual thread 
> oop -  it's not testing if the thread is owned by the virtual thread 
> scheduler.

I'm still thinking what identifier to use instead of 
`is_passive_carrier_thread`.
Just `is_carrier_thread` is going to be confusing as well.
What about `is_carrier_thread_waiting_for_virtual` or 
`is_carrier_thread_waiting_for_virtual_to_unmount`?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220435738

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v3]

[Alex Menkov]

On Tue, 6 Jun 2023 22:07:14 GMT, Serguei Spitsyn  wrote:

>> When a virtual thread is mounted, the carrier thread should be reported as 
>> "waiting" until the virtual thread unmounts. Right now, GetThreadState 
>> reports a state based the JavaThread status when it should return 
>> JVMTI_THREAD_STATE_WAITING | JVMTI_THREAD_STATE_WAITING_INDEFINITELY.
>> The fix adds:
>>  - a special case for passive carrier threads
>>  - necessary test coverage to the existing JVMTI test: 
>> `serviceability/jvmti/vthread/ThreadStateTest`.
>> 
>> Testing:
>>- tested with the updated test: 
>> `serviceability/jvmti/vthread/ThreadStateTest`
>>- submitted mach5 tiers 1-5
>>- TBD: to submit mach5 tier 6
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   review: removed JVMTI_THREAD_STATE_RUNNABLE from a carrier thread state

src/hotspot/share/prims/jvmtiEnvBase.cpp line 768:

> 766:   }
> 767:   return state;
> 768: }

You don't need to call get_thread_state_base in case "passive carrier thread":

if (is_passive_carrier_thread(jt, thread_oop)) {
  return JVMTI_THREAD_STATE_ALIVE | JVMTI_THREAD_STATE_WAITING
 | JVMTI_THREAD_STATE_WAITING_INDEFINITELY;
}
return get_thread_state_base(thread_oop, jt);

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220447606

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v4]

[Alex Menkov]

On Tue, 6 Jun 2023 21:37:25 GMT, Serguei Spitsyn  wrote:

>> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
>> VirtualThread blocked on a monitor when called for more than one thread. 
>> When called for a single VirtualThread it correctly returns a state that 
>> includes the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
>> The `VM_GetThreadListStackTraces::doit` should call the 
>> `get_threadOop_and_JavaThread` instead of 
>> `cv_external_thread_to_JavaThread`. But the `get_threadOop_and_JavaThread` 
>> has a check for the current thread by comparing with the 
>> JavaThread::current() which does not work for a `VM_op`. Some refactoring of 
>> the `get_threadOop_and_JavaThread` was made to make it working for a `VM_op`.
>> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
>> discovered during testing.
>> 
>> The list of changes is:
>> - minor refactoring of the function`get_threadOop_and_JavaThread`: added an 
>> overloaded version of this function with the extra parameter `JavaThread* 
>> cur_thread`. It is called instead of 
>> `JvmtiExport::cv_external_thread_to_JavaThread` from the 
>> `VM_GetThreadListStackTraces::doit`.
>> - `GetSingleStackTraceClosure::do_thread()`: The use of `jt->threadObj()` is 
>> replaced with the `JNIHandles::resolve_external_guard(_jthread)`.
>>  - added new test to provide needed coverage: 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  
>> Testing:
>>  - ran new test: 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  - TBD: tiers 1-6 (all are good)
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   addressed new test related review comments

Marked as reviewed by amenkov (Reviewer).

test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
 line 29:

> 27:  * @summary GetThreadListStackTraces returns wrong state for blocked 
> VirtualThread
> 28:  * @requires vm.continuations
> 29:  * @modules java.base/java.lang:+open

I think `@modules` it's not needed

test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
 line 34:

> 32: 
> 33: import java.util.concurrent.locks.ReentrantLock;
> 34: import java.util.concurrent.*;

unused imports

-

PR Review: https://git.openjdk.org/jdk/pull/14326#pullrequestreview-1466246664
PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220481289
PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220480268

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v4]

[Serguei Spitsyn]

> When a virtual thread is mounted, the carrier thread should be reported as 
> "waiting" until the virtual thread unmounts. Right now, GetThreadState 
> reports a state based the JavaThread status when it should return 
> JVMTI_THREAD_STATE_WAITING | JVMTI_THREAD_STATE_WAITING_INDEFINITELY.
> The fix adds:
>  - a special case for passive carrier threads
>  - necessary test coverage to the existing JVMTI test: 
> `serviceability/jvmti/vthread/ThreadStateTest`.
> 
> Testing:
>- tested with the updated test: 
> `serviceability/jvmti/vthread/ThreadStateTest`
>- submitted mach5 tiers 1-5
>- TBD: to submit mach5 tier 6

Serguei Spitsyn has updated the pull request incrementally with one additional 
commit since the last revision:

  review: call get_thread_state_base only when needed

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14298/files
  - new: https://git.openjdk.org/jdk/pull/14298/files/b5eb3835..1816

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14298&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14298&range=02-03

  Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/14298.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14298/head:pull/14298

PR: https://git.openjdk.org/jdk/pull/14298

Re: RFR: 8309510: com/sun/jdi/RedefineNestmateAttr/TestNestmateAttr.java no longer needs to override startup() method

[Alex Menkov]

On Tue, 6 Jun 2023 00:36:12 GMT, Chris Plummer  wrote:

> com/sun/jdi/RedefineNestmateAttr/TestNestmateAttr.java currently overrides 
> the TestScaffold.startup() method:
> 
> // override this to correct a bug so arguments can be passed to
> // the Target class
> protected void startUp(String targetName) {
> List argList = new ArrayList<>(Arrays.asList(args));
> argList.add(0, targetName); // pre-pend so it becomes the first "app" 
> arg
> println("run args: " + argList);
> connect((String[]) argList.toArray(args));
> waitForVMStart();
> }
> 
> This issue of passing app args was fixed recently by 
> [JDK-8308481](https://bugs.openjdk.org/browse/JDK-8308481), so the override 
> is no longer needed.

Marked as reviewed by amenkov (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14325#pullrequestreview-1466250289

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v3]

[Serguei Spitsyn]

On Tue, 6 Jun 2023 22:17:57 GMT, Alex Menkov  wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   review: removed JVMTI_THREAD_STATE_RUNNABLE from a carrier thread state
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 768:
> 
>> 766:   }
>> 767:   return state;
>> 768: }
> 
> You don't need to call get_thread_state_base in case "passive carrier thread":
> 
> if (is_passive_carrier_thread(jt, thread_oop)) {
>   return JVMTI_THREAD_STATE_ALIVE | JVMTI_THREAD_STATE_WAITING
>  | JVMTI_THREAD_STATE_WAITING_INDEFINITELY;
> }
> return get_thread_state_base(thread_oop, jt);

Thanks. Yes, noticed it. :) Fixed now.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220484149

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v5]

[Serguei Spitsyn]

> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
> VirtualThread blocked on a monitor when called for more than one thread. When 
> called for a single VirtualThread it correctly returns a state that includes 
> the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
> The `VM_GetThreadListStackTraces::doit` should call the 
> `get_threadOop_and_JavaThread` instead of `cv_external_thread_to_JavaThread`. 
> But the `get_threadOop_and_JavaThread` has a check for the current thread by 
> comparing with the JavaThread::current() which does not work for a `VM_op`. 
> Some refactoring of the `get_threadOop_and_JavaThread` was made to make it 
> working for a `VM_op`.
> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
> discovered during testing.
> 
> The list of changes is:
> - minor refactoring of the function`get_threadOop_and_JavaThread`: added an 
> overloaded version of this function with the extra parameter `JavaThread* 
> cur_thread`. It is called instead of 
> `JvmtiExport::cv_external_thread_to_JavaThread` from the 
> `VM_GetThreadListStackTraces::doit`.
> - `GetSingleStackTraceClosure::do_thread()`: The use of `jt->threadObj()` is 
> replaced with the `JNIHandles::resolve_external_guard(_jthread)`.
>  - added new test to provide needed coverage: 
> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  
> Testing:
>  - ran new test: 
> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  - TBD: tiers 1-6 (all are good)

Serguei Spitsyn has updated the pull request incrementally with one additional 
commit since the last revision:

  review: ThreadListStackTracesTest cleanup

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14326/files
  - new: https://git.openjdk.org/jdk/pull/14326/files/e982f97e..6b685ca3

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14326&range=04
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14326&range=03-04

  Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/14326.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14326/head:pull/14326

PR: https://git.openjdk.org/jdk/pull/14326

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v4]

[Serguei Spitsyn]

On Tue, 6 Jun 2023 21:37:25 GMT, Serguei Spitsyn  wrote:

>> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
>> VirtualThread blocked on a monitor when called for more than one thread. 
>> When called for a single VirtualThread it correctly returns a state that 
>> includes the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
>> The `VM_GetThreadListStackTraces::doit` should call the 
>> `get_threadOop_and_JavaThread` instead of 
>> `cv_external_thread_to_JavaThread`. But the `get_threadOop_and_JavaThread` 
>> has a check for the current thread by comparing with the 
>> JavaThread::current() which does not work for a `VM_op`. Some refactoring of 
>> the `get_threadOop_and_JavaThread` was made to make it working for a `VM_op`.
>> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
>> discovered during testing.
>> 
>> The list of changes is:
>> - minor refactoring of the function`get_threadOop_and_JavaThread`: added an 
>> overloaded version of this function with the extra parameter `JavaThread* 
>> cur_thread`. It is called instead of 
>> `JvmtiExport::cv_external_thread_to_JavaThread` from the 
>> `VM_GetThreadListStackTraces::doit`.
>> - `GetSingleStackTraceClosure::do_thread()`: The use of `jt->threadObj()` is 
>> replaced with the `JNIHandles::resolve_external_guard(_jthread)`.
>>  - added new test to provide needed coverage: 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  
>> Testing:
>>  - ran new test: 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  - TBD: tiers 1-6 (all are good)
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   addressed new test related review comments

Chris and Alex, thank you for review!

-

PR Comment: https://git.openjdk.org/jdk/pull/14326#issuecomment-1579560041

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v4]

[Serguei Spitsyn]

On Tue, 6 Jun 2023 22:34:33 GMT, Alex Menkov  wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   addressed new test related review comments
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
>  line 29:
> 
>> 27:  * @summary GetThreadListStackTraces returns wrong state for blocked 
>> VirtualThread
>> 28:  * @requires vm.continuations
>> 29:  * @modules java.base/java.lang:+open
> 
> I think `@modules` it's not needed

Thanks. Removed now.

> test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
>  line 34:
> 
>> 32: 
>> 33: import java.util.concurrent.locks.ReentrantLock;
>> 34: import java.util.concurrent.*;
> 
> unused imports

Thanks. Removed now.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220495535
PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220495047

Re: RFR: 8309509: com/sun/jdi/RedefineNestmateAttr/TestNestmateAttr.java fails with virtual test thread factory

[Alex Menkov]

On Tue, 6 Jun 2023 00:02:47 GMT, Chris Plummer  wrote:

> The test fails with the virtual test thread factory because it tries to find 
> the "main" thread in the list of threads returned by JDI, but "main" is a 
> virtual thread and will only be returned by JDI if the debug agent is 
> launched with includevirtualthreads=y. As a result the thread is not found 
> and the test asserts:
> 
> java.lang.RuntimeException: assertTrue: expected true, was false
>   at jdk.test.lib.Asserts.fail(Asserts.java:594)
>   at jdk.test.lib.Asserts.assertTrue(Asserts.java:486)
>   at jdk.test.lib.Asserts.assertTrue(Asserts.java:472)
>   at TestNestmateAttr.checkGoodTransforms(TestNestmateAttr.java:511)
>   at TestNestmateAttr.methodEntered(TestNestmateAttr.java:320)
>   at TestScaffold$EventHandler.notifyEvent(TestScaffold.java:205)
>   at TestScaffold$EventHandler.run(TestScaffold.java:279)
>   at java.base/java.lang.Thread.run(Thread.java:1583)
> 
> The fix is to always run the debug agent with includevirtualthreads=y.
> 
> Tested by running all com/sun/jdi tests locally with and without the virtual 
> test thread factory. Also ran tier1 and tier5 svc test tasks.

Marked as reviewed by amenkov (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14324#pullrequestreview-1466268017

Re: RFR: 8309420: com/sun/jdi/StepTest.java fails with virtual thread wrapper

[Alex Menkov]

On Mon, 5 Jun 2023 04:03:57 GMT, Chris Plummer  wrote:

> The test has two issues. The first is that it assume that once the VMStart 
> event has arrived and one "step into" is done, it will be in the main method 
> of the debuggee. Once there, it determines the debuggee class name by looking 
> at the classtype of topmost frame. The problems is when using virtual 
> threads, it is actually in TestScaffold.main() at this point, so the wrong 
> class name is gleaned from the frame. To fix this the test just saves away 
> the debuggee class name, which is passed to the test as the 4th argument.
> 
> The other issue is that the test assumes once it gets to the debuggee go() 
> method, there are only two frames on the stack. It's more like 16 when using 
> virtual threads. The test needs to account for this by counting the number of 
> frames when go() is entered rather than assuming it will be 2.
> 
> Tested locally with and without the wrapper and by running tier5 svc tests.

Marked as reviewed by amenkov (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14307#pullrequestreview-1466272033

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v5]

[Alex Menkov]

On Tue, 6 Jun 2023 22:54:04 GMT, Serguei Spitsyn  wrote:

>> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
>> VirtualThread blocked on a monitor when called for more than one thread. 
>> When called for a single VirtualThread it correctly returns a state that 
>> includes the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
>> The `VM_GetThreadListStackTraces::doit` should call the 
>> `get_threadOop_and_JavaThread` instead of 
>> `cv_external_thread_to_JavaThread`. But the `get_threadOop_and_JavaThread` 
>> has a check for the current thread by comparing with the 
>> JavaThread::current() which does not work for a `VM_op`. Some refactoring of 
>> the `get_threadOop_and_JavaThread` was made to make it working for a `VM_op`.
>> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
>> discovered during testing.
>> 
>> The list of changes is:
>> - minor refactoring of the function`get_threadOop_and_JavaThread`: added an 
>> overloaded version of this function with the extra parameter `JavaThread* 
>> cur_thread`. It is called instead of 
>> `JvmtiExport::cv_external_thread_to_JavaThread` from the 
>> `VM_GetThreadListStackTraces::doit`.
>> - `GetSingleStackTraceClosure::do_thread()`: The use of `jt->threadObj()` is 
>> replaced with the `JNIHandles::resolve_external_guard(_jthread)`.
>>  - added new test to provide needed coverage: 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  
>> Testing:
>>  - ran new test: 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  - TBD: tiers 1-6 (all are good)
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   review: ThreadListStackTracesTest cleanup

test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
 line 37:

> 35: import java.util.List;
> 36: import java.lang.reflect.Constructor;
> 37: import java.lang.reflect.InvocationTargetException;

I tried to comment all this lines, but something went wrong..
AFAIC the only required import is java.util.concurrent.locks.ReentrantLock, the 
rest are unused

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220526323

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v8]

[Kelvin Nilsen]

> OpenJDK Colleagues:
> 
> Please review this proposed integration of Generational mode for Shenandoah 
> GC under https://bugs.openjdk.org/browse/JDK-8307314.
> 
> Generational mode of Shenandoah is enabled by adding 
> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
> implementation automatically adjusts the sizes of old generation and young 
> generation to efficiently utilize the entire heap capacity.  Generational 
> mode of Shenandoah resembles G1 in the following regards:
> 
> 1. Old-generation marking runs concurrently during the time that multiple 
> young generation collections run to completion.
> 2. After old-generation marking completes, we perform a sequence of mixed 
> collections.  Each mixed collection combines collection of young generation 
> with evacuation of a portion of the old-generation regions identified for 
> collection based on old-generation marking information.
> 3. Unlike G1, young-generation collections and evacuations are entirely 
> concurrent, as with single-generation Shenandoah.
> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
> and survivor space within the young generation.  In practice, regions that 
> were most recently allocated tend to have large amounts of garbage and these 
> regions tend to be collected with very little effort.  Young-generation 
> objects that survive garbage collection tend to accumulate in regions that 
> hold survivor objects.  These regions tend to have smaller amounts of 
> garbage, and are less likely to be collected.  If they survive a sufficient 
> number of young-generation collections, the “survivor” regions are promoted 
> into the old generation.
> 
> We expect to refine heuristics as we gain experience with more production 
> workloads.  In the future, we plan to remove the “experimental” qualifier 
> from generational mode, at which time we expect that generational mode will 
> become the default mode for Shenandoah.
> 
> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
> HyperAlloc, and multiple AWS production workload simulators. We test on Linux 
> x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and Windows 
> x64.

Kelvin Nilsen has updated the pull request incrementally with one additional 
commit since the last revision:

  Improve efficiency of card-size alignment calculations

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14185/files
  - new: https://git.openjdk.org/jdk/pull/14185/files/cc149904..8f9e2a84

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14185&range=07
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14185&range=06-07

  Stats: 8 lines in 1 file changed: 0 ins; 4 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/14185.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14185/head:pull/14185

PR: https://git.openjdk.org/jdk/pull/14185

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v4]

[Kelvin Nilsen]

On Fri, 2 Jun 2023 17:55:56 GMT, Y. Srinivas Ramakrishna  
wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Force PLAB sizes to align on card-table size
>
> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 1285:
> 
>> 1283:   if (unalignment != 0) {
>> 1284: word_size = word_size - unalignment + 
>> CardTable::card_size_in_words();
>> 1285:   }
> 
> Probably not a big deal since this is only used when refilling a PLAB, which 
> is an infrequent operation, but `mod` is an expensive operation, in general, 
> and best to avoid in our code except in assertion checks (or even there given 
> recent experiences with debug tests timing out). Since card size is a power 
> of 2, may be we could use addition and masking instead. Something like 
> defining the following inline in the CardTable class and using it everywhere 
> where card alignment granularity is sought. There may even be a macro or 
> method defined for this already perhaps:
> 
> 
> (FOO + CardSize - 1) & ~((1 << LogCardSize) - 1)
> 
> 
> One could even store the mask to avoid the arithmetic to produce the mask 
> although it's pretty cheap.
> 
> This may turn out to be less expensive than mod, test, and branch, but as I 
> said probably not a big deal here. We should make sure we don't overuse mods 
> in our allocation paths much.

Thanks for this suggestion.  I've modified the code.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14185#discussion_r1220523992

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v4]

[Alex Menkov]

On Tue, 6 Jun 2023 22:06:14 GMT, Serguei Spitsyn  wrote:

>>> Okay, I see you point. Unfortunately, I've always referred the platform 
>>> thread with an executed FJP schedular as a carrier thread. The term 
>>> 'carrier' with this meaning is everywhere in the JVMTI code. It looks very 
>>> confusing to call a thread to be a carrier thread only during some phases 
>>> of its execution.
>> 
>> Okay, I'm just pointing out that is_passive_carrier_thread looks a bit 
>> strange here as it is testing if a JavaThread is carrying a virtual thread 
>> oop -  it's not testing if the thread is owned by the virtual thread 
>> scheduler.
>
> I'm still thinking what identifier to use instead of 
> `is_passive_carrier_thread`.
> Just `is_carrier_thread` is going to be confusing as well.
> What about `is_carrier_thread_waiting_for_virtual` or 
> `is_carrier_thread_waiting_for_virtual_to_unmount`?

`is_carrying_carrier_thread`? a bit artificial, but it's a carrier thread and 
it's carrying a virtual thread

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220541192

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v4]

[Kelvin Nilsen]

On Sat, 3 Jun 2023 15:17:37 GMT, Y. Srinivas Ramakrishna  
wrote:

>> Yes. And also from files which were changed by non-Amazon employees only, 
>> please.
>
> Thanks, Martin. Yes, we have noted that there were a few other files that 
> were inadvertently caught in a copyright header dragnet. These will be 
> reviewed and fixed in https://bugs.openjdk.org/browse/JDK-8309392 .

I'm fixing this copyright notice and others.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14185#discussion_r1220542561

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v4]

[Alex Menkov]

On Tue, 6 Jun 2023 22:41:54 GMT, Serguei Spitsyn  wrote:

>> When a virtual thread is mounted, the carrier thread should be reported as 
>> "waiting" until the virtual thread unmounts. Right now, GetThreadState 
>> reports a state based the JavaThread status when it should return 
>> JVMTI_THREAD_STATE_WAITING | JVMTI_THREAD_STATE_WAITING_INDEFINITELY.
>> The fix adds:
>>  - a special case for passive carrier threads
>>  - necessary test coverage to the existing JVMTI test: 
>> `serviceability/jvmti/vthread/ThreadStateTest`.
>> 
>> Testing:
>>- tested with the updated test: 
>> `serviceability/jvmti/vthread/ThreadStateTest`
>>- submitted mach5 tiers 1-5
>>- TBD: to submit mach5 tier 6
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   review: call get_thread_state_base only when needed

Marked as reviewed by amenkov (Reviewer).

src/hotspot/share/prims/jvmtiEnvBase.hpp line 386:

> 384: 
> 385:   // get platform thread state
> 386:   static jint get_thread_state_base(oop thread_oop, JavaThread* jt);

maybe rename it to `get_platform_thread_state`?

-

PR Review: https://git.openjdk.org/jdk/pull/14298#pullrequestreview-1466301122
PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220543652

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v4]

[Serguei Spitsyn]

On Tue, 6 Jun 2023 23:37:03 GMT, Alex Menkov  wrote:

> is_carrying_carrier_thread? a bit artificial, but it's a carrier thread and 
> it's carrying a virtual thread

I guess, your suggestion is `is_carrying_virtual_thread`. Is it right?
If so, I like this suggestion.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220546031

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v6]

[Serguei Spitsyn]

> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a 
> VirtualThread blocked on a monitor when called for more than one thread. When 
> called for a single VirtualThread it correctly returns a state that includes 
> the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
> The `VM_GetThreadListStackTraces::doit` should call the 
> `get_threadOop_and_JavaThread` instead of `cv_external_thread_to_JavaThread`. 
> But the `get_threadOop_and_JavaThread` has a check for the current thread by 
> comparing with the JavaThread::current() which does not work for a `VM_op`. 
> Some refactoring of the `get_threadOop_and_JavaThread` was made to make it 
> working for a `VM_op`.
> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was 
> discovered during testing.
> 
> The list of changes is:
> - minor refactoring of the function`get_threadOop_and_JavaThread`: added an 
> overloaded version of this function with the extra parameter `JavaThread* 
> cur_thread`. It is called instead of 
> `JvmtiExport::cv_external_thread_to_JavaThread` from the 
> `VM_GetThreadListStackTraces::doit`.
> - `GetSingleStackTraceClosure::do_thread()`: The use of `jt->threadObj()` is 
> replaced with the `JNIHandles::resolve_external_guard(_jthread)`.
>  - added new test to provide needed coverage: 
> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  
> Testing:
>  - ran new test: 
> `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>  - TBD: tiers 1-6 (all are good)

Serguei Spitsyn has updated the pull request incrementally with one additional 
commit since the last revision:

  review: remove unused imports from ThreadListStackTracesTest.java

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14326/files
  - new: https://git.openjdk.org/jdk/pull/14326/files/6b685ca3..7506b539

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14326&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14326&range=04-05

  Stats: 5 lines in 1 file changed: 0 ins; 5 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/14326.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14326/head:pull/14326

PR: https://git.openjdk.org/jdk/pull/14326

Re: RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v5]

[Serguei Spitsyn]

On Tue, 6 Jun 2023 23:20:53 GMT, Alex Menkov  wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   review: ThreadListStackTracesTest cleanup
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
>  line 37:
> 
>> 35: import java.util.List;
>> 36: import java.lang.reflect.Constructor;
>> 37: import java.lang.reflect.InvocationTargetException;
> 
> I tried to comment all this lines, but something went wrong..
> AFAIC the only required import is java.util.concurrent.locks.ReentrantLock, 
> the rest are unused

Thanks. Fixed now.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220538628

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v4]

[Serguei Spitsyn]

On Tue, 6 Jun 2023 23:39:54 GMT, Alex Menkov  wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   review: call get_thread_state_base only when needed
>
> src/hotspot/share/prims/jvmtiEnvBase.hpp line 386:
> 
>> 384: 
>> 385:   // get platform thread state
>> 386:   static jint get_thread_state_base(oop thread_oop, JavaThread* jt);
> 
> maybe rename it to `get_platform_thread_state`?

I was thinking about it. It will be inconsistent with`get_vthread_state`.
Ideally, then the `get_vthread_state` needs to be replaced with the 
`get_virtual_thread_state`.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220559583

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v9]

[Kelvin Nilsen]

> OpenJDK Colleagues:
> 
> Please review this proposed integration of Generational mode for Shenandoah 
> GC under https://bugs.openjdk.org/browse/JDK-8307314.
> 
> Generational mode of Shenandoah is enabled by adding 
> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
> implementation automatically adjusts the sizes of old generation and young 
> generation to efficiently utilize the entire heap capacity.  Generational 
> mode of Shenandoah resembles G1 in the following regards:
> 
> 1. Old-generation marking runs concurrently during the time that multiple 
> young generation collections run to completion.
> 2. After old-generation marking completes, we perform a sequence of mixed 
> collections.  Each mixed collection combines collection of young generation 
> with evacuation of a portion of the old-generation regions identified for 
> collection based on old-generation marking information.
> 3. Unlike G1, young-generation collections and evacuations are entirely 
> concurrent, as with single-generation Shenandoah.
> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
> and survivor space within the young generation.  In practice, regions that 
> were most recently allocated tend to have large amounts of garbage and these 
> regions tend to be collected with very little effort.  Young-generation 
> objects that survive garbage collection tend to accumulate in regions that 
> hold survivor objects.  These regions tend to have smaller amounts of 
> garbage, and are less likely to be collected.  If they survive a sufficient 
> number of young-generation collections, the “survivor” regions are promoted 
> into the old generation.
> 
> We expect to refine heuristics as we gain experience with more production 
> workloads.  In the future, we plan to remove the “experimental” qualifier 
> from generational mode, at which time we expect that generational mode will 
> become the default mode for Shenandoah.
> 
> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
> HyperAlloc, and multiple AWS production workload simulators. We test on Linux 
> x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and Windows 
> x64.

Kelvin Nilsen has updated the pull request incrementally with one additional 
commit since the last revision:

  Update copyright notices

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14185/files
  - new: https://git.openjdk.org/jdk/pull/14185/files/8f9e2a84..f6c073a5

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14185&range=08
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14185&range=07-08

  Stats: 7 lines in 6 files changed: 0 ins; 5 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/14185.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14185/head:pull/14185

PR: https://git.openjdk.org/jdk/pull/14185

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v4]

[Alex Menkov]

On Tue, 6 Jun 2023 23:42:24 GMT, Serguei Spitsyn  wrote:

> > is_carrying_carrier_thread? a bit artificial, but it's a carrier thread and 
> > it's carrying a virtual thread
> 
> I guess, your suggestion is `is_carrying_virtual_thread`. Is it right? If so, 
> I like this suggestion.

Up to you. I think any of this names is better than is_passive_carrier_thread.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220627250

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v9]

[Y . Srinivas Ramakrishna]

On Wed, 7 Jun 2023 00:39:52 GMT, Kelvin Nilsen  wrote:

>> OpenJDK Colleagues:
>> 
>> Please review this proposed integration of Generational mode for Shenandoah 
>> GC under https://bugs.openjdk.org/browse/JDK-8307314.
>> 
>> Generational mode of Shenandoah is enabled by adding 
>> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a 
>> command line that already specifies ` -XX:+UseShenandoahGC`.  The 
>> implementation automatically adjusts the sizes of old generation and young 
>> generation to efficiently utilize the entire heap capacity.  Generational 
>> mode of Shenandoah resembles G1 in the following regards:
>> 
>> 1. Old-generation marking runs concurrently during the time that multiple 
>> young generation collections run to completion.
>> 2. After old-generation marking completes, we perform a sequence of mixed 
>> collections.  Each mixed collection combines collection of young generation 
>> with evacuation of a portion of the old-generation regions identified for 
>> collection based on old-generation marking information.
>> 3. Unlike G1, young-generation collections and evacuations are entirely 
>> concurrent, as with single-generation Shenandoah.
>> 4. As with single-generation Shenandoah, there is no explicit notion of eden 
>> and survivor space within the young generation.  In practice, regions that 
>> were most recently allocated tend to have large amounts of garbage and these 
>> regions tend to be collected with very little effort.  Young-generation 
>> objects that survive garbage collection tend to accumulate in regions that 
>> hold survivor objects.  These regions tend to have smaller amounts of 
>> garbage, and are less likely to be collected.  If they survive a sufficient 
>> number of young-generation collections, the “survivor” regions are promoted 
>> into the old generation.
>> 
>> We expect to refine heuristics as we gain experience with more production 
>> workloads.  In the future, we plan to remove the “experimental” qualifier 
>> from generational mode, at which time we expect that generational mode will 
>> become the default mode for Shenandoah.
>> 
>> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, 
>> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, 
>> HyperAlloc, and multiple AWS production workload simulators. We test on 
>> Linux x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and 
>> Windows x64.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update copyright notices

> Hi, I have built this pr based on 
> [aa85a90](https://github.com/openjdk/jdk/commit/aa85a9073e2a71d6bf920409e739d555f9dcf302),
>  Tier1 tests failed on `gc/TestAllocHumongousFragment.java#generational` on 
> Linux/RISC-V with the following output:
> 
> ```
> #
> # A fatal error has been detected by the Java Runtime Environment:
> #
> #  Internal Error (shenandoahVerifier.cpp:1244), pid=2951116, tid=2951124
> #  Error: Verify init-mark remembered set violation; clean card should be 
> dirty
> #
> # JRE version: OpenJDK Runtime Environment (21.0) (build 
> 21-internal-adhoc.ubuntu.jdk)
> # Java VM: OpenJDK 64-Bit Server VM (21-internal-adhoc.ubuntu.jdk, mixed 
> mode, sharing, tiered, compressed oops, compressed class ptrs, shenandoah gc, 
> linux-riscv64)
> ```
> 
> Looks like Generational Shenandoah does not fully support RISC-V port, should 
> we disable this test on RISC-V port for now?

Fixed (platform disabled) by @kdnilsen in 
https://github.com/openjdk/jdk/pull/14185/commits/cc149904d76c78355fc994da171f0f21411e903f

-

PR Comment: https://git.openjdk.org/jdk/pull/14185#issuecomment-1579829038

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v4]

[Chris Plummer]

On Tue, 6 Jun 2023 22:41:54 GMT, Serguei Spitsyn  wrote:

>> When a virtual thread is mounted, the carrier thread should be reported as 
>> "waiting" until the virtual thread unmounts. Right now, GetThreadState 
>> reports a state based the JavaThread status when it should return 
>> JVMTI_THREAD_STATE_WAITING | JVMTI_THREAD_STATE_WAITING_INDEFINITELY.
>> The fix adds:
>>  - a special case for passive carrier threads
>>  - necessary test coverage to the existing JVMTI test: 
>> `serviceability/jvmti/vthread/ThreadStateTest`.
>> 
>> Testing:
>>- tested with the updated test: 
>> `serviceability/jvmti/vthread/ThreadStateTest`
>>- submitted mach5 tiers 1-5
>>- TBD: to submit mach5 tier 6
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   review: call get_thread_state_base only when needed

Marked as reviewed by cjplummer (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14298#pullrequestreview-1466591721

Re: RFR: 8307153: JVMTI GetThreadState on carrier should return STATE_WAITING [v4]

[Alan Bateman]

On Wed, 7 Jun 2023 01:05:07 GMT, Alex Menkov  wrote:

> I guess, your suggestion is `is_carrying_virtual_thread`. Is it right? If so, 
> I like this suggestion.

Good, I think will be easy to understand at the use sites.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14298#discussion_r1220891958