[SPARK-56548][CORE] Replace modulo with bitmask in BloomFilter hot paths - #58551
Open
Vivek1106-04 wants to merge 1 commit into
Open
[SPARK-56548][CORE] Replace modulo with bitmask in BloomFilter hot paths#58551Vivek1106-04 wants to merge 1 commit into
Vivek1106-04 wants to merge 1 commit into
Conversation
Both bloom filter implementations reduce a hash into a bit index with `hash % bitSize`, once per hash function, in the innermost loop of `put` and `mightContain`. `bitSize` is not a compile time constant, so this is a hardware division on every probe. `BitArray` rounds its allocation up to whole 64 bit words, so its bit size is very often a power of two - in particular for the Spark SQL runtime bloom filter, whose sizes are set by `spark.sql.optimizer.runtime.bloomFilter.numBits` (2^23 by default) and `maxNumBits` (2^26). Taking a non-negative value modulo a power of two is exactly the same as masking off its low bits, and both implementations already flip negative hashes before reducing them. `BitArray` now caches that mask, and the reduction goes through `BloomFilterBase#bitIndex`, which masks when the bit size is a power of two and falls back to the modulo otherwise. The V1 fallback also uses a 32 bit division when both its hash and the bit size fit in an int, rather than widening to a 64 bit one. The bit positions are unchanged, so this affects no serialized filter, no format version, no memory footprint and no false positive rate.
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
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.
What changes were proposed in this pull request?
Both bloom filter implementations reduce a hash into a bit index with
hash % bitSize, once per hash function, in the innermost loop ofputandmightContain:bitSizeis a field rather than a compile time constant, so this compiles down to a hardware division on every probe.BitArrayrounds its allocation up to whole 64 bit words, so its bit size is a power of two for a wide range of requested sizes. Taking a non-negative value modulo a power of two is exactly masking off its low bits, and both implementations already flip negative hashes before reducing them.This PR:
bitSize - 1when the bit size is a power of two, 0 otherwise) inBitArray. It is computed in the single private constructor that every construction path funnels through, includingBitArray#readFrom, so no deserialization path can miss it.BloomFilterImpl(V1) andBloomFilterImplV2through a newBloomFilterBase#bitIndex, which masks when the mask is set and falls back to the modulo otherwise. The condition is loop invariant, so it is hoisted out of the loop.combinedHashis anint, so today it pays a 64 bit division to reduce a 32 bit value.The bit positions produced are unchanged, so there is no new
BloomFilter.Version, no change to serialized filters, no change to the memory footprint and no change to the false positive rate.Why are the changes needed?
This is the hottest loop of the runtime bloom filter used by
bloom_filter_agg/might_containfor join pushdown, and the reduction happens once per hash function per row.The Spark SQL runtime bloom filter sizes are powers of two, so the masking path is what those filters take:
spark.sql.optimizer.runtime.bloomFilter.numBitsdefaults to8388608(2^23) andspark.sql.optimizer.runtime.bloomFilter.maxNumBitsto67108864(2^26).Benchmark, on the power of two sizes (
Bit Size Shape Impactsection added toSparkBloomFilterBenchmark), per row nanoseconds, lower is better. Two runs after the change are given because these numbers carry a few percent of run to run noise:The benchmark compares a filter of
2^kbits against one of2^k + 64bits, which hold practically the same number of bits but take the masking and the modulo path respectively. As a control, the same benchmark on unmodifiedmasterputs the two within 1-3% of each other for every case above, so the gap is the change rather than the bit size shape. The non power of two cases are unchanged by this PR, as expected.These numbers are from an Apple silicon machine, whose divider is comparatively fast; the benchmark result files are not regenerated here, since that should happen on the standard benchmark hardware.
Does this PR introduce any user-facing change?
No. The bit indices, and therefore the contents of every bloom filter, are identical to before.
How was this patch tested?
New
BloomFilterBitIndexSuite, which pins down the equivalence the change rests on:BitArraywrite/read round trip,bitIndex(hash, bitSize, mask) == hash % bitSizefor 10000 random hashes plus the edge values, over both power of two and non power of two bit sizes, for thelongand theintoverload, and for a bit size beyond the int range.New cases in
BloomFilterSuitecovering V1 and V2 against a power of two and a non power of two bit size: no false negatives, plus a serialization round trip.Existing suites:
sketch/test(44 tests) andBloomFilterAggregateQuerySuite(11 tests) pass.Benchmark cases added to
SparkBloomFilterBenchmark; the result files should be regenerated through the GitHub Actions benchmark workflow for a consistent environment.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)