RFR 8245289: Clean up offset code in JavaClasses

Frederic Parain frederic.parain at oracle.com
Thu May 28 17:32:10 UTC 2020



> On May 28, 2020, at 13:09, coleen.phillimore at oracle.com wrote:
> 
> 
> 
> On 5/28/20 12:57 PM, Frederic Parain wrote:
>> Coleen,
>> 
>> This is a huge patch, fortunately most of the changes are
>> related to the removal of the _in_bytes suffix and the encapsulation
>> of fields and were easy to review.
>> 
>> javaClasses.hpp/javaClasses.hpp: a lot of changes and code refactoring
>> there, that are not easy to track with a webrev. Most changes make the
>> code cleaner. However, there’re some inconsistencies on static fields
>> declaration: some are initialized with an explicit value, others are
>> not, and I’m not seeing a clear rule why.
> 
> Fred, Thank you for looking at the patch.  It is unfortunately hard to read with webrev by moving things.  I did move the static definitions and didn't remove the initializations in cases that had them.  The initialization to zero should be unnecessary, and I guess I can remove them.  I could also add initializations to the ones missing.  I don't have a preference to which to do, so I left them as they were.  What do you think?

I don’t have a preference either, so I would ask: what is the recommended way to code this in HotSpot?
The inconsistencies are not causing issues, it’s more of a style question, so I’m fine if you push
your patch as is.

Thank you,

Fred


> 
> thanks,
> Coleen
>> 
>> Otherwise, looks good.
>> 
>> Regards,
>> 
>> Fred
>> 
>> 
>>> On May 28, 2020, at 08:48, 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.
>>> 
>>> 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
> 



More information about the hotspot-dev mailing list