Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 66 additions & 13 deletions lib/logstorage/bloomfilter.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package logstorage

import (
"fmt"
bitutil "math/bits"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't a good idea to use aliases for packages, since it complicates investigating and reading the code. It is better to rename local variables, which clash with the package names instead.

"sync"
"unsafe"

Expand Down Expand Up @@ -82,12 +83,33 @@ func (bf *bloomFilter) mustInitTokens(tokens []string) {
// mustInitHashes initializes bf with the given hashes
func (bf *bloomFilter) mustInitHashes(hashes []uint64) {
bitsCount := len(hashes) * bloomFilterBitsPerItem
wordsCount := (bitsCount + 63) / 64
bits := slicesutil.SetLength(bf.bits, wordsCount)
wordsCount := int64((bitsCount + 63) / 64)
/*
If the length of the array is a power of 2, then subsequent calculations can use bitwise operations instead of modulo operations to improve performance.
However, the increase in array length also increases the length of the data, resulting in negative optimization.
A wider range of trade-offs may be required.
*/
//wordsCount = roundUpPowerOf2(wordsCount)
bits := slicesutil.SetLength(bf.bits, int(wordsCount))
bloomFilterAddHashes(bits, hashes)
bf.bits = bits
}

func roundUpPowerOf2(x int64) int64 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function isn't used, so it must be removed. Also, it might be better from the readability and performance PoV to use bits.Len64() instead of the manual calculations (if this function will be used in the future):

func roundUpPowerOf(x int64) int64 {
  return int64(1 << bits.Len(uint64(x)))
}

if x <= 0 {
return 1
}
x--
x |= x >> 1
x |= x >> 2
x |= x >> 4
x |= x >> 8
x |= x >> 16
x |= x >> 32
x++
return x
}

// bloomFilterAddTokens adds the given tokens to the bloom filter bits
func bloomFilterAddTokens(bits []uint64, tokens []string) {
hashesCount := len(tokens) * bloomFilterHashesCount
Expand All @@ -107,15 +129,30 @@ func bloomFilterAddHashes(bits, hashes []uint64) {
}

func initBloomFilter(bits, hashes []uint64) {
maxBits := uint64(len(bits)) * 64
maxBits := uint64(len(bits)) << 6 // x<<6 means x*64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This optimization is performed by Go compiler, so it isn't needed - it just complicates reading and maintaining the code.

ptr := unsafe.Pointer(unsafe.SliceData(bits))
if bitutil.OnesCount64(maxBits) == 1 { // Determine whether it is a power of 2
Copy link
Contributor

@valyala valyala Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fast path will be triggered in very rare cases, so its' usefulness is questionable. Suppose the average size of the bloom filter is 32*128 bits. Then this optimization will be triggered only 6 out of 31 times - for (64*1, 64*2, 64*4, 64*8, 64*16, 64*32) bit lengths, or in 6/31=19% of cases. It isn't worth complicating the code for such cases. Additionally, more code in hot paths means the CPU must fit more code into instruction cache. This may lead to slowdowns if the needed instructions cannot fit the instruction cache.

// fast path
for _, h := range hashes {
idx := h & (maxBits - 1) // Fast modulo algorithm: 1,bit operation instead of division; 2,no judgment on whether the divisor is 0
i := idx >> 6
j := idx & 63
mask := uint64(1) << j
w := (*uint64)(unsafe.Add(ptr, i<<3))
if (*w & mask) == 0 {
*w |= mask
}
}
return
}
for _, h := range hashes {
idx := h % maxBits
i := idx / 64
j := idx % 64
i := idx >> 6
j := idx & 63
Comment on lines +150 to +151
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These optimizations aren't needed - Go compiler performs them on itself.

mask := uint64(1) << j
w := bits[i]
if (w & mask) == 0 {
bits[i] = w | mask
w := (*uint64)(unsafe.Add(ptr, i<<3))
if (*w & mask) == 0 {
*w |= mask
}
}
}
Expand Down Expand Up @@ -175,14 +212,30 @@ func (bf *bloomFilter) containsAll(hashes []uint64) bool {
if len(bits) == 0 {
return true
}
maxBits := uint64(len(bits)) * 64
maxBits := uint64(len(bits)) << 6 // x<<6 means x*64
ptr := unsafe.Pointer(unsafe.SliceData(bits))
if bitutil.OnesCount64(maxBits) == 1 { // Determine whether it is a power of 2
// fast path
for _, h := range hashes {
idx := h & (maxBits - 1) // Fast modulo algorithm: 1,bit operation instead of division; 2,no judgment on whether the divisor is 0
i := idx >> 6
j := idx & 63
mask := uint64(1) << j
w := (*uint64)(unsafe.Add(ptr, i<<3))
if (*w & mask) == 0 {
// The token is missing
return false
}
}
return true
}
for _, h := range hashes {
idx := h % maxBits
i := idx / 64
j := idx % 64
i := idx >> 6
j := idx & 63
mask := uint64(1) << j
w := bits[i]
if (w & mask) == 0 {
w := (*uint64)(unsafe.Add(ptr, i<<3))
if (*w & mask) == 0 {
// The token is missing
return false
}
Expand Down