Re: RFR(S) 8242162: convert clhsdb "sysprops" command from javascript to java

2020-04-07 Thread serguei.spit...@oracle.com
Chris, Refactored version looks nice. I was thinking to suggest the same in previous review but decided there is not a big profit for small blocks of code. But it is obvious now that it is clearly better. :) Thanks, Serguei On 4/7/20 22:15, Chris Plummer wrote: Hello, Please review the fol

Re: RFR(XS) 8242265: serviceability/sa/ClhsdbScanOops.java fails due to bad @requires expression

2020-04-07 Thread Leonid Mesnik
Looks good, thank you for fixing this. Leonid > On Apr 7, 2020, at 9:57 PM, Chris Plummer wrote: > > Hello, > > Please review the following: > > https://bugs.openjdk.java.net/browse/JDK-8242265 > > diff --git a/test/hotspot/jtreg/serviceability/sa/ClhsdbScanOops.java > b/test/hotspot/jtreg/s

Re: RFR(XS) 8242265: serviceability/sa/ClhsdbScanOops.java fails due to bad @requires expression

2020-04-07 Thread serguei.spit...@oracle.com
It looks good and trivial. Thanks, Serguei On 4/7/20 21:57, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8242265 diff --git a/test/hotspot/jtreg/serviceability/sa/ClhsdbScanOops.java b/test/hotspot/jtreg/serviceability/sa/ClhsdbScanOops.

Re: RFR(M) 8240990: convert clhsdb "dumpclass" command from javascript to java

2020-04-07 Thread serguei.spit...@oracle.com
Hi Chris, It looks good to me. Thanks, Serguei On 4/7/20 20:12, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8240990 http://cr.openjdk.java.net/~cjplummer/8240990/webrev.00 The javascript code was just a few lines like other recent comman

RFR(S) 8242162: convert clhsdb "sysprops" command from javascript to java

2020-04-07 Thread Chris Plummer
Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8242162 http://cr.openjdk.java.net/~cjplummer/8242162/webrev.00 http://cr.openjdk.java.net/~cjplummer/8242162/webrev.01 Note there are two webrevs. You can skip the first if you want. It's the initial straight forward

RFR(XS) 8242265: serviceability/sa/ClhsdbScanOops.java fails due to bad @requires expression

2020-04-07 Thread Chris Plummer
Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8242265 diff --git a/test/hotspot/jtreg/serviceability/sa/ClhsdbScanOops.java b/test/hotspot/jtreg/serviceability/sa/ClhsdbScanOops.java --- a/test/hotspot/jtreg/serviceability/sa/ClhsdbScanOops.java +++ b/test/hotsp

RFR(M) 8240990: convert clhsdb "dumpclass" command from javascript to java

2020-04-07 Thread Chris Plummer
Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8240990 http://cr.openjdk.java.net/~cjplummer/8240990/webrev.00 The javascript code was just a few lines like other recent commands, but it had quite a bit of support on the java side in JSJavaScriptEngine.dumpClass(

Re: RFR: 8242295: Move files from vmTestbase/nsk/monitoring/ThreadMBean into ThreadMXBean

2020-04-07 Thread Igor Ignatyev
JFYI: I also remembered that I did file an RFE for this renaming -- https://bugs.openjdk.java.net/browse/JDK-8208245 . and as it seems that your patch addresses it fully, I'll close 8208245 as a dup of 8242295. -- Igor > On Apr 7, 2020, at 4:4

Re: RFR: 8242295: Move files from vmTestbase/nsk/monitoring/ThreadMBean into ThreadMXBean

2020-04-07 Thread Leonid Mesnik
Thank you for review. Filed https://bugs.openjdk.java.net/browse/JDK-8242328 Leonid On 4/7/20 4:31 PM, Igor Ignatyev wrote: On Apr 7, 2020, at 4:26 PM, Leonid Mesnik > wrote: I didn't want to touch unrelated files and spread this fix outside vmTestbase/ns

Re: RFR: 8242295: Move files from vmTestbase/nsk/monitoring/ThreadMBean into ThreadMXBean

2020-04-07 Thread Igor Ignatyev
> On Apr 7, 2020, at 4:26 PM, Leonid Mesnik wrote: > > I didn't want to touch unrelated files and spread this fix outside > vmTestbase/nsk/monitoring/ThreadMBean package. I wanted just to merge folders > to don't confuse people by layout. > > The ThreadMBean is mentioned also in > > test/j

