[GitHub] dlg99 commented on issue #1108: Replace DoubleByteBuf with CompositeByteBuf because of perf regression with Netty > 4.1.12

[GitBox <gi...@apache.org>]

dlg99 commented on issue #1108: Replace DoubleByteBuf with CompositeByteBuf because of perf regression with Netty > 4.1.12
URL: https://github.com/apache/bookkeeper/pull/1108#issuecomment-362440987
 
 
   I experimented with getting rid of DoubleByteBuffer completely (on older/internal codebase).
   
   allocations are from its
   
   ```java
       public ByteBuffer nioBuffer(int index, int length) {
           ByteBuffer dst = isDirect() ? ByteBuffer.allocateDirect(length) : ByteBuffer.allocate(length);
   ```
   Composite also ends up doing 
   ```java
           ByteBuffer merged = ByteBuffer.allocate(length).order(order());
   ```
   if it is wrapped around > 1 buffers.
   so you are trading a chance for direct buffer allocation for always on-heap allocation. 
   
   We use DoubleByteBuf to wrap (header, data) tuple i.e. in computeDigestAndPackageForSending.
   Header is of fixed size for given ledger. so I got rid of DoubleByteBuf and preallocated enough space in single buffer to have data after offset for header and populated it. 
   I am not saying that this is the best way to do this but either getting rid of it, or having header/digest as a separate field in proto, or pooling buffers in nioBuffer().
   
   hope I am making sense.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services