[GitHub] eolivelli commented on issue #1277: WIP - Use ByteBufOutputStream to serialize ProtoBuf messages
eolivelli commented on issue #1277: WIP - Use ByteBufOutputStream to serialize ProtoBuf messages URL: https://github.com/apache/bookkeeper/pull/1277#issuecomment-375366545 Closing this PR in favour of #1286 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1277: WIP - Use ByteBufOutputStream to serialize ProtoBuf messages
eolivelli commented on issue #1277: WIP - Use ByteBufOutputStream to serialize ProtoBuf messages URL: https://github.com/apache/bookkeeper/pull/1277#issuecomment-374552243 It seems that this code is working as well: ``` private static ByteBuf serializeProtobuf(MessageLite msg, ByteBufAllocator allocator) { int size = msg.getSerializedSize(); ByteBuf buf = allocator.heapBuffer(size, size); try { msg.writeTo(CodedOutputStream.newInstance(buf.array(), buf.arrayOffset() + buf.writerIndex(), size)); buf.writerIndex(buf.capacity()); } catch (IOException e) { // This is in-memory serialization, should not fail throw new RuntimeException(e); } for (int i = 0; i < size; i++) { } return buf; } ``` It seems that adding some code after the write makes the system work as expected. Typical message size is about 120 bytes , ad we are talking about READ_ENTRY responses as reported on Slack I have added some assertions, but just adding assertions makes the system work 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1277: WIP - Use ByteBufOutputStream to serialize ProtoBuf messages
eolivelli commented on issue #1277: WIP - Use ByteBufOutputStream to serialize ProtoBuf messages URL: https://github.com/apache/bookkeeper/pull/1277#issuecomment-374236185 I am marking this PR as Work-in-progress in order to gather reviews and comments. This fix makes the staging environment work on Java 9. This is the email thread on user list http://mail-archives.apache.org/mod_mbox/bookkeeper-user/201803.mbox/%3CCACcefge48TTvEocqDbP%2BOCsBH6tuOOpJSS_tLnmHX3bKxXLysg%40mail.gmail.com%3E Another fix is to use Unpooled ByteBufs instead of using the ByteBufAllocator returned by the Channel. This fix may impact somehow performances because previously we were writing directly to the byte[] inside the ByteBuf. This fix may open the way to using direct memory buffers on the v3 encoder as an heapBuffer is no more needed 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: us...@infra.apache.org With regards, Apache Git Services