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

[Coleen Phillimore]

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.

I think what the bug is complaining about is that NMT returned the 
number of entries in the SystemDictionary which includes entries for 
classes for their initiating loader.   So for each VM representation of 
class, there were several entries, and only one is useful for counting 
how many classes were loaded (where class loader of the dictionary entry 
matches the class loader for the Klass*).   See code in 
systemDictionary.cpp classes_do().

When the class is parsed, the state is set to InstanceKlass::allocated.  
Right before the class is added to the system dictionary, it's changed 
to InstanceKlass::loaded, it's linked and initialized later.

Zhengyu's change counts the classes at the point where they are 
allocated.   So he's actually counting the allocated classes before they 
are loaded.  This is a relatively small window, though.   It's probably 
better to move the atomic add of this counter to the point where the 
class is added to the system dictionary (dictionary()->add_klass()) and 
remove it when the class is unloaded.   Note that the counter enables 
NMT not to lock the system dictionary to count the entries.

This change as is misses deleting anonymous classes, but counts adding 
them, since they are not added to the system dictionary. That count 
atomic add should be added to SystemDictionary::parse_stream() and 
decremented in ClassLoaderData::unload() for the anonymous class case.

Thanks,
Coleen

>
>> 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
>
> ---
>
> 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?
>
> ---
>
> I can't comment on the detailed memory management of the GenerationData.
>
> David
> -----
>
>
>>
>> Thanks,
>>
>> -Zhengyu