Re: RFD: Remove Hotspot-internal MBeans from sun.management

[mandy . chung]

Would a PR to remove these APIs be welcome?



Good with me.

Mandy

On 3/19/24 9:41 AM, Eirik Bjørsnøs wrote:


Hi,

Last September, Volker shared the observation that we have 
Hotspot-internal MBeans in sun.management which are strongly 
encapsulated and not used internally by OpenJDK besides their unit tests:


https://www.mail-archive.com/core-libs-dev@openjdk.org/msg19878.html 



A summary of the email thread:

Mandy pointed out:

We added these HotSpot internal MBeans in JDK 5 to expose the
internal metrics.  Most of these internal metrics are exposed via
jstat tool too.   We didn't receive much feedback regarding these
HotSpot internal MBeans.    Removing them is fine and good cleanup
effort.


Alan made a similar point:

It's left over from experiments on exposing some internal metrics,
I think during JDK 5. It's code that should probably have been
removed a long time ago.


Kirk P raised a concern:

It would be a shame to lose these metrics because many of them
have been very
useful over time and some would be even more useful with some
modifications.

To which Mandy responded:

What we're referring to is to remove sun.management.Hotspot*, the
internal MBeans which are never exposed and registered in the
platform MBeanServer.   The internal metrics in HotSpot VM should
be retained as they are exposed through other ways like jstat, GC
logs, etc.


The email thread seems to have ended here without further action taken.

My interpretation of the above is that we have a consensus that these 
Hotspot-internal MBeans can be removed. Since I was not part of the 
initial discussion and some time has passed, I'd like some 
confirmation that my interpretation is correct.


Would a PR to remove these APIs be welcome?

(This would remove HotspotInternalMBean, HotspotMemoryMBean, 
HotspotRuntimeMBean, HotspotThreadMBean, with associated 
implementation, factory methods, tests and probably also some native 
code in libmanagement. Details can be discussed in a PR)


Cheers,
Eirik.

Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v12]

[Mandy Chung]

On Thu, 14 Mar 2024 12:23:09 GMT, Kevin Walls  wrote:

>> The deprecated Subject Delegation feature in JMX will be removed.
>> 
>> This was marked in JDK 21 as deprecated for removal (JDK-8298966).
>
> Kevin Walls has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   RMIConnectionImpl_Stub also should explicity inherit the unchecked UOE.

The spec updates are good with me.   Caught one formatting nit in the 
`RMIConnection` class spec.

src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
 line 85:

> 83:  *
> 84:  * JMX Subject Delegation has been removed. All methods that take a
> 85:  * delegationSubject parameter will throw UnsupportedOperationException if

Suggestion:

 * delegation subject parameter will throw {@code 
UnsupportedOperationException} if

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18025#pullrequestreview-1937202285
PR Review Comment: https://git.openjdk.org/jdk/pull/18025#discussion_r1525173122

Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v10]

[Mandy Chung]

On Tue, 12 Mar 2024 19:58:24 GMT, Kevin Walls  wrote:

>> The deprecated Subject Delegation feature in JMX will be removed.
>> 
>> This was marked in JDK 21 as deprecated for removal (JDK-8298966).
>
> Kevin Walls has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   typo

The spec change looks good to me.I leave to others to review the 
implementation and test changes.

src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
 line 85:

> 83:  *
> 84:  * JMX Subject Delegation has been removed. All methods that take a
> 85:  * delegationSubject parameter will throw UnsupportedOperationException if

Suggestion:

 * JMX Subject Delegation has been removed. All methods that take a
 * delegation subject parameter will throw {@code 
UnsupportedOperationException} if

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18025#pullrequestreview-1934819628
PR Review Comment: https://git.openjdk.org/jdk/pull/18025#discussion_r1523673795

Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v7]

[Mandy Chung]

On Mon, 11 Mar 2024 18:49:43 GMT, Daniel Fuchs  wrote:

>> I was wondering if we should add a note at the class-level API documentation 
>> to say that subject delegation has been removed (or is no longer supported), 
>> and that methods will throw UOE if a non-null delegation subject is passed. 
>> It is otherwise strange to see a parameter that must be null. It could be 
>> worthwhile to mention it just once, at the class-level API.
>
> Maybe mention there too that the delegationSubject parameter is kept for 
> interoperability with older remote clients.

It's a good suggestion.   Something like this:

All methods that take a `Subject` parameter will throw UOE if called with a 
non-null subject delegation.  JMX subject delegation feature is no longer 
supported.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18025#discussion_r1520310935

Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v7]

[Mandy Chung]

On Mon, 11 Mar 2024 10:09:28 GMT, Kevin Walls  wrote:

>> The deprecated Subject Delegation feature in JMX will be removed.
>> 
>> This was marked in JDK 21 as deprecated for removal (JDK-8298966).
>
> Kevin Walls has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   (C) oops

src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
 line 126:

> 124:  * @param className The class name of the MBean to be instantiated.
> 125:  * @param name The object name of the MBean. May be null.
> 126:  * @param delegationSubject No longer supported and should be null.

I think it can be simplified to:

Suggestion:

 * @param delegationSubject must be {@code null}.


I see no need to mention this feature no longer supported as no one is using 
and will use this feature.

Same comment applies to all methods that take `delegationSubject`.

src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
 line 157:

> 155:  * @throws IOException if a general communication exception occurred.
> 156:  * @throws UnsupportedOperationException if a non-null 
> delegationSubject
> 157:  * is specified. Subject Delegation has been removed.

This can be simplified to:

Suggestion:

 * @throws UnsupportedOperationException if {@code delegationSubject} is 
non-null


Same comment applies to all methods that take `delegationSubject`.

src/java.management/share/classes/javax/management/remote/JMXConnector.java 
line 139:

