Merged
Conversation
Add a bounds check (panic if len(bms) < 1) and a doc comment explaining the ownership-transfer contract and why no-copy is intentional. The primary caller (bsiDocFreqs.ReadFrom in caterwaul) reads bitmaps freshly from a stream, reconstructs the existence bitmap by ORing the bit planes (the eBM is not stored on disk to save space), then hands the whole slice to FromBitmaps before it goes out of scope. All bitmaps are newly allocated from the stream at that point, so copying would be pure waste. The design also leaves the door open for a future zero-copy path once roaring gains FromUnsafeBytes support.
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
2 participants
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.
Summary
FromBitmapsinitializes a BSI from a pre-built slice of bitmaps, wherebms[0]is the existence bitmap andbms[1:]are the bit planes inleast-to-most-significant order — matching the layout of
MarshalBinary/UnmarshalBinary.The caller transfers ownership of the slice; the BSI aliases it directly
without copying. This is intentional: the primary use case is a
deserialization pipeline where the existence bitmap is not stored on
disk (it is reconstructed by ORing the bit planes on read, saving storage
space), and all bitmaps are freshly allocated from the stream. Copying at
that point would be pure waste. Because the caller's slice goes out of
scope immediately after the call, aliasing is safe.
A bounds check (panic if
len(bms) < 1) is included since an empty slicewould cause a silent panic or corrupt state.