bitmap: use bits.LeadingZeros64 intrinsic in maximum()#529
Merged
Conversation
bitmapContainer.maximum() relied on a hand-rolled clz() to find the highest set bit in the most significant non-empty word. That helper reimplemented, in portable shift/compare arithmetic, what the standard library already exposes as math/bits.LeadingZeros64, which the Go compiler lowers to a single hardware instruction (LZCNT on amd64, CLZ on arm64). The package already wraps it as countLeadingZeros (clz.go), so maximum() now calls that and the redundant clz() helper is deleted.
Contributor
Author
|
Just for the record / clarification:
|
Member
|
@TheHippo Yes. I know. Thanks for the PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
bitmapContainer.maximum()finds the highest set bit by counting the leading zeros of the top word. It used a hand-written helper,clz(), to do that counting. This change makes it use the standard library'smath/bits.LeadingZeros64instead (already wrapped in this package ascountLeadingZeros), and removesclz().Type of Change
Changes Made
What was changed?
bitmapContainer.maximum()now computes the highest set bit usingcountLeadingZeros(w)(i.e. math/bits.LeadingZeros64) instead of the privateclz()` function.clz(i uint64) inthelper inbitmapcontainer.gois deleted (it had no other callers).Why was it changed?
clz()counted the leading zero bits of a word using a chain ofifchecks and bit shifts. The standard library already does the same thing withmath/bits.LeadingZeros64, which the Go compiler turns into a single CPU instruction (LZCNTon amd64,CLZon arm64). In short,clz()was a slower, handwritten version of something the standard library does in one instruction.How was it changed?
clz()withcountLeadingZeros()the body ofmaximum().clz()function.Testing
Passes.
maximum()is exercised by the existing root-package tests (roaring_test.go) and the randomizedproperty_test.go, which compareMaximum()against reference behavior over many bitmaps - these cover the substitution directly. No new test was required.Fuzzing
Not affected by this change (no serialization/parsing surface is touched), but the existing fuzz harness continues to build and run.
Performance Impact
Negligible-to-positive.
Performance Analysis
benchstatreports a 3.58% improvement over the entire benchmark suite (geomean).Breaking Changes
None. Public API and behavior are unchanged.