vert.x icon indicating copy to clipboard operation
vert.x copied to clipboard

Save intermediate text encoded buffer copies

Open franz1981 opened this issue 1 year ago • 1 comments

Websocket txt messages perform an additional byte[] copy, because:

  1. Buffer::buffer(String) is performing String::getBytes -> first byte[] allocation
  2. It than allocates VertxByteBufAllocator.DEFAULT.heapBuffer to write the copy in -> second byte[] allocation + copy

VertxByteBufAllocator.DEFAULT.heapBuffer could allocate, based on Unsafe presence:

  • UnpooledHeapByteBuf
  • UnpooledUnsafeHeapByteBuf

Both the Vertx childrens of these 2 Netty buffers are NOT exposing directly the ability to wrap a given byte[] and that's why we currently perform the copy: in theory UnpooledHeapByteBuf vertx variant could be modified to do it, but UnpooledUnsafeHeapByteBuf nope, because it doesn't expose its parent constructor which enable setting the byte[] array without copying it (see https://github.com/netty/netty/blob/netty-4.1.114.Final/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java#L69).

Fixing the additional copy should require to change both Vertx and Netty, for not much gains, given that sharing a given byte[] is likely to disappear in Netty 5, regardless, so, not a good solution.

A different path would be to "guesstimate" the length of the utf8 array (there are 2 Netty method for such: eager O(1) or precise O(n)) - and using Netty's ByteBuf encoder to perform it: this could be beneficial, but not ideal because the JVM can perform String checks to estimate the length of the encoded byte[] way faster thanks to its vectorized instructions. This still could be benchmarked, to make sure, is a valid assumption.

For completeness, this same path is used by https://github.com/eclipse-vertx/vert.x/blob/4.5.10/src/main/java/io/vertx/core/http/impl/WebSocketImplBase.java#L240 which is using Unpooled::copiedBuffer under the hood to do what's described above and allocates io.netty.buffer.UnpooledByteBufAllocator$InstrumentedUnpooledUnsafeHeapByteBuf or io.netty.buffer.UnpooledByteBufAllocator$InstrumentedUnpooledHeapByteBuf.

The approach taken for this PR is instead to allocates a UnpooledHeapByteBuf without performing any copy:

  • it has the disadvantages of bringing another Netty buffer type in
  • it has another disadvantages because it is still a Netty reference counted type, while the Vertx heap buffers are modified to not perform reference counting
  • it has the advantage of using the (supposly) faster String::getBytes and not performing any additional copy from it (including the wrappers BufferImpl and subsequent Netty slices

franz1981 avatar Oct 15 '24 08:10 franz1981

@mkouba @vietj

franz1981 avatar Oct 15 '24 08:10 franz1981