New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Netty's optimized UTF-8 encoding if available [SPR-17558] #22090

Closed
spring-issuemaster opened this Issue Dec 3, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Dec 3, 2018

Mark Paluch opened SPR-17558 and commented

CharSequenceEncoder uses CharBuffer.wrap and charset.encode to encode String data to its binary representation. There can be two optimizations made here:

  1. CharSequenceEncoder uses typically UTF-8 encoding and Java's UTF-8 encoder requires significant computing time. It would make sense to detect this case and whether netty is on the class path to use netty's optimized UTF-8 encoding via ByteBufUtil.writeUtf8(…)
  2. Encoding creates a new unpooled ByteBuffer when calling Charset.encode. Netty's ByteBufUtil.encodeString() can encode a String to a pooled buffer that reduces GC pressure.

See also attached profiling snapshot.


Attachments:

Referenced from: commits 5a8b8b1, 6361b0c, a00be62, 4955d08

0 votes, 5 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Dec 4, 2018

Arjen Poutsma commented

It would make sense to detect this case and whether netty is on the class path.

It would not make sense, unfortunately: whether netty is on the class path is not a valid trigger. Users can choose to use a different DataBuffer implementation, or—when using the webclient on Tomcat, Jetty, or Undertow—ByteBuf will be on the classpath, but should only be used for the client-side; not server-side.

We could check whether a data buffer is an instance of NettyDataBuffer, and then call getNativeBuffer() to get access to the ByteBuf.

However, I think that's a bit ugly, so I'd rather expose the necessary functionality through the DataBuffer and DataBufferFactory abstractions, delegate to Netty's ByteBufUtil for NettyDataBuffer, and use a different, less optimal implementation for the DefaultDataBuffer.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Dec 4, 2018

Brian Clozel commented

Hey Arjen, that’s what I had in mind as well.
I can work on that and ask for a review or feel free to reassign this issue to yourself. I self-assigned it initially to make sure to benchmark the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment