Re: 8231585: java/lang/management/ThreadMXBean/MaxDepthForThreadInfoTest.java fails with java.lang.NullPointerException

2020-04-20 Thread Daniil Titov
Hi David, Serguei, and Chris, Thank you for reviewing this change. The issue is not easy to reproduce. I tried to run the test (without the fix) with Graal on in Mach5 about 600 times and so far the failure didn't reproduce. I will continue running these tests in the next few days and will

Re: RFR: JDK-8242943 Fix all remaining unchecked warnings in jdk.hotspot.agent

2020-04-20 Thread David Holmes
Hi Magnus, That all seems reasonable to me. Thanks, David - On 16/04/2020 10:47 pm, Magnus Ihse Bursie wrote: This is the final part of removing all warnings from the build of jdk.hotspot.agent. This patch includes a number of non-trivial fixes for the few remaining unchecked warnings.

Re: RFR(XS) 8242789: sun/tools/jhsdb/HeapDumpTestWithActiveProcess.java fails with 'JShellToolProvider' missing from stdout/stderr

2020-04-20 Thread Alex Menkov
+1 --alex On 04/20/2020 12:03, serguei.spit...@oracle.com wrote: Hi Chris, LGTM Thanks, Serguei On 4/20/20 10:46, Chris Plummer wrote: Ping. This is a very simple change. thanks, Chris On 4/17/20 10:30 AM, Chris Plummer wrote: Hello, Please review the following:

Re: RFR: 8242425: JVMTI monitor operations should use Thread-Local Handshakes

2020-04-20 Thread Yasumasa Suenaga
Hi Rechard, Thank you for telling it to me. I grep'ed jdk.hotspot.agent with VMOps (it is just enum), it does not seem to be used. In addition, VMOps has not already synced to HotSpot. For example, VM_HandshakeOneThread does not appear in VMOps. (I wonder how do we use VMOps in SA) Thus I

Re: RFR: JDK-8242943 Fix all remaining unchecked warnings in jdk.hotspot.agent

2020-04-20 Thread serguei.spit...@oracle.com
Hi Magnus, This looks pretty good to me. I hope, Joe gave some insight for Comparable's. Thanks, Serguei On 4/16/20 05:47, Magnus Ihse Bursie wrote: This is the final part of removing all warnings from the build of jdk.hotspot.agent. This patch includes a number of non-trivial fixes for the

Re: RFR(XS) 8243206: Cleanup error checking and handling in serviceability/sa/JhsdbThreadInfoTest.java

2020-04-20 Thread serguei.spit...@oracle.com
LGTM++ Thanks, Serguei On 4/20/20 16:37, Alex Menkov wrote: Hi Chris, LGTM --alex On 04/20/2020 13:56, Chris Plummer wrote: Hello, Please review the following simple fixes to JhsdbThreadInfoTest.java. Details are in the CR description. https://bugs.openjdk.java.net/browse/JDK-8243206

Re: RFR(XS) 8243206: Cleanup error checking and handling in serviceability/sa/JhsdbThreadInfoTest.java

2020-04-20 Thread Alex Menkov
Hi Chris, LGTM --alex On 04/20/2020 13:56, Chris Plummer wrote: Hello, Please review the following simple fixes to JhsdbThreadInfoTest.java. Details are in the CR description. https://bugs.openjdk.java.net/browse/JDK-8243206 http://cr.openjdk.java.net/~cjplummer/8243206/webrev.00/

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-20 Thread Chris Plummer
On 4/20/20 1:57 PM, Mandy Chung wrote: On 4/20/20 1:06 PM, Chris Plummer wrote: On 4/18/20 3:28 PM, Mandy Chung wrote: Hi Chris, Serguei, On 4/18/20 12:18 AM, Chris Plummer wrote: Yes. I'd like to see all this as part of the Class/Classloading spec in some sort of section that gives an

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-20 Thread Mandy Chung
On 4/20/20 1:06 PM, Chris Plummer wrote: On 4/18/20 3:28 PM, Mandy Chung wrote: Hi Chris, Serguei, On 4/18/20 12:18 AM, Chris Plummer wrote: Yes. I'd like to see all this as part of the Class/Classloading spec in some sort of section that gives an overview of all these topics, but mostly

