RFR: 8221477: Inject os/cpu-specific constants into Unsafe from JVM

[Andrew Dinn]

Hi Coleen,

Thanks for the follow-up reply.

On 04/04/2019 01:25, coleen.phillimore at oracle.com wrote:
> Yes, I see the advantage to it.  I wish it were more compact but it's
> fine the way it you've written it.

Ok, I'll keep this as is then.

>>> The cast is unnecessary.  UnsafeConstants_klass returns an
>>> InstanceKlass.
>> Ah, ok, I cut and pasted this from the equivalent code for String.
>> Perhaps I should fix that as well.
> 
> Please, fix that one too.  I've had several passes removing
> InstanceKlass::cast()s but they keep sneaking in.

Sure, will do.

> Actually the field names are fine, I don't like that we have to add them
> to vmSymbols.hpp.  But that's how the code works.
> 
> Thank you for the rationale of the naming though.  It makes sense.
Well, thank you for asking me to provide an explanation as it has made
me understand better and reconsider slightly the choices. If it is not
the actual choice of names that concerns you then I think it would be an
improvement if I tried harder to follow my original plan and statically
import all the UnsafeConstants fields as replacements for the existing
Unsafe ones (where present). I believe I can finesse the problem that
Unsafe.ADDRESS_SIZE is public. So, that means:

  adding 'import static UnsafeConstants.*;' to Unsafe.java

  renaming UnsafeConstants.BIG_ENDIAN to UnsafeConstants.BE and removing
the corresponding field Unsafe.BE

  finessing the public visibility of Unsafe.ADDRESS_SIZE by renaming
UnsafeConstants.ADDRESS_SIZE to UnsafeConstants.ADDRESS_SIZE0 while
retaining public field Unsafe.ADDRESS_SIZE but initializing it to
UnsafeConstants.ADDRESS_SIZE0

The last change is to avoid a name clash from the 'import static ...'

I'll post a follow-up linking to a new webrev which answers your
comments and David's.

regards,


Andrew Dinn
-----------