review (L) for 7017732: move static fields into Class to prepare for perm gen removal

[Tom Rodriguez]

On Feb 9, 2011, at 5:09 PM, Vladimir Kozlov wrote:

> Tom,
> 
> Could you rename "o" (which looks like 0) in set_offset_of_static_fields(int o)?
> You did not initialize static field (it is C++) but you have assert to check that it is 0:
> instanceMirrorKlass::_offset_of_static_fields;

Fixed.

> 
> Could you add/modify layout decription in instanceKlass.hpp for instanceMirrorKlass and static fields.

I removed the no longer correct part.  Did you want something added?
> 
> I don't understand changes in Unsafe_StaticFieldBaseFromClass(),
> it should return handle but you just return original jobject.

We're supposed to return the object to use as the base of unsafe static field references.  That's just the java.lang.Class instance which was passed in so we simply return it.  What would you expect it to do?  I guess we might want to unhandle it and return a new local one in case the original one wasn't a local handle.  Is that what you're getting at?

tom

> 
> Thanks,
> Vladimir
> 
> Tom Rodriguez wrote:
>> I've updated the webrev with this change, fixed a couple macro names that still referenced InstanceKlass and fixed the alignment of the continuation slashes.
>> tom
>> On Feb 9, 2011, at 3:52 AM, Christian Thalinger wrote:
>>> On Feb 8, 2011, at 11:01 PM, Tom Rodriguez wrote:
>>>> http://cr.openjdk.java.net/~never/7017732
>>> src/cpu/x86/vm/c1_CodeStubs_x86.cpp:
>>> 
>>> 315     Register tmp = rax;
>>> 316     Register tmp2 = rbx;
>>> 317     if (_obj == tmp) tmp = rcx;
>>> 318     else if (_obj == tmp2) tmp = rcx;
>>> 319     __ push(tmp);
>>> 320     __ push(tmp2);
>>> 321     __ get_thread(tmp);
>>> 322     __ movptr(tmp2, Address(_obj, java_lang_Class::klass_offset_in_bytes()));
>>> 323     __ cmpptr(tmp, Address(tmp2, instanceKlass::init_thread_offset_in_bytes() + sizeof(klassOopDesc)));
>>> 324     __ pop(tmp2);
>>> 325     __ pop(tmp);
>>> 
>>> Why do you change tmp if _obj == tmp2?  I think line 318 isn't required at all since if _obj == tmp2 it gets saved and restored anyway.  What about this:
>>> 
>>> 315     Register tmp = rax;
>>> 316     Register tmp2 = rbx;
>>> 319     __ push(tmp);
>>> 320     __ push(tmp2);
>>> 322     __ movptr(tmp2, Address(_obj, java_lang_Class::klass_offset_in_bytes()));
>>> 321     __ get_thread(tmp);
>>> 323     __ cmpptr(tmp, Address(tmp2, instanceKlass::init_thread_offset_in_bytes() + sizeof(klassOopDesc)));
>>> 324     __ pop(tmp2);
>>> 325     __ pop(tmp);
>>> 
>>> -- Christian