> On 27 Oct 2016, at 20:15, David Holmes <david.hol...@oracle.com> wrote: > > On 27/10/2016 6:36 PM, 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. > > Right - it doesn't make sense to have to use UnlockDiagnosticVMOptions to run > any diagnostic Dcmd. Otherwise it should be a requirement for all. > >> 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. > > The only reason a Dcmd should require a specific VM option is if the Dcmd > will not be able to function unless the VM was started with that option - > IMHO :) Is that the case for LogTouchedMethods?
I believe LogTouchedMethods stores a long list of all methods ever being run. We don’t want to have that enabled by default. It would maybe be a good future enhancement to be able to turn this on and off? > > Thanks, > David > >> 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/ >> >> Thanks, >> /Staffan >>
