Skip to content

HBASE-28025 Enhance ByteBufferUtils.findCommonPrefix to compare 8 bytes each time - #5354

Merged
Apache9 merged 1 commit into
apache:masterfrom
jbewing:HBASE-28025-optimize-find-common-prefix-compare
Aug 20, 2023
Merged

HBASE-28025 Enhance ByteBufferUtils.findCommonPrefix to compare 8 bytes each time#5354
Apache9 merged 1 commit into
apache:masterfrom
jbewing:HBASE-28025-optimize-find-common-prefix-compare

Conversation

@jbewing

@jbewingjbewing commented Aug 16, 2023

Copy link
Copy Markdown
Contributor

What

This PR updates ByteBufferUtils#findCommonPrefix and Bytes#findCommonPrefix to compare 8 bytes from the input buffers/arrays if Unsafe access is available. On platforms where Unsafe is unavailable, we use the current implementations. This is a similar optimization as to what is already done with ByteBufferUtils#compareToUnsafe.

Implementation Notes

There was a Bytes#findCommonPrefix method and a ByteBufferUtils#findCommonPrefix method that both accepted byte[] args. I've updated the ByteBufferUtils#findCommonPrefix method to delegate to Bytes#findCommonPrefix and applied the optimization for 8 byte at a time comparison to the Bytes class.

Overall, the implementation draws a ton of inspiration from ByteBufferUtils#compareToUnsafe. The only large change that I made is for how we handle mismatches in the big endian case. I used the number of leading zeros intrinsic there instead of the number of trailing zeros intrinsic to find which byte was mismatched.

Testing

I've added some unit tests to cover testing the path with unsafe enabled and disabled.

Benchmarking

I haven't done any micro-benchmarking of the new "faster" implementations vs. the current implementations. I'll update the JIRA with a link to those when I get a chance to write them. For now, I'm assuming that this method of finding common prefixes is faster than the current one based off the previous micro-benchmarking results for compareTo (as this is very similar code). I've done some microbenchmarking with JMH. The results are in this JIRA comment

HBASE-28025

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 13sDocker mode activated.
-0 ⚠️yetus0m 2sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall2m 28smaster passed
+1 💚compile0m 16smaster passed
+1 💚shadedjars4m 31sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 16smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 18sthe patch passed
+1 💚compile0m 17sthe patch passed
+1 💚javac0m 17sthe patch passed
+1 💚shadedjars4m 29spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 14sthe patch passed
_ Other Tests _
+1 💚unit1m 44shbase-common in the patch passed.
18m 10s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5354/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#5354
Optional Testsjavac javadoc unit shadedjars compile
unameLinux d48a3a195633 5.4.0-156-generic #173-Ubuntu SMP Tue Jul 11 07:25:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 2fb2ae1
Default JavaTemurin-1.8.0_352-b08
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5354/1/testReport/
Max. process+thread count354 (vs. ulimit of 30000)
modulesC: hbase-common U: hbase-common
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5354/1/console
versionsgit=2.34.1 maven=3.8.6
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 24sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall2m 54smaster passed
+1 💚compile0m 15smaster passed
+1 💚shadedjars4m 58sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 15smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 37sthe patch passed
+1 💚compile0m 15sthe patch passed
+1 💚javac0m 15sthe patch passed
+1 💚shadedjars4m 56spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 13sthe patch passed
_ Other Tests _
+1 💚unit2m 7shbase-common in the patch passed.
20m 21s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5354/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#5354
Optional Testsjavac javadoc unit shadedjars compile
unameLinux b693bd2ca6af 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 2fb2ae1
Default JavaEclipse Adoptium-11.0.17+8
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5354/1/testReport/
Max. process+thread count396 (vs. ulimit of 30000)
modulesC: hbase-common U: hbase-common
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5354/1/console
versionsgit=2.34.1 maven=3.8.6
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 42sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+1 💚mvninstall3m 0smaster passed
+1 💚compile0m 36smaster passed
+1 💚checkstyle0m 17smaster passed
+1 💚spotless0m 44sbranch has no errors when running spotless:check.
+1 💚spotbugs0m 34smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 37sthe patch passed
+1 💚compile0m 34sthe patch passed
-0 ⚠️javac0m 34shbase-common generated 2 new + 34 unchanged - 0 fixed = 36 total (was 34)
+1 💚checkstyle0m 13sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚hadoopcheck9m 14sPatch does not cause any errors with Hadoop 3.2.4 3.3.5.
+1 💚spotless0m 41spatch has no errors when running spotless:check.
+1 💚spotbugs0m 39sthe patch passed
_ Other Tests _
+1 💚asflicense0m 12sThe patch does not generate ASF License warnings.
25m 56s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5354/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#5354
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
unameLinux f007f107ca19 5.4.0-156-generic #173-Ubuntu SMP Tue Jul 11 07:25:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 2fb2ae1
Default JavaEclipse Adoptium-11.0.17+8
javachttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5354/1/artifact/yetus-general-check/output/diff-compile-javac-hbase-common.txt
Max. process+thread count78 (vs. ulimit of 30000)
modulesC: hbase-common U: hbase-common
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5354/1/console
versionsgit=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache9
Apache9 merged commit dae078e into apache:masterAug 20, 2023
Apache9 pushed a commit that referenced this pull request Aug 20, 2023
…es each time (#5354)
Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit dae078e)
Apache9 pushed a commit that referenced this pull request Aug 20, 2023
…es each time (#5354)
Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit dae078e)
Apache9 pushed a commit that referenced this pull request Aug 20, 2023
…es each time (#5354)
Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit dae078e)
Apache9 pushed a commit that referenced this pull request Aug 20, 2023
…es each time (#5354)
Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit dae078e)
bbeaudreault pushed a commit to HubSpot/hbase that referenced this pull request Aug 21, 2023
…x to compare 8 bytes each time (apache#5354)
Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit dae078e)
vinayakphegde pushed a commit to vinayakphegde/hbase that referenced this pull request Apr 4, 2024
…es each time (apache#5354)
Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit dae078e)
(cherry picked from commit 6596ef6)
Change-Id: I046c51b34f9cd6809df3e940b42f927e75fdc85f
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jbewing@Apache-HBase@Apache9