From 52bf8569e8e9881d0999156c31d99b99f28e6e73 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 7 Aug 2018 16:27:10 -0400 Subject: [PATCH] [LOG4J2-2391] Improve ThrowableProxy performace on java 9+ Share the SecurityManager implementation of getCurrentStackTrace with the java 9 implementation. --- log4j-api-java9/src/assembly/java9.xml | 1 + .../PrivateSecurityManagerStackTraceUtil.java | 34 +++++++++ .../logging/log4j/util/StackLocator.java | 4 ++ .../PrivateSecurityManagerStackTraceUtil.java | 71 +++++++++++++++++++ .../logging/log4j/util/StackLocator.java | 38 +--------- 5 files changed, 112 insertions(+), 36 deletions(-) create mode 100644 log4j-api-java9/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java create mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java diff --git a/log4j-api-java9/src/assembly/java9.xml b/log4j-api-java9/src/assembly/java9.xml index 41a0da0699d..58c63876ee7 100644 --- a/log4j-api-java9/src/assembly/java9.xml +++ b/log4j-api-java9/src/assembly/java9.xml @@ -35,6 +35,7 @@ **/Dummy.class **/spi/Provider.class **/util/PropertySource.class + **/util/PrivateSecurityManagerStackTraceUtil.class **/message/ThreadDumpMessage.class **/message/ThreadDumpMessage$ThreadInfoFactory.class diff --git a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java new file mode 100644 index 00000000000..e334bc81537 --- /dev/null +++ b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.logging.log4j.util; + +import java.util.Stack; + +/** + * This is a dummy class and is only here to allow this module to compile. It will not + * be copied into the log4j-api module. + */ +final class PrivateSecurityManagerStackTraceUtil { + + static boolean isEnabled() { + return false; + } + + static Stack> getCurrentStackTrace() { + return null; + } +} 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 b0661d0fe14..2cd658ff3db 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 @@ -61,6 +61,10 @@ public Class getCallerClass(final int depth) { } public Stack> getCurrentStackTrace() { + // benchmarks show that using the SecurityManager is much faster than looping through getCallerClass(int) + if (PrivateSecurityManagerStackTraceUtil.isEnabled()) { + return PrivateSecurityManagerStackTraceUtil.getCurrentStackTrace(); + } Stack> stack = new Stack>(); List> classes = walker.walk(s -> s.map(f -> f.getDeclaringClass()).collect(Collectors.toList())); stack.addAll(classes); diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java new file mode 100644 index 00000000000..51bca15fc92 --- /dev/null +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.logging.log4j.util; + +import java.util.Stack; + +/** + * Internal utility to share a fast implementation of {@link #getCurrentStackTrace()} + * with the java 9 implementation of {@link StackLocator}. + */ +final class PrivateSecurityManagerStackTraceUtil { + + private static final PrivateSecurityManager SECURITY_MANAGER; + + static { + PrivateSecurityManager psm; + try { + final SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + sm.checkPermission(new RuntimePermission("createSecurityManager")); + } + psm = new PrivateSecurityManager(); + } catch (final SecurityException ignored) { + psm = null; + } + + SECURITY_MANAGER = psm; + } + + private PrivateSecurityManagerStackTraceUtil() { + // Utility Class + } + + static boolean isEnabled() { + return SECURITY_MANAGER != null; + } + + // benchmarks show that using the SecurityManager is much faster than looping through getCallerClass(int) + static Stack> getCurrentStackTrace() { + final Class[] array = SECURITY_MANAGER.getClassContext(); + final Stack> classes = new Stack<>(); + classes.ensureCapacity(array.length); + for (final Class clazz : array) { + classes.push(clazz); + } + return classes; + } + + private static final class PrivateSecurityManager extends SecurityManager { + + @Override + protected Class[] getClassContext() { + return super.getClassContext(); + } + + } +} 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 fa7df446970..4eba44bff62 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 @@ -46,8 +46,6 @@ */ public final class StackLocator { - private static final PrivateSecurityManager SECURITY_MANAGER; - // Checkstyle Suppress: the lower-case 'u' ticks off CheckStyle... // CHECKSTYLE:OFF static final int JDK_7u25_OFFSET; @@ -82,19 +80,6 @@ public final class StackLocator { java7u25CompensationOffset = -1; } - PrivateSecurityManager psm; - try { - final SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(new RuntimePermission("createSecurityManager")); - } - psm = new PrivateSecurityManager(); - } catch (final SecurityException ignored) { - psm = null; - } - - SECURITY_MANAGER = psm; - GET_CALLER_CLASS = getCallerClass; JDK_7u25_OFFSET = java7u25CompensationOffset; @@ -167,14 +152,8 @@ public Class getCallerClass(final Class anchor) { @PerformanceSensitive public Stack> getCurrentStackTrace() { // benchmarks show that using the SecurityManager is much faster than looping through getCallerClass(int) - if (getSecurityManager() != null) { - final Class[] array = getSecurityManager().getClassContext(); - final Stack> classes = new Stack<>(); - classes.ensureCapacity(array.length); - for (final Class clazz : array) { - classes.push(clazz); - } - return classes; + if (PrivateSecurityManagerStackTraceUtil.isEnabled()) { + return PrivateSecurityManagerStackTraceUtil.getCurrentStackTrace(); } // slower version using getCallerClass where we cannot use a SecurityManager final Stack> classes = new Stack<>(); @@ -251,17 +230,4 @@ private boolean isValid(final StackTraceElement element) { // any others? return true; } - - protected PrivateSecurityManager getSecurityManager() { - return SECURITY_MANAGER; - } - - private static final class PrivateSecurityManager extends SecurityManager { - - @Override - protected Class[] getClassContext() { - return super.getClassContext(); - } - - } }