RFR (M) JDK-6479360 - detailed dumping of class size statistics
Ioi Lam
ioi.lam at oracle.com
Thu Jan 24 11:52:10 PST 2013
Hi,
I have updated the patch:
http://cr.openjdk.java.net/~iklam/6479360/class_stats_012/
[1] Removed FOREACH_COLUMN and SELECTED per Harold
[2] Modified HeapInspection constructor per Coleen.
[3] Renamed a few columns in the output to match the capitalizations
in source code:
Constmethod -> ConstMethod
Methoddata -> MethodData, etc
Thanks!
- Ioi
On 01/24/2013 08:34 AM, harold seigel wrote:
> Hi Ioi,
>
> I think we want to get away from using lots of macros, such as these
> that were added to heapinspection.cpp:
>
> +#define FOREACH_COLUMN(c) \
> + for (int c=0; c<KlassSizeStats::_num_columns; c++)
> +
> +#define SELECTED(c) selected_columns_table[c]
>
> Would you consider removing them?
>
> Thanks, Harold
>
> On 1/23/2013 8:24 PM, Coleen Phillimore wrote:
>>
>> 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/20130124/0d12d689/attachment.html
More information about the hotspot-runtime-dev
mailing list