You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ck...@apache.org on 2021/03/24 19:21:07 UTC

[logging-log4j2] branch release-2.x updated: LOG4J2-2940: Reduce StackWalker interactions accessing an slf4j logger instance

This is an automated email from the ASF dual-hosted git repository.

ckozak pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


The following commit(s) were added to refs/heads/release-2.x by this push:
     new cefa593  LOG4J2-2940: Reduce StackWalker interactions accessing an slf4j logger instance
cefa593 is described below

commit cefa59383c560af7ba03300601a61d82480ffd67
Author: Carter Kozak <ck...@apache.org>
AuthorDate: Mon Mar 22 14:38:51 2021 -0400

    LOG4J2-2940: Reduce StackWalker interactions accessing an slf4j logger instance
    
    Previously we walked the stack twice, once to find the
    `org.slf4j.LoggerFactory` interaction, and a second time to
    discover the application code which called it. Now we use a new
    API to skip a frame at the end to find the final result in a single
    interaction.
---
 .../org/apache/logging/log4j/util/StackLocator.java  | 15 ++++++++++++---
 .../org/apache/logging/log4j/util/StackLocator.java  | 12 +++++++++++-
 .../apache/logging/log4j/util/StackLocatorUtil.java  | 20 +++++++++++++++++++-
 .../org/apache/logging/slf4j/Log4jLoggerFactory.java |  4 ++--
 .../org/apache/logging/slf4j/Log4jLoggerFactory.java |  4 ++--
 src/changes/changes.xml                              |  3 +++
 6 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/StackLocator.java b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/StackLocator.java
index c19670c..2740fcf 100644
--- a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/StackLocator.java
+++ b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/StackLocator.java
@@ -45,9 +45,18 @@ public class StackLocator {
     }
 
     public Class<?> getCallerClass(final String fqcn, final String pkg) {
-        return walker.walk(s -> s.dropWhile(f -> !f.getClassName().equals(fqcn)).
-                dropWhile(f -> f.getClassName().equals(fqcn)).dropWhile(f -> !f.getClassName().startsWith(pkg)).
-                findFirst()).map(StackWalker.StackFrame::getDeclaringClass).orElse(null);
+        return getCallerClass(fqcn, pkg, 0);
+    }
+
+    public Class<?> getCallerClass(final String fqcn, final String pkg, final int skipDepth) {
+        return walker.walk(s -> s
+                .dropWhile(f -> !f.getClassName().equals(fqcn))
+                .dropWhile(f -> f.getClassName().equals(fqcn))
+                .dropWhile(f -> !f.getClassName().startsWith(pkg))
+                .skip(skipDepth)
+                .findFirst())
+                .map(StackWalker.StackFrame::getDeclaringClass)
+                .orElse(null);
     }
 
     public Class<?> getCallerClass(final Class<?> anchor) {
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java
index e83dc12..cbe70a9 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java
@@ -119,6 +119,14 @@ public final class StackLocator {
     // migrated from Log4jLoggerFactory
     @PerformanceSensitive
     public Class<?> getCallerClass(final String fqcn, final String pkg) {
+        return getCallerClass(fqcn, pkg, 0);
+    }
+
+    @PerformanceSensitive
+    public Class<?> getCallerClass(final String fqcn, final String pkg, final int skipDepth) {
+        if (skipDepth < 0) {
+            throw new IllegalArgumentException("skipDepth cannot be negative");
+        }
         boolean next = false;
         Class<?> clazz;
         for (int i = 2; null != (clazz = getCallerClass(i)); i++) {
@@ -127,7 +135,9 @@ public final class StackLocator {
                 continue;
             }
             if (next && clazz.getName().startsWith(pkg)) {
-                return clazz;
+                return skipDepth == 0
+                        ? clazz
+                        : getCallerClass(i + skipDepth);
             }
         }
         // TODO: return Object.class
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java
index 3503229..7f1dc7e 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java
@@ -48,18 +48,36 @@ public final class StackLocatorUtil {
         return stackLocator.getStackTraceElement(depth + 1);
     }
 
+    /**
+     * Equivalent to {@link #getCallerClass(String, String)} with an empty {@code pkg}.
+     */
     // migrated from ClassLoaderContextSelector
     @PerformanceSensitive
     public static Class<?> getCallerClass(final String fqcn) {
         return getCallerClass(fqcn, Strings.EMPTY);
     }
 
-    // migrated from Log4jLoggerFactory
+    /**
+     * Equivalent to {@link #getCallerClass(String, String, int)} with {@code skipDepth = 0}.
+     */
     @PerformanceSensitive
     public static Class<?> getCallerClass(final String fqcn, final String pkg) {
         return stackLocator.getCallerClass(fqcn, pkg);
     }
 
+    /**
+     * Search for a calling class.
+     *
+     * @param fqcn Root class name whose caller to search for.
+     * @param pkg Package name prefix that must be matched after the {@code fqcn} has been found.
+     * @param skipDepth Number of stack frames to skip after the {@code fqcn} and {@code pkg} have been matched.
+     * @return The caller class that was matched, or null if one could not be located.
+     */
+    @PerformanceSensitive
+    public static Class<?> getCallerClass(final String fqcn, final String pkg, final int skipDepth) {
+        return stackLocator.getCallerClass(fqcn, pkg, skipDepth);
+    }
+
     // added for use in LoggerAdapter implementations mainly
     @PerformanceSensitive
     public static Class<?> getCallerClass(final Class<?> anchor) {
diff --git a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
index d4b460b..f513f0d 100644
--- a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
+++ b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
@@ -42,11 +42,11 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements
     @Override
     protected LoggerContext getContext() {
         final Class<?> anchor = LogManager.getFactory().isClassLoaderDependent()
-                ? StackLocatorUtil.getCallerClass(FQCN, PACKAGE)
+                ? StackLocatorUtil.getCallerClass(FQCN, PACKAGE, 1)
                 : null;
         return anchor == null
                 ? LogManager.getContext()
-                : getContext(StackLocatorUtil.getCallerClass(anchor));
+                : getContext(anchor);
     }
     private LoggerContext validateContext(final LoggerContext context) {
         if (TO_SLF4J_CONTEXT.equals(context.getClass().getName())) {
diff --git a/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java b/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
index eb5539f..3404e0b 100644
--- a/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
+++ b/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
@@ -48,11 +48,11 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements
     @Override
     protected LoggerContext getContext() {
         final Class<?> anchor = LogManager.getFactory().isClassLoaderDependent()
-                ? StackLocatorUtil.getCallerClass(FQCN, PACKAGE)
+                ? StackLocatorUtil.getCallerClass(FQCN, PACKAGE, 1)
                 : null;
         return anchor == null
                 ? LogManager.getContext()
-                : getContext(StackLocatorUtil.getCallerClass(anchor));
+                : getContext(anchor);
     }
 
     Log4jMarkerFactory getMarkerFactory() {
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 540819b..13f9d7a 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -53,6 +53,9 @@
       <action issue="LOG4J2-3054" dev="ckozak" type="fix">
         BasicContextSelector hasContext and shutdown take the default context into account
       </action>
+      <action issue="LOG4J2-2940" dev="ckozak" type="fix">
+        Slf4j implementations walk the stack at most once rather than twice to determine the caller's class loader.
+      </action>
     </release>
     <release version="2.14.1" date="2021-03-06" description="GA Release 2.14.1">
       <!-- FIXES -->