-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Bit-packing works on blocks larger than 128 longs #137811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
parkertimmins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Been looking at the numeric compression lately, so went ahead and gave this a review. 😃
Just a couple of nits, logic looks correct.
| private static final int BITS_IN_SIX_BYTES = 6 * Byte.SIZE; | ||
| private static final int BITS_IN_SEVEN_BYTES = 7 * Byte.SIZE; | ||
|
|
||
| private static final int BITBACK_BLOCK_SHIFT = 7; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BITBACK -> BITPACK?
| assert numericBlockSize >= BITBACK_BLOCK_SIZE && (numericBlockSize & (BITBACK_BLOCK_SIZE - 1)) == 0 | ||
| : "expected to get a block size that a multiple of " + BITBACK_BLOCK_SIZE + ", got " + numericBlockSize; | ||
| this.blockSize = numericBlockSize; | ||
| this.encoded = new byte[numericBlockSize * Byte.SIZE]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: It be the same value, but would be clearer ifByte.SIZE were Long.BYTES`
Existing bit-packing logic is customized to apply to blocks of 128 longs. For larger blocks that are multiple of 128, the bit-packing logic is extended to be applied to blocks of size 128 iteratively. This is expected to be a no-op, since the codec block size is 128, but unlocks experimenting with larger blocks.
Related to #136307