Re: RFR: 8331385: G1: Prefix HeapRegion helper classes with G1

2024-07-05 Thread Thomas Schatzl
On Tue, 2 Jul 2024 10:21:35 GMT, Albert Mingkun Yang  wrote:

>> Hi all,
>> 
>>   after [JDK-8330694](https://bugs.openjdk.org/browse/JDK-8330694) which 
>> renamed HeapRegion to G1HeapRegion, there were  a few related helper classes 
>> in this CR that were not renamed.
>> 
>> It's purely mechanical renaming without even further renaming of files etc.
>> 
>> This change updates them.
>> 
>> (Fwiw, the "Viewed" checkbox at the top right of the file change helps a lot 
>> review this change incrementally)
>> 
>> Testing: tier1, tier4, tier5
>> 
>> Thanks,
>>   Thomas
>
> Marked as reviewed by ayang (Reviewer).

Thanks @albertnetymk @dholmes-ora for your reviews.

-

PR Comment: https://git.openjdk.org/jdk/pull/19967#issuecomment-2210331294


Integrated: 8331385: G1: Prefix HeapRegion helper classes with G1

2024-07-05 Thread Thomas Schatzl
On Mon, 1 Jul 2024 09:35:00 GMT, Thomas Schatzl  wrote:

> Hi all,
> 
>   after [JDK-8330694](https://bugs.openjdk.org/browse/JDK-8330694) which 
> renamed HeapRegion to G1HeapRegion, there were  a few related helper classes 
> in this CR that were not renamed.
> 
> It's purely mechanical renaming without even further renaming of files etc.
> 
> This change updates them.
> 
> (Fwiw, the "Viewed" checkbox at the top right of the file change helps a lot 
> review this change incrementally)
> 
> Testing: tier1, tier4, tier5
> 
> Thanks,
>   Thomas

This pull request has now been integrated.

Changeset: 4ec1ae10
Author:Thomas Schatzl 
URL:   
https://git.openjdk.org/jdk/commit/4ec1ae109710aa150e27acf5706475d335c4655c
Stats: 887 lines in 68 files changed: 163 ins; 165 del; 559 mod

8331385: G1: Prefix HeapRegion helper classes with G1

Reviewed-by: ayang, dholmes

-

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


Re: RFR: 8335619: Add an @apiNote to j.l.i.ClassFileTransformer to warn about recursive class loading and ClassCircularityErrors [v2]

2024-07-05 Thread Volker Simonis
On Thu, 4 Jul 2024 10:49:47 GMT, Volker Simonis  wrote:

>> Since Java 5 the `java.lang.instrument` package provides services that allow 
>> Java programming language agents to instrument (i.e. modify the bytecode) of 
>> programs running on the Java Virtual Machine. The `java.lang.instrument` 
>> functionality is based and implemented on top of the native Java Virtual 
>> Machine Tool Interface (JVMTI) also introduced in Java 5. But because the 
>> `java.lang.instrument` API is a pure Java API and uses Java classes to 
>> instrument Java classes it imposes some usage restrictions which are not 
>> very well documented in its API specification.
>> 
>> E.g. the section on ["Bytecode 
>> Instrumentation"](https://docs.oracle.com/en/java/javase/21/docs/specs/jvmti.html#bci)
>>  in the JVMTI specification explicitly warns that special "*Care must be 
>> taken to avoid perturbing dependencies, especially when instrumenting core 
>> classes*". The risk of such "perturbing dependencies" is obviously much 
>> higher in a Java API like `java.lang.instrument`, but a more detailed 
>> explanation and warning is missing from its API documentation.
>> 
>> The most evident class file transformation restriction is that while a class 
>> A is being loaded and transformed it is not possible to use this same class 
>> directly or transitively from the `ClassFileTransformer::transform()` 
>> method. Violating this rule will result in a `ClassCircularityError` (the 
>> exact error type is disputable as can be seen in [8164165: JVM throws 
>> incorrect exception when ClassFileTransformer.transform() triggers class 
>> loading of class already being 
>> loaded](https://bugs.openjdk.org/browse/JDK-8164165), but the result would 
>> be a `LinkageError in any case).
>> 
>> The risk to run into such a `ClassCircularityError` error increases with the 
>> amount of code a transforming agent is transitively using from the 
>> `transform()` method. Using popular libraries like ASM, ByteBuddy, etc. for 
>> transformation further increases the probability of running into such 
>> issues, especially if the agent aims to transform core JDK library classes.
>> 
>> By default, the occurrence of a `ClassCircularityError` in 
>> `ClassFileTransformer::transform()` will be handled gracefully with the only 
>> consequence that the current transformation target will be loaded unmodified 
>> (see `ClassFileTransformer` API spec: "*throwing an exception has the same 
>> effect as returning null*"). But unfortunately, it can also have a subtle 
>> but at the same time much more far-reaching consequence. If the 
>> `ClassCircularityError` occurs during the resolution of a constant pool ...
>
> Volker Simonis has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixed text based on reviewer comments. Also added a '@jvms' tag to another 
> reference to the JVMS and fixed its section number

@AlanBateman would you mind having a quick look at this trivial, 
documentation-only PR and let me know if you're OK with it?

Thank you and best regards,
Volker

-

PR Comment: https://git.openjdk.org/jdk/pull/20011#issuecomment-2210345568


Re: RFR: 8335688: Fix -Wzero-as-null-pointer-constant warnings from fflush calls in jvmti tests

2024-07-05 Thread Julian Waters
On Thu, 4 Jul 2024 12:15:09 GMT, Kim Barrett  wrote:

> Please review this change to some jvmti tests, which were calling fflush with
> an argument of 0. Most of these are in C++ code, where we change them to use
> nullptr as the argument.  A couple are in C, where we change them to use NULL.
> This removes a bunch of -Wzero-as-null-pointer-constant when building with
> that enabled.
> 
> Testing: mach5 tier1

Marked as reviewed by jwaters (Committer).

-

PR Review: https://git.openjdk.org/jdk/pull/20032#pullrequestreview-2160159717


Re: RFR: 8335643: serviceability/dcmd/vm tests fail for ZGC after JDK-8322475

2024-07-05 Thread Thomas Stuefe
On Fri, 5 Jul 2024 06:20:27 GMT, Severin Gehwolf  wrote:

> @tstuefe Please also remove the problem list entry. See the error from the 
> skara bot. Thanks!

Oh, Skara tells me this now? Or did I never notice :) In any case that's cool. 
Thanks!

-

PR Comment: https://git.openjdk.org/jdk/pull/20034#issuecomment-2210464002


Re: RFR: 8335643: serviceability/dcmd/vm tests fail for ZGC after JDK-8322475 [v2]

2024-07-05 Thread Thomas Stuefe
> The new System.map facility fails its tests when the JVM is using ZGC. The 
> facility is working fine, but the test checks for the java heap to appear as 
> committed non-shared memory segment, but on ZGC we reserve the memory as 
> shared memory.

Thomas Stuefe has updated the pull request incrementally with one additional 
commit since the last revision:

  problemlist

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/20034/files
  - new: https://git.openjdk.org/jdk/pull/20034/files/5d89a936..50a73e14

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

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

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


RFR: 8335743: jhsdb jstack cannot print some information on the waiting thread

2024-07-05 Thread KIRIYAMA Takuya
This bug was introduced by JDK-8284161. "Object.wait (long timeoutMillis)" was 
changed to call "Object.wait0 (long timeoutMillis)" in JDK-8284161.

When "jhdsb jstack" is executed, the stack and lock information are printed in 
"sun.jvm.hotspot.runtime.JavaVFrame.printLockInfo(PrintStream tty, int 
frameCount)". This method checks whether the method is java.lang.Object.wait 
(...) and then reports the waitstate only if the check passes.
https://github.com/openjdk/jdk/blob/7bc8f9c150cbf457edf6144adba734ecd5ca5a0f/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaVFrame.java#L103


  if (getMethod().getName().asString().equals("wait") &&
  
getMethod().getMethodHolder().getName().asString().equals("java/lang/Object")) {


However, after JDK-8284161, the waiting thread waits on "java.lang.Object.wait0 
(long timeoutMillis)", so it no longer reports the waitstate.

I changed "printLockInfo(PrintStream tty, int frameCount)" to check for 
"java.lang.Object.wait0 (...)".
I confirmed that the lock information is correctly printed with this fix.
I tested hotspot/jtreg/serviceability/sa/ and jdk/sun/tools/jhsdb/heapconfig/, 
and there were no regressions by this fix.

Would anyone review this change, please?

-

Commit messages:
 - 8335743: jhsdb jstack cannot print some information on the waiting thread

Changes: https://git.openjdk.org/jdk/pull/20049/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20049&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8335743
  Stats: 24 lines in 4 files changed: 17 ins; 0 del; 7 mod
  Patch: https://git.openjdk.org/jdk/pull/20049.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20049/head:pull/20049

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


[jdk23] Integrated: 8322812: Manpage for jcmd is missing JFR.view command

2024-07-05 Thread Erik Gahlin
On Thu, 4 Jul 2024 10:51:48 GMT, Erik Gahlin  wrote:

> 8322812: Manpage for jcmd is missing JFR.view command

This pull request has now been integrated.

Changeset: 10b28bab
Author:Erik Gahlin 
URL:   
https://git.openjdk.org/jdk/commit/10b28babe53821fcdeef3a1aa0712feb7cd67529
Stats: 49 lines in 1 file changed: 49 ins; 0 del; 0 mod

8322812: Manpage for jcmd is missing JFR.view command

Reviewed-by: mgronlun
Backport-of: 350f9c1947b0eab3ee233516ceefca1e25de9583

-

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


Re: RFR: 8335643: serviceability/dcmd/vm tests fail for ZGC after JDK-8322475 [v2]

2024-07-05 Thread Severin Gehwolf
On Fri, 5 Jul 2024 08:53:30 GMT, Thomas Stuefe  wrote:

>> The new System.map facility fails its tests when the JVM is using ZGC. The 
>> facility is working fine, but the test checks for the java heap to appear as 
>> committed non-shared memory segment, but on ZGC we reserve the memory as 
>> shared memory.
>
> Thomas Stuefe has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   problemlist

Seems fine to me.

Apparently also in `ProblemList-generational-zgc.txt`, which needs to remove 
those lines.

-

Marked as reviewed by sgehwolf (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20034#pullrequestreview-2160393678
Changes requested by sgehwolf (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20034#pullrequestreview-2160395464


Re: RFR: 8335643: serviceability/dcmd/vm tests fail for ZGC after JDK-8322475 [v3]

2024-07-05 Thread Thomas Stuefe
> The new System.map facility fails its tests when the JVM is using ZGC. The 
> facility is working fine, but the test checks for the java heap to appear as 
> committed non-shared memory segment, but on ZGC we reserve the memory as 
> shared memory.

Thomas Stuefe has updated the pull request incrementally with one additional 
commit since the last revision:

  problemlist

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/20034/files
  - new: https://git.openjdk.org/jdk/pull/20034/files/50a73e14..7cbeb260

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

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

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


[jdk23] Integrated: 8324089: Fix typo in the manual page for "jcmd" (man jcmd)

2024-07-05 Thread Erik Gahlin
On Thu, 4 Jul 2024 10:54:22 GMT, Erik Gahlin  wrote:

> 8324089: Fix typo in the manual page for "jcmd" (man jcmd)

This pull request has now been integrated.

Changeset: 90d5b5b4
Author:Erik Gahlin 
URL:   
https://git.openjdk.org/jdk/commit/90d5b5b4c497ac99d0e2ade689b6459fffea3e2a
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod

8324089: Fix typo in the manual page for "jcmd" (man jcmd)

Reviewed-by: mgronlun
Backport-of: 0bb9c76288b5f63fe965c3276bb566cef5f51c50

-

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


RFR: 8335775: Remove extraneous 's' in comment of rawmonitor.cpp test file

2024-07-05 Thread Severin Gehwolf
Trivial comment only change in a test. Please review!

Thanks!

-

Commit messages:
 - 8335775: Remove extraneous 's' in comment of rawmonitor.cpp test file

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

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


Re: RFR: 8335775: Remove extraneous 's' in comment of rawmonitor.cpp test file

2024-07-05 Thread George Adams
On Fri, 5 Jul 2024 11:14:10 GMT, Severin Gehwolf  wrote:

> Trivial comment only change in a test. Please review!
> 
> Thanks!

Marked as reviewed by gdams (Author).

-

PR Review: https://git.openjdk.org/jdk/pull/20051#pullrequestreview-2160518890


Re: RFR: 8335775: Remove extraneous 's' in comment of rawmonitor.cpp test file

2024-07-05 Thread Thomas Stuefe
On Fri, 5 Jul 2024 11:14:10 GMT, Severin Gehwolf  wrote:

> Trivial comment only change in a test. Please review!
> 
> Thanks!

Good and trivial

Marked as reviewed by stuefe (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/20051#pullrequestreview-2160533824
PR Review: https://git.openjdk.org/jdk/pull/20051#pullrequestreview-2160534056


Re: RFR: 8335643: serviceability/dcmd/vm tests fail for ZGC after JDK-8322475 [v3]

2024-07-05 Thread Severin Gehwolf
On Fri, 5 Jul 2024 10:56:37 GMT, Thomas Stuefe  wrote:

>> The new System.map facility fails its tests when the JVM is using ZGC. The 
>> facility is working fine, but the test checks for the java heap to appear as 
>> committed non-shared memory segment, but on ZGC we reserve the memory as 
>> shared memory.
>
> Thomas Stuefe has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   problemlist

Marked as reviewed by sgehwolf (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/20034#pullrequestreview-2160590870


Re: RFR: 8335775: Remove extraneous 's' in comment of rawmonitor.cpp test file

2024-07-05 Thread Severin Gehwolf
On Fri, 5 Jul 2024 11:14:10 GMT, Severin Gehwolf  wrote:

> Trivial comment only change in a test. Please review!
> 
> Thanks!

Thanks for the review!

-

PR Comment: https://git.openjdk.org/jdk/pull/20051#issuecomment-2210765310


Integrated: 8335775: Remove extraneous 's' in comment of rawmonitor.cpp test file

2024-07-05 Thread Severin Gehwolf
On Fri, 5 Jul 2024 11:14:10 GMT, Severin Gehwolf  wrote:

> Trivial comment only change in a test. Please review!
> 
> Thanks!

This pull request has now been integrated.

Changeset: ff49f677
Author:Severin Gehwolf 
URL:   
https://git.openjdk.org/jdk/commit/ff49f677ee5017019c90823bc412ceb90068ffbd
Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod

8335775: Remove extraneous 's' in comment of rawmonitor.cpp test file

Reviewed-by: gdams, stuefe

-

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


RFR: 8267887: RMIConnector_NPETest.java fails after removal of RMI Activation (JDK-8267123)

2024-07-05 Thread Kevin Walls
The test 
test/jdk/javax/management/remote/mandatory/connection/RMIConnector_NPETest.java 
should be removed.

This test was added when 6984520 was fixed in 6u25.  It has been problemlisted 
since JDK-8267123 removed RMI Activation (it does not use RMI Activation, it 
just wanted something "RMI" to connect with).

Looking at the change it tested, the code is no longer the same, and is no 
longer at risk of this NPE.

The original NPE fix was to check that jxmServerviceURL was not null before 
calling methods on it, but the current RMIConnector.java does not make those 
same calls.

It is best to remove the test and its problemlist entry.

-

Commit messages:
 - 8267887: RMIConnector_NPETest.java fails after removal of RMI Activation 
(JDK-8267123)

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

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


Re: RFR: 8335684: Test ThreadCpuTime.java should pause like ThreadCpuTimeArray.java

2024-07-05 Thread Serguei Spitsyn
On Thu, 4 Jul 2024 10:08:30 GMT, Kevin Walls  wrote:

> There are two similarly names tests.
> Recently:
> JDK-8335124: com/sun/management/ThreadMXBean/ThreadCpuTimeArray.java failed 
> with CPU time out of expected range
> ...made a simple change to try and avoid noisy test failures.  The same fix 
> should be applied here to ThreadCpuTime.java.
> 
> Also removing an old comment about a Solaris issue.

Okay to me.

-

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20025#pullrequestreview-2161247996