From c5a7dd735877d926d12b77f94de119d32122b197 Mon Sep 17 00:00:00 2001 From: Tony Astolfi Date: Mon, 22 Jun 2026 20:14:44 -0400 Subject: [PATCH] Fix bug with bloom filter query cache. --- conan.lock | 6 +- conanfile.py | 2 +- src/llfs/bloom_filter.cpp | 30 ++++++--- src/llfs/bloom_filter.hpp | 88 ++++++++++++++++----------- src/llfs/packed_bloom_filter_page.hpp | 11 ++++ 5 files changed, 89 insertions(+), 48 deletions(-) diff --git a/conan.lock b/conan.lock index 18597a4..9545608 100644 --- a/conan.lock +++ b/conan.lock @@ -8,7 +8,7 @@ "gflags/2.2.2", "glog/0.7.1", "gtest/1.17.0", - "libbacktrace/cci.20240730", + "libbacktrace/cci.20210118", "libfuse/3.16.2", "libunwind/1.8.1", "liburing/2.11", @@ -49,7 +49,7 @@ "libunwind/1.8.1" ], "libbacktrace/[>=cci.20210118]": [ - "libbacktrace/cci.20240730" + "libbacktrace/cci.20210118" ], "openssl/[>=3 <4]": [ "openssl/[>=3.6.0 <4]" @@ -65,7 +65,7 @@ "zlib/[>=1.3.1 <2]" ], "libbacktrace/cci.20210118": [ - "libbacktrace/[>=cci.20240730]" + "libbacktrace/[>=cci.20210118]" ], "libunwind/1.8.0": [ "libunwind/[>=1.8.1 <2]" diff --git a/conanfile.py b/conanfile.py index a41ef60..09fb3bc 100644 --- a/conanfile.py +++ b/conanfile.py @@ -80,7 +80,7 @@ def requirements(self): self.requires("boost/[>=1.88.0 <2]", **VISIBLE) self.requires("cli11/[>=2.5.0 <3]", **VISIBLE) self.requires("glog/[>=0.7.1 <1]", **VISIBLE) - self.requires("libbacktrace/[>=cci.20240730]", **VISIBLE) + self.requires("libbacktrace/[>=cci.20210118]", **VISIBLE) self.requires("openssl/[>=3.6.0 <4]", **VISIBLE) self.requires("xxhash/[>=0.8.3 <1]", **VISIBLE) self.requires("zlib/[>=1.3.1 <2]") diff --git a/src/llfs/bloom_filter.cpp b/src/llfs/bloom_filter.cpp index aeba308..7ed8243 100644 --- a/src/llfs/bloom_filter.cpp +++ b/src/llfs/bloom_filter.cpp @@ -10,6 +10,8 @@ // #include +#include + namespace llfs { //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - @@ -307,19 +309,26 @@ std::ostream& operator<<(std::ostream& out, const BloomFilterConfig& t) //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // -void PackedBloomFilter::initialize(const BloomFilterConfig& config) noexcept +bool avx512_enabled() noexcept { - [[maybe_unused]] static const bool init = [] { - LLFS_LOG_INFO() << + static const bool enabled = [] { + bool value = false; #ifdef __AVX512F__ - "AVX512 enabled" -#else - "AVX512 NOT enabled" + value = batt::getenv_as("LLFS_AVX512").value_or(true); #endif - ; - return true; + LLFS_LOG_INFO() << "AVX512 " << (value ? "enabled" : "NOT enabled"); + return value; }(); + return enabled; +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// +void PackedBloomFilter::initialize(const BloomFilterConfig& config) noexcept +{ + [[maybe_unused]] static const bool init = avx512_enabled(); + BATT_CHECK_NE(config.word_count(), 0); BATT_CHECK_NE(config.block_count(), 0); @@ -332,6 +341,11 @@ void PackedBloomFilter::initialize(const BloomFilterConfig& config) noexcept this->block_count_pre_mul_shift_ = batt::log2_ceil(this->block_count_) + 1; this->block_count_post_mul_shift_ = 64 - this->block_count_pre_mul_shift_; + const std::uintptr_t addr_int = reinterpret_cast((const void*)this->words); + BATT_CHECK_EQ(addr_int % (512 / 8), 0) + << BATT_INSPECT(addr_int) << BATT_INSPECT((const void*)this) + << BATT_INSPECT((const void*)this->words); + this->check_invariants(); } diff --git a/src/llfs/bloom_filter.hpp b/src/llfs/bloom_filter.hpp index aea3ef0..e7f0f30 100644 --- a/src/llfs/bloom_filter.hpp +++ b/src/llfs/bloom_filter.hpp @@ -39,6 +39,8 @@ namespace llfs { +bool avx512_enabled() noexcept; + /** \brief Physical layout of a Bloom filter; each carries different trade-offs. */ enum struct BloomFilterLayout : u8 { @@ -185,7 +187,7 @@ std::ostream& operator<<(std::ostream& out, const BloomFilterConfig& t); template struct BloomFilterQuery { T key; - batt::SmallVec hash; + batt::SmallVec hash; batt::SmallVec mask; u32 mask_n_hashes = 0; @@ -282,6 +284,7 @@ struct BloomFilterQuery { // The mask should exactly as large as the block; all 0's initially. // + this->mask.clear(); this->mask.resize(kBlockWord64Size, 0); // Each time through the loop, we pull enough bits from this->hash[1..] to locate a single bit @@ -318,6 +321,7 @@ struct BloomFilterQuery { shift = 0; ++src_val_j; } + BATT_ASSERT_LT(shift, kShiftOverflow); } // Finally store the new number of hashes. @@ -400,7 +404,7 @@ struct PackedBloomFilter { // little_u8 block_count_post_mul_shift_; - // Align to 64-bit boundary. + // Align to 512-bit boundary. // little_u8 reserved_[41]; @@ -532,6 +536,8 @@ struct PackedBloomFilter { const usize block_i = this->block_index_from_hash(query.hash[0]); + BATT_ASSERT_LT(block_i, this->block_count_.value()); + return (this->words[block_i] & query.mask[0]) == query.mask[0]; } @@ -545,36 +551,41 @@ struct PackedBloomFilter { const usize block_i = this->block_index_from_hash(query.hash[0]); const usize block_first_word_i = block_i * 8; + BATT_ASSERT_LT(block_i, this->block_count_.value()); + BATT_ASSERT_LT(block_first_word_i + 7, this->word_count_.value()); + const i64* const block_p = (const i64*)&this->words[block_first_word_i]; const i64* const query_p = (const i64*)&query.mask[0]; BATT_CHECK_EQ(query.mask.size(), 8); + if (avx512_enabled()) { #ifdef __AVX512F__ - __m512i block_512 = {block_p[0], block_p[1], block_p[2], block_p[3], - block_p[4], block_p[5], block_p[6], block_p[7]}; + __m512i block_512 = {block_p[0], block_p[1], block_p[2], block_p[3], + block_p[4], block_p[5], block_p[6], block_p[7]}; - __m512i query_512 = {query_p[0], query_p[1], query_p[2], query_p[3], - query_p[4], query_p[5], query_p[6], query_p[7]}; + __m512i query_512 = {query_p[0], query_p[1], query_p[2], query_p[3], + query_p[4], query_p[5], query_p[6], query_p[7]}; - const bool match = _mm512_cmp_epi64_mask(_mm512_and_epi64(block_512, query_512), query_512, - _MM_CMPINT_EQ) == __mmask8{0b11111111}; + const bool match = _mm512_cmp_epi64_mask(_mm512_and_epi64(block_512, query_512), query_512, + _MM_CMPINT_EQ) == __mmask8{0b11111111}; - return match; + return match; #else - - return ((block_p[0] & query_p[0]) == query_p[0]) && // - ((block_p[1] & query_p[1]) == query_p[1]) && //; - ((block_p[2] & query_p[2]) == query_p[2]) && //; - ((block_p[3] & query_p[3]) == query_p[3]) && //; - ((block_p[4] & query_p[4]) == query_p[4]) && //; - ((block_p[5] & query_p[5]) == query_p[5]) && //; - ((block_p[6] & query_p[6]) == query_p[6]) && //; - ((block_p[7] & query_p[7]) == query_p[7]); - + BATT_PANIC() << "AVX512 not compiled in!"; #endif + } else { + return ((block_p[0] & query_p[0]) == query_p[0]) && // + ((block_p[1] & query_p[1]) == query_p[1]) && //; + ((block_p[2] & query_p[2]) == query_p[2]) && //; + ((block_p[3] & query_p[3]) == query_p[3]) && //; + ((block_p[4] & query_p[4]) == query_p[4]) && //; + ((block_p[5] & query_p[5]) == query_p[5]) && //; + ((block_p[6] & query_p[6]) == query_p[6]) && //; + ((block_p[7] & query_p[7]) == query_p[7]); + } } template @@ -583,34 +594,39 @@ struct PackedBloomFilter { const usize block_i = this->block_index_from_hash(query.hash[0]); const usize block_first_word_i = block_i * 8; + BATT_ASSERT_LT(block_i, this->block_count_.value()); + BATT_ASSERT_LT(block_first_word_i + 7, this->word_count_.value()); + const i64* const block_p = (const i64*)&this->words[block_first_word_i]; const i64* const query_p = (const i64*)query.mask.data(); + if (avx512_enabled()) { #ifdef __AVX512F__ - __m512i block_512 = {block_p[0], block_p[1], block_p[2], block_p[3], - block_p[4], block_p[5], block_p[6], block_p[7]}; + __m512i block_512 = {block_p[0], block_p[1], block_p[2], block_p[3], + block_p[4], block_p[5], block_p[6], block_p[7]}; - __m512i query_512 = {query_p[0], query_p[1], query_p[2], query_p[3], - query_p[4], query_p[5], query_p[6], query_p[7]}; + __m512i query_512 = {query_p[0], query_p[1], query_p[2], query_p[3], + query_p[4], query_p[5], query_p[6], query_p[7]}; - const bool match = _mm512_cmp_epi64_mask(_mm512_and_epi64(block_512, query_512), query_512, - _MM_CMPINT_EQ) == __mmask8{0b11111111}; + const bool match = _mm512_cmp_epi64_mask(_mm512_and_epi64(block_512, query_512), query_512, + _MM_CMPINT_EQ) == __mmask8{0b11111111}; - return match; + return match; #else - - return ((block_p[0] & query_p[0]) == query_p[0]) && // - ((block_p[1] & query_p[1]) == query_p[1]) && //; - ((block_p[2] & query_p[2]) == query_p[2]) && //; - ((block_p[3] & query_p[3]) == query_p[3]) && //; - ((block_p[4] & query_p[4]) == query_p[4]) && //; - ((block_p[5] & query_p[5]) == query_p[5]) && //; - ((block_p[6] & query_p[6]) == query_p[6]) && //; - ((block_p[7] & query_p[7]) == query_p[7]); - + BATT_PANIC() << "AVX512 not compiled in!"; #endif + } else { + return ((block_p[0] & query_p[0]) == query_p[0]) && // + ((block_p[1] & query_p[1]) == query_p[1]) && //; + ((block_p[2] & query_p[2]) == query_p[2]) && //; + ((block_p[3] & query_p[3]) == query_p[3]) && //; + ((block_p[4] & query_p[4]) == query_p[4]) && //; + ((block_p[5] & query_p[5]) == query_p[5]) && //; + ((block_p[6] & query_p[6]) == query_p[6]) && //; + ((block_p[7] & query_p[7]) == query_p[7]); + } } template diff --git a/src/llfs/packed_bloom_filter_page.hpp b/src/llfs/packed_bloom_filter_page.hpp index 3e19b45..2758ed1 100644 --- a/src/llfs/packed_bloom_filter_page.hpp +++ b/src/llfs/packed_bloom_filter_page.hpp @@ -68,11 +68,19 @@ struct PackedBloomFilterPage { */ little_u64 bit_count; + /** \brief The number of keys in this filter. + */ + little_u64 key_count; + /** \brief If this filter is associated with another page, the page id of that page is stored * here. */ PackedPageId src_page_id; + // Needed to make sure bloom_filter's word array starts on a 512-bit boundary. + // + u8 pad_[24]; + /** \brief The Bloom filter header; the actual filter bits follow this struct, and may extend to * the end of the page. */ @@ -102,6 +110,8 @@ struct PackedBloomFilterPage { void check_integrity() const; }; +static_assert(sizeof(PackedBloomFilterPage) == 128); + //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // /** \brief Builds a PackedBloomFilterPage in the passed page buffer from the items in the passed @@ -155,6 +165,7 @@ StatusOr build_bloom_filter_page( // Grab the item count to figure out how to size the filter. // const u64 item_count = std::distance(std::begin(items), std::end(items)); + packed_filter_page->key_count = item_count; // Calculate the total filter bits to use. //