RFR 8245289: Clean up offset code in JavaClasses

[coleen.phillimore at oracle.com]

Hi David, I already checked this in.

On 6/1/20 1:51 AM, David Holmes wrote:
> Hi Coleen,
>
> On 28/05/2020 10:48 pm, coleen.phillimore at oracle.com wrote:
>> Summary: Make offset member names consistent and private, move static 
>> initializations near owning classes
>>
>> This one is better.  I gave up on the X macros because they didn't 
>> save typing.  The changes here were to add underscores to offset 
>> field names, remove _in_bytes for offset accessors and add asserts 
>> that the offset is non-zero, made java_lang_ref_Reference offset 
>> fields private, and moved static member definitions closer to the 
>> class instead of some random place in javaClasses.cpp.
>
> I focused on the javaClasses.* changes rather than the incidental 
> changes due to the _in_bytes removal.
>
> src/hotspot/share/classfile/javaClasses.hpp
>
>  892   static void serialize_offsets(SerializeClosure* f) NOT_CDS_RETURN;

I did remove that unintentionally in a merge.  If the minimal build 
fails, we can fix it.
>
> You dropped the NOT_CDS_RETURN in the new code.
>
> ---
>
> src/hotspot/share/classfile/javaClasses.inline.hpp
>
> I'm confused about the removal of the initialization assertions. I 
> would have expected this, for example,
>
>  33 void java_lang_String::set_coder(oop string, jbyte coder) {
>  34   string->byte_field_put(_coder_offset, coder);
>
> to be expressed using coder_offset():
>
>  33 void java_lang_String::set_coder(oop string, jbyte coder) {
>  34   string->byte_field_put(coder_offset(), coder);
>
> so that we still get an initialization check. Though I see that 
> initialization checks are an area where there remains a lot of 
> inconsistency - it's not clear when we can rely solely on the 
> "_x_offset != 0" check versus the "_initialized==true" check, and 
> there are many methods with no initialization check at all.
Yeah, the initialization check seemed pointless to me in a lot of places 
where we've already created an instance of a java.lang.Blah and we're 
just doing getters and setters on them.

One could do further cleanup with the initialization fields, which seem 
redundant with checking that the offset is non-zero, and keep cleaning 
this up,  I presume.  This is where I stopped.

Thanks,
Coleen
>
> Thanks,
> David
> -----
>
>> See discussion for 8243996 Remove hardcoded field offsets for more 
>> details. 
>> https://mail.openjdk.java.net/pipermail/hotspot-dev/2020-May/041732.html
>>
>> Tested with tier1-6.
>>
>> open webrev at 
>> http://cr.openjdk.java.net/~coleenp/2020/8245289.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8245289
>>
>> Thanks,
>> Coleen