Add byte-oriented sizing and validation for Bloom filters - #841
Open
yuweih205 wants to merge 1 commit into
Open
Conversation
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.
Callers with a byte budget currently have to convert it to block counts and repeat the policy's sizing constraints. Add
bloom_filter_size_bytesconstruction,aligned_size(bytes)to round down and cap a budget, andmax_size()to expose the byte limit.The byte constructor checks positive, block-aligned sizes, capacity limits, and static extent compatibility before delegating to the block-count constructor. Capacity calculations account for the policy, extent value type, and
std::size_trange. The new Catch2 cases join the existingBLOOM_FILTER_TESTtarget and cover byte sizing and construction.Fixes #829.
Validation of the current revision
BLOOM_FILTER_TESTtarget; run[sizing][host]BLOOM_FILTER_TESTtarget; run[sizing][host]git diff --check[sizing][gpu]The compiler checks used an x86_64 Ubuntu 24.04 environment. They cover the two toolchains above, not the complete repository CI matrix. The host tests were rerun before submission with source hashes matching this commit.
The host cases exercise rounding down, policy capping, budgets smaller than one block, zero/misaligned/oversized construction, a narrow extent's capacity limit, and static extent mismatch. The GPU cases compare byte and block construction for dynamic extents and
extent<uint8_t, 2>.Static extent boundary experiment
A separate allocator probe stopped at the allocation boundary without allocating device memory. In the pre-fix candidate,
extent<uint8_t, 257>truncated to one block and incorrectly reached the allocator for a one-block request when warnings were not treated as errors. The probe failed as expected, exposing the missing validation.With the final guard, that instantiation fails to compile with
Static extent must be representable by its size type. Valid static extents of 2 and 255 blocks still reach the allocator, while a one-block request for a two-block extent throwscuco::logic_errorbefore allocation. All three valid/mismatch probe checks passed. These diagnostic probes were run separately from the repository test suite.GPU validation status
The current revision passed on one NVIDIA H200 from an 8×H200 node (driver 595.58.03). The job verified the test binary and all four changed source files by SHA-256 before execution:
BLOOM_FILTER_TEST: 52 cases / 155 assertions passed.[sizing][gpu]: 2 cases / 4 assertions passed, with 0 errors and 0 bytes leaked.The completed host checks can be repeated with
./build/tests/BLOOM_FILTER_TEST '[sizing][host]'. Full repository CI remains to be run.