Repository: netty/netty
Issue: Http2FrameLogger to many char[] allocation when logger disabled
Labels: defect

### Expected behavior
No char[] allocation when logger disabled

### Actual behavior
The logger always performing a hex dump of the ByteBufs regarless whether or not the log would take place. I faced with huge allocation during run grpc benchmarks
![screenshot from 2017-07-26 15 40 17](https://user-images.githubusercontent.com/10699594/28621281-17ec8cfa-7219-11e7-8579-d51b7489072d.png)

### Netty version
io.netty:netty-codec-http2:4.1.13.Final

### JVM version (e.g. `java -version`)
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)

Issue comments:
Check enable or not logger before encode already was introduced in #3492 but then lost in #6877
Will fix ...
@normanmaurer sorry, I already prepare PR #7026
Ha! Even better

> Am 26.07.2017 um 15:02 schrieb Vladimir Gordiychuk <notifications@github.com>:
> 
> @normanmaurer sorry, I already prepare PR #7026
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 

Fixed by https://github.com/netty/netty/pull/7026.

Pull request review comments:
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`:

```java
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) + "...";
    }
```
@Gordiychuk please address and once done we can merge it :)
just one last nit: `StringUtil.EMPTY`

Patch that resolved the issue:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameLogger.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameLogger.java
index 5a29d7f4616..f0297f9b1e9 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameLogger.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameLogger.java
@@ -20,6 +20,7 @@
 import io.netty.channel.ChannelHandlerAdapter;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.logging.LogLevel;
+import io.netty.util.internal.StringUtil;
 import io.netty.util.internal.UnstableApi;
 import io.netty.util.internal.logging.InternalLogLevel;
 import io.netty.util.internal.logging.InternalLogger;
@@ -132,6 +133,10 @@ public void logUnknownFrame(Direction direction, ChannelHandlerContext ctx, byte
     }
 
     private String toString(ByteBuf buf) {
+        if (!logger.isEnabled(level)) {
+            return StringUtil.EMPTY_STRING;
+        }
+
         if (level == InternalLogLevel.TRACE || buf.readableBytes() <= BUFFER_LENGTH_THRESHOLD) {
             // Log the entire buffer.
             return ByteBufUtil.hexDump(buf);


Test patch: