-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Optimize Netty Frame Decoding #44664
Optimize Netty Frame Decoding #44664
Conversation
* We should not create a new wrapper object if there's no bytes in the `ByteBuf` * We should not create a new wrapped `ByteBuf` if it can't contain a message anyway because it doesn't even have enough bytes for a header left
Pinging @elastic/es-distributed |
@@ -35,7 +35,7 @@ | |||
@Override | |||
protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception { | |||
try { | |||
boolean continueDecode = true; | |||
boolean continueDecode = in.readableBytes() >= HEADER_SIZE; | |||
while (continueDecode) { | |||
int messageLength = TcpTransport.readMessageLength(Netty4Utils.toBytesReference(in)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the cost of some duplication (between Netty and NIO impls.) we could just remove creating the wrapping BytesReference
just for reading the length here as well. I'd look into that in a follow up.
...rt-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4SizeHeaderFrameDecoder.java
Outdated
Show resolved
Hide resolved
@ywelsch fixed :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
thanks @ywelsch ! |
* We should not create a new wrapper object if there's no bytes in the `ByteBuf` * We should not create a new wrapped `ByteBuf` if it can't contain a message anyway because it doesn't even have enough bytes for a header left
* We should not create a new wrapper object if there's no bytes in the `ByteBuf` * We should not create a new wrapped `ByteBuf` if it can't contain a message anyway because it doesn't even have enough bytes for a header left
ByteBuf
ByteBuf
if it can't contain a message anyway because it doesn't even have enough bytes for a header leftByteBuf
we still ran the decode loop once again (because of the missing update tocontinueDecode
that I now added) so without this change every message (if it's the only buffered message) cost an additional object here