> On 27 Oct 2016, at 11:41, Marcus Larsson <marcus.lars...@oracle.com> 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

Well spotted!

new webrev: http://cr.openjdk.java.net/~sla/8168305/webrev.02/ 
<http://cr.openjdk.java.net/~sla/8168305/webrev.02/>


> Apart from that this looks good to me!

Thanks.

> 
> Thanks,
> Marcus
> 
>> 
>> Thanks,
>> /Staffan

Reply via email to