Re: RFR (M) Close alignment gaps in InstanceKlass

Hi Dean, Thank you for looking at the JVMCI changes and the suggestion to add the test.  I did this and found a bug.  The new test is quite limited because there's no good test to see if a source file name can assertNotNull(type.getSourceFileName()), so I couldn't iterate through the list

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

Hi Chris, I filed it to JBS: https://bugs.openjdk.java.net/browse/JDK-8243450 I will send review request later. Thanks, Yasumasa On 2020/04/23 5:02, Chris Plummer wrote: Hi Yasumasa, Yes, it looks like VMOps in SA can be removed. It's probably bit rotted code. My guess is at some

Re: RFR (M) Close alignment gaps in InstanceKlass

On 4/22/20 9:00 PM, Dean Long wrote: It looks like calling the JVMCI getSourceFileName() on an array would have accessed random memory because it was expecting an InstanceKlass.  Instead of returning null we might want to throw an exception like in HotSpotResolvedPrimitiveType. It was never

Re: RFR: 8243450: Reomve VMOps from jdk.hotspot.agent

Looks good. Chris On 4/22/20 7:22 PM, Yasumasa Suenaga wrote: Hi all, Please review removing file from jdk.hotspot.agent:   JBS: https://bugs.openjdk.java.net/browse/JDK-8243450   webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8243450/webrev.00/ VMOps is enum class which lists all of VM

Re: RFR (M) Close alignment gaps in InstanceKlass

Can you compare the result to some string, like "Object.java"?  That seems to be what HotSpotResolvedObjectTypeTest.java is doing. Also, did getSourceFileName() return null for arrays before your change? dl On 4/22/20 8:18 AM, coleen.phillim...@oracle.com wrote: Hi Dean, Thank you for

RFR: 8243450: Reomve VMOps from jdk.hotspot.agent

Hi all, Please review removing file from jdk.hotspot.agent: JBS: https://bugs.openjdk.java.net/browse/JDK-8243450 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8243450/webrev.00/ VMOps is enum class which lists all of VM Operation, but it has not been synced with HotSpot source. In

Re: RFR: 8243450: Reomve VMOps from jdk.hotspot.agent

Hi Yasumasa, On 23/04/2020 12:22 pm, Yasumasa Suenaga wrote: Hi all, Please review removing file from jdk.hotspot.agent:   JBS: https://bugs.openjdk.java.net/browse/JDK-8243450   webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8243450/webrev.00/ VMOps is enum class which lists all of VM

Re: RFR: 8243450: Reomve VMOps from jdk.hotspot.agent

Thanks Chris and David for quick review! Yasumasa On 2020/04/23 11:22, Yasumasa Suenaga wrote: Hi all, Please review removing file from jdk.hotspot.agent:   JBS: https://bugs.openjdk.java.net/browse/JDK-8243450   webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8243450/webrev.00/ VMOps

Re: RFR(L): 8215624: add parallel heap inspection support for jmap histo(G1)(Internet mail)

Thanks Paul! I agree with using "parallel", will make the update in next patch, Thanks for help update the CSR. BRs, Lin On 2020/4/23, 4:42 AM, "Hohensee, Paul" wrote: For the interface, I'd use "parallel" instead of "parallelThreadNum". All the other options are lower case, and it's

Re: RFR (M) Close alignment gaps in InstanceKlass

It looks like calling the JVMCI getSourceFileName() on an array would have accessed random memory because it was expecting an InstanceKlass.  Instead of returning null we might want to throw an exception like in HotSpotResolvedPrimitiveType. dl On 4/22/20 5:39 PM, Dean Long wrote: Can you

Re: RFR(L): 8215624: add parallel heap inspection support for jmap histo(G1)

Hi Lin, I took a look at this earlier and saw that the heap inspection code is strongly coupled with the CollectedHeap and G1CollectedHeap. I'd prefer if we'd abstract this away, so that the GCs only provide a "parallel object iteration" interface, and the heap inspection code is kept

Re: RFR(L): 8215624: add parallel heap inspection support for jmap histo(G1)(Internet mail)

Dear Stefan, Thanks a lot! I agree with you to decouple the heap inspection code with GC's. I will start from your POC code, may discuss with you later. BRs, Lin On 2020/4/22, 5:14 PM, "Stefan Karlsson" wrote: Hi Lin, I took a look at this earlier and

RFR: 8238561 serviceability/sa tests continue to run out of memory on Win* machines

Please review the change [1] that ensures that VM and test options are forwarded to j*-tools when they are launched from serviceability/sa tests. In particular, it will ensure that passed to the tests maximum heap size settings ( -XX:MaxRAMPercentage) are also honored by j*-tools

Re: RFR(XS) 8243210: ClhsdbScanOops fails with NullPointerException in FileMapHeader.inCopiedVtableSpace

Hi Chris, the change looks good to me, too. Thanks - Ioi On 4/21/20 6:35 PM, serguei.spit...@oracle.com wrote: Hi Chris, It looks reasonable. Thanks, Serguei On 4/21/20 16:07, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8243210

RFR (L): 8241214: Test debugging of hidden classes using jdb

Please, review a fix for the sub-task:   https://bugs.openjdk.java.net/browse/JDK-8241214 This fix has already been reviewed internally (in Vlahalla project) by Mandy, Chris and Alex. This RFR is to collect more comments (if there are any) before

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

Hi Yasumasa, Yes, it looks like VMOps in SA can be removed. It's probably bit rotted code. My guess is at some point there was support, or were plans for supporting, the debugging of VMOps from within SA. Given that there are no references to the VMOps class, it looks like none of that

Re: RFR (L): 8241214: Test debugging of hidden classes using jdb

Looks good, Here are several comments, mostly about code style/compliance. No another review is needed if you want to fix them. http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/valhalla-hidden-jdb.7/test/hotspot/jtreg/vmTestbase/nsk/jdb/hidden_class/hc001/hc001.java.html 36 interface

Re: RFR (L): 8241214: Test debugging of hidden classes using jdb

Hi Leonid, Thank you for the review! I'll address your comments. Thanks, Serguei On 4/22/20 13:06, Leonid Mesnik wrote: Looks good, Here are several comments, mostly about code style/compliance. No

RE: RFR(L): 8215624: add parallel heap inspection support for jmap histo(G1)(Internet mail)

For the interface, I'd use "parallel" instead of "parallelThreadNum". All the other options are lower case, and it's a lot easier to type "parallel". I took the liberty of updating the CSR. If you're ok with it, you might want to change variable names and such, plus of course JMap.usage.

Re: RFR(XS) 8243210: ClhsdbScanOops fails with NullPointerException in FileMapHeader.inCopiedVtableSpace

Thanks Ioi and Serguei! Chris On 4/22/20 11:42 AM, Ioi Lam wrote: Hi Chris, the change looks good to me, too. Thanks - Ioi On 4/21/20 6:35 PM, serguei.spit...@oracle.com wrote: Hi Chris, It looks reasonable. Thanks, Serguei On 4/21/20 16:07, Chris Plummer wrote: Hello, Please review

Re: RFR: 8238561 serviceability/sa tests continue to run out of memory on Win* machines

Hi Daniil, Thanks for cleaning this up. I think this should be fixed under JDK-8242009. JDK-8238561 involves more than just this one issue. Is there a reason why you didn't just change JDKToolLauncher to have an option or API to add the args? Why are you calling Utils.addTestJavaOpts()