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

2013-05-21 Thread Stefan Karlsson

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




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

2013-05-21 Thread Coleen Phillimore


On 05/20/2013 11:39 PM, David Holmes wrote:

Hi Coleen,

On 21/05/2013 8:39 AM, Coleen Phillimore wrote:
Summary: Inject protection_domain, signers, init_lock into 
java_lang_Class


Basic VM changes look fine to me.


Thanks!



Net footprint change is zero except that these fields are in Java heap
rather than metaspace.  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.


Unsure about the SA changes. Basically you just removed access to the 
pd and signers, rather than changing it to allow access via the new 
path. That said I don't know SA so don't know whether it makes sense 
for SA to access things that are logically part of java.lang.Class; or 
whether it can access them more directly anyway because they are 
logically part of java.lang.Class.




I did remove these from instanceKlass.  It doesn't appear in the SA that 
it digs into the java mirror so there was nowhere to put these fields.   
The code that I took out was in places that may not be used and may be 
bit rotted.   I added serviceability team to the review request so 
someone could comment.  I already asked Staffan about this.


Thanks,
Coleen

Thanks,
David
-


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

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

Thanks,
Coleen




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

2013-05-21 Thread Staffan Larsen

 Net footprint change is zero except that these fields are in Java heap
 rather than metaspace.  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.
 
 Unsure about the SA changes. Basically you just removed access to the pd and 
 signers, rather than changing it to allow access via the new path. That said 
 I don't know SA so don't know whether it makes sense for SA to access things 
 that are logically part of java.lang.Class; or whether it can access them 
 more directly anyway because they are logically part of java.lang.Class.
 
 
 I did remove these from instanceKlass.  It doesn't appear in the SA that it 
 digs into the java mirror so there was nowhere to put these fields.   The 
 code that I took out was in places that may not be used and may be bit 
 rotted.   I added serviceability team to the review request so someone could 
 comment.  I already asked Staffan about this.

Reading this in more detail, I'm a little worried about the change in hprof 
output. The change in HeapHprofBinWriter actually breaks the hprof binary 
format, leading to unparseable files. Granted, this is hprof output when 
invoked either in SA or with jmap -F so it won't be used by very many people 
and I can't find any tests that do this. 

I think we need to find a way to add this information back.

/Staffan

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

2013-05-21 Thread Coleen Phillimore


On 05/21/2013 05:11 AM, Stefan Karlsson wrote:

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


Yes, I'm hoping for someone to 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.


That's right.


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.




I left these accessor functions in with a comment that JVMTI spec 
defined these fields in InstanceKlass and we have to simulate that they 
are still there for compatibility.


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?




Yes.
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 ...




That would add a field to all mirrors though.  It's a bit harder but it 
would be nice to have smaller mirrors for array klasses vs. instanceKlasses.



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)



Yes, we want to move in this direction!  Not with this change though.

Coleen

thanks,
StefanK



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


Thanks,
Coleen






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

2013-05-21 Thread Stefan Karlsson

On 21 maj 2013, at 16:12, Coleen Phillimore coleen.phillim...@oracle.com 
wrote:

 
 On 05/21/2013 05:11 AM, Stefan Karlsson wrote:
 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
 
 Yes, I'm hoping for someone to 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.
 
 That's right.
 
 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.
 
 I left these accessor functions in with a comment that JVMTI spec defined 
 these fields in InstanceKlass and we have to simulate that they are still 
 there for compatibility.

Where in the spec does it mention InstanceKlasses?

I still see no reason to keep the setters.

 
 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?
 
 Yes.
 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 ...
 
 That would add a field to all mirrors though.

Unless we reuse the pd field, which isn't used for the arrays. But that's a 
hack.

  It's a bit harder but it would be nice to have smaller mirrors for array 
 klasses vs. instanceKlasses.

The size of the mirrors are already of variable size, because of the static 
fields, so this would probably be a good idea. Something for the Embedded team 
to pursue maybe?

 
 
 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)
 
 Yes, we want to move in this direction!  Not with this change though.

Of course. I'm fine with your current changes as they are.

StefanK

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


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

2013-05-21 Thread Coleen Phillimore


Thanks Stefan,  I have 2 small comments.

On 05/21/2013 10:38 AM, Stefan Karlsson wrote:

