[9] RFR(XS): 8176513 Poor code quality for ByteBuffers

[John Rose]

On Mar 15, 2017, at 2:48 AM, Roland Westrelin <rwestrel at redhat.com> wrote:
> 
> This changes relaxes the condition under which MemBarCPUOrder nodes are
> added around unsafe accesses. If the object being accessed is an array
> and the access is mismatched, the current code add useless barriers
> (there's only one memory slice for the entire array so no risk of
> breaking alias analysis).

This is reasonable.  The CPUOrder is intended to prevent an unsafe access
from reordering with a nominal (non-unsafe) field access, since it is impossible
(in general!) to tell if an unsafe access might alias with a field access.

From the JMM point of view, a *single* array element works like a field
(both are variables in the heap).  Thus, we also don't want to reorder
an unsafe access *to a particular array element* with a normal "*aload"
or "*astore" access.

Reordering accesses between unsafe loads or stores is less of a problem
I think, because, well, they are unsafe.  User responsibility is to avoid alias
issues, and even fix with explicit fences if necessary.

There also a possible argument of "user responsibility" for the interaction
of unsafe access with normal access; when a bunch of unsafe access code
is done, the effects must post correctly to normal access, without reordering.
(Same point backwards in time/HB-relation also.)  We might say users should
put fences around all unsafe code, but they don't (today) so a little more
care is appropriate here.  I recommend aiming to put implicit CPU order
fences between unsafe-access code and "innocent" normal code, when
possible.  The current overuse of fences conservatively achieves this.

> If the base is not known to be not null,
> membars are added as well.

These are present "just in case" the accessed field was in fact volatile.
They are not relevant to normal fields.  They are not relevant to array
elements (since array variables are never volatile, nor, alas, final).

> These membars are not added for arrays in 8u
> and that hasn't caused any known problem. So in order to keep this
> change simple, we propose removing that condition (this was suggested by
> Vladimir Ivanov in the comments for this bug).

Thank you for working on this.

— John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20170315/f99d8c4f/attachment.html>