siddharthteotia commented on a change in pull request #6710:
URL: https://github.com/apache/incubator-pinot/pull/6710#discussion_r599240463
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableImplV2V3.java
##########
@@ -344,6 +395,20 @@ public void addException(ProcessingException
processingException) {
return byteArrayOutputStream.toByteArray();
}
+ private byte[] serializePositionalData()
Review comment:
This is actually not doing the serialization to the main output stream
opened by the caller toByte().
This function like the other serialization functions first writes to a
temporary output stream and then converts to byte array which is returned to
the caller and written to the main stream. I think the reason for doing that is
upfront we don't know the length of byte[] array to allocate.
However, for this footer we can probably do different and it might be faster
- Write a loop to go over each entry and keep a running sum of size
- At the end of loop, allocate byte array of that size
- Start another loop and go over each entry again and fill out the
pre-allocated byte array.
- Return the filled byte array
This will prevent the unnecessary creation of streams at lined 400,401 and
then writing to them followed by converting to byte array. We can directly
write to byte array. I think this can be faster.
For the other Serialization functions which follow this approach, we can fix
them later outside this PR if need be
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]