RFR 8003421: NPG: Move oops out of InstanceKlass into mirror

Stefan Karlsson stefan.karlsson at oracle.com
Tue May 21 02:11:14 PDT 2013


Hi Coleen,

Good to see all these oops moving to the mirrors. I think the changes 
look good. I let someone else review the SA changes

Some comments below:

On 05/21/2013 12:39 AM, Coleen Phillimore wrote:
> Summary: Inject protection_domain, signers, init_lock into 
> java_lang_Class
>
> Net footprint change is zero except that these fields are in Java heap 
> rather than metaspace.

There should be some memory saved since we now use compressed oops for 
the embedded fields.

> This helps a little with InstanceKlass size which is in fixed size 
> space with UseCompressedKlassPointers. Included serviceability because 
> there were SA changes to code that I don't know is used.
>
> Future work is to remove the signers field and the unused 
> SetProtectionDomain function.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8003421/
> bug link at http://bugs.sun.com/view_bug.do?bug_id=8003421

http://cr.openjdk.java.net/~coleenp/8003421/src/share/vm/oops/instanceKlass.hpp.patch

    // protection domain
-  oop protection_domain()                  { return _protection_domain; }
-  void set_protection_domain(oop pd)       { klass_oop_store(&_protection_domain, pd); }
+  oop protection_domain() const;
+  void set_protection_domain(Handle pd);
...
    // signers
-  objArrayOop signers() const              { return _signers; }
-  void set_signers(objArrayOop s)          { klass_oop_store((oop*)&_signers, s); }
+  objArrayOop signers() const;
+  void set_signers(objArrayOop s);

You don't really need the setters on the InstanceKlass anymore. They are 
only used in jvm.cpp where they take a couple of unnecessary 
indirections: mirror -> IK -> mirror->set_protection_domain/set_signers.

http://cr.openjdk.java.net/~coleenp/8003421/src/share/vm/oops/arrayKlass.cpp.udiff.html

-  java_lang_Class::create_mirror(k, CHECK);
+  java_lang_Class::create_mirror(k, Handle(NULL), CHECK);

You use NULL here since typeArrays always return a NULL pd, and 
objArrays always returns the pd of the bottom klass?

http://cr.openjdk.java.net/~coleenp/8003421/src/share/vm/oops/instanceKlass.cpp.patch

-void InstanceKlass::oops_do(OopClosure* cl) {
-  Klass::oops_do(cl);
-
-  cl->do_oop(adr_protection_domain());
-  cl->do_oop(adr_signers());
-  cl->do_oop(adr_init_lock());
-
-  // Don't walk the arrays since they are walked from the ClassLoaderData objects.
-}

If we could move ArrayKlass::_component_mirror into the j.l.Class, then 
_java_mirror would be the only oop in the klasses and we could make 
Klass::oops_do non-virtual ...

Another thing. If we could direct-allocate the java mirrors in the old 
gen, then we wouldn't have to walk all the klasses during the young GCs. 
This would make the GCs a bit less complicated and we could get rid of 
these fields in Klass:

   // Remembered sets support for the oops in the klasses.
   jbyte _modified_oops;             // Card Table Equivalent (YC/CMS 
support)
   jbyte _accumulated_modified_oops; // Mod Union Equivalent (CMS support)

thanks,
StefanK

>
> Tested with vm.quick.testlist, JPRT, jtreg java/security tests and 
> jck8 tests.
>
> Thanks,
> Coleen



More information about the serviceability-dev mailing list