Re: RFR: 8287200: Test java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java timed out after JDK-8287103

2022-05-25 Thread Alan Bateman
On Wed, 25 May 2022 23:04:50 GMT, Leonid Mesnik  wrote:

>> test/jdk/java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java line 
>> 100:
>> 
>>> 98: while (thread.getState() != Thread.State.BLOCKED) {
>>> 99: Thread.sleep(10);
>>> 100: if (thread.getState() == Thread.State.TERMINATED) {
>> 
>> Thread.State.TERMINATED == thread.getState(). Does it make sense?
>
> Not sure. What is the goal?

I assume this test is catch the catch where the thread terminates without 
blocking. Using isAlive might be cleaner for people to understand.

-

PR: https://git.openjdk.java.net/jdk/pull/8874


Re: RFR: 8287135: Calculation of jmm_GetMemoryUsage is wrong

2022-05-25 Thread Ioi Lam
On Mon, 23 May 2022 07:28:41 GMT, Yi Yang  wrote:

> It seems that calculation of 
> MemoryMXBean.getNonHeapMemoryUsage(jmm_GetMemoryUsage) is wrong.
> 
> Currently, 
> `NonHeapUsage=CodeCache+Metaspace(ClassTypeSpace+NonClassTypeSpace)+CompressedClassSpace(ClassTypeSpace)`
> 
> ==> CodeHeap 'non-nmethods' 1532544 (Used)
> ==> CodeHeap 'profiled nmethods' 0
> ==> CodeHeap 'non-profiled nmethods' 13952
> ==> Metaspace 506696
> ==> Compressed Class Space 43312
> init = 7667712(7488K) used = 2096504(2047K) committed = 8454144(8256K) max = 
> -1(-1K)
> 
> In this way, getNonHeapMemoryUsage is larger than it ought to be, it should 
> be `NonHeapUsage = CodeCache + Metaspace`.

src/hotspot/share/services/management.cpp line 753:

> 751: for (int i = 0; i < MemoryService::num_memory_pools(); i++) {
> 752:   MemoryPool* pool = MemoryService::get_memory_pool(i);
> 753:   if (pool->is_codeheap() || pool->is_metaspace()) {

Our only special case is that all the memory reported by 
`CompressedKlassSpacePool` are already reported by `MetaspacePool`, so 
shouldn't we do this:


if (pool->is_non_heap() && !pool->is_compressed_klass_space()) {
  // skip CompressedKlassSpacePool since its memory is already reported by
  // MetaspacePool

-

PR: https://git.openjdk.java.net/jdk/pull/8831


Re: RFR: 8286983: Add mention of "Preview Feature" for new loom related jdb and debug agent options [v2]

2022-05-25 Thread Chris Plummer
On Thu, 26 May 2022 02:35:08 GMT, David Holmes  wrote:

>>> If virtual threads become permanent then the usage message would minimally 
>>> be updated to drop the sentence that virtual threads are a preview feature.
>> 
>> But that's precisely the commitment to this flag that I want to avoid.  I 
>> want something that is the equivalent of `EXPERIMENTAL` for `-XX` flags.
>
> Again I have to say I agree with Chris. The flag itself is a "preview" flag 
> and relates to a "preview feature". If the feature goes obviously the flag 
> does too, but the flag could go even if the feature stays.

Please see the discussion with Alex in 
[JDK-8286983](https://bugs.openjdk.java.net/browse/JDK-8286983)

-

PR: https://git.openjdk.java.net/jdk/pull/8780


Re: RFR: 8286983: Add mention of "Preview Feature" for new loom related jdb and debug agent options [v2]

2022-05-25 Thread David Holmes
On Wed, 25 May 2022 16:11:52 GMT, Chris Plummer  wrote:

>> If virtual threads become permanent then the usage message would minimally 
>> be updated to drop the sentence that virtual threads are a preview feature. 
>> At that point the debugger APIs may have been built out further and it might 
>> be that there is no need for the -trackvthreads option but that's too far 
>> out to know at this point.
>> 
>> If virtual threads were to be removed then the -trackvthreads command line 
>> option would be removed too.
>
>> If virtual threads become permanent then the usage message would minimally 
>> be updated to drop the sentence that virtual threads are a preview feature.
> 
> But that's precisely the commitment to this flag that I want to avoid.  I 
> want something that is the equivalent of `EXPERIMENTAL` for `-XX` flags.

Again I have to say I agree with Chris. The flag itself is a "preview" flag and 
relates to a "preview feature". If the feature goes obviously the flag does 
too, but the flag could go even if the feature stays.

-

PR: https://git.openjdk.java.net/jdk/pull/8780


Re: RFR: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

2022-05-25 Thread David Holmes
On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn  wrote:

> A part of this issue was contributed with the following changeset:
> 
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +
> 
> 8249004: Reduce ThreadsListHandle overhead in relation to direct 
> handshakes
> Reviewed-by: coleenp, sspitsyn, dholmes, rehn
> 
> The following change in `src/hotspot/share/runtime/thread.cpp` added new 
> assert:
> 
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on 
> ThreadsList, no suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
>  + "missing ThreadsListHandle in calling context.");
>   return this->handshake_state()->suspend();
> }
> 
> This new assert misses a check for target thread as being current 
> `JavaThread`.
> 
> Also, the JVMTI SuspendThread is protected with TLH:
> 
> JvmtiEnv::SuspendThread(jthread thread) {
>   JavaThread* current = JavaThread::current();
>   ThreadsListHandle tlh(current);  <= TLS defined here!!!
> 
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true); 
> 
> 
> However, it is possible that a new carrier thread (and an associated 
> `JavaThread`) can be created after the `TLH` was set and the target virtual 
> thread can be mounted on new carrier thread. Then target virtual thread will 
> be associated with newly created `JavaThread` which is unprotected by the TLH.
> The right way to be protected from this situation it is to prevent mount 
> state transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as 
> in the change below:
> 
> 
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, 
> jthread** threads_ptr) {
>  jvmtiError
>  JvmtiEnv::SuspendThread(jthread thread) {
>JavaThread* current = JavaThread::current();
> -  ThreadsListHandle tlh(current);
> 
>jvmtiError err;
>JavaThread* java_thread = NULL;
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true);
> +ThreadsListHandle tlh(current);
> 
>  err = get_threadOop_and_JavaThread(tlh.list(), thread, _thread, 
> _oop);
>  if (err != JVMTI_ERROR_NONE) {
> 
> 
> 
> This problem exist in all JVMTI Suspend functions:
>  `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

I was expecting there to be an existing non-vthread-related test for 
self-suspension. That seems to be a hole in our test coverage.

-

PR: https://git.openjdk.java.net/jdk/pull/8878


Re: How can Java developers best understand their impact on the start-up of the JVM?

2022-05-25 Thread David Holmes

cc'ing hotspot-jfr-dev

On 26/05/2022 9:45 am, Mat Carter wrote:

The Microsoft Java Engineering Group (JEG) have been looking at how Java 
developers can best understand the impact of their decisions on start-up time. 
Specifically, how developers can identify and mitigate the dominant activities 
that occur during start-up, through to their app / service reaching a steady 
state (warmed up).

Upon investigation into the available tooling (i.e., jstat, jcmd, jmc, 
VisualVM, MXBeans), it seems that Performance Counters (PCs) and JDK Flight 
Recorder events (JFREs) are the main source of the available startup metrics 
that get surfaced.

Whilst the tooling does provide insight to specific areas (GC, JIT Compilation 
etc.), there does not seem to be one view to help a developer identify the 
dominant activities that are impacting start-up.

Q: Has the above been discussed before and are there any existing plans to 
address the outcomes that I might have missed?

Looking at the specific PCs and JFREs there are a few “missing” metrics, for 
example a timestamp at the very beginning entry point of the JVM. Also, it 
appears that the JFREs represent a subset of the PCs - does this indicate that 
either some PCs should be removed (or only enabled for JVM development) or that 
there remain JFREs to be added?

Q: Is there a current position on whether PCs and JFREs will continue to 
co-exist (the former supporting JMX MXBeans, et al.), or is there a plan to 
phase out PCs in favor of JFREs?

Q: Assuming PCs should continue to exist, do folks see the value in documenting 
them and making them readily accessible to the running application? Especially 
in the deployment scenario where running a second application or tool (to 
sample the PCs remotely) is not feasible. FYI - during our initial 
investigations we wrote some scripts to find all possible PCs. We did this 
because jcmd only lists instantiated PCs and the PC names are procedurally 
generated, making them non-trivial to search for.

Q: Do folks see value in documenting the JFREs, especially since the 
application can access the JFR Event stream? (There is non-official 
documentation available [1])

Q: While Project Leyden concerns itself primarily with improving start-up 
times, are there any other known activities targeting profiling of Java 
applications (esp. start-up)?

Answers and discussion of the above is most welcome so that our group moves 
forward in helping our customers, whilst staying aligned with and contributing 
to the OpenJDK community!

Thanks in advance
Mat Carter

[1] https://bestsolution-at.github.io/jfr-doc/index.html


Re: RFR: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

2022-05-25 Thread Serguei Spitsyn
On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn  wrote:

> A part of this issue was contributed with the following changeset:
> 
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +
> 
> 8249004: Reduce ThreadsListHandle overhead in relation to direct 
> handshakes
> Reviewed-by: coleenp, sspitsyn, dholmes, rehn
> 
> The following change in `src/hotspot/share/runtime/thread.cpp` added new 
> assert:
> 
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on 
> ThreadsList, no suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
>  + "missing ThreadsListHandle in calling context.");
>   return this->handshake_state()->suspend();
> }
> 
> This new assert misses a check for target thread as being current 
> `JavaThread`.
> 
> Also, the JVMTI SuspendThread is protected with TLH:
> 
> JvmtiEnv::SuspendThread(jthread thread) {
>   JavaThread* current = JavaThread::current();
>   ThreadsListHandle tlh(current);  <= TLS defined here!!!
> 
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true); 
> 
> 
> However, it is possible that a new carrier thread (and an associated 
> `JavaThread`) can be created after the `TLH` was set and the target virtual 
> thread can be mounted on new carrier thread. Then target virtual thread will 
> be associated with newly created `JavaThread` which is unprotected by the TLH.
> The right way to be protected from this situation it is to prevent mount 
> state transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as 
> in the change below:
> 
> 
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, 
> jthread** threads_ptr) {
>  jvmtiError
>  JvmtiEnv::SuspendThread(jthread thread) {
>JavaThread* current = JavaThread::current();
> -  ThreadsListHandle tlh(current);
> 
>jvmtiError err;
>JavaThread* java_thread = NULL;
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true);
> +ThreadsListHandle tlh(current);
> 
>  err = get_threadOop_and_JavaThread(tlh.list(), thread, _thread, 
> _oop);
>  if (err != JVMTI_ERROR_NONE) {
> 
> 
> 
> This problem exist in all JVMTI Suspend functions:
>  `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

Thank you for review, Alex!

-

PR: https://git.openjdk.java.net/jdk/pull/8878


Integrated: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

2022-05-25 Thread Serguei Spitsyn
On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn  wrote:

> A part of this issue was contributed with the following changeset:
> 
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +
> 
> 8249004: Reduce ThreadsListHandle overhead in relation to direct 
> handshakes
> Reviewed-by: coleenp, sspitsyn, dholmes, rehn
> 
> The following change in `src/hotspot/share/runtime/thread.cpp` added new 
> assert:
> 
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on 
> ThreadsList, no suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
>  + "missing ThreadsListHandle in calling context.");
>   return this->handshake_state()->suspend();
> }
> 
> This new assert misses a check for target thread as being current 
> `JavaThread`.
> 
> Also, the JVMTI SuspendThread is protected with TLH:
> 
> JvmtiEnv::SuspendThread(jthread thread) {
>   JavaThread* current = JavaThread::current();
>   ThreadsListHandle tlh(current);  <= TLS defined here!!!
> 
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true); 
> 
> 
> However, it is possible that a new carrier thread (and an associated 
> `JavaThread`) can be created after the `TLH` was set and the target virtual 
> thread can be mounted on new carrier thread. Then target virtual thread will 
> be associated with newly created `JavaThread` which is unprotected by the TLH.
> The right way to be protected from this situation it is to prevent mount 
> state transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as 
> in the change below:
> 
> 
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, 
> jthread** threads_ptr) {
>  jvmtiError
>  JvmtiEnv::SuspendThread(jthread thread) {
>JavaThread* current = JavaThread::current();
> -  ThreadsListHandle tlh(current);
> 
>jvmtiError err;
>JavaThread* java_thread = NULL;
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true);
> +ThreadsListHandle tlh(current);
> 
>  err = get_threadOop_and_JavaThread(tlh.list(), thread, _thread, 
> _oop);
>  if (err != JVMTI_ERROR_NONE) {
> 
> 
> 
> This problem exist in all JVMTI Suspend functions:
>  `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

This pull request has now been integrated.

Changeset: 94811c0d
Author:Serguei Spitsyn 
URL:   
https://git.openjdk.java.net/jdk/commit/94811c0dc7c20b0e7cb2649fe8da5061ce3d6246
Stats: 17 lines in 2 files changed: 8 ins; 7 del; 2 mod

8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing 
ThreadsListHandle in calling context

Reviewed-by: dholmes, pchilanomate, amenkov

-

PR: https://git.openjdk.java.net/jdk/pull/8878


How can Java developers best understand their impact on the start-up of the JVM?

2022-05-25 Thread Mat Carter
The Microsoft Java Engineering Group (JEG) have been looking at how Java 
developers can best understand the impact of their decisions on start-up time. 
Specifically, how developers can identify and mitigate the dominant activities 
that occur during start-up, through to their app / service reaching a steady 
state (warmed up). 

Upon investigation into the available tooling (i.e., jstat, jcmd, jmc, 
VisualVM, MXBeans), it seems that Performance Counters (PCs) and JDK Flight 
Recorder events (JFREs) are the main source of the available startup metrics 
that get surfaced. 

Whilst the tooling does provide insight to specific areas (GC, JIT Compilation 
etc.), there does not seem to be one view to help a developer identify the 
dominant activities that are impacting start-up. 

Q: Has the above been discussed before and are there any existing plans to 
address the outcomes that I might have missed? 

Looking at the specific PCs and JFREs there are a few “missing” metrics, for 
example a timestamp at the very beginning entry point of the JVM. Also, it 
appears that the JFREs represent a subset of the PCs - does this indicate that 
either some PCs should be removed (or only enabled for JVM development) or that 
there remain JFREs to be added? 

Q: Is there a current position on whether PCs and JFREs will continue to 
co-exist (the former supporting JMX MXBeans, et al.), or is there a plan to 
phase out PCs in favor of JFREs? 

Q: Assuming PCs should continue to exist, do folks see the value in documenting 
them and making them readily accessible to the running application? Especially 
in the deployment scenario where running a second application or tool (to 
sample the PCs remotely) is not feasible. FYI - during our initial 
investigations we wrote some scripts to find all possible PCs. We did this 
because jcmd only lists instantiated PCs and the PC names are procedurally 
generated, making them non-trivial to search for. 

Q: Do folks see value in documenting the JFREs, especially since the 
application can access the JFR Event stream? (There is non-official 
documentation available [1]) 

Q: While Project Leyden concerns itself primarily with improving start-up 
times, are there any other known activities targeting profiling of Java 
applications (esp. start-up)? 

Answers and discussion of the above is most welcome so that our group moves 
forward in helping our customers, whilst staying aligned with and contributing 
to the OpenJDK community! 

Thanks in advance 
Mat Carter 

[1] https://bestsolution-at.github.io/jfr-doc/index.html 

Re: RFR: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

2022-05-25 Thread Alex Menkov
On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn  wrote:

> A part of this issue was contributed with the following changeset:
> 
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +
> 
> 8249004: Reduce ThreadsListHandle overhead in relation to direct 
> handshakes
> Reviewed-by: coleenp, sspitsyn, dholmes, rehn
> 
> The following change in `src/hotspot/share/runtime/thread.cpp` added new 
> assert:
> 
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on 
> ThreadsList, no suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
>  + "missing ThreadsListHandle in calling context.");
>   return this->handshake_state()->suspend();
> }
> 
> This new assert misses a check for target thread as being current 
> `JavaThread`.
> 
> Also, the JVMTI SuspendThread is protected with TLH:
> 
> JvmtiEnv::SuspendThread(jthread thread) {
>   JavaThread* current = JavaThread::current();
>   ThreadsListHandle tlh(current);  <= TLS defined here!!!
> 
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true); 
> 
> 
> However, it is possible that a new carrier thread (and an associated 
> `JavaThread`) can be created after the `TLH` was set and the target virtual 
> thread can be mounted on new carrier thread. Then target virtual thread will 
> be associated with newly created `JavaThread` which is unprotected by the TLH.
> The right way to be protected from this situation it is to prevent mount 
> state transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as 
> in the change below:
> 
> 
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, 
> jthread** threads_ptr) {
>  jvmtiError
>  JvmtiEnv::SuspendThread(jthread thread) {
>JavaThread* current = JavaThread::current();
> -  ThreadsListHandle tlh(current);
> 
>jvmtiError err;
>JavaThread* java_thread = NULL;
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true);
> +ThreadsListHandle tlh(current);
> 
>  err = get_threadOop_and_JavaThread(tlh.list(), thread, _thread, 
> _oop);
>  if (err != JVMTI_ERROR_NONE) {
> 
> 
> 
> This problem exist in all JVMTI Suspend functions:
>  `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

Marked as reviewed by amenkov (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/8878


Re: RFR: 8287200: Test java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java timed out after JDK-8287103

2022-05-25 Thread Leonid Mesnik
On Wed, 25 May 2022 21:18:24 GMT, master-code-java  
wrote:

>> Need to use proper synchronization.
>> 
>> The CyclicBarriers might move the thread to WAITING state but not BLOCKED. 
>> So it should not confuse existing checks.
>
> test/jdk/java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java line 
> 98:
> 
>> 96: 
>> 97: private static void awaitBlocked(Thread thread) throws 
>> InterruptedException {
>> 98: while (thread.getState() != Thread.State.BLOCKED) {
> 
> Thread.State.BLOCKED == thread.getState(). Does it make sense?

Do you mean '==' or '!='? Both don't make sense to me, honestly. Could you 
please elaborate?

> test/jdk/java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java line 
> 100:
> 
>> 98: while (thread.getState() != Thread.State.BLOCKED) {
>> 99: Thread.sleep(10);
>> 100: if (thread.getState() == Thread.State.TERMINATED) {
> 
> Thread.State.TERMINATED == thread.getState(). Does it make sense?

Not sure. What is the goal?

-

PR: https://git.openjdk.java.net/jdk/pull/8874


Re: RFR: 8287200: Test java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java timed out after JDK-8287103

2022-05-25 Thread master-code-java
On Tue, 24 May 2022 19:52:57 GMT, Leonid Mesnik  wrote:

> Need to use proper synchronization.
> 
> The CyclicBarriers might move the thread to WAITING state but not BLOCKED. So 
> it should not confuse existing checks.

test/jdk/java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java line 98:

> 96: 
> 97: private static void awaitBlocked(Thread thread) throws 
> InterruptedException {
> 98: while (thread.getState() != Thread.State.BLOCKED) {

Thread.State.BLOCKED == thread.getState(). Does it make sense?

test/jdk/java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java line 100:

> 98: while (thread.getState() != Thread.State.BLOCKED) {
> 99: Thread.sleep(10);
> 100: if (thread.getState() == Thread.State.TERMINATED) {

Thread.State.TERMINATED == thread.getState(). Does it make sense?

-

PR: https://git.openjdk.java.net/jdk/pull/8874


Re: RFR: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

2022-05-25 Thread Serguei Spitsyn
On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn  wrote:

> A part of this issue was contributed with the following changeset:
> 
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +
> 
> 8249004: Reduce ThreadsListHandle overhead in relation to direct 
> handshakes
> Reviewed-by: coleenp, sspitsyn, dholmes, rehn
> 
> The following change in `src/hotspot/share/runtime/thread.cpp` added new 
> assert:
> 
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on 
> ThreadsList, no suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
>  + "missing ThreadsListHandle in calling context.");
>   return this->handshake_state()->suspend();
> }
> 
> This new assert misses a check for target thread as being current 
> `JavaThread`.
> 
> Also, the JVMTI SuspendThread is protected with TLH:
> 
> JvmtiEnv::SuspendThread(jthread thread) {
>   JavaThread* current = JavaThread::current();
>   ThreadsListHandle tlh(current);  <= TLS defined here!!!
> 
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true); 
> 
> 
> However, it is possible that a new carrier thread (and an associated 
> `JavaThread`) can be created after the `TLH` was set and the target virtual 
> thread can be mounted on new carrier thread. Then target virtual thread will 
> be associated with newly created `JavaThread` which is unprotected by the TLH.
> The right way to be protected from this situation it is to prevent mount 
> state transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as 
> in the change below:
> 
> 
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, 
> jthread** threads_ptr) {
>  jvmtiError
>  JvmtiEnv::SuspendThread(jthread thread) {
>JavaThread* current = JavaThread::current();
> -  ThreadsListHandle tlh(current);
> 
>jvmtiError err;
>JavaThread* java_thread = NULL;
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true);
> +ThreadsListHandle tlh(current);
> 
>  err = get_threadOop_and_JavaThread(tlh.list(), thread, _thread, 
> _oop);
>  if (err != JVMTI_ERROR_NONE) {
> 
> 
> 
> This problem exist in all JVMTI Suspend functions:
>  `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

Thank you for quick review, Patricio!

-

PR: https://git.openjdk.java.net/jdk/pull/8878


Re: RFR: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

2022-05-25 Thread Patricio Chilano Mateo
On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn  wrote:

> A part of this issue was contributed with the following changeset:
> 
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +
> 
> 8249004: Reduce ThreadsListHandle overhead in relation to direct 
> handshakes
> Reviewed-by: coleenp, sspitsyn, dholmes, rehn
> 
> The following change in `src/hotspot/share/runtime/thread.cpp` added new 
> assert:
> 
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on 
> ThreadsList, no suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
>  + "missing ThreadsListHandle in calling context.");
>   return this->handshake_state()->suspend();
> }
> 
> This new assert misses a check for target thread as being current 
> `JavaThread`.
> 
> Also, the JVMTI SuspendThread is protected with TLH:
> 
> JvmtiEnv::SuspendThread(jthread thread) {
>   JavaThread* current = JavaThread::current();
>   ThreadsListHandle tlh(current);  <= TLS defined here!!!
> 
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true); 
> 
> 
> However, it is possible that a new carrier thread (and an associated 
> `JavaThread`) can be created after the `TLH` was set and the target virtual 
> thread can be mounted on new carrier thread. Then target virtual thread will 
> be associated with newly created `JavaThread` which is unprotected by the TLH.
> The right way to be protected from this situation it is to prevent mount 
> state transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as 
> in the change below:
> 
> 
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, 
> jthread** threads_ptr) {
>  jvmtiError
>  JvmtiEnv::SuspendThread(jthread thread) {
>JavaThread* current = JavaThread::current();
> -  ThreadsListHandle tlh(current);
> 
>jvmtiError err;
>JavaThread* java_thread = NULL;
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true);
> +ThreadsListHandle tlh(current);
> 
>  err = get_threadOop_and_JavaThread(tlh.list(), thread, _thread, 
> _oop);
>  if (err != JVMTI_ERROR_NONE) {
> 
> 
> 
> This problem exist in all JVMTI Suspend functions:
>  `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

LGTM.

-

Marked as reviewed by pchilanomate (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8878


Re: RFR: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

2022-05-25 Thread Serguei Spitsyn
On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn  wrote:

> A part of this issue was contributed with the following changeset:
> 
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +
> 
> 8249004: Reduce ThreadsListHandle overhead in relation to direct 
> handshakes
> Reviewed-by: coleenp, sspitsyn, dholmes, rehn
> 
> The following change in `src/hotspot/share/runtime/thread.cpp` added new 
> assert:
> 
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on 
> ThreadsList, no suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
>  + "missing ThreadsListHandle in calling context.");
>   return this->handshake_state()->suspend();
> }
> 
> This new assert misses a check for target thread as being current 
> `JavaThread`.
> 
> Also, the JVMTI SuspendThread is protected with TLH:
> 
> JvmtiEnv::SuspendThread(jthread thread) {
>   JavaThread* current = JavaThread::current();
>   ThreadsListHandle tlh(current);  <= TLS defined here!!!
> 
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true); 
> 
> 
> However, it is possible that a new carrier thread (and an associated 
> `JavaThread`) can be created after the `TLH` was set and the target virtual 
> thread can be mounted on new carrier thread. Then target virtual thread will 
> be associated with newly created `JavaThread` which is unprotected by the TLH.
> The right way to be protected from this situation it is to prevent mount 
> state transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as 
> in the change below:
> 
> 
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, 
> jthread** threads_ptr) {
>  jvmtiError
>  JvmtiEnv::SuspendThread(jthread thread) {
>JavaThread* current = JavaThread::current();
> -  ThreadsListHandle tlh(current);
> 
>jvmtiError err;
>JavaThread* java_thread = NULL;
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true);
> +ThreadsListHandle tlh(current);
> 
>  err = get_threadOop_and_JavaThread(tlh.list(), thread, _thread, 
> _oop);
>  if (err != JVMTI_ERROR_NONE) {
> 
> 
> 
> This problem exist in all JVMTI Suspend functions:
>  `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

The test `serviceability/jvmti/vthread/SelfSuspendDisablerTest` is triggering 
this guarantee.
But it needs this update in the `SuspendThread` in order to do it.


#  Internal Error 
(/scratch/sspitsyn/loom5/open/src/hotspot/share/runtime/thread.cpp:1781), 
pid=31157, tid=31182
#  guarantee(Thread::is_JavaThread_protected_by_TLH( this)) failed: missing 
ThreadsListHandle in calling context.
#
# JRE version: Java(TM) SE Runtime Environment (19.0) (fastdebug build 
19-internal-2022-05-19-0744187.sspitsyn...)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug 
19-internal-2022-05-19-0744187.sspitsyn..., mixed mode, sharing, tiered, 
compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# V  [libjvm.so+0x1a41724]  JavaThread::java_suspend()+0xa4
. . . . .
---  S U M M A R Y 

Command Line: -Dtest.vm.opts=-XX:+CreateCoredumpOnCrash --enable-preview 
-Djava.util.concurrent.ForkJoinPool.common.parallelism=1 
-XX:-VerifyContinuations -XX:+CheckUnhandledOops -Xcheck:jni 
-Dmain.wrapper=Virtual -Dtest.tool.vm.opts=-J-XX:+CreateCoredumpOnCrash 
-J--enable-preview -J-Djava.util.concurrent.ForkJoinPool.common.parallelism=1 
-J-XX:-VerifyContinuations -J-XX:+CheckUnhandledOops -J-Xcheck:jni 
-J-Dmain.wrapper=Virtual -Dtest.compiler.opts= -Dtest.java.opts= 
-Dtest.jdk=/scratch/sspitsyn/loom5/build/linux-x64-debug/images/jdk 
-Dcompile.jdk=/scratch/sspitsyn/loom5/build/linux-x64-debug/images/jdk 
-Dtest.timeout.factor=1.0 
-Dtest.nativepath=/scratch/sspitsyn/loom5/build/linux-x64-debug/images/test/hotspot/jtreg/native
 -Dtest.root=/scratch/sspitsyn/loom5/open/test/hotspot/jtreg 
-Dtest.name=serviceability/jvmti/vthread/SelfSuspendDisablerTest/SelfSuspendDisablerTest.java
 
-Dtest.file=/scratch/sspitsyn/loom5/open/test/hotspot/jtreg/serviceability/jvmti/vthread/SelfSuspendDisablerT
 est/SelfSuspendDisablerTest.java 
-Dtest.src=/scratch/sspitsyn/loom5/open/test/hotspot/jtreg/serviceability/jvmti/vthread/SelfSuspendDisablerTest
 
-Dtest.src.path=/scratch/sspitsyn/loom5/open/test/hotspot/jtreg/serviceability/jvmti/vthread/SelfSuspendDisablerTest:/scratch/sspitsyn/loom5/open/test/lib
 
-Dtest.classes=/scratch/sspitsyn/tst/loom5/JTwork/classes/serviceability/jvmti/vthread/SelfSuspendDisablerTest/SelfSuspendDisablerTest.d
 

Re: RFR: 8286983: Add mention of "Preview Feature" for new loom related jdb and debug agent options [v2]

2022-05-25 Thread Chris Plummer
On Wed, 25 May 2022 06:23:48 GMT, Alan Bateman  wrote:

> If virtual threads become permanent then the usage message would minimally be 
> updated to drop the sentence that virtual threads are a preview feature.

But that's precisely the commitment to this flag that I want to avoid.  I want 
something that is the equivalent of `EXPERIMENTAL` for `-XX` flags.

-

PR: https://git.openjdk.java.net/jdk/pull/8780


Re: RFR: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

2022-05-25 Thread David Holmes
On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn  wrote:

> A part of this issue was contributed with the following changeset:
> 
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +
> 
> 8249004: Reduce ThreadsListHandle overhead in relation to direct 
> handshakes
> Reviewed-by: coleenp, sspitsyn, dholmes, rehn
> 
> The following change in `src/hotspot/share/runtime/thread.cpp` added new 
> assert:
> 
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on 
> ThreadsList, no suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
>  + "missing ThreadsListHandle in calling context.");
>   return this->handshake_state()->suspend();
> }
> 
> This new assert misses a check for target thread as being current 
> `JavaThread`.
> 
> Also, the JVMTI SuspendThread is protected with TLH:
> 
> JvmtiEnv::SuspendThread(jthread thread) {
>   JavaThread* current = JavaThread::current();
>   ThreadsListHandle tlh(current);  <= TLS defined here!!!
> 
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true); 
> 
> 
> However, it is possible that a new carrier thread (and an associated 
> `JavaThread`) can be created after the `TLH` was set and the target virtual 
> thread can be mounted on new carrier thread. Then target virtual thread will 
> be associated with newly created `JavaThread` which is unprotected by the TLH.
> The right way to be protected from this situation it is to prevent mount 
> state transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as 
> in the change below:
> 
> 
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, 
> jthread** threads_ptr) {
>  jvmtiError
>  JvmtiEnv::SuspendThread(jthread thread) {
>JavaThread* current = JavaThread::current();
> -  ThreadsListHandle tlh(current);
> 
>jvmtiError err;
>JavaThread* java_thread = NULL;
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true);
> +ThreadsListHandle tlh(current);
> 
>  err = get_threadOop_and_JavaThread(tlh.list(), thread, _thread, 
> _oop);
>  if (err != JVMTI_ERROR_NONE) {
> 
> 
> 
> This problem exist in all JVMTI Suspend functions:
>  `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

One query though - don't we have a self-suspension test that would have 
triggered the guarantee failure? If we don't then we should (obviously the 
resume will be a bit racy).

-

PR: https://git.openjdk.java.net/jdk/pull/8878


Re: RFR: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

2022-05-25 Thread David Holmes
On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn  wrote:

> A part of this issue was contributed with the following changeset:
> 
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +
> 
> 8249004: Reduce ThreadsListHandle overhead in relation to direct 
> handshakes
> Reviewed-by: coleenp, sspitsyn, dholmes, rehn
> 
> The following change in `src/hotspot/share/runtime/thread.cpp` added new 
> assert:
> 
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on 
> ThreadsList, no suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
>  + "missing ThreadsListHandle in calling context.");
>   return this->handshake_state()->suspend();
> }
> 
> This new assert misses a check for target thread as being current 
> `JavaThread`.
> 
> Also, the JVMTI SuspendThread is protected with TLH:
> 
> JvmtiEnv::SuspendThread(jthread thread) {
>   JavaThread* current = JavaThread::current();
>   ThreadsListHandle tlh(current);  <= TLS defined here!!!
> 
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true); 
> 
> 
> However, it is possible that a new carrier thread (and an associated 
> `JavaThread`) can be created after the `TLH` was set and the target virtual 
> thread can be mounted on new carrier thread. Then target virtual thread will 
> be associated with newly created `JavaThread` which is unprotected by the TLH.
> The right way to be protected from this situation it is to prevent mount 
> state transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as 
> in the change below:
> 
> 
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, 
> jthread** threads_ptr) {
>  jvmtiError
>  JvmtiEnv::SuspendThread(jthread thread) {
>JavaThread* current = JavaThread::current();
> -  ThreadsListHandle tlh(current);
> 
>jvmtiError err;
>JavaThread* java_thread = NULL;
>oop thread_oop = NULL;
>{
>  JvmtiVTMSTransitionDisabler disabler(true);
> +ThreadsListHandle tlh(current);
> 
>  err = get_threadOop_and_JavaThread(tlh.list(), thread, _thread, 
> _oop);
>  if (err != JVMTI_ERROR_NONE) {
> 
> 
> 
> This problem exist in all JVMTI Suspend functions:
>  `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

These changes look good to me - as per our discussion in JBS issue.

Thanks,
David

-

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8878


Integrated: 8287254: Clean up Xcode sysroot logic

2022-05-25 Thread Magnus Ihse Bursie
On Tue, 24 May 2022 17:09:10 GMT, Magnus Ihse Bursie  wrote:

> The logic in BASIC_SETUP_DEVKIT for setting a correct sysroot for Xcode is 
> hard to follow. This should be straightened out. We also expose variables 
> that are no longer used. So there's a bit of related cleanup. 
> 
> The new code is more or less functionally equivalent to the old. There were 
> some corner cases which the old code did not handle well; this has now been 
> improved. I've also added yet another method of trying to get the SDK root 
> before falling back to the system library, by using `xcrun --show-sdk-path`.
> 
> In an ideal world, the sysroot flags to `mig` should be set in configure, 
> e.g. as `MIG_FLAGS` or `MIG_SYSROOT_FLAGS`. I can't be bothered to do that 
> for a single call to `mig`, though.
> 
> As always, changes like this that depend on the environment is tricky to 
> test. I've tried running it on a couple of different macs, with and without a 
> devkit.

This pull request has now been integrated.

Changeset: d889319a
Author:Magnus Ihse Bursie 
URL:   
https://git.openjdk.java.net/jdk/commit/d889319a86101e944aefd4ad7f300505abbe5b30
Stats: 200 lines in 4 files changed: 91 ins; 98 del; 11 mod

8287254: Clean up Xcode sysroot logic

Reviewed-by: erikj

-

PR: https://git.openjdk.java.net/jdk/pull/8872


RFR: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

2022-05-25 Thread Serguei Spitsyn
A part of this issue was contributed with the following changeset:

commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@openjdk.org)>
Date: Mon Nov 8 14:45:04 2021 +

8249004: Reduce ThreadsListHandle overhead in relation to direct handshakes
Reviewed-by: coleenp, sspitsyn, dholmes, rehn

The following change in `src/hotspot/share/runtime/thread.cpp` added new assert:

bool JavaThread::java_suspend() {
- ThreadsListHandle tlh;
- if (!tlh.includes(this)) {
- log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on ThreadsList, 
no suspension", p2i(this));
- return false;
- }
+ guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
 + "missing ThreadsListHandle in calling context.");
  return this->handshake_state()->suspend();
}

This new assert misses a check for target thread as being current `JavaThread`.

Also, the JVMTI SuspendThread is protected with TLH:

JvmtiEnv::SuspendThread(jthread thread) {
  JavaThread* current = JavaThread::current();
  ThreadsListHandle tlh(current);  <= TLS defined here!!!

   oop thread_oop = NULL;
   {
 JvmtiVTMSTransitionDisabler disabler(true); 


However, it is possible that a new carrier thread (and an associated 
`JavaThread`) can be created after the `TLH` was set and the target virtual 
thread can be mounted on new carrier thread. Then target virtual thread will be 
associated with newly created `JavaThread` which is unprotected by the TLH.
The right way to be protected from this situation it is to prevent mount state 
transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as in the 
change below:


@@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, 
jthread** threads_ptr) {
 jvmtiError
 JvmtiEnv::SuspendThread(jthread thread) {
   JavaThread* current = JavaThread::current();
-  ThreadsListHandle tlh(current);

   jvmtiError err;
   JavaThread* java_thread = NULL;
   oop thread_oop = NULL;
   {
 JvmtiVTMSTransitionDisabler disabler(true);
+ThreadsListHandle tlh(current);

 err = get_threadOop_and_JavaThread(tlh.list(), thread, _thread, 
_oop);
 if (err != JVMTI_ERROR_NONE) {



This problem exist in all JVMTI Suspend functions:
 `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

-

Commit messages:
 - 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing 
ThreadsListHandle in calling context

Changes: https://git.openjdk.java.net/jdk/pull/8878/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8878=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8286960
  Stats: 17 lines in 2 files changed: 8 ins; 7 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8878.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8878/head:pull/8878

PR: https://git.openjdk.java.net/jdk/pull/8878


Re: RFR: 8286983: Add mention of "Preview Feature" for new loom related jdb and debug agent options [v2]

2022-05-25 Thread Alan Bateman
On Tue, 24 May 2022 19:36:02 GMT, Chris Plummer  wrote:

>> I don't think it is correct to say that this command line option is a 
>> preview feature. Maybe it can be clarified on a second line:
>> 
>> 
>> -trackvthreadstrack virtual threads as they are created
>>  Virtual threads are a preview feature of the 
>> Java platform.
>
> Does that imply that `-trackvthreads` can be changed or removed?

If virtual threads become permanent then the usage message would minimally be 
updated to drop the sentence that virtual threads are a preview feature. At 
that point the debugger APIs may have been built out further and it might be 
that there is no need for the -trackvthreads option but that's too far out to 
know at this point.

If virtual threads were to be removed then the -trackvthreads command line 
option would be removed too.

-

PR: https://git.openjdk.java.net/jdk/pull/8780


Integrated: 8286490: JvmtiEventControllerPrivate::set_event_callbacks CLEARING_MASK computation is incorrect

2022-05-25 Thread Serguei Spitsyn
On Tue, 24 May 2022 05:45:17 GMT, Serguei Spitsyn  wrote:

> It was a typo when the original CLEARING_MASK was initially introduced:
> 
> 
>  // Mask to clear normal event bits.
>   const jlong CLEARING_MASK = (1L >> (TOTAL_MIN_EVENT_TYPE_VAL - 
> TOTAL_MIN_EVENT_TYPE_VAL)) - 1L;
>   // Avoid cleaning extension event bits.
>   jlong enabled_bits = CLEARING_MASK & 
> env->env_event_enable()->_event_callback_enabled.get_bits();
> 
> ``` 
> 
> The first TOTAL_MIN_EVENT_TYPE_VAL has to be JVMTI_MIN_EVENT_TYPE_VAL as 
> below:
> `  const jlong CLEARING_MASK = (1L >> (JVMTI_MIN_EVENT_TYPE_VAL - 
> TOTAL_MIN_EVENT_TYPE_VAL)) - 1L;`
> 
> Let me provide some background to understand how this mask is used.
> 
> Normal JVMTI event types are numbered from 50 to 88. There are two constants:
> 
>   JVMTI_MIN_EVENT_TYPE_VAL = 50
>   JVMTI_MAX_EVENT_TYPE_VAL = 88
> 
> The extension event types are numbered from 47 to 49. There are also two 
> constants:
> 
>   EXT_MIN_EVENT_TYPE_VAL = 47
>   EXT_MAX_EVENT_TYPE_VAL = 49
> 
> 
> There are also two additional constants:
> 
>   TOTAL_MIN_EVENT_TYPE_VAL = 47
>   TOTAL_MAX_EVENT_TYPE_VAL = 88
> 
> 
> The `enabled_bits` are shifted left by 47, so that the 0-bit in 
> `enabled_bits` is for first extension event type. And the first normal JVMTI 
> event type is numbered 3 in the `enabled_bits` bit mask. The `CLEARING_MASK` 
> is used to clear only normal JVMTI event types but keep the extension event 
> bits (0-2) unchanged.
> 
> Testing:
> I'll run all JVMTI tests including nsk.jvmti tests and serviceability/jvmti 
> tests including those added in Loom for virtual thread coverage.

This pull request has now been integrated.

Changeset: a0cccb54
Author:Serguei Spitsyn 
URL:   
https://git.openjdk.java.net/jdk/commit/a0cccb54791d954bf08da5aac9b9794e370617c8
Stats: 40 lines in 3 files changed: 35 ins; 4 del; 1 mod

8286490: JvmtiEventControllerPrivate::set_event_callbacks CLEARING_MASK 
computation is incorrect

Reviewed-by: jbachorik, lmesnik, amenkov

-

PR: https://git.openjdk.java.net/jdk/pull/8860


Re: RFR: 8287200: Test java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java timed out after JDK-8287103

2022-05-25 Thread Alan Bateman
On Tue, 24 May 2022 19:52:57 GMT, Leonid Mesnik  wrote:

> Need to use proper synchronization.
> 
> The CyclicBarriers might move the thread to WAITING state but not BLOCKED. So 
> it should not confuse existing checks.

Marked as reviewed by alanb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/8874