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 allow 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

Reply via email to