RFR 8151163 All Buffer implementations should leverage Unsafe unaligned accessors <was> Re: ByteBuffer views, alignment, word tearing

[Paul Sandoz]

Hi Andrew,

Would you mind formally reviewing the patches?

Including hotspot-dev as i have updated a hotspot test so should probably push to hs.

I rebased to hs and fixed a silly bug in generated heap view buffers that were incorrectly calculating the unsafe byte offset:

  http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151163-buffer-unsafe-unaligned-access/webrev/ <http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151163-buffer-unsafe-unaligned-access/webrev/>

  http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151163-buffer-unsafe-unaligned-access-hotspot/webrev/ <http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151163-buffer-unsafe-unaligned-access-hotspot/webrev/>

JPRT runs for core and hotspot pass.


> On 11 Jul 2016, at 18:34, Andrew Haley <aph at redhat.com> wrote:
> 
> On 11/07/16 17:04, Paul Sandoz wrote:
> 
>> I opted to directly use Unsafe rather than defer. I am not sure it’s
>> always possible to defer to the underlying buffer since its
>> positional and order state is mutable.
> 
> Eww.  The view has its own position, and the byte ordering is fixed
> when the view is created.  The wrapped byte buffer is in a protected
> field, and the view is a package-private class so it can't be
> subclassed by the user.  So I would have thought that could be made to
> work, but but never mind, I can see you've gone a long way down this
> road already.

Yes, and i laid some ground work in a previous fix to enable support for Unsafe double addressing mode on heap and direct buffers, with a grander plan to try and unify some of the implementations (careful performance analysis is required for direct buffers that access the base field whose value is null).


> 
> The SPARC regression is very odd: I would have thought that if it's
> broken here it would also be broken in HeapByteBuffer.getLong().
> 

I did not get a chance to look at the generated code, so there could be a number of factors involved. My feeling is the three alignment checks are causing more issues on SPARC (perhaps combined less efficient addressing calculations [*], with loop unrolling), which led to the suggestion of a modifying the unaligned Unsafe methods to take an extra arg that is the constant offset to check alignment, thus could be hoisted out of a loop making constant strides.

Paul.

[*] some work on x86 was recently done by Roland to improve this area