Skip to content

Conversation

@frsyuki
Copy link
Member

@frsyuki frsyuki commented Dec 25, 2015

This pull-request makes it possible that MessageBufferOutput implementations can allocate memory from external buffer pools such as Netty (netty-io) PooledByteBufAllocator or Jetty ByteBufferPool.
This pull-request consists with following changes:

  • Improvements of MessageBufferOutput interface. The old flush method is divided into 3 methods: writeBuffer, write, add.
    • writeBuffer method writes the internal buffer. This lets MessageBufferOutput to know that the previously allocated buffer by next() is ready to release.
    • write and add method write external buffer which were NOT allocated by MessageBufferOutput.next. This lets MessageBufferOutput to tell that this buffer should not be returned to a memory pool. write method requires MessageBufferOutput to copy the contents of it immediately. add method moves ownership of the buffer to MessageBufferOutput so that it doesn't have to copy the buffer.
  • Update of packString method to support the new API. This implementation tries to optimize performance by assuming that strings serialized using MessagePack are most likely ASCII-only or mostly ASCII. As long as this assumption is correct, number of flush calls to the final destination is reduced (buffering works better). If a lot of multi-byte UTF-8 sequences are given, this implementation causes one extra copy.
    • If source string is long (longer than 16K characters), this implementation immediately allocates a new buffer by calling String.getBytes. This is slightly inefficient because buffer is not reused. But In this case, this implementation can do another optimization that gives ownership of the new buffer to MessageBufferOutput.add method. Therefore MessageBufferOutput.add has chance to optimize by using zero-copy. Optimized MessageBufferOutput that takes advantage of it should be done in another pull-request. The reason of this overall design is to simplify the string encoding code.

@xerial
Copy link
Member

xerial commented Jan 4, 2016

Merged with #310

@xerial xerial closed this Jan 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants