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

[Karen Kinnear]

Sorry I missed this. Good points Coleen about catching anonymous classes.

So I should have been clearer - the goal is to print the information about the number of classes for which
there is metadata using space.

So if we were to increment when the IK is allocated and decrement when the IK is freed, this would be 
the accuracy that we are looking for.

I believe that would also cover the anonymous classes - right?

So if you don't want to call it "defined" we can call it "allocated" - would that be clearer?

thanks,
Karen

On Dec 19, 2012, at 3:41 PM, Coleen Phillimore wrote:

> 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
>