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

[Thomas Stüfe]

Thanks Andrew. Looks all good to me now.

Cheers, Thomas

On Fri, Mar 29, 2019 at 9:43 AM Andrew Dinn <adinn at redhat.com> wrote:

> 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
>