Code review request: JDK-8005048,NMT: #loaded classes needs to just show the # defined classes

[David Holmes]

On 19/12/2012 12:43 AM, Zhengyu Gu wrote:
> Hi David,
>
> Thanks for reviewing.
>
> On 12/17/2012 9:41 PM, David Holmes wrote:
>> Hi Zhengyu,
>>
>> On 18/12/2012 6:11 AM, Zhengyu Gu wrote:
>>> Current NMT implementation reports number of loaded classes at query
>>> time, but number of defined classes is what is expected.
>>>
>>> This changset reflects two major changes:
>>>
>>> 1. It counts number of defined classes vs. number of loaded classes
>>
>> Can you explain what the distinction is please? I don't know what a
>> "defined class" is.
>>
> "defined class" refers to class definition (InstanceKlass), "loaded
> class" refers to instance.

Sorry that didn't help. I don't understand what these two different 
things are (InstanceKlass vs. "instance" ???). Can you explain in terms 
of Java level class loading - when is a class "defined" versus "loaded"?

Thanks,
David

>
>>> 2. It counts number of defined classes for each generation, vs. counts
>>> at query time. In this way, the number of defined classes that NMT
>>> reports, should match the corresponding class metadata data. As the
>>> result, the data should be more accurate.
>>>
>>>
>>> Webrev: http://cr.openjdk.java.net/~zgu/8005048/webrev.00/
>>
>> I think we could benefit from NMT_ONLY(x) macros to get rid of the
>> one-line conditional blocks eg in instanceKlass.cpp
> After consulting Chris Plummer, it does not have to exclude for
> embedded. But yes, have NMT_ONLY() macros will be very helpful, I will
> keep this in mind.
>
>>
>> ---
>>
>> src/share/vm/oops/instanceKlass.hpp
>>
>> + #if INCLUDE_NMT
>> + static int number_of_instance_classes() { return
>> (int)_total_instanceKlass_count; }
>> +
>> + private:
>> + static volatile jint _total_instanceKlass_count;
>> + #endif
>>
>> Why are we mixing int and jint here?
> Yes, it is bad idea. Will change to "int"
>
> Thanks,
>
> -Zhengyu
>>
>> ---
>>
>> I can't comment on the detailed memory management of the GenerationData.
>>
>> David
>> -----
>>
>>
>>>
>>> Thanks,
>>>
>>> -Zhengyu