[11] RFR 8193085 Vectorize the nio Buffer equals and compareTo implementations (original) (raw)
Alan Bateman Alan.Bateman at oracle.com
Mon Dec 18 14:07:36 UTC 2017
- Previous message: [11] RFR 8193085 Vectorize the nio Buffer equals and compareTo implementations
- Next message: [11] RFR 8193085 Vectorize the nio Buffer equals and compareTo implementations
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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
- Previous message: [11] RFR 8193085 Vectorize the nio Buffer equals and compareTo implementations
- Next message: [11] RFR 8193085 Vectorize the nio Buffer equals and compareTo implementations
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]