[common] Using a faster deserialization method in RoaringBitmap32 #4765
Conversation
|
Can you provide the benchmark result? |
|
I'm trying to understand how to use the benchmark in paimon, and I hope my benchmark using in correct.
|
|
Thanks for your information. |
| @@ -89,6 +91,10 @@ public void deserialize(DataInput in) throws IOException { | |||
| roaringBitmap.deserialize(in); | |||
| } | |||
|
|
|||
| public void deserialize(DataInput in, @Nullable byte[] buffer) throws IOException { | |||
| roaringBitmap.deserialize(in, buffer); | |||
There was a problem hiding this comment.
if deserialize(in, null) is better, can you just modify RoaringBitmap32.deserialize(DataInput)?
There was a problem hiding this comment.
I totally agree with this, but be aware that it may take up 8KB of memory during deserialization, although I think it's reasonable.
| @@ -86,7 +86,7 @@ public void serialize(DataOutput out) throws IOException { | |||
| } | |||
|
|
|||
| public void deserialize(DataInput in) throws IOException { | |||
| roaringBitmap.deserialize(in); | |||
| roaringBitmap.deserialize(in, null); | |||
There was a problem hiding this comment.
Maybe we can tested roaringBitmap.deserialize(ByteBuffer bbf) directly? We actually know the exact byte count of the bitmap
There was a problem hiding this comment.
Did you mean adding a benchmark?
There was a problem hiding this comment.
From the test results, it is more optimal, and I have checked other formats, which also use it. we can just use it.
There was a problem hiding this comment.
The premise of ByteBuffer is that we need to know the actual size of the Bitmap. This seems to require recording the actual size at serialisation time.
There was a problem hiding this comment.
We can using it in delection vector bitmap and rbm aggregate function, but the bitmap and bsi index can not for now.
There was a problem hiding this comment.
roaringBitmap.deserialize(ByteBuffer bbf)
Does the bitmap index not record the byte size? Would you be willing to integrate roaringBitmap.deserialize(ByteBuffer bbf) into a usable place such as dv?
|
Add the
|
There was a problem hiding this comment.
Hi @Tan-JiaLiang , can you use null for bitmap index, and use ByteBuffer for deletion vectors?
Tan-JiaLiang
force-pushed
the
faster_bitmap_deserialize
branch
from
1b0cecd to
f5272c2
Compare
Tan-JiaLiang
changed the title
|
@JingsongLi @Zouxxyy Can you help to review again? |
| if (magicNum == BitmapDeletionVector.MAGIC_NUMBER) { | ||
| return BitmapDeletionVector.deserializeFromDataInput(dis); | ||
| return BitmapDeletionVector.deserializeFromByteBuffer(buffer.slice()); |
There was a problem hiding this comment.
This is really just a programming convention to ensure that the ByteBuffer passed in are bitmaps only. I take a look that it is already using in RoaringArray#deserialize, so it's redundant here.
|
Seems the CDC test case is still having some thread safe problem, I will take a look in next week. Can someone help to trigger the build one more time? |
…ache#4765) (cherry picked from commit 8157be9)
Purpose
In
RoaringBitmap, thedeserialize(java.io.DataInput, byte[])function has a better performance to deserialize theBitmapContainer.Tests
No.
API and Format
No.
Documentation
No.