Re: RFR: 8331385: G1: Prefix HeapRegion helper classes with G1
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
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]
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
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
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]
> 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
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
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]
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]
> 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)
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
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
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
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]
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
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
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)
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
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