RFR: 8198286: Direct memory accessors in typeArrayOop.hpp should use Access API
Kim Barrett
kim.barrett at oracle.com
Sat Feb 17 19:54:13 UTC 2018
> On Feb 16, 2018, at 11:18 AM, Roman Kennke <rkennke at redhat.com> wrote:
>
> The direct memory accessors in typeArrayOop.hpp, which are usually
> used for bulk memory access operations, should use the Access API, in
> order to give the garbage collector a chance to intercept the access
> (for example, employ read- or write-barriers on the target array).
> This also means it's necessary to distinguish between write-accesses
> and read-accesses (for example, GCs might want to use a
> copy-on-write-barrier for write-accesses only).
>
> This changeset introduces two new APIs in access.hpp: load_at_addr()
> and store_at_addr(), and links it up to the corresponding X_get_addr()
> and X_put_addr() in typeArrayOop.hpp. All uses of the previous
> X_addr() accessors have been renamed to match their use (load or store
> of primitive array elements).
>
> The changeset is based on the previously proposed:
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2018-February/026426.html
>
> Webrev:
> http://cr.openjdk.java.net/~rkennke/8198286/webrev.00/
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8198286
>
> Please review!
>
> Thanks,
> Roman
Not a review, but suggestions and questions.
In order to reduce opportunities for bugs, I suggest a change of
signatures:
T* T_at_get_addr(int which) const
T* T_at_put_addr(int which) const
=>
const T* T_at_get_addr(int which) const
T* T_at_put_addr(int which)
This prevents (a) writing to the result of a get_addr, (b) writing to
a constant object. The latter is probably matters less, given our
level of const-cleanliness.
Obviously there will be fanout, particularly from the get_addr
change, as there are some missing const qualifiers. Addressing those
is probably a good thing though.
But I'm also wondering whether the overall approach is right. If this
is about bulk transfers, as suggested in the CR and RFR, then shouldn't
this be involving Access::arraycopy? Why isn't changing these copies
to use that operation, along with an appropriate implementation of
that operation by the barrier set, a better solution?
More information about the hotspot-runtime-dev
mailing list