Conversation
This passes an accurate size estimate for bloom filter blocks when we build them so we can appropriately size the FBuf for it. The problem surfaced when I tested a different FBuf grow policy to strictly grow to the requested size instead of power of two increments (not included in this commit). I do think this can be a problem in practice: say 9 GiB bloom filter is built it will grow the allocation from 4 KiB to 16 GiB. The other place where we used this was for footer blocks, where 4KiB was a fine estimate. Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
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
3 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.
This passes an accurate size estimate for bloom filter blocks when we build them so we can appropriately size the FBuf for it in advance.
The problem surfaced when I changed the FBuf grow policy to strictly grow to the requested size instead of power of two increments. I do think this can be a problem in practice: say a 9 GiB bloom filter is built we will grow the buffer from 4 KiB all the way to 16 GiB. With this it should just do one allocation of 9 GiB upfront.
The other place where we used this was for footer blocks, where 4KiB is a fine (over)estimate.
Describe Manual Test Plan
I ran a bunch of tests looking at FBuf resizing and confirmed there is no resize anymore for writing the bloom filter.