Http2FrameLogger avoid hex dump of the ByteBufs when log disabled#7026
Http2FrameLogger avoid hex dump of the ByteBufs when log disabled#7026Gordiychuk wants to merge 4 commits intonetty:4.1from
Conversation
Currentry logger create hex dump even if log write will not apply. It's unecessary GC overhead. Restore optiomization from netty#3492 Close netty#7025
|
@Gordiychuk please fix checkstyle error Also please sign out icla and let me know once done |
done |
| int windowSizeIncrement) { | ||
| logger.log(level, "{} {} WINDOW_UPDATE: streamId={} windowSizeIncrement={}", ctx.channel(), | ||
| direction.name(), streamId, windowSizeIncrement); | ||
| if (isEnable()) { |
There was a problem hiding this comment.
The log level will be enforced by the logger. We should only have to "double check" the log level if creation of one of the arguments may potentially be expensive. We should only need this double check on methods with a toString(..) as a parameter.
Alternatively we could isolate the check in toString:
private String toString(ByteBuf buf) {
if (!logger.isEnabled(level)) {
return "";
}
if (level == InternalLogLevel.TRACE || buf.readableBytes() <= BUFFER_LENGTH_THRESHOLD) {
// Log the entire buffer.
return ByteBufUtil.hexDump(buf);
}
// Otherwise just log the first 64 bytes.
int length = Math.min(buf.readableBytes(), BUFFER_LENGTH_THRESHOLD);
return ByteBufUtil.hexDump(buf, buf.readerIndex(), length) + "...";
}There was a problem hiding this comment.
@Gordiychuk please address and once done we can merge it :)
|
|
||
| private String toString(ByteBuf buf) { | ||
| if (!logger.isEnabled(level)) { | ||
| return ""; |
There was a problem hiding this comment.
just one last nit: StringUtil.EMPTY
|
Squashed and cherry-picked into 4.1 (fe8ecea). @Gordiychuk thanks! |
Currentry logger create hex dump even if log write will not apply.
It's unecessary GC overhead. Restore optiomization from #3492
Close #7025