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

David Holmes david.holmes at oracle.com
Fri Mar 29 01:25:00 UTC 2019

Hi Andrew,

This seems fine in general but I have a few queries on some details:


     f(java_lang_Thread) \
+   f(jdk_internal_misc_UnsafeConstants) \
     f(java_lang_ThreadGroup) \

Is there a reason this needs to be shoved in there? Similarly with 

java_lang_Thread                                      ) \
+   do_klass(UnsafeConstants_klass, 
jdk_internal_misc_UnsafeConstants                     ) \
java_lang_ThreadGroup                                 ) \




+   // initialize the hardware-specific constants needed by Unsafe
+   initialize_class(vmSymbols::jdk_internal_misc_UnsafeConstants(), CHECK);
+   jdk_internal_misc_UnsafeConstants::set_unsafe_constants();
     // Set thread status to running since main thread has
     // been started and running.

That seems a very odd place to insert the new initialization. I can't 
see any reason you'd need to split the Thread object initialization like 
that. ??

More generally exactly when do you need to initialize this new class by 
and how does the initialization order change before/after this fix? (I'm 
expecting only to see the new class inserted where needed without any 
other perturbations.)



+   template(big_endian_name,                           "BIG_ENDIAN") 
+   template(use_unaligned_access_name, 
"UNALIGNED_ACCESS")                        \

Nit: There's an extra space before "UNALIGNED...



31  * package-protected  ...


  37  * The JVM injects values into all the static fields of this class
  43  * hardware-specific constants installed by the JVM.

I get this gist of this note but still found it rather difficult to 
intepret. There are I think two key points:

1. The fields must not be constant variables, hence the need for the 
static initialization block.
2. The true value of each field is injected by the VM.

How about:

* The JVM injects hardware-specific values into all the static fields
* of this class during JVM initialization. The static initialization
* block exists to prevent the fields from being considered constant
* variables, so the field values will be not be compiled directly into
* any class that uses them.

60      * @implNote
61      * The actual value for this field is injected by JVM. A static
62      * initialization block is used to set the value here to
63      * communicate that this static final field is not statically
64      * foldable, and to avoid any possible circular dependency during
65      * vm initialization.

I think all you need for each field is just:

* @implNote
* The actual value for this field is injected by _the_ JVM.

85      * flag whose value ...
92      * flag whose value ...



On 29/03/2019 2:56 am, Andrew Dinn wrote:
> On 28/03/2019 15:22, Thomas Stüfe wrote:
>>      The second of those was actually intended to be iff. This is a common
>>      abbreviation used by English/US mathematicians and logicians to write
>>      'if and only if' (it is also sometimes written as <=>). Since you didn't
>>      recognize it I guess I really need to write it out in full.
>> Oh, don't worry on my account. I am not a native speaker nor a
>> mathematician. You could leave iff and add [sic] to make everyone
>> curious and start googling "iff" :)
> I changed it nevertheless. It would be remiss to presume readers need be
> conversant with elliptical, English logico-mathematical parlance (I'm
> explaining this in as plain English as I can ... no, wait! ;-).
> Anyway, I addressed this and your other concerns in a new webrev.
>    JIRA:   https://bugs.openjdk.java.net/browse/JDK-8221477
>    Webrev: http://cr.openjdk.java.net/~adinn/8221477/webrev.02
> Input from a second reviewer would be welcome ...
> regards,
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

More information about the core-libs-dev mailing list