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

Wed Jan 23 15:59:57 PST 2013

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/700378b3/attachment.html 