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

Thomas Stüfe thomas.stuefe at gmail.com
Thu Mar 28 12:17:26 UTC 2019


Hi,

this looks fine, nits only:

UnsafeConstantsFixup::do_field()

you could shrink that coding a bit by factoring out the checks, e.g.:

static oop mirror_with_checks_from_field_descriptor(fieldDescriptor* fd) {
 oop mirror = fd->field_holder()->java_mirror();
 assert(fd->field_holder() == SystemDictionary::UnsafeConstants_klass(),
"Should be UnsafeConstants");
 assert(mirror != NULL, "UnsafeConstants must have mirror already");
 return oop;
}

Also, is there a reason that code from thread.cpp:

    jint address_size = sizeof(void*);
    jint page_size = os::vm_page_size();
    jboolean is_big_endian = LITTLE_ENDIAN_ONLY(false)
BIG_ENDIAN_ONLY(true);
    jint use_unaligned_access = UseUnalignedAccesses;

could not just happen right in UnsafeConstantsFixup::do_field()? You would
not have to pass all these arguments around, param list could be empty.


src/hotspot/share/classfile/vmSymbols.hpp

Alignment of trailing '\' broken


src/java.base/share/classes/jdk/internal/misc/UnsafeConstants.java

s/intitialize/initialize
s/iff/if


Cheers, Thomas

On Thu, Mar 28, 2019 at 11:44 AM Andrew Dinn <adinn at redhat.com> wrote:

> Could I please have reviews for this patch which changes the
> initialization of four os/cpu-specific static final constants used by
> class Unsafe. The patch injects values during JVM startup (along similar
> lines to how String field COMPACT_STRINGS is initialized) rather than
> retrieving them via native method calls. This localizes the computation
> of the assigned values in one place, relocates the constants into a
> separate final, static-only Java class and avoids the need to maintain
> four separate native methods.
>
> A further motive for making this change is to pave the way for adding
> the writeback cache line size/address mask as Unsafe constants for use
> by the JEP proposed in JDK-8207851.
>
> JIRA:   https://bugs.openjdk.java.net/browse/JDK-8221477
> Webrev: http://cr.openjdk.java.net/~adinn/8221477/webrev.01
>
> Testing:
> submit repo tests passed
>
> 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