colinmjj commented on PR #157: URL: https://github.com/apache/incubator-uniffle/pull/157#issuecomment-1214715832
> > @summaryzb For this optimization, I think there has once more `System.arraycopy` to add record, and it will impact performance a lot with previous test. For this PR, I think it is target to make size of block larger than before. To reduce memory pressure in client side, how about change the parameter for flush strategy? > > Yeah,it indeed has once more `System.arraycopy` when record splits across writer buffer, however it result in reducing the num of `System.arraycopy` when transfer all the buffer into one byte array. Additionally, we should consider the total `System.arraycopy` for a certain total number of bytes to be written, rather than for a certain buffer. For the situation with buffer = 3k, length of record = 2.8k, after insert 1000 records: with current implementation, calls of `System.arraycopy` is about **2000** with PR, calls of `System.arraycopy` is about **2800** I agree that memory waste happen in your example, but I prefer to avoid possible performance regression with more resources. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
