jooby icon indicating copy to clipboard operation
jooby copied to clipboard

MessageEncoder: must returns a ByteBuffer (not a byte[])

Open jknack opened this issue 3 years ago • 6 comments

So:

@Nullable byte[] encode(@Nonnull Context ctx, @Nonnull Object value) throws Exception;

Must be:

@Nullable ByteBuffer encode(@Nonnull Context ctx, @Nonnull Object value) throws Exception;

This give us opportunity to reuse a byte buffer due we can wrap a byte array

jknack avatar Nov 08 '20 14:11 jknack

I was kind of surprised to find this as well however if you do this you have to change the API more than just returning ByteBuffer.

The issue being thread safety. RingBuffer Libraries like the Disruptor have a commit to signify that the ByteBuffer can be reused and or a "translation" isolation API where you do the translation of ByteBuffer to something else and the expectation is you don't share that with other threads: http://lmax-exchange.github.io/disruptor/user-guide/index.html#_publishing_using_translators

Now some libraries cheat and use ThreadLocals like JRocker (and I believe Log4j2 before they switched to disruptor) but this has its own perils.

Also I believe reading somewhere that the JVM is especially good at dealing with byte arrays that were originally strings.

Regardless I recommend checking out disruptor as preallocation and garbage free can produce amazing results.

agentgt avatar Nov 27 '20 15:11 agentgt

I added a threadlocal+byte[] to rocker template engine. This is what most of the Java Frameworks does in the TEB site. So with this pattern a ByteBuffer.wrap makes sense.

Also I believe reading somewhere that the JVM is especially good at dealing with byte arrays that were originally strings.

I believe the same.. but never read anything about it. Can you share it?

Would you like to do a POC with ringbuffer? I would like to see it in action.

jknack avatar Nov 30 '20 14:11 jknack

Would you like to do a POC with ringbuffer? I would like to see it in action.

I will have to look into Netty to see what would happen as I'm not sure the programming models would line up correctly. Not knowing the low levels of Netty and Undertow are one of the reasons why I use libraries like Jooby 😄 . I have some idea but I just haven't used the modern version of the API's directly in some time.

Netty and HTTP event lopps seems to be more like reactive streams aka Java 9 flow and not a queue. You can certainly transform back and forth between the two but I'm not sure what happens (ie queue -> flow -> queue) perf wise. Traditionally it is a bad idea because blocking queues are slow but disruptor is kind of an exception.

While Disruptor does apparently have an Event API its more like an alternative to a blocking queue. I have used it for RabbitMQ and Logging but those are naturally queue like and most (modern) HTTP is not due to the request reply nature (ie it is not fire and forget).

The idea in this scenario though is to create a barrier to prevent extensions from screwing up the event queue and or accidentally writing to bytebuffers when they are in use by something else.

I'll look later this weekend.

agentgt avatar Nov 30 '20 21:11 agentgt

@agentgt what if we add caffeine-cache that works as ByteBuffer pool (not thread-local). What do you think?

cache should only works while running in event-loop mode.

jknack avatar Apr 07 '23 13:04 jknack

Sorry I missed this comment. I think if it is event-loop it is fine. I assume we would just make an abstraction so that the encoder does not have to know it is event-loop and the cache in worker mode would just return a new buffer every time?

The other option besides returning a ByteBuffer is having the encoder return a ReadableByteChannel which is actually how Rocker wants to do it. This would remove the need for the encoder to even know about a cache for buffer pool.

@Nullable ReadableByteChannel encode(@Nonnull Context ctx, @Nonnull Object value) throws Exception;

https://docs.oracle.com/javase/8/docs/api/java/nio/channels/ReadableByteChannel.html

That is probably the most reactive way to do it but it leaves a lot of work on the implementer. However the issue there is the encoder might want to give a hint on roughly how big the response will be so that you can decided how big of a ByteBuffer you want to use on its read method.

Now that we are on Java 17 you could make it future proof by using a sealed interface:

Encoded encode(@Nonnull Context ctx, @Nonnull Object value) throws Exception;

public sealed interface Encoded {
   public interface ByteBufferEncoded extends Encoded {}
   public interface ChannelEncoded extends Encoded, ReadableByteChannel {
      public int estimatedSize();
   }
   public enum Ignore implements Encoded {
      INSTANCE;
   }
}

agentgt avatar Jun 02 '23 15:06 agentgt

Here is Spring's BTW:

https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/core/codec/Encoder.html

Massively overcomplicated as usual for Spring (and surprisingly not very abstract as you have to use Reactor).

JStachio's implementation from Spring WebFlux is here:

https://github.com/jstachio/jstachio/blob/main/opt/jstachio-spring-webflux/src/main/java/io/jstach/opt/spring/webflux/JStachioEncoder.java

Perhaps providing a buffer factory and the sealed interface Encoded is the best option:

Encoded encode(@Nonnull Context ctx, @Nonnull Object value, BufferFactory bf) throws Exception;

agentgt avatar Jun 02 '23 15:06 agentgt