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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org