[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(S): 8168305 GC.class_stats should not require -XX:+UnlockDiagnosticVMOptions
From:       Staffan Larsen <staffan.larsen () oracle ! com>
Date:       2016-10-28 6:41:13
Message-ID: B487BF26-E165-4221-80FF-E05ABBA26A5B () oracle ! com
[Download RAW message or body]

Thanks Ioi!

> On 27 Oct 2016, at 19:21, Ioi Lam <ioi.lam@oracle.com> wrote:
> 
> Looks good.
> 
> Thanks
> - Ioi
> 
> On 10/27/16 2:41 AM, Marcus Larsson wrote:
> > Hi Staffan,
> > 
> > 
> > On 2016-10-27 10:36, Staffan Larsen wrote:
> > > All,
> > > 
> > > Please review this small patch to remove the requirement \
> > > -XX:+UnlockDiagnosticVMOptions when running the GC.class_stats diagnostic \
> > > command. Diagnostic commands are used for diagnosing problems and should not \
> > > require restarting the JVM with a different command line flag if this can be \
> > > avoided. 
> > > While fixing this I also noticed that VM.print_touched_methods said it required \
> > > -XX:+UnlockDiagnosticVMOptions, while what it really required was \
> > > -XX:+LogTouchedMethods (which in turn requires -XX:+UnlockDiagnosticVMOptions) \
> > > so I changed the error message here. In this case I think it is ok to require a \
> > > special command line flag since collecting the information comes with an \
> > > overhead. 
> > > I have verified the fix manually and by running these tests: \
> > > hotspot/test/runtime/CommandLine/PrintTouchedMethods.java \
> > > hotspot/test/serviceability/sa/TestInstanceKlassSize.java \
> > > hotspot/test/serviceability/sa/TestInstanceKlassSizeForInterface.java 
> > > bug: https://bugs.openjdk.java.net/browse/JDK-8168305
> > > webrev: http://cr.openjdk.java.net/~sla/8168305/webrev.01/
> > 
> > The command description still mentions the flag, see
> > src/share/vm/services/diagnosticCommand.hpp:389
> > 
> > Apart from that this looks good to me!
> > 
> > Thanks,
> > Marcus
> > 
> > > 
> > > Thanks,
> > > /Staffan
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]