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

[yumin qi]

Hi, Zhengyu

  Looks good.

Thanks
Yumin
 (open id: minqi)


On Mon, Feb 5, 2018 at 5:45 PM, Zhengyu Gu <zgu at redhat.com> wrote:

>
> Updated: http://cr.openjdk.java.net/~zgu/8193184/webrev.02/
>
> Thanks,
>
> -Zhengyu
>
>
> On 02/05/2018 05:17 PM, coleen.phillimore at oracle.com wrote:
>
>>
>> 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/hotspo
>>>> t/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
>>>>>
>>>>>
>>>>
>>