RFR(S) 8193184: NMT: Report array class count in NMT summary

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Mon Feb 5 22:17:22 UTC 2018


Yes, this looks good.  A couple of small issues.  Where you include 
classLoaderData.inline.hpp, you don't have to include 
classLoaderData.hpp because it's already included in the inlne.hpp file.
Also, you are supposed to put the Oracle copyright at the top before the 
RedHat one in the test.

When you get another reviewer, I'll sponsor it for you.
thanks!
Coleen

On 2/5/18 5:11 PM, Zhengyu Gu wrote:
> Hi Coleen,
>
> Thanks for the review and suggestions.
>
> Webrev is updated according to your comments.
>
> Webrev: http://cr.openjdk.java.net/~zgu/8193184/webrev.01/index.html
>
> Reran hotspot_runtime test on Linux 64 (fastdebug and release)
>
> -Zhengyu
>
>
> On 02/05/2018 01:15 PM, coleen.phillimore at oracle.com wrote:
>>
>> Hi Zhengyu,
>>
>> This looks really good.  The instanceKlass count does seem to belong 
>> to ClassLoaderData.
>>
>> http://cr.openjdk.java.net/~zgu/8193184/webrev.00/src/hotspot/share/classfile/classLoaderData.hpp.udiff.html 
>>
>>
>> + static void inc_instance_classes(size_t count) {
>> + Atomic::add(count, &_num_instance_classes);
>> + }
>> +
>> + static void dec_instance_classes(size_t count) {
>> + assert(count <= _num_instance_classes, "Sanity");
>> + Atomic::sub(count, &_num_instance_classes);
>> + }
>> +
>> + static void inc_array_classes(size_t count) {
>> + Atomic::add(count, &_num_array_classes);
>> + }
>> +
>> + static void dec_array_classes(size_t count) {
>> + assert(count <= _num_array_classes, "Sanity");
>> + Atomic::sub(count, &_num_array_classes);
>> + }
>>
>>
>> Can you move these functions to classLoaderData.inline.hpp?
>>
>> Maybe you should add an NMT test for this to verify the output?
>>
>> Thanks,
>> Coleen
>>
>>
>> On 2/2/18 3:28 PM, Zhengyu Gu wrote:
>>> Please review this patch that adds array class count to NMT report, 
>>> so the class count can closely match the output from GC.class_stats.
>>>
>>> For example:
>>> Summary:
>>> -                     Class (reserved=1069781KB, committed=1069269KB)
>>>                             (classes #3362)
>>>                             (  instance classes# 3105, array 
>>> classes#257)
>>>
>>> Summary diff:
>>> -                     Class (reserved=1071843KB +2061KB, 
>>> committed=1070051KB +781KB)
>>>                             (classes #3405 +43)
>>>                             (  instance classes# 3143 +38, array 
>>> classes#262 +5)
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8193184
>>> Webrev: http://cr.openjdk.java.net/~zgu/8193184/webrev.00/
>>>
>>> Test:
>>>   hotspot_tier1_runtime on Linux 64 (fastdebug and release)
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>



More information about the hotspot-runtime-dev mailing list