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

[Vladimir Kozlov]

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;

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

I don't understand changes in Unsafe_StaticFieldBaseFromClass(),
it should return handle but you just return original jobject.

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
>