[11] RFR 8193085 Vectorize the nio Buffer equals and compareTo implementations

[Alan Bateman]

On 15/12/2017 20:29, Paul Sandoz wrote:
> Hi,
>
> Please review this patch to vectorize the buffer equals and compareTo implementations, using the same approach that was used for arrays:
>
>    http://cr.openjdk.java.net/~psandoz/jdk/JDK-8193085-buffer-equals-compareTo-vectorize/webrev/ <http://cr.openjdk.java.net/~psandoz/jdk/JDK-8193085-buffer-equals-compareTo-vectorize/webrev/>
>
> This patch expands on using the double address mode of unsafe to uniformly access a memory region covered by a buffer be it on or off heap. Only buffers with the same endianness can support optimized equality and comparison.
I've looked through the changes and it looks quite good.

It might be useful to add a comment in StringCharBuffer to explain why 
equals/compareTo are overridden. Also it might be safer if the mismatch 
method that takes CharBuffers checks if one of charRegionOrders is null 
to guarantee that it will never do a vectorizedMismatch test with a 
wrapped char sequence (it would catch a serious bug if someone were to 
misuse to call it with two StringCharBuffers).

Not for this patch but XXXBuffer.compareTo doesn't specify how it 
compares when the remaining elements in one buffer is a proper prefix of 
the remaining elements in the other buffer.

It's hard to see the changes to ArraySupport. I assume it's just the 
package name and changing the methods to public. I can't tell why webrev 
doesn't handle the move in this case. Another one is Arrays where they 
are some re-formatting in the patch that webrev doesn't show (I can't 
tell if this is intended or not).

It would be good if the really long lines in BufferMismatch could be 
trimmed down (maybe import static ArraySupport) as it will very annoying 
to do side-by-side diffs when reviewing future changes. There are 
several places where the styles differs to the style in this area but 
it's probably not worth spending time on.

The test looks okay although it overlaps with the existing tests.

-Alan