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

Coleen Phillimore coleen.phillimore at oracle.com
Wed Jan 23 17:24:24 PST 2013 

In vmGCOperations.cpp the type HeapInspection inspect looks like the 
only initialization of these fields is in the set functions.

It would be cleaner to declare the constructor to take the initial 
values in the declaration, ie:

     HeapInspection inspect(_csv_format, _print_help, 
_print_class_stats, _colmns);

and then you can delete the setter functions.

The rest of the code looks good.
Coleen

On 1/23/2013 6:59 PM, Karen Kinnear wrote:
> Ioi,
>
> Thanks for finding that - I had missed that. That makes a lot of 
> sense. Thank you.
> So - you have my approval for checking this in. You need one more code 
> reviewer.
>
> thanks,
> Karen
>
> On Jan 23, 2013, at 6:57 PM, Ioi Lam wrote:
>
>> Karen,
>>
>> Thanks for the review.
>>
>> hotspot/make/excludeSrc.make already excludes the entire file if 
>> INCLUDE_SERVICES=0. I verified that heapInspection.o does not exist 
>> for the "linux_i486_minimal1" build, which has INCLUDE_SERVICES=0.
>>
>> ifeq ($(INCLUDE_SERVICES), false)
>>       CXXFLAGS += -DINCLUDE_SERVICES=0
>>       CFLAGS += -DINCLUDE_SERVICES=0
>>
>>       Src_Files_EXCLUDE += heapDumper.cpp heapInspection.cpp \
>>     attachListener_linux.cpp attachListener.cpp
>> endif
>>
>> - Ioi
>>
>> On 01/23/2013 03:31 PM, Karen Kinnear wrote:
>>> Ioi,
>>>
>>> Thank you for the cleanups. Looks good. Ok to check in.
>>>
>>> Or - if you are up to adding INCLUDE_SERVICES around a bit more -
>>>
>>> including heapInspection.cpp/hpp (at least that entire table of
>>> strings) and print_class_stats and the call to it. The goal is to 
>>> reduce memory
>>> usage when not needed.
>>>
>>> And thank you for testing with that flag off.
>>>
>>> thanks,
>>> Karen
>>>
>>> (p.s. you might ask Jiangli for a code review)
>>>
>>> On Jan 23, 2013, at 5:18 PM, Ioi Lam wrote:
>>>
>>>> Thanks to Karen for the feedback. I have updated the webrev:
>>>>
>>>> http://cr.openjdk.java.net/~iklam/6479360/class_stats_011/
>>>>
>>>> Changes since last time
>>>>
>>>>     + Cleaned up typos found by Karen
>>>>
>>>>     + Use JULONG_FORMAT. Remove all uses of PRIu64 and
>>>>       UINT64_FORMAT which may be unportable on MacOS (see 7102489).
>>>>
>>>>     + Updated copyright to 2013
>>>>
>>>>     + Added more #if INCLUDE_SERVICES. GC.class_histogram and
>>>> GC.class_stats are no longer accessible by jcmd if
>>>> INCLUDE_SERVICES=0.
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>>>
>>>> On 01/22/2013 02:07 PM, Karen Kinnear 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 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
>>>>>
>>>>>
>>>>> 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)
>>>>>
>>>>> 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/20130123/93323520/attachment.html

More information about the hotspot-runtime-dev mailing list