Check if Log level is enabled before creating log statements (netty#8022

) Motivation There is a cost to concatenating strings and calling methods that will be wasted if the Logger's level is not enabled. Modifications Check if Log level is enabled before producing log statement. These are just a few cases found by RegEx'ing in the code. Result Tiny bit more efficient code.
qerub · Jun 14, 2018 · 3e3e515 · 3e3e515
1 parent 3521530
commit 3e3e515
Show file tree

Hide file tree

Showing 11 changed files with 59 additions and 26 deletions.
diff --git a/common/src/main/java/io/netty/util/HashedWheelTimer.java b/common/src/main/java/io/netty/util/HashedWheelTimer.java
@@ -437,10 +437,12 @@ public long pendingTimeouts() {
     }
 
     private static void reportTooManyInstances() {
-        String resourceType = simpleClassName(HashedWheelTimer.class);
-        logger.error("You are creating too many " + resourceType + " instances. " +
-                resourceType + " is a shared resource that must be reused across the JVM," +
-                "so that only a few instances are created.");
+        if (logger.isErrorEnabled()) {
+            String resourceType = simpleClassName(HashedWheelTimer.class);
+            logger.error("You are creating too many " + resourceType + " instances. " +
+                    resourceType + " is a shared resource that must be reused across the JVM," +
+                    "so that only a few instances are created.");
+        }
     }
 
     private final class Worker implements Runnable {

diff --git a/common/src/main/java/io/netty/util/concurrent/DefaultPromise.java b/common/src/main/java/io/netty/util/concurrent/DefaultPromise.java
@@ -510,7 +510,9 @@ private static void notifyListener0(Future future, GenericFutureListener l) {
         try {
             l.operationComplete(future);
         } catch (Throwable t) {
-            logger.warn("An exception was thrown by " + l.getClass().getName() + ".operationComplete()", t);
+            if (logger.isWarnEnabled()) {
+                logger.warn("An exception was thrown by " + l.getClass().getName() + ".operationComplete()", t);
+            }
         }
     }
 
@@ -740,7 +742,9 @@ private static void notifyProgressiveListener0(
         try {
             l.operationProgressed(future, progress, total);
         } catch (Throwable t) {
-            logger.warn("An exception was thrown by " + l.getClass().getName() + ".operationProgressed()", t);
+            if (logger.isWarnEnabled()) {
+                logger.warn("An exception was thrown by " + l.getClass().getName() + ".operationProgressed()", t);
+            }
         }
     }
 

diff --git a/common/src/main/java/io/netty/util/concurrent/SingleThreadEventExecutor.java b/common/src/main/java/io/netty/util/concurrent/SingleThreadEventExecutor.java
@@ -896,9 +896,11 @@ public void run() {
 
                     // Check if confirmShutdown() was called at the end of the loop.
                     if (success && gracefulShutdownStartTime == 0) {
-                        logger.error("Buggy " + EventExecutor.class.getSimpleName() + " implementation; " +
-                                SingleThreadEventExecutor.class.getSimpleName() + ".confirmShutdown() must be called " +
-                                "before run() implementation terminates.");
+                        if (logger.isErrorEnabled()) {
+                            logger.error("Buggy " + EventExecutor.class.getSimpleName() + " implementation; " +
+                                    SingleThreadEventExecutor.class.getSimpleName() + ".confirmShutdown() must " +
+                                    "be called before run() implementation terminates.");
+                        }
                     }
 
                     try {
@@ -915,9 +917,10 @@ public void run() {
                             STATE_UPDATER.set(SingleThreadEventExecutor.this, ST_TERMINATED);
                             threadLock.release();
                             if (!taskQueue.isEmpty()) {
-                                logger.warn(
-                                        "An event executor terminated with " +
-                                                "non-empty task queue (" + taskQueue.size() + ')');
+                                if (logger.isWarnEnabled()) {
+                                    logger.warn("An event executor terminated with " +
+                                            "non-empty task queue (" + taskQueue.size() + ')');
+                                }
                             }
 
                             terminationFuture.setSuccess(null);

diff --git a/handler/src/main/java/io/netty/handler/ssl/util/InsecureTrustManagerFactory.java b/handler/src/main/java/io/netty/handler/ssl/util/InsecureTrustManagerFactory.java
@@ -44,12 +44,16 @@ public final class InsecureTrustManagerFactory extends SimpleTrustManagerFactory
     private static final TrustManager tm = new X509TrustManager() {
         @Override
         public void checkClientTrusted(X509Certificate[] chain, String s) {
-            logger.debug("Accepting a client certificate: " + chain[0].getSubjectDN());
+            if (logger.isDebugEnabled()) {
+                logger.debug("Accepting a client certificate: " + chain[0].getSubjectDN());
+            }
         }
 
         @Override
         public void checkServerTrusted(X509Certificate[] chain, String s) {
-            logger.debug("Accepting a server certificate: " + chain[0].getSubjectDN());
+            if (logger.isDebugEnabled()) {
+                logger.debug("Accepting a server certificate: " + chain[0].getSubjectDN());
+            }
         }
 
         @Override

diff --git a/handler/src/main/java/io/netty/handler/ssl/util/SelfSignedCertificate.java b/handler/src/main/java/io/netty/handler/ssl/util/SelfSignedCertificate.java
@@ -175,7 +175,9 @@ public SelfSignedCertificate(String fqdn, SecureRandom random, int bits, Date no
                 try {
                     certificateInput.close();
                 } catch (IOException e) {
-                    logger.warn("Failed to close a file: " + certificate, e);
+                    if (logger.isWarnEnabled()) {
+                        logger.warn("Failed to close a file: " + certificate, e);
+                    }
                 }
             }
         }
@@ -288,15 +290,19 @@ static String[] newSelfSignedCertificate(
 
     private static void safeDelete(File certFile) {
         if (!certFile.delete()) {
-            logger.warn("Failed to delete a file: " + certFile);
+            if (logger.isWarnEnabled()) {
+                logger.warn("Failed to delete a file: " + certFile);
+            }
         }
     }
 
     private static void safeClose(File keyFile, OutputStream keyOut) {
         try {
             keyOut.close();
         } catch (IOException e) {
-            logger.warn("Failed to close a file: " + keyFile, e);
+            if (logger.isWarnEnabled()) {
+                logger.warn("Failed to close a file: " + keyFile, e);
+            }
         }
     }
 }
diff --git a/handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java b/handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java
@@ -178,7 +178,9 @@ private void discard(Throwable cause) {
                     closeInput(in);
                 } catch (Exception e) {
                     currentWrite.fail(e);
-                    logger.warn(ChunkedInput.class.getSimpleName() + ".isEndOfInput() failed", e);
+                    if (logger.isWarnEnabled()) {
+                        logger.warn(ChunkedInput.class.getSimpleName() + ".isEndOfInput() failed", e);
+                    }
                     closeInput(in);
                 }
             } else {

diff --git a/handler/src/main/java/io/netty/handler/traffic/AbstractTrafficShapingHandler.java b/handler/src/main/java/io/netty/handler/traffic/AbstractTrafficShapingHandler.java
@@ -441,11 +441,15 @@ public void run() {
                 // Anything else allows the handler to reset the AutoRead
                 if (logger.isDebugEnabled()) {
                     if (config.isAutoRead() && !isHandlerActive(ctx)) {
-                        logger.debug("Unsuspend: " + config.isAutoRead() + ':' +
-                                isHandlerActive(ctx));
+                        if (logger.isDebugEnabled()) {
+                            logger.debug("Unsuspend: " + config.isAutoRead() + ':' +
+                                    isHandlerActive(ctx));
+                        }
                     } else {
-                        logger.debug("Normal unsuspend: " + config.isAutoRead() + ':'
-                                + isHandlerActive(ctx));
+                        if (logger.isDebugEnabled()) {
+                            logger.debug("Normal unsuspend: " + config.isAutoRead() + ':'
+                                    + isHandlerActive(ctx));
+                        }
                     }
                 }
                 channel.attr(READ_SUSPENDED).set(false);

diff --git a/resolver/src/main/java/io/netty/resolver/HostsFileParser.java b/resolver/src/main/java/io/netty/resolver/HostsFileParser.java
@@ -75,7 +75,9 @@ public static HostsFileEntries parseSilently() {
         try {
             return parse(hostsFile);
         } catch (IOException e) {
-            logger.warn("Failed to load and parse hosts file at " + hostsFile.getPath(), e);
+            if (logger.isWarnEnabled()) {
+                logger.warn("Failed to load and parse hosts file at " + hostsFile.getPath(), e);
+            }
             return HostsFileEntries.EMPTY;
         }
     }

diff --git a/transport-sctp/src/main/java/io/netty/channel/sctp/oio/OioSctpChannel.java b/transport-sctp/src/main/java/io/netty/channel/sctp/oio/OioSctpChannel.java
@@ -402,7 +402,9 @@ private static void closeSelector(String selectorName, Selector selector) {
         try {
             selector.close();
         } catch (IOException e) {
-            logger.warn("Failed to close a " + selectorName + " selector.", e);
+            if (logger.isWarnEnabled()) {
+                logger.warn("Failed to close a " + selectorName + " selector.", e);
+            }
         }
     }
 

diff --git a/transport/src/main/java/io/netty/channel/ChannelInitializer.java b/transport/src/main/java/io/netty/channel/ChannelInitializer.java
@@ -88,7 +88,9 @@ public final void channelRegistered(ChannelHandlerContext ctx) throws Exception
      */
     @Override
     public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
-        logger.warn("Failed to initialize a channel. Closing: " + ctx.channel(), cause);
+        if (logger.isWarnEnabled()) {
+            logger.warn("Failed to initialize a channel. Closing: " + ctx.channel(), cause);
+        }
         ctx.close();
     }
 

diff --git a/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java b/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java
@@ -394,7 +394,9 @@ private void rebuildSelector0() {
             }
         }
 
-        logger.info("Migrated " + nChannels + " channel(s) to the new Selector.");
+        if (logger.isInfoEnabled()) {
+            logger.info("Migrated " + nChannels + " channel(s) to the new Selector.");
+        }
     }
 
     @Override