diff --git a/src/filter_match_proxy.cpp b/src/filter_match_proxy.cpp index ce33435402..4c19b6e546 100644 --- a/src/filter_match_proxy.cpp +++ b/src/filter_match_proxy.cpp @@ -7,16 +7,16 @@ template void bitmask_filter_match::build_query_mask(const std::vector& filter_labels, LabelT unv_label) { // _query_bitmask_buf is already bound (by the ctor init list) to either the - // caller-supplied scratch buffer or the owned buffer. Size it to at least - // AVX2_TAIL_PADDING words so the 256-bit load in test_full_mask_val never - // reads past the end. + // caller-supplied scratch buffer or the owned buffer. Size it to + // _bitmask_size + AVX2_TAIL_PADDING words so the unconditional 256-bit load + // in test_full_mask_val's fast path never reads past the end (matches the + // node-side simple_bitmask_buf padding scheme). // // clear() first so the resize zero-fills every word: a reused scratch buffer // may already be at padded_size, in which case a bare resize is a no-op that // leaves stale high words from a previous query -- test_full_mask_val's // unconditional 256-bit AND would then treat them as real filter bits. - const std::uint64_t padded_size = - std::max(_bitmask_filters._bitmask_size, simple_bitmask_buf::AVX2_TAIL_PADDING); + const std::uint64_t padded_size = _bitmask_filters._bitmask_size + simple_bitmask_buf::AVX2_TAIL_PADDING; _query_bitmask_buf.clear(); _query_bitmask_buf.resize(padded_size, 0); _bitmask_full_val._mask = _query_bitmask_buf.data(); diff --git a/src/index.cpp b/src/index.cpp index 76d7422598..7433b9fbca 100644 --- a/src/index.cpp +++ b/src/index.cpp @@ -2577,12 +2577,12 @@ void Index::aggregate_points_by_bitmask_label( std::advance(itr, lbl); auto& x = *itr; - // Pad to >=AVX2_TAIL_PADDING words: test_full_mask_val issues an + // Pad by AVX2_TAIL_PADDING words: test_full_mask_val issues an // unconditional 256-bit load over this query mask, so it must be at least // 4 words long. clear() first so resize zero-fills every word (no stale // bits carried across label iterations). label_bitmask.clear(); - label_bitmask.resize(std::max(_bitmask_buf._bitmask_size, simple_bitmask_buf::AVX2_TAIL_PADDING), 0); + label_bitmask.resize(_bitmask_buf._bitmask_size + simple_bitmask_buf::AVX2_TAIL_PADDING, 0); simple_bitmask_full_val bitmask_full_val; bitmask_full_val._mask = label_bitmask.data(); diff --git a/tests/filter_match_proxy_tests.cpp b/tests/filter_match_proxy_tests.cpp index 76b7513f74..326aa6b54e 100644 --- a/tests/filter_match_proxy_tests.cpp +++ b/tests/filter_match_proxy_tests.cpp @@ -51,16 +51,14 @@ BOOST_AUTO_TEST_SUITE(FilterMatchProxy_tests) // ---- (2) AVX2 padding of query_bitmask_buf ---------------------------------- // -// The optimization changed the ctor from: -// query_bitmask_buf.resize(_bitmask_filters._bitmask_size, 0); -// to: -// size_t padded_size = std::max(_bitmask_size, (uint64_t)4); -// query_bitmask_buf.resize(padded_size, 0); -// This ensures a 256-bit (4x uint64) AVX2 load starting at -// query_bitmask_buf.data() never runs past the buffer end even when the -// natural bitmask_size is 1, 2, or 3 words. - -BOOST_AUTO_TEST_CASE(bitmask_ctor_pads_query_buf_to_at_least_4_words_when_size_1) +// build_query_mask sizes the query buffer to: +// bitmask_size + AVX2_TAIL_PADDING +// (unified with the node-side simple_bitmask_buf padding scheme). This ensures a +// 256-bit (4x uint64) AVX2 load starting at query_bitmask_buf.data() never runs +// past the buffer end for any bitmask_size, and the trailing padding words stay +// zero so they never contribute false bits to the intersection test. + +BOOST_AUTO_TEST_CASE(bitmask_ctor_pads_query_buf_when_size_1) { // 1 word covers up to 64 bits; use 64 bits total. auto filters = make_bitmask_buf(/*num_points*/ 8, /*total_bits*/ 64, @@ -71,8 +69,8 @@ BOOST_AUTO_TEST_CASE(bitmask_ctor_pads_query_buf_to_at_least_4_words_when_size_1 std::vector filter_labels = {3}; bitmask_filter_match m(filters, qbuf, filter_labels, /*unv*/ 0); - // Must be padded up to 4 for AVX2 safety. - BOOST_TEST(qbuf.size() == (size_t)4); + // size + 4 padding. + BOOST_TEST(qbuf.size() == (size_t)5); } BOOST_AUTO_TEST_CASE(bitmask_ctor_pads_query_buf_when_size_2) @@ -85,7 +83,7 @@ BOOST_AUTO_TEST_CASE(bitmask_ctor_pads_query_buf_when_size_2) std::vector filter_labels = {1}; bitmask_filter_match m(filters, qbuf, filter_labels, 0); - BOOST_TEST(qbuf.size() == (size_t)4); + BOOST_TEST(qbuf.size() == (size_t)6); } BOOST_AUTO_TEST_CASE(bitmask_ctor_pads_query_buf_when_size_3) @@ -98,12 +96,12 @@ BOOST_AUTO_TEST_CASE(bitmask_ctor_pads_query_buf_when_size_3) std::vector filter_labels = {0}; bitmask_filter_match m(filters, qbuf, filter_labels, 0); - BOOST_TEST(qbuf.size() == (size_t)4); + BOOST_TEST(qbuf.size() == (size_t)7); } -BOOST_AUTO_TEST_CASE(bitmask_ctor_no_padding_when_size_ge_4) +BOOST_AUTO_TEST_CASE(bitmask_ctor_pads_query_buf_when_size_4) { - // 256 bits -> bitmask_size == 4; no extra padding needed. + // 256 bits -> bitmask_size == 4. auto filters = make_bitmask_buf(4, 256, 0, {}); BOOST_TEST(filters._bitmask_size == (std::uint64_t)4); @@ -111,12 +109,12 @@ BOOST_AUTO_TEST_CASE(bitmask_ctor_no_padding_when_size_ge_4) std::vector filter_labels = {0}; bitmask_filter_match m(filters, qbuf, filter_labels, 0); - BOOST_TEST(qbuf.size() == (size_t)4); + BOOST_TEST(qbuf.size() == (size_t)8); } -BOOST_AUTO_TEST_CASE(bitmask_ctor_keeps_size_when_larger_than_4) +BOOST_AUTO_TEST_CASE(bitmask_ctor_pads_query_buf_when_size_larger_than_4) { - // 512 bits -> bitmask_size == 8; must not be truncated. + // 512 bits -> bitmask_size == 8; padded to 8 + 4. auto filters = make_bitmask_buf(4, 512, 0, {}); BOOST_TEST(filters._bitmask_size == (std::uint64_t)8); @@ -124,7 +122,7 @@ BOOST_AUTO_TEST_CASE(bitmask_ctor_keeps_size_when_larger_than_4) std::vector filter_labels = {0}; bitmask_filter_match m(filters, qbuf, filter_labels, 0); - BOOST_TEST(qbuf.size() == (size_t)8); + BOOST_TEST(qbuf.size() == (size_t)12); } BOOST_AUTO_TEST_CASE(bitmask_ctor_size_zero_leaves_query_buf_empty) @@ -389,7 +387,7 @@ BOOST_AUTO_TEST_CASE(bitmask_reused_query_buffer_is_rezeroed) std::vector f1 = {5}; bitmask_filter_match m1(filters, qbuf, f1, /*unv*/ 0); BOOST_TEST(m1.contain_filtered_label(2) == true); - BOOST_TEST(qbuf.size() == (size_t)4); // padded + BOOST_TEST(qbuf.size() == (size_t)5); // size 1 + 4 padding } // qbuf now retains proxy #1's bits (filter label 5 in word 0). A second // proxy filtering a DISJOINT label must not inherit label 5.