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

[Vitaly Davidovich]

OK yeah, that makes sense.  Looking at oops/InstanceKlass.hpp, it looks
like there's a precursor state(ClassState::allocated) in the VM.

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

> On 19/12/2012 11:05 AM, Vitaly Davidovich wrote:
>
>> You sure define is last step? Even classloader javadoc says it still
>> needs to be resolved (linked and verified, I presume).
>>
>
> Sorry I was being imprecise. After class definition the class is loaded
> but may still need to be resolved (linked).
>
> A loaded Class at the Java level can be in one of three states:
> - loaded, unlinked, uninitialized
> - loaded, linked, uninitialized
> - loaded, linked, initialized
>
> Thanks,
> David
>
>  Sent from my phone
>>
>> On Dec 18, 2012 7:59 PM, "David Holmes" <david.holmes at oracle.com
>> <mailto:david.holmes at oracle.**com <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>>
>>         <mailto: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/>
>> >
>>         <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
>>
>>