Re: RFR: 8242295: Move files from vmTestbase/nsk/monitoring/ThreadMBean into ThreadMXBean

2020-04-07 Thread Leonid Mesnik
I didn't want to touch unrelated files and spread this fix outside vmTestbase/nsk/monitoring/ThreadMBean package. I wanted just to merge folders to don't confuse people by layout. The ThreadMBean is mentioned also in test/jdk/sun/management/jmxremote/bootstrap/ files and src/jdk.management.a

Re: RFR: 8242295: Move files from vmTestbase/nsk/monitoring/ThreadMBean into ThreadMXBean

2020-04-07 Thread Igor Ignatyev
great! what about nsk/monitoring/stress/thread/ ? they all have lines 'and states gotten via the ThreadMBean interface.' (strace010.java has 3 occurrences, other files just one) -- Igor > On Apr 7, 2020, at 3:56 PM, Leonid Mesnik wrote: > > Sure > > I've updated ThreadMBean to ThreadMXBea

Re: RFR: 8242295: Move files from vmTestbase/nsk/monitoring/ThreadMBean into ThreadMXBean

2020-04-07 Thread Leonid Mesnik
Sure I've updated ThreadMBean to ThreadMXBean in test descriptions for ThreadMXBean tests . http://cr.openjdk.java.net/~lmesnik/8242295/webrev.01/ Leonid On 4/7/20 3:00 PM, Igor Ignatyev wrote: Hi Leonid, looks good and trivial to me. one question, will it also make sense to replace Threa

RFR (XS): 8242241: add assert to ClassUnloadEventImpl::className

2020-04-07 Thread serguei.spit...@oracle.com
Please, review a fix for minor JDI enhancement:   https://bugs.openjdk.java.net/browse/JDK-8242241 Webrev:   http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/8242241-jdi-add-assert.1/ Summary:   A useful assert is introduced to check the class

Re: RFR: 8242295: Move files from vmTestbase/nsk/monitoring/ThreadMBean into ThreadMXBean

2020-04-07 Thread Igor Ignatyev
Hi Leonid, looks good and trivial to me. one question, will it also make sense to replace ThreadMBean w/ ThreadMXBean in test descriptions, e.g. at L#33 of test/hotspot/jtreg/vmTestbase/nsk/monitoring/ThreadMXBean/isCurrentThreadCpuTimeSupported/curthcputime001/TestDescription.java? Thanks, --

RFR: 8242295: Move files from vmTestbase/nsk/monitoring/ThreadMBean into ThreadMXBean

2020-04-07 Thread Leonid Mesnik
Hi Could you please review following fix which just moves content of ThreadMBean back into ThreadMXBean. webrev: http://cr.openjdk.java.net/~lmesnik/8242295/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8242295

Re: RFR(M) 8242165: SA sysprops support fails to dump all system properties

2020-04-07 Thread Chris Plummer
Since JDK-8193639 has now been pushed, this test no longer needs to be problem listed on solaris, so I undid that change to ProblemList.txt. However, it does fail on with ZGC due to JDK-8220624, so it is now problem listed in ProblemList-zgc.txt. I also fixed a minor sort order in the problem l

Re: RFR(M) 8242165: SA sysprops support fails to dump all system properties

2020-04-07 Thread serguei.spit...@oracle.com
Hi Chris, I'm okay with your approaches. Please, consider it reviewed. Thanks, Serguei On 4/7/20 00:17, Chris Plummer wrote: Hi Serguei, Thanks for the review. I considered a refactoring like that. However, I'm going to expand this test when I push my fix to add support for the clhsdb "sysp

Re: RFR(M) 8242165: SA sysprops support fails to dump all system properties

2020-04-07 Thread Chris Plummer
Hi Serguei, Thanks for the review. I considered a refactoring like that. However, I'm going to expand this test when I push my fix to add support for the clhsdb "sysprops" commands (one of the clhsdb commands that was lost when we lost JS support). So I'll be executing that command also in thi

Re: RFR(M) 8242165: SA sysprops support fails to dump all system properties

2020-04-07 Thread serguei.spit...@oracle.com
Hi Chris, It looks good in general. But a couple of comments. http://cr.openjdk.java.net/~cjplummer/8242165/webrev.03/test/hotspot/jtreg/serviceability/sa/TestSysProps.java.html I'm thinking if it'd make sense to do this kind of refactoring:     OutputAnalyzer runSACommand(String cmd, String a