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

[Zhengyu Gu]

Sorry for replying late. This CR actually is less about class state, but 
about the accuracy between the number of classes vs. size of class metadata.

Current implementation uses the number of  entries in system dictionary 
for the number of classes, which is inaccurate, which it does not 
contain anonymous classes and can have multiple entries

Thanks,

-Zhengyu

On 12/18/2012 8:41 PM, Vitaly Davidovich wrote:
>
> 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 
> <mailto: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>
>         <mailto:david.holmes at oracle.com
>         <mailto: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>
>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>>
>         <mailto:david.holmes at oracle. <mailto:david.holmes at oracle.>__com
>         <mailto:david.holmes at oracle.com
>         <mailto: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/%7E____zgu/8005048/webrev.00/>
>         <http://cr.openjdk.java.net/~__zgu/8005048/webrev.00/
>         <http://cr.openjdk.java.net/%7E__zgu/8005048/webrev.00/>>
>         <http://cr.openjdk.java.net/~__zgu/8005048/webrev.00/
>         <http://cr.openjdk.java.net/%7E__zgu/8005048/webrev.00/>
>         <http://cr.openjdk.java.net/~zgu/8005048/webrev.00/
>         <http://cr.openjdk.java.net/%7Ezgu/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
>