> 137: /**
> 138:  * This method remains for compatibility reasons, but has no more 
> meaning
> 139:  * than {@link #getMBeanServerConnection()}.

Suggestion:

 * This method is equivalent to calling {@link 
#getMBeanServerConnection()}.

src/java.management/share/classes/javax/management/remote/JMXConnector.java 
line 142:

> 140:  *
> 141:  * @param delegationSubject must be null, since the removal of the
> 142:  * Subject Delegation feature.

Suggestion:

 * @param delegationSubject must be {@code null}

src/java.management/share/classes/javax/management/remote/JMXConnector.java 
line 153:

> 151:  *
> 152:  * @exception UnsupportedOperationException if a non-null 
> delegationSubject
> 153:  * is specified. Subject Delegation has been removed.

Suggestion:

 * @exception UnsupportedOperationException if {@code delegationSubject} is 
non-null.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18025#discussion_r1520188392
PR Review Comment: https://git.openjdk.org/jdk/pull/18025#discussion_r1520183154
PR Review Comment: https://git.openjdk.org/jdk/pull/18025#discussion_r1520165902
PR Review Comment: https://git.openjdk.org/jdk/pull/18025#discussion_r1520166471
PR Review Comment: https://git.openjdk.org/jdk/pull/18025#discussion_r1520170490

Re: RFR: 8324845: management.properties text "interface name" is misleading [v2]

[Mandy Chung]

On Wed, 31 Jan 2024 21:29:14 GMT, Kevin Walls  wrote:

>> We have the text "host-or-interface-name" describing the 
>> com.sun.management.jmxremote.host property in management.properties, but 
>> interface names (like "eth0" or "lo", or "ens3"...) are not what it accepts. 
>>  It should just say it needs to be a host name or address.
>> 
>> This change only affects comment text in a properties file.
>> 
>> (We don't currently mention this property in other docs, e.g. in the M 
>> guide.)
>
> Kevin Walls has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   text update

Marked as reviewed by mchung (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/17615#pullrequestreview-1855055246

Re: RFR: 8320652: ThreadInfo.isInNative needs to be updated to say what executing native code means [v3]

[Mandy Chung]

On Fri, 1 Dec 2023 12:16:29 GMT, Alan Bateman  wrote:

>> This is a docs only change to j.l.management.ThreadInfo::isInNative. 
>> 
>> The method currently specifies that it tests if the thread is  "executing 
>> native code via the Java Native Interface (JNI)".  It would be clearer to 
>> say that it tests if the thread is executing a native method, and expand it 
>> to include native code invoked using the new foreign linker APIs. For now, 
>> I've left out the detail of a downcall handle created with the "critical" 
>> linker option.
>
> Alan Bateman has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains seven additional 
> commits since the last revision:
> 
>  - Extend description
>  - Merge
>  - Change link to downcallHandle
>  - Merge
>  - Simplify wording
>  - Merge
>  - Initial commit

The updated spec change looks good.

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16791#pullrequestreview-1760317046

Re: RFR: 8320652: ThreadInfo.isInNative needs to be updated to say what executing native code means [v2]

[Mandy Chung]

On Mon, 27 Nov 2023 20:31:23 GMT, Alan Bateman  wrote:

>> This is a docs only change to j.l.management.ThreadInfo::isInNative. 
>> 
>> The method currently specifies that it tests if the thread is  "executing 
>> native code via the Java Native Interface (JNI)".  It would be clearer to 
>> say that it tests if the thread is executing a native method, and expand it 
>> to include native code invoked using the new foreign linker APIs. For now, 
>> I've left out the detail of a downcall handle created with the "critical" 
>> linker option.
>> 
>> The existing javadoc has wording to try to clarify that "native code" does 
>> not include runtime or compiled code. I went through a few iterations to 
>> work this in but all attempts just invite more questions and nit picking. To 
>> keep it simple, this sentence is dropped so that the method description is 
>> focused on the cases where the method returns true.
>
> Alan Bateman has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Change link to downcallHandle
>  - Merge
>  - Simplify wording
>  - Merge
>  - Initial commit

Marked as reviewed by mchung (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/16791#pullrequestreview-1751563823

Re: RFR: 8320652: ThreadInfo.isInNative needs to be updated to say what executing native code means [v2]

[Mandy Chung]

On Mon, 27 Nov 2023 20:38:03 GMT, Bernd  wrote:

>> src/java.management/share/classes/java/lang/management/ThreadInfo.java line 
>> 552:
>> 
>>> 550:  * java.lang.invoke.MethodHandle method handle} obtained from the
>>> 551:  * {@linkplain java.lang.foreign.Linker native linker}.
>>> 552:  *
>> 
>> This area is new to me, but I happened to be in this code few days back. I'm 
>> mostly curious on what the actual definition of a thread being in native 
>> means.
>> When a thread is executing any of the following,  does it end up being 
>> considered as being in a "native method":
>> 
>> - A syscall (for example, `write()`)
>> - A C function exposed by a platform specific library
>> - A JNI method (either part of the JDK or the application) which then may or 
>> may not do any syscall or C function call on a platform specific library
>
> I would agree, it should state if runtime functions (including those doing a 
> syscall) will be counted here. (For JNi i would not need it to be spelled 
> out, on the other hand it would help, since it makes clear we don’t mean c2 
> code)

This `isInNative` method intends to provide a way to tell if the thread has 
transitioned from executing Java method to a native method.  JVM interpreter 
and compiler provide the support for Java language which is why I think such 
clarification might not be highly necessary.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16791#discussion_r1406737205

Re: RFR: 8320652: ThreadInfo.isInNative needs to be updated to say what executing native code means

[Mandy Chung]

On Thu, 23 Nov 2023 10:25:31 GMT, Alan Bateman  wrote:

> This is a docs only change to j.l.management.ThreadInfo::isInNative. 
> 
> The method currently specifies that it tests if the thread is  "executing 
> native code via the Java Native Interface (JNI)".  It would be clearer to say 
> that it tests if the thread is executing a native method, and expand it to 
> include native code invoked using the new foreign linker APIs. For now, I've 
> left out the detail of a downcall handle created with the "critical" linker 
> option.
> 
> The existing javadoc has wording to try to clarify that "native code" does 
> not include runtime or compiled code. I went through a few iterations to work 
> this in but all attempts just invite more questions and nit picking. To keep 
> it simple, this sentence is dropped so that the method description is focused 
> on the cases where the method returns true.

It's ok to drop the sentence about "native code" does not include runtime or 
compiled code.

I can see why you left out the detail of a downcall handle.   Maybe better to 
link the text "method handle" to `Linker::downcallHandle` that is how such a 
method handle is created.

-

PR Comment: https://git.openjdk.org/jdk/pull/16791#issuecomment-1828467569

Re: RFR: 8315702: jcmd Thread.dump_to_file slow with millions of virtual threads

[Mandy Chung]

On Wed, 6 Sep 2023 07:02:53 GMT, Alan Bateman  wrote:

> `HotSpotDiagnosticMXBean.dumpThreads` and `jcmd Thread.dump_to_file` are slow 
> when there is a large number of threads.
> 
> The thread dump can be sped up significantly with some small changes:
> - Using println rather than format when print thread info and thread stacks
> - Wrap the underlying file output stream in a BufferedOutputStream
> 
> With 200k virtual threads on macOS, the plain format thread dump goes from 
> 22s to 1.8s, and the json format thread dump goes from 31s to 2.8s on one 
> system that I tried. On a Linux system, also with 200k threads, the plain 
> thread dump goes from 8.7s to 2.9s, and the json format thread dump from 
> 12.4s to 4.5s.

Looks good to me.

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15581#pullrequestreview-1615959218

Re: Question on why sun.management MBeans are not exported?

[mandy . chung]

What we're referring to is to remove sun.management.Hotspot*, the 
internal MBeans which are never exposed and registered in the platform 
MBeanServer.   The internal metrics in HotSpot VM should be retained as 
they are exposed through other ways like jstat, GC logs, etc.


Mandy

On 9/6/23 11:27 PM, Kirk Pepperdine wrote:


Hi,

It would be a shame to lose these metrics because many of them have been very 
useful over time and some would be even more useful with some modifications. 
For example, the CPU breakouts found in GC logs has been incredibly useful as a 
proxy measure in helping sort out other issues in systems. So much so that I 
have analytics built specifically around this in my tooling.

Kind regards,
Kirk Pepperdine



On Sep 6, 2023, at 10:50 AM, Alan Bateman  wrote:

On 06/09/2023 16:17, Volker Simonis wrote:

:
I'm familiar with JEP 260. But wouldn't you agree that an
"encapsulated" monitoring API is an oxymoron? A monitoring API is by
design intended for external usage and completely useless to the
platform itself. There's no single usage of the "sun.management"
MBeans in the JDK itself (except for jconsole where the encapsulation
broke it). My assumption is that the corresponding MBeans in
"sun.management" are there for historic reasons (added in JDK 1.5) and
would have made much more sense in "com.sun.management" package. But I
doubt that they can be classified in the "internal implementation
details of the JDK and never intended for external use² category of
JEP 260.

It's left over from experiments on exposing some internal metrics, I think 
during JDK 5. It's code that should probably have been removed a long time ago.

-Alan

Re: Question on why sun.management MBeans are not exported?

[mandy . chung]

On 9/6/23 8:17 AM, Volker Simonis wrote:

Anyway, if you classify the MBeans in "sun.management" as non-critical
internal APIs (with respect to JEP 260) but without any "internal"
usage, than we should really remove them, right, because an internal
API without any internal usage doesn't make any sense?



We added these HotSpot internal MBeans in JDK 5 to expose the internal 
metrics.  Most of these internal metrics are exposed via jstat tool 
too.   We didn't receive much feedback regarding these HotSpot internal 
MBeans.    Removing them is fine and good cleanup effort.



Mandy




I'll then try to come up with a proposal to port some of the more
useful MBeans functionality in "sun.management" to
"com.sun.management".

Thank you and best regards,
Volker




:

So to cut a long story short, I see several options:

1. Publicly export sun.management and restore the JDK 8 (or pre JDK
16) behavior. This would certainly require some polishing (e.g. some
of the corresponding JVM functionality has already been removed [1])
but I think it could still be quite useful.
2. Port the useful functionality from the "sun.management" MBeans to
corresponding "com.sun.management" MBeans and remove the
"sun.management" MBeans.
3. Remove the "sun.management" MBeans without substitution.

What do you think?

If there are JDK-specific or HotSpot VM specific features where there is
a compelling case for a management interface then com.sun.management is
good place to prototype new APIs. You may already be familiar with
com.sun.management.HotSpotDiagnosticMBean.

-Alan

Re: [jdk21] RFR: 8300937: Update nroff pages in JDK 21 before RC

[Mandy Chung]

On Mon, 31 Jul 2023 08:33:07 GMT, David Holmes  wrote:

> Main changes are to use 21 instead of 21-ea. In addition the following files 
> contain additional updates from the closed sources:
> 
> - src/java.base/share/man/java.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
>  [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
> StartFlightRecording: consider moving mention of multiple comma-separated 
> parameters to the front
> 
> There are also some formatting differences related to:
> 
>  [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help 
> output for --module-path / -p is incomplete
> 
> Likely a different version of pandoc was used.
> 
> - src/java.base/share/man/keytool.1
> 
>  [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> 
> - src/jdk.compiler/share/man/javac.1
> 
>  [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac 
> tool page for -proc:full
> 
> also some formatting differences.
> 
> - src/jdk.jartool/share/man/jarsigner.1
> 
>  [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
> man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> 
> - src/jdk.jcmd/share/man/jcmd.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
> 
> - src/jdk.jdeps/share/man/jdeps.1
> 
>  [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> - src/jdk.jfr/share/man/jfr.1
> 
> Formatting changes.
> 
> - src/jdk.jshell/share/man/jshell.1
> 
>  [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
> manpage to include TOOLING description

Changes in java and jdeps are good.

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk21/pull/151#pullrequestreview-1555334390

Re: RFR: 8310816: GcInfoBuilder float/double signature mismatch

[Mandy Chung]

On Fri, 23 Jun 2023 18:10:59 GMT, Kevin Walls  wrote:

> Simple typo in a signature which is passed to JNU_NewObjectByName.  The 
> method clearly intentds to pass Float, but uses Double.
> 
> This code is probably not invoked, unless there is a GC MXBean with such 
> fields.  I see no straightforward way of testing this explicitly, but the 
> change is tiny.
> 
> All tests in test/jdk/com/sun/management still pass.

Marked as reviewed by mchung (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14631#pullrequestreview-1496015078

Re: RFR: 8310628: GcInfoBuilder.c missing JNI Exception checks

[Mandy Chung]

On Thu, 22 Jun 2023 10:10:07 GMT, Kevin Walls  wrote:

> JNI calls were identified, where we do not check for a pending Exception 
> afterwards.
> 
> (JDK-8162530 cleaned up up some of these kind of issues some years back, but 
> more were found.)
> 
> I tested a code change to manually create an Exception before some of the new 
> ExceptionCheck calls, and this is correctly recognised, we see an Exception 
> thrown by the native getLastGcInfo0, e.g. 
> 
> java.lang.NullPointerException: XXX Test Exception
> at 
> jdk.management/com.sun.management.internal.GcInfoBuilder.getLastGcInfo0(Native
>  Method)
> at 
> jdk.management/com.sun.management.internal.GcInfoBuilder.getLastGcInfo(GcInfoBuilder.java:77)
> at 
> jdk.management/com.sun.management.internal.GarbageCollectorExtImpl.getLastGcInfo(GarbageCollectorExtImpl.java:69)
> at LastGCInfo.main(LastGCInfo.java:53)
> at 
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
> at java.base/java.lang.reflect.Method.invoke(Method.java:580)
> at 
> com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
> at java.base/java.lang.Thread.run(Thread.java:1570)
> 
> Tested with all of test/jdk/com/sun/management including 
> test/jdk/com/sun/management/GarbageCollectorMXBean/LastGCInfo.java

Marked as reviewed by mchung (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14613#pullrequestreview-1495483988

Re: RFR: 8310628: GcInfoBuilder.c missing JNI Exception checks

[Mandy Chung]

On Thu, 22 Jun 2023 10:10:07 GMT, Kevin Walls  wrote:

> JNI calls were identified, where we do not check for a pending Exception 
> afterwards.
> 
> (JDK-8162530 cleaned up up some of these kind of issues some years back, but 
> more were found.)
> 
> I tested a code change to manually create an Exception before some of the new 
> ExceptionCheck calls, and this is correctly recognised, we see an Exception 
> thrown by the native getLastGcInfo0, e.g. 
> 
> java.lang.NullPointerException: XXX Test Exception
> at 
> jdk.management/com.sun.management.internal.GcInfoBuilder.getLastGcInfo0(Native
>  Method)
> at 
> jdk.management/com.sun.management.internal.GcInfoBuilder.getLastGcInfo(GcInfoBuilder.java:77)
> at 
> jdk.management/com.sun.management.internal.GarbageCollectorExtImpl.getLastGcInfo(GarbageCollectorExtImpl.java:69)
> at LastGCInfo.main(LastGCInfo.java:53)
> at 
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
> at java.base/java.lang.reflect.Method.invoke(Method.java:580)
> at 
> com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
> at java.base/java.lang.Thread.run(Thread.java:1570)
> 
> Tested with all of test/jdk/com/sun/management including 
> test/jdk/com/sun/management/GarbageCollectorMXBean/LastGCInfo.java

Looks fine.   It may worth refactoring the code to set an element in an utility 
method.

jobject obj = JNU_NewObjectByName(env, class_name, signature, value);
if ((*env)->ExceptionCheck(env)) {
return;
}
(*env)->SetObjectArrayElement(env, array, index, obj);

-

PR Review: https://git.openjdk.org/jdk/pull/14613#pullrequestreview-1493765649

Re: RFR: 8304478: Initial nroff manpage generation for JDK 22

[Mandy Chung]

On Wed, 14 Jun 2023 04:53:58 GMT, David Holmes  wrote:

> Updated the version to 22-ea and year to 2024.
> 
> The following unpublished changes will also be included in this update:
> - [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> - [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update 
> jarsigner man page after 
> [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> - [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> The following changes, to `javac.1`, were never applied to the closed sources 
> and are "lost" by this update. These changes will need to be re-applied 
> directly in JDK 21 and JDK 22:
> - [JDK-8296656](https://bugs.openjdk.org/browse/JDK-8296656): 
> java.lang.NoClassDefFoundError exception on running fully legitimate code
> - [JDK-8015831](https://bugs.openjdk.org/browse/JDK-8015831): Add lint check 
> for calling overridable methods from a constructor
> 
> Thanks.

Marked as reviewed by mchung (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14462#pullrequestreview-1480088199

Re: RFR: 8309406: Change jdk.trackAllThreads to default to true

[Mandy Chung]

On Sat, 3 Jun 2023 13:41:53 GMT, Alan Bateman  wrote:

> Virtual threads created directly with the Thread API are not included in the 
> new thread dump by default. This was a source of confusion in JDK 19/20 when 
> virtual threads were in preview. We have decided to switch the default so 
> that all virtual threads are observable. Future work will reduce/remove the 
> overhead of tracking threads in the root container. The change proposed here 
> has no impact to virtual threads created with the virtual 
> ThreadPerTaskExecutor or StructuredTaskScope.
> 
> With jdk.trackAllThreads defaulting to true then the need for 
> SharedThreadContainer to count threads can go away, as does the need for 
> RootContainer::threads to return virtual threads blocked on I/O.
> 
> Most of the changes are to the test HotSpotDiagnosticMXBean/DumpThreads.java 
> as this is expanded to cover more cases, and specifically more cases with 
> thread groupings that correspond to the ThreadPerTaskExecutor and thread 
> pools.
> 
> Testing: Test1-5

Looks okay to me.

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14300#pullrequestreview-1463111013

Re: RFR: 8304074: [JMX] Add an approximation of total bytes allocated on the Java heap by the JVM [v28]

[Mandy Chung]

On Fri, 26 May 2023 16:41:32 GMT, Paul Hohensee  wrote:

>> Please review this addition to com.sun.management.ThreadMXBean that returns 
>> the total number of bytes allocated on the Java heap since JVM launch by 
>> both terminated and live threads.
>> 
>> Because this PR adds a new interface method, I've updated the JMM_VERSION to 
>> 4, but would be happy to update it to 3_1 instead.
>
> Paul Hohensee has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 100 commits:
> 
>  - 8304074: Remove extra file
>  - 8304074: Add getTotalAllocatedBytes high water mark
>  - Merge branch 'master' into 8304074
>  - 8304074: Refactor test exception handling
>  - Merge master
>  - Merge master
>  - 8304074: Change UnsupportedOperationException in javadoc comment to {@code 
> UnsupportedOperationException}
>  - 8306507: [linux] Print number of memory mappings in error reports
>
>Reviewed-by: adinn, sgehwolf
>  - 8300086: Replace NULL with nullptr in share/c1/
>
>Reviewed-by: thartmann, chagedorn
>  - 8308465: Reduce memory accesses in AArch64 MD5 intrinsic
>
>Reviewed-by: aph, phh
>  - ... and 90 more: https://git.openjdk.org/jdk/compare/77c5adb0...5f2f86bb

Marked as reviewed by mchung (Reviewer).

Thanks for updating the test.   The spec looks good.   I'll leave the hotspot 
implementation for David and others to approve.

-

PR Review: https://git.openjdk.org/jdk/pull/13814#pullrequestreview-1446662964
PR Comment: https://git.openjdk.org/jdk/pull/13814#issuecomment-1564701378

Re: RFR: 8304074: [JMX] Add an approximation of total bytes allocated on the Java heap by the JVM [v23]

[Mandy Chung]

On Wed, 24 May 2023 16:50:59 GMT, Paul Hohensee  wrote:

>> test/jdk/com/sun/management/ThreadMXBean/ThreadAllocatedMemory.java line 160:
>> 
>>> 158: try {
>>> 159: curThread.join();
>>> 160: } catch (InterruptedException e) {
>> 
>> should it just let `InterruptedException` be thrown rather than swallowing 
>> it?  This pattern appears multiple places in this test and 
>> ThreadAllocatedMemoryArray.java test.
>
> The original implementation grabbed this code from, e.g., 
> test/jdk/java/lang/management/ThreadMXBean/ThreadUserTime.java. In the 
> latter, InterruptedException causes the test to fail, but for some reason now 
> lost, ThreadAllocatedMemory.java doesn't do that, but 
> ThreadAllocatedMemoryArray.java does.

Since you are on this file, it may be good to clean them up.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/13814#discussion_r1205773318

Re: RFR: 8304074: [JMX] Add an approximation of total bytes allocated on the Java heap by the JVM [v23]

[Mandy Chung]

On Fri, 19 May 2023 13:54:13 GMT, Paul Hohensee  wrote:

>> Please review this addition to com.sun.management.ThreadMXBean that returns 
>> the total number of bytes allocated on the Java heap since JVM launch by 
>> both terminated and live threads.
>> 
>> Because this PR adds a new interface method, I've updated the JMM_VERSION to 
>> 4, but would be happy to update it to 3_1 instead.
>
> Paul Hohensee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8304074: atomic load needed in exited_allocated_bytes

The specification looks good.   A minor comment in the test.

src/jdk.management/share/classes/com/sun/management/ThreadMXBean.java line 116:

> 114:  * recorded.
> 115:  *
> 116:  * @implSpec The default implementation throws 
> UnsupportedOperationException

Suggestion:

 * @implSpec The default implementation throws {@code 
UnsupportedOperationException}

test/jdk/com/sun/management/ThreadMXBean/ThreadAllocatedMemory.java line 160:

> 158: try {
> 159: curThread.join();
> 160: } catch (InterruptedException e) {

should it just let `InterruptedException` be thrown rather than swallowing it?  
This pattern appears multiple places in this test and 
ThreadAllocatedMemoryArray.java test.

-

PR Review: https://git.openjdk.org/jdk/pull/13814#pullrequestreview-1440326691
PR Review Comment: https://git.openjdk.org/jdk/pull/13814#discussion_r1202819575
PR Review Comment: https://git.openjdk.org/jdk/pull/13814#discussion_r1202824396

Re: RFR: 8304919: Implementation of Virtual Threads [v4]

[Mandy Chung]

On Wed, 29 Mar 2023 08:00:36 GMT, Alan Bateman  wrote:

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

The stack walker change looks good.  I also reviewed other changes which look 
fine to me.

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13203#pullrequestreview-1364091585

Integrated: JDK-8304163: Move jdk.internal.module.ModuleInfoWriter to the test library

[Mandy Chung]

On Fri, 17 Mar 2023 22:01:46 GMT, Mandy Chung  wrote:

> `ModuleInfoWriter` is not used by the runtime.   Move it to the test library 
> as `jdk.test.lib.util.ModuleInfoWriter`.   The tests are updated to use the 
> test library instead.   `ModuleInfoWriter` depends on `jdk.internal.module` 
> types and the Classfile API.   Hence `@modules 
> java.base/jdk.internal.classfile` and other classfile subpackages are added.

This pull request has now been integrated.

Changeset: 622f2394
Author:Mandy Chung 
URL:   
https://git.openjdk.org/jdk/commit/622f239448c2a96a74202621ee84c181d79fbde4
Stats: 154 lines in 17 files changed: 91 ins; 19 del; 44 mod

8304163: Move jdk.internal.module.ModuleInfoWriter to the test library

Reviewed-by: jpai, alanb

-

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

Re: RFR: JDK-8304163: Move jdk.internal.module.ModuleInfoWriter to the test library [v2]

[Mandy Chung]

On Mon, 20 Mar 2023 06:57:32 GMT, Jaikiran Pai  wrote:

>> test/jdk/java/lang/ModuleTests/AnnotationsTest.java line 61:
>> 
>>> 59:  *  java.base/jdk.internal.module
>>> 60:  * @library /test/lib
>>> 61:  * @build jdk.test.lib.util.ModuleInfoWriter
>> 
>> You don't need to build library classes explicitly. I think @library 
>> /test/lib it enough.
>
> Hello @lmesnik, on the contrary, these build directives are recommended (and 
> based on some of the issues we have encountered, are in fact necessary). The 
> jtreg documentation has this to say https://openjdk.org/jtreg/tag-spec.html:
> 
>> In general, classes in library directories are not automatically compiled as 
>> part of a compilation command explicitly naming the source files containing 
>> those classes. A test that relies upon library classes should contain 
>> appropriate @build directives to ensure that the classes will be compiled. 
>> It is strongly recommended that tests do not rely on the use of implicit 
>> compilation by the Java compiler. Such an approach is generally fragile, and 
>> may lead to incomplete recompilation when a test or library code has been 
>> modified.

Explicit compilation is exactly the reason of adding `@build`

-

PR Review Comment: https://git.openjdk.org/jdk/pull/13085#discussion_r1142447529

Re: RFR: JDK-8304163: Move jdk.internal.module.ModuleInfoWriter to the test library

[Mandy Chung]

On Fri, 17 Mar 2023 22:01:46 GMT, Mandy Chung  wrote:

> `ModuleInfoWriter` is not used by the runtime.   Move it to the test library 
> as `jdk.test.lib.util.ModuleInfoWriter`.   The tests are updated to use the 
> test library instead.   `ModuleInfoWriter` depends on `jdk.internal.module` 
> types and the Classfile API.   Hence `@modules 
> java.base/jdk.internal.classfile` and other classfile subpackages are added.

Thanks for the pointer to the recommended ordering.   Tests updated.

-

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

Re: RFR: JDK-8304163: Move jdk.internal.module.ModuleInfoWriter to the test library [v2]

[Mandy Chung]

> `ModuleInfoWriter` is not used by the runtime.   Move it to the test library 
> as `jdk.test.lib.util.ModuleInfoWriter`.   The tests are updated to use the 
> test library instead.   `ModuleInfoWriter` depends on `jdk.internal.module` 
> types and the Classfile API.   Hence `@modules 
> java.base/jdk.internal.classfile` and other classfile subpackages are added.

Mandy Chung has updated the pull request incrementally with one additional 
commit since the last revision:

  move @library after @modules per the recommended ordering

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/13085/files
  - new: https://git.openjdk.org/jdk/pull/13085/files/3eda19b5..6b7611ad

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=13085=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=13085=00-01

  Stats: 30 lines in 14 files changed: 15 ins; 15 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/13085.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/13085/head:pull/13085

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

RFR: JDK-8304163: Move jdk.internal.module.ModuleInfoWriter to the test library

[Mandy Chung]

`ModuleInfoWriter` is not used by the runtime.   Move it to the test library as 
`jdk.test.lib.util.ModuleInfoWriter`.   The tests are updated to use the test 
library instead.   `ModuleInfoWriter` depends on `jdk.internal.module` types 
and the Classfile API.   Hence `@modules java.base/jdk.internal.classfile` and 
other classfile subpackages are added.

-

Commit messages:
 - JDK-8304163: Move jdk.internal.module.ModuleInfoWriter to the test library

Changes: https://git.openjdk.org/jdk/pull/13085/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=13085=00
  Issue: https://bugs.openjdk.org/browse/JDK-8304163
  Stats: 146 lines in 17 files changed: 87 ins; 15 del; 44 mod
  Patch: https://git.openjdk.org/jdk/pull/13085.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/13085/head:pull/13085

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

Re: RFR: 8298966: Deprecate JMX Subject Delegation and the method JMXConnector.getMBeanServerConnection(Subject) for removal. [v2]

[Mandy Chung]

On Fri, 3 Mar 2023 11:46:49 GMT, Kevin Walls  wrote:

>> Deprecate the Java Management Extension (JMX) Subject Delegation feature for 
>> removal in a future release.
>> 
>> Given no known usage, there is no replacement feature for JMX Subject 
>> Delegation.
>> 
>> CSR is https://bugs.openjdk.org/browse/JDK-8298967
>
> Kevin Walls 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 five additional 
> commits since the last revision:
> 
>  - deprecation text update
>  - Revert "RMIConnection throw comments"
>
>This reverts commit aceb4fe44189245ac702f0c74c2bb1100a6d17fa.
>  - Merge remote-tracking branch 'upstream/master' into 
> Deprecate_SubjectDelegation
>  - RMIConnection throw comments
>  - 8298966: Deprecate JMX Subject Delegation and the method 
> JMXConnector.getMBeanServerConnection(Subject) for removal.

Marked as reviewed by mchung (Reviewer).

-

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

Re: RFR: 8303242: ThreadMXBean issues with virtual threads [v4]

[Mandy Chung]

On Thu, 2 Mar 2023 08:18:03 GMT, Alan Bateman  wrote:

>> This PR covers a number of issues with j.l.management.ThreadMXBean, and the 
>> JDK-specific extension c.s.management.ThreadMXBean, when there are virtual 
>> threads in use.
>> 
>> As background, ThreadMXBean was re-specified in Java 19 to support the 
>> monitoring and management of platform threads. It does not support virtual 
>> threads as their potential number, and the need to find a thread by id, does 
>> not make sense for this API. At the same time, JDK 19 introduced an 
>> alternative implementation of virtual threads for Zero and ports without 
>> continuations support in the VM. This alternative implementation of virtual 
>> threads means a JavaThread per virtual thread and so requires filtering to 
>> ensure that the API behaves as specified. For the initial implementation, 
>> the filtering was done in the ThreadMXBean implementation. That works for 
>> most functions but not for getThreadXXXTime(long[]) and 
>> getThreadAllocatedBytes(long[]) where the filtering needs to be pushed down 
>> to the management code.
>> 
>> The changes in this PR move the filtering to the management functions 
>> (jmm_XXX) so they only return information about platform threads. It also 
>> fixes ThreadMXBean.getCurrentThreadCpuTime and getCurrentThreadUserTime to 
>> not throw UOE when CPU time measurement from a platform thread is supported. 
>> There are some small adjustments to the API docs (see linked CSR). Test 
>> coverage is expanded as we didn't include tests for 
>> c.s.management.ThreadMXBean with virtual threads in JDK 19.
>> 
>> Testing tier1-3 (jdk_management test group is in test/jdk/:tier3), plus 
>> sanity checking that --with-jvm-variants=minimal builds as some of this code 
>> is not compiled in with minimal VM builds.
>
> Alan Bateman has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains eight additional 
> commits since the last revision:
> 
>  - Tweak javadoc to avoid listing too many conditions in @return description
>  - Merge
>  - Update isXXXThreadCpuTimeSupported descriptions
>  - Clarify Thread CPU time seciton of spec
>  - Merge
>  - Fix minimal build
>  - Fix minimal build
>  - Initial commit

Marked as reviewed by mchung (Reviewer).

Looks good.

-

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

Re: RFR: 8303242: ThreadMXBean issues with virtual threads [v3]

[Mandy Chung]

On Wed, 1 Mar 2023 12:39:52 GMT, Alan Bateman  wrote:

>> This PR covers a number of issues with j.l.management.ThreadMXBean, and the 
>> JDK-specific extension c.s.management.ThreadMXBean, when there are virtual 
>> threads in use.
>> 
>> As background, ThreadMXBean was re-specified in Java 19 to support the 
>> monitoring and management of platform threads. It does not support virtual 
>> threads as their potential number, and the need to find a thread by id, does 
>> not make sense for this API. At the same time, JDK 19 introduced an 
>> alternative implementation of virtual threads for Zero and ports without 
>> continuations support in the VM. This alternative implementation of virtual 
>> threads means a JavaThread per virtual thread and so requires filtering to 
>> ensure that the API behaves as specified. For the initial implementation, 
>> the filtering was done in the ThreadMXBean implementation. That works for 
>> most functions but not for getThreadXXXTime(long[]) and 
>> getThreadAllocatedBytes(long[]) where the filtering needs to be pushed down 
>> to the management code.
>> 
>> The changes in this PR move the filtering to the management functions 
>> (jmm_XXX) so they only return information about platform threads. It also 
>> fixes ThreadMXBean.getCurrentThreadCpuTime and getCurrentThreadUserTime to 
>> not throw UOE when CPU time measurement from a platform thread is supported. 
>> There are some small adjustments to the API docs (see linked CSR). Test 
>> coverage is expanded as we didn't include tests for 
>> c.s.management.ThreadMXBean with virtual threads in JDK 19.
>> 
>> Testing tier1-3 (jdk_management test group is in test/jdk/:tier3), plus 
>> sanity checking that --with-jvm-variants=minimal builds as some of this code 
>> is not compiled in with minimal VM builds.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update isXXXThreadCpuTimeSupported descriptions

src/java.management/share/classes/java/lang/management/ThreadMXBean.java line 
479:

> 477:  * if the thread of the specified ID exists, the thread is alive,
> 478:  * and CPU time measurement is enabled; {@code -1} if not enabled
> 479:  * or the specified ID is a virtual thread

It should be "{@code -1} if not enabled or the specified ID is a virtual thread 
or the thread does not exist or not alive."

Would this be simpler:


 * @return the total CPU time for a thread of the specified ID
 * if the thread of the specified ID is a platform thread, the thread is 
alive,
 * and CPU time measurement is enabled; {@code -1} otherwise.


I'm fine with either way.  Same comment for `getThreadUserTime(long)`

-

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

Re: RFR: 8303242: ThreadMXBean issues with virtual threads [v3]

[Mandy Chung]

On Wed, 1 Mar 2023 11:40:26 GMT, Alan Bateman  wrote:

>> test/jdk/java/lang/management/ThreadMXBean/VirtualThreads.java line 258:
>> 
>>> 256: long tid = Thread.currentThread().threadId();
>>> 257: long cpuTime = bean.getThreadCpuTime(tid);
>>> 258: assertEquals(-1L, cpuTime);
>> 
>> Am I correct that `getThreadCpuTime(tid)` returns -1 for the current thread 
>> is a virtual thread whereas `getCurrentThreadCpuTime` throws UOE in the 
>> current implementation?
>> 
>> `getCurrentThreadCpuTime` is specified to be equivalent to calling 
>> `getThreadCpuTime(Thread.currentThread().threadId()`.
>
> We didn't get this quite right in JDK 19 but I think I've fixed all those 
> issues now.  So assuming the VM supports CPU time for all platform threads, 
> it means:
> 
> - isThreadCpuTimeEnabled and isCurrentThreadCpuTimeSupported will return true.
> - If getThreadCpuTime(long) is called with the thread ID of a virtual thread 
> then -1 will be returned.
> - If getCurrentThreadCpuTime() is called from a virtual thread then -1 will 
> be  returned.
> 
> I did another pass over the API docs, update the "Thread CPU time" section, 
> so I hope it is clearer now.

This looks better.  I see `testGetCurrentThreadCpuTime` and 
`testGetCurrentThreadUserTime` test cases fixed.

-

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

Re: RFR: 8303242: ThreadMXBean issues with virtual threads [v3]

[Mandy Chung]

On Wed, 1 Mar 2023 12:39:52 GMT, Alan Bateman  wrote:

>> This PR covers a number of issues with j.l.management.ThreadMXBean, and the 
>> JDK-specific extension c.s.management.ThreadMXBean, when there are virtual 
>> threads in use.
>> 
>> As background, ThreadMXBean was re-specified in Java 19 to support the 
>> monitoring and management of platform threads. It does not support virtual 
>> threads as their potential number, and the need to find a thread by id, does 
>> not make sense for this API. At the same time, JDK 19 introduced an 
>> alternative implementation of virtual threads for Zero and ports without 
>> continuations support in the VM. This alternative implementation of virtual 
>> threads means a JavaThread per virtual thread and so requires filtering to 
>> ensure that the API behaves as specified. For the initial implementation, 
>> the filtering was done in the ThreadMXBean implementation. That works for 
>> most functions but not for getThreadXXXTime(long[]) and 
>> getThreadAllocatedBytes(long[]) where the filtering needs to be pushed down 
>> to the management code.
>> 
>> The changes in this PR move the filtering to the management functions 
>> (jmm_XXX) so they only return information about platform threads. It also 
>> fixes ThreadMXBean.getCurrentThreadCpuTime and getCurrentThreadUserTime to 
>> not throw UOE when CPU time measurement from a platform thread is supported. 
>> There are some small adjustments to the API docs (see linked CSR). Test 
>> coverage is expanded as we didn't include tests for 
>> c.s.management.ThreadMXBean with virtual threads in JDK 19.
>> 
>> Testing tier1-3 (jdk_management test group is in test/jdk/:tier3), plus 
>> sanity checking that --with-jvm-variants=minimal builds as some of this code 
>> is not compiled in with minimal VM builds.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update isXXXThreadCpuTimeSupported descriptions

Marked as reviewed by mchung (Reviewer).

-

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

Re: RFR: 8303242: ThreadMXBean issues with virtual threads

[Mandy Chung]

On Mon, 27 Feb 2023 12:23:09 GMT, Alan Bateman  wrote:

> This PR covers a number of issues with j.l.management.ThreadMXBean, and the 
> JDK-specific extension c.s.management.ThreadMXBean, when there are virtual 
> threads in use.
> 
> As background, ThreadMXBean was re-specified in Java 19 to support the 
> monitoring and management of platform threads. It does not support virtual 
> threads as their potential number, and the need to find a thread by id, does 
> not make sense for this API. At the same time, JDK 19 introduced an 
> alternative implementation of virtual threads for Zero and ports without 
> continuations support in the VM. This alternative implementation of virtual 
> threads means a JavaThread per virtual thread and so requires filtering to 
> ensure that the API behaves as specified. For the initial implementation, the 
> filtering was done in the ThreadMXBean implementation. That works for most 
> functions but not for getThreadXXXTime(long[]) and 
> getThreadAllocatedBytes(long[]) where the filtering needs to be pushed down 
> to the management code.
> 
> The changes in this PR move the filtering to the management functions 
> (jmm_XXX) so they only return information about platform threads. There are 
> some minor adjustments to the API docs (see linked CSR). Test coverage is 
> expanded as we didn't include tests for c.s.management.ThreadMXBean with 
> virtual threads in JDK 19.
> 
> Testing tier1-3 (jdk_management test group is in test/jdk/:tier3), plus 
> sanity checking that --with-jvm-variants=minimal builds as some of this code 
> is not compiled in with minimal VM builds.

The "Thread CPU time" section in the class spec of ThreadMXBean can also be 
clarified.

src/java.management/share/classes/java/lang/management/ThreadMXBean.java line 
529:

> 527: /**
> 528:  * Tests if the Java virtual machine implementation supports CPU time
> 529:  * measurement for any platform thread.

This change can also apply in `@return` (line 538)

test/jdk/java/lang/management/ThreadMXBean/VirtualThreads.java line 258:

> 256: long tid = Thread.currentThread().threadId();
> 257: long cpuTime = bean.getThreadCpuTime(tid);
> 258: assertEquals(-1L, cpuTime);

Am I correct that `getThreadCpuTime(tid)` returns -1 for the current thread is 
a virtual thread whereas `getCurrentThreadCpuTime` throws UOE in the current 
implementation?

`getCurrentThreadCpuTime` is specified to be equivalent to calling 
`getThreadCpuTime(Thread.currentThread().threadId()`.

-

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

Re: RFR: 8293810: Remove granting of RuntimePermission("stopThread") from tests

[Mandy Chung]

On Wed, 5 Oct 2022 15:01:15 GMT, Alan Bateman  wrote:

> This is a test only change to remove the granting of 
> RuntimePermission("stopThread") from the tests. With Thread.stop changed to 
> throw UOE it means there is nothing that requires this permission.

LGTM

-

Marked as reviewed by mchung (Reviewer).

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

Re: RFR: 8289610: Degrade Thread.stop [v2]

[Mandy Chung]

On Wed, 14 Sep 2022 14:09:52 GMT, Alan Bateman  wrote:

>> Degrade Thread.stop to throw UOE unconditionally, deprecate ThreadDeath for 
>> removal, and remove the remaining special handling of ThreadDeath from the 
>> JDK.
>> 
>> Thread.stop is inherently unsafe and has been deprecated since JDK 1.2 
>> (1998) with a link to a supplementary page that explains the rationale. Some 
>> of the API surface has already been degraded or removed: 
>> Thread.stop(Throwable) was degraded to throw UOE in Java 8 and removed in 
>> Java 11, and ThreadGroup.stop was degraded to throw UOE in Java 19. As of 
>> Java 19, the no-arg Thread.stop continues to work as before for platform 
>> threads but throws UOE for virtual threads. The next step in the glacial 
>> pace removal is the degrading of the no-arg Thread.stop method to throw UOE 
>> for all threads.
>> 
>> To keep things manageable, the change proposed here leaves JVM_StopThread in 
>> place. A separate issue will remove it and do other cleanup/removal in the 
>> VM. We have another JBS issue for the updates to the JLS and JVMS where 
>> asynchronous exceptions are defined. There is also some remaining work on a 
>> test class used by 6 jshell tests - if they aren't done in time then we will 
>> temporarily exclude them.
>> 
>> The change here has no  impact on the debugger APIs (JVM TI StopThread, JDWP 
>> ThreadReference/Stop, and JDI ThreadReference.stop). Debuggers can continue 
>> to cause threads to throw an asynchronous exception, as might be done when 
>> simulating code throwing an exception at some point in the code.
>
> Alan Bateman has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains six additional 
> commits since the last revision:
> 
>  - Merge
>  - Remove stopThread permission from RuntimePermission table, exclude jshell 
> tests
>  - Deprecate for removal
>  - Next iteration, update dates in headers
>  - Merge
>  - Initial commit

Marked as reviewed by mchung (Reviewer).

-

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

Re: RFR: 8289610: Degrade Thread.stop

[Mandy Chung]

On Fri, 9 Sep 2022 12:44:31 GMT, Alan Bateman  wrote:

> Degrade Thread.stop to throw UOE unconditionally, deprecate ThreadDeath for 
> removal, and remove the remaining special handling of ThreadDeath from the 
> JDK.
> 
> Thread.stop is inherently unsafe and has been deprecated since JDK 1.2 (1998) 
> with a link to a supplementary page that explains the rationale. Some of the 
> API surface has already been degraded or removed: Thread.stop(Throwable) was 
> degraded to throw UOE in Java 8 and removed in Java 11, and ThreadGroup.stop 
> was degraded to throw UOE in Java 19. As of Java 19, the no-arg Thread.stop 
> continues to work as before for platform threads but throws UOE for virtual 
> threads. The next step in the glacial pace removal is the degrading of the 
> no-arg Thread.stop method to throw UOE for all threads.
> 
> To keep things manageable, the change proposed here leaves JVM_StopThread in 
> place. A separate issue will remove it and do other cleanup/removal in the 
> VM. We have another JBS issue for the updates to the JLS and JVMS where 
> asynchronous exceptions are defined. There is also some remaining work on a 
> test class used by 6 jshell tests - if they aren't done in time then we will 
> temporarily exclude them.
> 
> The change here has no  impact on the debugger APIs (JVM TI StopThread, JDWP 
> ThreadReference/Stop, and JDI ThreadReference.stop). Debuggers can continue 
> to cause threads to throw an asynchronous exception, as might be done when 
> simulating code throwing an exception at some point in the code.

The change looks good.   There are other tests that reference `ThreadDeath` for 
example `test/lib/jdk/test/lib/process/ProcessTools.java`, 
`test/jdk/java/util/Timer/KillThread.java` , 
`test/jdk/java/net/httpclient/reactivestreams-tck/org/reactivestreams/tck/flow/support/NonFatal.java`
 etc.Is it expected to update them in a follow-on issue?

-

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

Re: [jdk19] RFR: 8290562: ThreadMXBean.getThread{Cpu, User}Time fails with -XX:-VMContinuations [v2]

[Mandy Chung]

On Wed, 3 Aug 2022 14:37:52 GMT, Alan Bateman  wrote:

>> ThreadMXBean.getThread{Cpu,User}Time is specified to return -1L when invoked 
>> with the id of a virtual thread. This isn't so when running with 
>> -XX:-VMContinuations (or ports without support for continuations in the VM) 
>> as it returns the cpu/user time of the OS thread that that the virtual 
>> thread is bound. A small oversight with JDK-8287496, and missed because our 
>> unit test only exercises these methods with the id of the "current virtual 
>> thread". The code path when the called with the id that is not the current 
>> thread is a different code path.
>> 
>> The change is limited to jmm_GetThreadCpuTimeWithKind.  I didn't change 
>> jmm_GetThreadCpuTimesWithKind because it seems to be unused/dead code. I'll 
>> create a separate issue to look at that (it doesn't need to be 
>> removed/changed for JDK 19).
>> 
>> The test case for this API is expanded to cover more cases where the current 
>> thread is special cased in the implementation.
>> 
>> JDK 19 is in RDP2 so this change will require additional approval.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix typo in comment

Looks good to me.

-

Marked as reviewed by mchung (Reviewer).

PR: https://git.openjdk.org/jdk19/pull/157