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

[Vitaly Davidovich]

You sure define is last step? Even classloader javadoc says it still needs
to be resolved (linked and verified, I presume).

Sent from my phone
On Dec 18, 2012 7:59 PM, "David Holmes" <david.holmes at oracle.com> wrote:

> On 19/12/2012 10:54 AM, Vitaly Davidovich wrote:
>
>> David,
>>
>> I suspect this is referring to the fact that InstanceKlass (VM
>> representation of java.lang.Class) can be in several states:
>> defined/allocated, loaded, linked/verified, etc.
>>
>
> Okay. It would be good to be clear on that.
>
>  Perhaps Classloader.define() and loadClass() are the classloader level
>> distinctions you're interested in?
>>
>
> define() is the final step of loadClass(): first find the bytes for the
> class then send them to the VM via defineClass to actually load the class.
> "define" is not a state at the Java level.
>
> Thanks,
> David
>
>
>> Sent from my phone
>>
>> On Dec 18, 2012 6:30 PM, "David Holmes" <david.holmes at oracle.com
>> <mailto:david.holmes at oracle.**com <david.holmes at oracle.com>>> wrote:
>>
>>     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/<http://cr.openjdk.java.net/~__zgu/8005048/webrev.00/>
>>                 <http://cr.openjdk.java.net/~**zgu/8005048/webrev.00/<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
>>
>>