RFR (M) JDK-6479360 - detailed dumping of class size statistics

[Ioi Lam]

Hi Karen,

Thanks for the review. Please see my comments in-line

On Jan 22, 2013, at 2:07 PM, Karen Kinnear <karen.kinnear at oracle.com> wrote:

> Ioi,
> 
> Thank you for doing this. I like the way you have the size collection in the file
> with the metadata definitions.
> 
> 1. diagnosticCommand.hpp line 200 - inspeactheap -> inspectheap
> 
> 2. please update copyrights on all files you changed to 2013
> 
> 3. I sent an email to the embedded folks to find out if they want this
>    under INCLUDE_SERVICES.
>    If they do ...
> 
>    - did you build with and without INCLUDE_SERVICES?
>    What happens when you build with it off and run the jcmd ?
>    I am confused that in diagnosticCommand.cpp ClassStatsDcmd is available without
>    INCLUDE_SERVICES, but the actual details below the collect_statistics are not?
>    Did you talk to the embedded team about whether they want this or not?
> 

I did test without INCLUDE_SERVICE -- this was done by building JPRT on the hotspotwest, queue, which includes all the embedded SE builds (x86, ARM, PPC, etc) which turn off INCLUDE_ SERVICES.


>    - I would recommend that include files also add the new methods conditional
>   on INCLUDE_SERVICES - you did that in some of them, but I think you missed:
>   method.hpp, methodData.hpp, constantPool.hpp, constMethod.hpp, annotations.hpp

Will do. I was trying to avoid clutter in the header files, but I think it's better to explicitly mark them by #ifdefs.

> 4. heapInspection.hpp line 56: is InstBytesi supposed to be InstBytes?
> 
> 5. heapInspection.hpp line 122 - you have some funny "\" - is that intentional?
> 
> 6. INT64_FORMAT: I think Harold just put back a change for julong handling
> that switched to using JULONG_FORMAT- see KlassInfoHisto::print_class_stats - 2 places
> (to ensure this also works on Mac OS/X)
> 

I will double check these and fix as necessary.

Thanks.

Ioi

> thanks,
> Karen
> 
> On Jan 16, 2013, at 7:39 PM, Ioi Lam wrote:
> 
>> Please review:
>>     http://cr.openjdk.java.net/~acorn/class_stats_010/
>> 
>> Bug: RFE: PrintClassHistogram improvements
>>     https://jbs.oracle.com/bugs/browse/JDK-6479360
>> 
>> Sponsor: Karen Kinnear
>> 
>> Summary:
>> 
>> A new diagnostic command "jcmd GC.class_stats" is added. The user
>> can invoke this command to connect to a live JVM and dump a detailed
>> report of size statistics of all loaded classes (including array
>> classes and anonymous classes).
>> 
>>     ==========SYNOPSIS===================================
>>     $ jcmd $PID help GC.class_stats
>>     Provide statistics about Java class meta data.
>>     Impact: High: Depends on Java heap size and content.
>> 
>>     Syntax : GC.class_stats [options] [<columns>]
>> 
>>     Arguments:
>> 	columns : [optional] Comma-separated list of all the columns to
>>                   show. If not specified, the following columns are shown:
>>                   InstBytes,KlassBytes,CpAll,annotations,MethodCount,Bytecodes,
>>                   MethodAll,ROAll,RWAll,Total (STRING, no default value)
>> 
>>     Options: (options must be specified using the <key> or <key>=<value> syntax)
>> 	-all : [optional] Show all columns (BOOLEAN, false)
>> 	-csv : [optional] Print in CSV (comma-separated values) format for 
>>                 spreadsheets (BOOLEAN, false)
>> 	-help : [optional] Show meaning of all the columns (BOOLEAN, false)
>>     ======================================================
>> 
>> By default, the output is human-readable tabulated text format. The
>> user can also select CSV format (comma-separated values) to be
>> used with spreadsheets.
>> 
>> A large number of columns are available. By default, a few "summary 
>> columns" (e.g., size of Klass objects, total read-only data, etc) 
>> are printed. The user can also manually select the columns.
>> 
>> Please see the attachments in the bug for sample output, as well as
>> a listing of all the columns and their meanings:
>> 
>> https://jbs.oracle.com/bugs/browse/JDK-6479360?focusedCommentId=13290360&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13290360
>> 
>> 
>> 
>> Impact:
>> 
>> If this diagnostic command is not used, there should be no
>> impact to the JVM's execution, except for
>> 
>>    + libjvm.so footprint increase (about 10KB larger on Linux/x64)
>>    + one additional virtual method in Klass.
>> 
>> This feature is excluded when INCLUDE_SERVICES=0 (e.g., 
>> embedded builds).
>> 
>> 
>> 
>> Tests run:
>> 
>> + JPRT -- (hotspot only) to verify build-ability
>> 
>> + Manually ran "jcmd $PID help GC.class_stats" on Linux/x64
>> 
>> + I intend to add a new testcase once this is committed:
>>   https://jbs.oracle.com/bugs/browse/JDK-8006413
>>   Add utility classes for writing better multiprocess tests in jtreg
>> 
>> Thanks,
>> Ioi
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130122/3cc3976a/attachment.html