Bit-packing works on blocks larger than 128 longs #137811
Conversation
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
parkertimmins
left a comment
parkertimmins
left a comment
There was a problem hiding this comment.
Been looking at the numeric compression lately, so went ahead and gave this a review. 😃
Just a couple of nits, logic looks correct.
server/src/main/java/org/elasticsearch/index/codec/tsdb/DocValuesForUtil.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/codec/tsdb/DocValuesForUtil.java
Outdated
Show resolved
Hide resolved
martijnvg
left a comment
martijnvg
left a comment
There was a problem hiding this comment.
This looks good Kostas 👍
I do have one question, can you both run the EncodeBenchmark and DecodeBenchmark micro benchmarks and check whether no regression was introduced by this change. I don't think so, but better be safe than sorry.
| private static final int BITPACK_BLOCK_SHIFT = 7; | ||
| private static final int BITBACK_BLOCK_SIZE = 1 << BITPACK_BLOCK_SHIFT; |
There was a problem hiding this comment.
Maybe turn this unto fields? And let both es87 and es819 doc values formats set this using their own block shift and size constants?
There was a problem hiding this comment.
This actually needs to be separated from the block sizes used in the es87 and es819. The only value that works here is 128.
server/src/main/java/org/elasticsearch/index/codec/ForUtil.java
Outdated
Show resolved
Hide resolved
4 participants
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