On 21 maj 2013, at 16:12, Coleen Phillimore coleen.phillim...@oracle.com 
wrote:


On 05/21/2013 05:11 AM, Stefan Karlsson wrote:

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

Yes, I'm hoping for someone to 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.

That's right.

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.

I left these accessor functions in with a comment that JVMTI spec defined these 
fields in InstanceKlass and we have to simulate that they are still there for 
compatibility.

Where in the spec does it mention InstanceKlasses?


The hprof format and jvmtiTagMap spec assumes they are dumped and the 
callbacks are there.


I still see no reason to keep the setters.


Oh, yes, I deleted them.  Thanks.



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?

Yes.

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 ...

That would add a field to all mirrors though.

Unless we reuse the pd field, which isn't used for the arrays. But that's a 
hack.


  It's a bit harder but it would be nice to have smaller mirrors for array 
klasses vs. instanceKlasses.

The size of the mirrors are already of variable size, because of the static 
fields, so this would probably be a good idea. Something for the Embedded team 
to pursue maybe?


Yes, or a separate change for runtime.






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)

Yes, we want to move in this direction!  Not with this change though.

Of course. I'm fine with your current changes as they are.


Thanks!   I've made some changes so am retesting.   I also changed the 
SA code, so I have to send out another review.


Coleen


StefanK


Coleen

thanks,
StefanK


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

Thanks,
Coleen




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

2013-05-21 Thread Coleen Phillimore


On 05/21/2013 09:18 AM, Staffan Larsen wrote:

Net footprint change is zero except that these fields are in Java heap
rather than metaspace.  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.

Unsure about the SA changes. Basically you just removed access to the pd and 
signers, rather than changing it to allow access via the new path. That said I 
don't know SA so don't know whether it makes sense for SA to access things that 
are logically part of java.lang.Class; or whether it can access them more 
directly anyway because they are logically part of java.lang.Class.


I did remove these from instanceKlass.  It doesn't appear in the SA that it 
digs into the java mirror so there was nowhere to put these fields.   The code 
that I took out was in places that may not be used and may be bit rotted.   I 
added serviceability team to the review request so someone could comment.  I 
already asked Staffan about this.

Reading this in more detail, I'm a little worried about the change in hprof output. The 
change in HeapHprofBinWriter actually breaks the hprof binary format, leading to 
unparseable files. Granted, this is hprof output when invoked either in SA or with 
jmap -F so it won't be used by very many people and I can't find any tests 
that do this.

I think we need to find a way to add this information back.


Hi,  I didn't want to break the hprof format so I added it back to 
return null for protection domain and signers.  There doesn't seem to be 
a way in SA to read from the mirror but I posted suggested code once 
this sort of thing is implemented.   See:


http://cr.openjdk.java.net/~coleenp/8003421_2/agent/src/share/classes/sun/jvm/hotspot/utilities/HeapGXLWriter.java.udiff.html
http://cr.openjdk.java.net/~coleenp/8003421_2/agent/src/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java.udiff.html

I can file a bug against the SA to get the correct information or decide 
whether this is something you need to support in this manner.


thanks,
Coleen



/Staffan




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

2013-05-20 Thread Dean Long
It seems like you could take this opportunity to make these declared 
fields of java.lang.Class,
allowing, for example, getProtectionDomain0() to be a simple Java method 
instead of a native method.


dl

On 05/20/2013 03:39 PM, 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.  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

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


Thanks,
Coleen




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

2013-05-20 Thread Coleen Phillimore

On 5/20/2013 8:42 PM, Dean Long wrote:
It seems like you could take this opportunity to make these declared 
fields of java.lang.Class,
allowing, for example, getProtectionDomain0() to be a simple Java 
method instead of a native method.


We actually use the protection domain and init_lock from within the vm, 
so we want to be able to see it.   Signers can be moved out eventually 
though.


Thanks,
Coleen



dl

On 05/20/2013 03:39 PM, 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.  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

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


Thanks,
Coleen






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

2013-05-20 Thread David Holmes

Hi Coleen,

On 21/05/2013 8:39 AM, Coleen Phillimore wrote:

Summary: Inject protection_domain, signers, init_lock into java_lang_Class


Basic VM changes look fine to me.