RFR(XS) 8243206: Cleanup error checking and handling in serviceability/sa/JhsdbThreadInfoTest.java

2020-04-20 Thread Chris Plummer
Hello, Please review the following simple fixes to JhsdbThreadInfoTest.java. Details are in the CR description. https://bugs.openjdk.java.net/browse/JDK-8243206 http://cr.openjdk.java.net/~cjplummer/8243206/webrev.00/ thanks, Chris

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-20 Thread Chris Plummer
On 4/18/20 3:28 PM, Mandy Chung wrote: Hi Chris, Serguei, On 4/18/20 12:18 AM, Chris Plummer wrote: Yes. I'd like to see all this as part of the Class/Classloading spec in some sort of section that gives an overview of all these topics, but mostly clarifies what an initiating loader is, and

Re: RFR: JDK-8242943 Fix all remaining unchecked warnings in jdk.hotspot.agent

2020-04-20 Thread Joe Darcy
Hello, On 4/16/2020 5:47 AM, Magnus Ihse Bursie wrote: [snip] The tricky one here is the helper TableModelComparator. This one took me a while to wrap my head around. It implements Comparator, but the compare() method takes "rows" from the model, which are just expressed as Objects, and left

Re: RFR(XS) 8242789: sun/tools/jhsdb/HeapDumpTestWithActiveProcess.java fails with 'JShellToolProvider' missing from stdout/stderr

2020-04-20 Thread serguei.spit...@oracle.com
Hi Chris, LGTM Thanks, Serguei On 4/20/20 10:46, Chris Plummer wrote: Ping. This is a very simple change. thanks, Chris On 4/17/20 10:30 AM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8242789

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-20 Thread serguei.spit...@oracle.com
Hi Mandy, Thank you for the update! I like it as it is a good compromise. I do not see any wording issues. Thanks, Serguei On 4/18/20 15:28, Mandy Chung wrote: Hi Chris, Serguei, On 4/18/20 12:18 AM, Chris Plummer wrote: Yes. I'd like to see all this as part of the Class/Classloading spec

Re: RFR(XS) 8242789: sun/tools/jhsdb/HeapDumpTestWithActiveProcess.java fails with 'JShellToolProvider' missing from stdout/stderr

2020-04-20 Thread Chris Plummer
Ping. This is a very simple change. thanks, Chris On 4/17/20 10:30 AM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8242789 http://cr.openjdk.java.net/~cjplummer/8242789/webrev.00 JShellHeapDumpTest.java has two variants, one that does a

RE: RFR: 8242425: JVMTI monitor operations should use Thread-Local Handshakes

2020-04-20 Thread Reingruber, Richard
Hi Yasumasa, I'm obviously late to this review. Still I wanted to let you know, that there's another file in the source tree that lists the VM operations in the VM (just found out about it myself): src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/VMOps.java Probably you should

Re: RFR(L) 8237354: Add option to jcmd to write a gzipped heap dump

2020-04-20 Thread coleen . phillimore
Hi, I don't want to review this but could you put this new code in its own file?  heapDumper only needs CompressionBackend to be exported, from what I can tell. Thanks, Coleen On 4/20/20 6:12 AM, Reingruber, Richard wrote: Hi Ralf, 767: I think _current->in_used doesn't take the final 9

RE: RFR(L) 8237354: Add option to jcmd to write a gzipped heap dump

2020-04-20 Thread Reingruber, Richard
Hi Ralf, > > 767: I think _current->in_used doesn't take the final 9 bytes into account > > that are written in > > DumperSupport::end_of_dump() after the last dump segment has been finished. > > You could call get_new_buffer() instead of the if clause. > Wow, how did you found this? I've fixed

RE: RFR(L) 8237354: Add option to jcmd to write a gzipped heap dump

2020-04-20 Thread Schmelter, Ralf
Hi Richard, thanks for the review. I have incorporated your remarks into a new webrev: http://cr.openjdk.java.net/~rschmelter/webrevs/8237354/webrev.2/ Some remarks to specific points: > ### src/hotspot/share/services/heapDumper.cpp > 762: assert(_active, "Must be active"); > > It appears to me