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

Andrew Dinn adinn at redhat.com
Fri Mar 29 08:43:10 UTC 2019


On 28/03/2019 17:09, Thomas Stüfe wrote:
> src/hotspot/share/classfile/javaClasses.cpp
> 
> Pure nitpicking, but you could remove the member variables too and just
> set the Unsafe members directly, e.g.:
> 
>     if (fd->name() == vmSymbols::address_size_name()) {
>       mirror->int_field_put(fd->offset(), (jint)sizeof(void*));
>     } else if (fd->name() == vmSymbols::page_size_name()) {
>       mirror->int_field_put(fd->offset(), (jint) os::vm_page_size());
> ..
> 
> and so on. But I leave this up to you. This is already better than before.

I thought about that but it seemed to me to be better for clarity and
readability to init all the members in a single block in the constructor
rather than spread the computations of the field values through the code
that updates the fields. I think it's easier that way to see where all
the values are coming from and align them up with the fields in the
target class.

> +    }else {
> space missing before else
Yes.

> src/hotspot/share/classfile/vmSymbols.hpp
> "UNALIGNED_ACCESS" off by one space

Yes.

> src/java.base/share/classes/java/nio/channels/FileChannel.java
> 
> This may have creeped in from another change.

Ah, yes. That was the next patch that I forgot to back out. I have
updated the webrev in place to remove it and also fix the whitespace &
alignment issues.

> Good otherwise.
Thanks.

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