Net footprint change is zero except that these fields are in Java heap
rather than metaspace.  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.


Unsure about the SA changes. Basically you just removed access to the pd 
and signers, rather than changing it to allow access via the new path. 
That said I don't know SA so don't know whether it makes sense for SA to 
access things that are logically part of java.lang.Class; or whether it 
can access them more directly anyway because they are logically part of 
java.lang.Class.


Thanks,
David
-


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

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

Thanks,
Coleen


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

2013-05-20 Thread Dean Long

On 5/20/2013 7:50 PM, Coleen Phillimore wrote:

On 5/20/2013 8:42 PM, Dean Long wrote:
It seems like you could take this opportunity to make these declared 
fields of java.lang.Class,
allowing, for example, getProtectionDomain0() to be a simple Java 
method instead of a native method.


We actually use the protection domain and init_lock from within the 
vm, so we want to be able to see it.   Signers can be moved out 
eventually though.


The VM can already see Java fields.  You would just need to initialize 
_protection_domain_offset using compute_offset() like we do for other 
fields.  Java fields would also have the advantage of working with the 
new @Contended annotation, which probably doesn't work for injected 
fields.  However for backwards compatability with an older class library 
you could use compute_optional_offset() and inject the field only if 
it's missing.


dl


Thanks,
Coleen



dl

On 05/20/2013 03:39 PM, 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.  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

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


Thanks,
Coleen








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

2013-05-20 Thread David Holmes

On 21/05/2013 12:50 PM, Coleen Phillimore wrote:

On 5/20/2013 8:42 PM, Dean Long wrote:

It seems like you could take this opportunity to make these declared
fields of java.lang.Class,
allowing, for example, getProtectionDomain0() to be a simple Java
method instead of a native method.


We actually use the protection domain and init_lock from within the vm,
so we want to be able to see it.   Signers can be moved out eventually
though.


But you can access those fields from the VM regardless - just as we 
access a bunch of fields in the Java level objects.


That said having these as injected fields hides them in the VM, so no 
issue with reflective access etc.


David



Thanks,
Coleen



dl

On 05/20/2013 03:39 PM, 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.  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

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

Thanks,
Coleen






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

2013-05-20 Thread Ioi Lam
But if you move these fields into Class.java (in JDK8), then hsx25 will 
not run on JDK7 anymore, unless these fields are also added in 
Class.java in JDK7.


- Ioi

On 05/20/2013 05:42 PM, Dean Long wrote:
It seems like you could take this opportunity to make these declared 
fields of java.lang.Class,
allowing, for example, getProtectionDomain0() to be a simple Java 
method instead of a native method.


dl

On 05/20/2013 03:39 PM, 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.  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

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


Thanks,
Coleen






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

2013-05-20 Thread Dean Long

Right, unless there is a way to inject them conditionally.

dl

On 5/20/2013 9:02 PM, Ioi Lam wrote:
But if you move these fields into Class.java (in JDK8), then hsx25 
will not run on JDK7 anymore, unless these fields are also added in 
Class.java in JDK7.


- Ioi

On 05/20/2013 05:42 PM, Dean Long wrote:
It seems like you could take this opportunity to make these declared 
fields of java.lang.Class,
allowing, for example, getProtectionDomain0() to be a simple Java 
method instead of a native method.


dl

On 05/20/2013 03:39 PM, 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.  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

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


Thanks,
Coleen








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

2013-05-20 Thread David Holmes

On 21/05/2013 2:50 PM, Dean Long wrote:

Right, unless there is a way to inject them conditionally.


You can inject them conditionally based on the JRE version, but then why 
bother moving to the Java level if you will still need the code in the VM ?



On 5/20/2013 9:02 PM, Ioi Lam wrote:

But if you move these fields into Class.java (in JDK8), then hsx25
will not run on JDK7 anymore, unless these fields are also added in
Class.java in JDK7.


As I understand it hs25 is already limited in the versions of jdk7 that 
it can run in.


David
-



- Ioi

On 05/20/2013 05:42 PM, Dean Long wrote:

It seems like you could take this opportunity to make these declared
fields of java.lang.Class,
allowing, for example, getProtectionDomain0() to be a simple Java
method instead of a native method.

dl

On 05/20/2013 03:39 PM, 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.  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

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

Thanks,
Coleen