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
