HADOOP-19810. Replace FastByteComparisons with JDK Arrays.compareUnsigned#8239
HADOOP-19810. Replace FastByteComparisons with JDK Arrays.compareUnsigned#8239pan3793 wants to merge 2 commits intoapache:trunkfrom
Conversation
|
cc @aajisaka since you reviewed and committed HADOOP-14313, also cc @cnauroth |
aajisaka
left a comment
There was a problem hiding this comment.
+1 pending Jenkins.
I'm happy to see sun.misc.Unsafe API usage is dropped.
|
@pan3793 Writable object is usually small, so would you also benchmark against smaller array size like https://issues.apache.org/jira/browse/HADOOP-14313? I want to check there's no regression in 128, 256, 512, 1028, 2048.
The above numbers are very small. How can we compare these? |
|
@aajisaka, thanks for the information, let me tune the code to test small size, will update benchmark results later. |
|
💔 -1 overall
This message was automatically generated. |
|
@aajisaka, I updated the benchmark result - on Linux, the result matches Guava's statement, the JDK impl is a little slower for 128 bytes, but faster for larger bytes. Note: I'm using a quite old Intel CPU, given |
aajisaka
left a comment
There was a problem hiding this comment.
Thank you for sharing the benchmark result. Looks good to me.
I think the unit test failures are related. Would you fix them?
Maybe Arrays.compareUnsigned(b1, s1, s1+l1, b2, s2, s2+l2);?
|
@aajisaka Oops, you are right, I fixed it and ensure |
|
💔 -1 overall
This message was automatically generated. |
ba66c6c to
cf758f7
Compare
|
💔 -1 overall
This message was automatically generated. |
Description of PR
Replace
FastByteComparisons(derived from Guava) with JDKArrays.compareUnsigned(added in JDK 9, and optimized in modern JDKs)Guava v33.4.5 starts to use
Arrays.compareUnsignedwhen it is available.google/guava@b3bb29a
How was this patch tested?
Create a JMH benchmark https://github.com/pan3793/HADOOP-19810.
TL;DR - JDK
Arrays.compareUnsignedhas better performance (except for small bytes like 128) on the x86_64 platform, and the same performance on Apple SiliconBenchmark results
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
No AI usage.