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 3bb92e0 LOG4J2-3171: Reduce PatternLayout + PatternConverter branching
3bb92e0 is described below
commit 3bb92e0f6fe9abf14d7ce161d916888e47ee8471
Author: Carter Kozak <[email protected]>
AuthorDate: Sun Aug 29 11:48:16 2021 -0400
LOG4J2-3171: Reduce PatternLayout + PatternConverter branching
Prefer fewer conditionals based on types known ahead of time.
Serializer extends Serializer2 with a default implementation
of the encode method -- all of our implementations of
the Serializer interface also implement Serializer2, so the
conditional path was never used. This may allow more code to
be inlined.
Simplify literal pattern converters to avoid overhead
Simplify LineSeparatorPatternConverter, non-variable, directly from static
unwrap PatternFormaters in PatternLayout
Split MessagePatternConverter into individual components
Complexity only exists in the call-path if it's needed. Note
that we are building a more complex object graph here, but
it should allow more effective inlining.
Remove verbose properties code from logger hot paths
LogEventFactory implements LocationAwareLogEventFactory to reduce
overhead due to branching on instanceof checks.
Factor out uncommon instance creation path in ReusableLogEventFactory
---
.../logging/log4j/core/config/LoggerConfig.java | 92 ++++++------
.../logging/log4j/core/impl/LogEventFactory.java | 15 +-
.../log4j/core/impl/ReusableLogEventFactory.java | 37 +++--
.../log4j/core/layout/AbstractStringLayout.java | 3 +-
.../logging/log4j/core/layout/PatternLayout.java | 150 ++++++++++++++-----
.../log4j/core/pattern/LevelPatternConverter.java | 41 ++++--
.../pattern/LineSeparatorPatternConverter.java | 23 +--
.../core/pattern/LiteralPatternConverter.java | 6 +-
.../core/pattern/MessagePatternConverter.java | 158 +++++++++++++--------
.../logging/log4j/core/pattern/PatternParser.java | 21 ++-
.../pattern/SimpleLiteralPatternConverter.java | 118 +++++++++++++++
.../log4j/core/pattern/PatternParserTest.java | 2 +-
.../pattern/SimpleLiteralPatternConverterTest.java | 47 ++++++
src/changes/changes.xml | 3 +
14 files changed, 540 insertions(+), 176 deletions(-)
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java
index 53913cb..a3bf60a 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java
@@ -381,31 +381,9 @@ public class LoggerConfig extends AbstractFilterable
implements LocationAware {
@PerformanceSensitive("allocation")
public void log(final String loggerName, final String fqcn, final Marker
marker, final Level level,
final Message data, final Throwable t) {
- List<Property> props = null;
- if (!propertiesRequireLookup) {
- props = properties;
- } else if (properties != null) {
- props = new ArrayList<>(properties.size());
- final LogEvent event = Log4jLogEvent.newBuilder()
- .setMessage(data)
- .setMarker(marker)
- .setLevel(level)
- .setLoggerName(loggerName)
- .setLoggerFqcn(fqcn)
- .setThrown(t)
- .build();
- for (int i = 0; i < properties.size(); i++) {
- final Property prop = properties.get(i);
- final String value = prop.isValueNeedsLookup() // since
LOG4J2-1575
- ? config.getStrSubstitutor().replace(event,
prop.getValue()) //
- : prop.getValue();
- props.add(Property.createProperty(prop.getName(), value));
- }
- }
- final LogEvent logEvent = logEventFactory instanceof
LocationAwareLogEventFactory ?
- ((LocationAwareLogEventFactory)
logEventFactory).createEvent(loggerName, marker, fqcn, requiresLocation() ?
- StackLocatorUtil.calcLocation(fqcn) : null, level, data,
props, t) :
- logEventFactory.createEvent(loggerName, marker, fqcn, level, data,
props, t);
+ final List<Property> props = getProperties(loggerName, fqcn, marker,
level, data, t);
+ final LogEvent logEvent = logEventFactory.createEvent(
+ loggerName, marker, fqcn, location(fqcn), level, data, props,
t);
try {
log(logEvent, LoggerConfigPredicate.ALL);
} finally {
@@ -414,6 +392,11 @@ public class LoggerConfig extends AbstractFilterable
implements LocationAware {
}
}
+ private StackTraceElement location(String fqcn) {
+ return requiresLocation() ?
+ StackLocatorUtil.calcLocation(fqcn) : null;
+ }
+
/**
* Logs an event.
*
@@ -428,12 +411,40 @@ public class LoggerConfig extends AbstractFilterable
implements LocationAware {
@PerformanceSensitive("allocation")
public void log(final String loggerName, final String fqcn, final
StackTraceElement location, final Marker marker,
final Level level, final Message data, final Throwable t) {
- List<Property> props = null;
- if (!propertiesRequireLookup) {
- props = properties;
- } else if (properties != null) {
- props = new ArrayList<>(properties.size());
- final LogEvent event = Log4jLogEvent.newBuilder()
+ final List<Property> props = getProperties(loggerName, fqcn, marker,
level, data, t);
+ final LogEvent logEvent = logEventFactory.createEvent(loggerName,
marker, fqcn, location, level, data, props, t);
+ try {
+ log(logEvent, LoggerConfigPredicate.ALL);
+ } finally {
+ // LOG4J2-1583 prevent scrambled logs when logging calls are
nested (logging in toString())
+ ReusableLogEventFactory.release(logEvent);
+ }
+ }
+
+ private List<Property> getProperties(
+ final String loggerName,
+ final String fqcn,
+ final Marker marker,
+ final Level level,
+ final Message data,
+ final Throwable t) {
+ List<Property> snapshot = properties;
+ if (snapshot == null || !propertiesRequireLookup) {
+ return snapshot;
+ }
+ return getPropertiesWithLookups(loggerName, fqcn, marker, level, data,
t, snapshot);
+ }
+
+ private List<Property> getPropertiesWithLookups(
+ final String loggerName,
+ final String fqcn,
+ final Marker marker,
+ final Level level,
+ final Message data,
+ final Throwable t,
+ final List<Property> props) {
+ List<Property> results = new ArrayList<>(props.size());
+ final LogEvent event = Log4jLogEvent.newBuilder()
.setMessage(data)
.setMarker(marker)
.setLevel(level)
@@ -441,23 +452,14 @@ public class LoggerConfig extends AbstractFilterable
implements LocationAware {
.setLoggerFqcn(fqcn)
.setThrown(t)
.build();
- for (int i = 0; i < properties.size(); i++) {
- final Property prop = properties.get(i);
- final String value = prop.isValueNeedsLookup() // since
LOG4J2-1575
+ for (int i = 0; i < props.size(); i++) {
+ final Property prop = props.get(i);
+ final String value = prop.isValueNeedsLookup() // since LOG4J2-1575
? config.getStrSubstitutor().replace(event,
prop.getValue()) //
: prop.getValue();
- props.add(Property.createProperty(prop.getName(), value));
- }
- }
- final LogEvent logEvent = logEventFactory instanceof
LocationAwareLogEventFactory ?
- ((LocationAwareLogEventFactory)
logEventFactory).createEvent(loggerName, marker, fqcn, location, level,
- data, props, t) : logEventFactory.createEvent(loggerName,
marker, fqcn, level, data, props, t);
- try {
- log(logEvent, LoggerConfigPredicate.ALL);
- } finally {
- // LOG4J2-1583 prevent scrambled logs when logging calls are
nested (logging in toString())
- ReusableLogEventFactory.release(logEvent);
+ results.add(Property.createProperty(prop.getName(), value));
}
+ return results;
}
/**
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/LogEventFactory.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/LogEventFactory.java
index ad9128c..c9e5511 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/LogEventFactory.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/LogEventFactory.java
@@ -28,8 +28,21 @@ import org.apache.logging.log4j.message.Message;
/**
*
*/
-public interface LogEventFactory {
+public interface LogEventFactory extends LocationAwareLogEventFactory {
LogEvent createEvent(String loggerName, Marker marker, String fqcn, Level
level, Message data,
List<Property> properties, Throwable t);
+
+ @Override
+ default LogEvent createEvent(
+ String loggerName,
+ Marker marker,
+ String fqcn,
+ @SuppressWarnings("unused") StackTraceElement location,
+ Level level,
+ Message data,
+ List<Property> properties,
+ Throwable t) {
+ return createEvent(loggerName, marker, fqcn, level, data, properties,
t);
+ }
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ReusableLogEventFactory.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ReusableLogEventFactory.java
index fdb9237..ae2a154 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ReusableLogEventFactory.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ReusableLogEventFactory.java
@@ -38,7 +38,7 @@ public class ReusableLogEventFactory implements
LogEventFactory, LocationAwareLo
private static final ThreadNameCachingStrategy
THREAD_NAME_CACHING_STRATEGY = ThreadNameCachingStrategy.create();
private static final Clock CLOCK = ClockFactory.getClock();
- private static ThreadLocal<MutableLogEvent> mutableLogEventThreadLocal =
new ThreadLocal<>();
+ private static final ThreadLocal<MutableLogEvent>
mutableLogEventThreadLocal = new ThreadLocal<>();
private final ContextDataInjector injector =
ContextDataInjectorFactory.createInjector();
/**
@@ -77,21 +77,10 @@ public class ReusableLogEventFactory implements
LogEventFactory, LocationAwareLo
public LogEvent createEvent(final String loggerName, final Marker marker,
final String fqcn,
final StackTraceElement location, final Level
level, final Message message,
final List<Property> properties, final
Throwable t) {
- MutableLogEvent result = mutableLogEventThreadLocal.get();
- if (result == null || result.reserved) {
- final boolean initThreadLocal = result == null;
- result = new MutableLogEvent();
-
- // usually no need to re-initialize thread-specific fields since
the event is stored in a ThreadLocal
- result.setThreadId(Thread.currentThread().getId());
- result.setThreadName(Thread.currentThread().getName()); //
Thread.getName() allocates Objects on each call
- result.setThreadPriority(Thread.currentThread().getPriority());
- if (initThreadLocal) {
- mutableLogEventThreadLocal.set(result);
- }
- }
+ MutableLogEvent result = getOrCreateMutableLogEvent();
result.reserved = true;
- result.clear(); // ensure any previously cached values (thrownProxy,
source, etc.) are cleared
+ // No need to clear here, values are cleared in release when reserved
is set to false.
+ // If the event was dirty we'd create a new one.
result.setLoggerName(loggerName);
result.setMarker(marker);
@@ -111,6 +100,24 @@ public class ReusableLogEventFactory implements
LogEventFactory, LocationAwareLo
return result;
}
+ private static MutableLogEvent getOrCreateMutableLogEvent() {
+ MutableLogEvent result = mutableLogEventThreadLocal.get();
+ return result == null || result.reserved ? createInstance(result) :
result;
+ }
+
+ private static MutableLogEvent createInstance(MutableLogEvent existing) {
+ MutableLogEvent result = new MutableLogEvent();
+
+ // usually no need to re-initialize thread-specific fields since the
event is stored in a ThreadLocal
+ result.setThreadId(Thread.currentThread().getId());
+ result.setThreadName(Thread.currentThread().getName()); //
Thread.getName() allocates Objects on each call
+ result.setThreadPriority(Thread.currentThread().getPriority());
+ if (existing == null) {
+ mutableLogEventThreadLocal.set(result);
+ }
+ return result;
+ }
+
/**
* Switches the {@code reserved} flag off if the specified event is a
MutableLogEvent, otherwise does nothing.
* This flag is used internally to verify that a reusable log event is no
longer in use and can be reused.
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
index d0b405e..8997d41 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
@@ -88,9 +88,10 @@ public abstract class AbstractStringLayout extends
AbstractLayout<String> implem
return false;
}
- public interface Serializer {
+ public interface Serializer extends Serializer2 {
String toSerializable(final LogEvent event);
+ @Override
default StringBuilder toSerializable(final LogEvent event, final
StringBuilder builder) {
builder.append(toSerializable(event));
return builder;
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java
index e276237..8e62c90 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java
@@ -35,6 +35,7 @@ import
org.apache.logging.log4j.core.config.plugins.PluginConfiguration;
import org.apache.logging.log4j.core.config.plugins.PluginElement;
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
import org.apache.logging.log4j.core.impl.LocationAware;
+import org.apache.logging.log4j.core.pattern.FormattingInfo;
import org.apache.logging.log4j.core.pattern.LogEventPatternConverter;
import org.apache.logging.log4j.core.pattern.PatternFormatter;
import org.apache.logging.log4j.core.pattern.PatternParser;
@@ -222,11 +223,7 @@ public final class PatternLayout extends
AbstractStringLayout {
@Override
public void encode(final LogEvent event, final ByteBufferDestination
destination) {
- if (!(eventSerializer instanceof Serializer2)) {
- super.encode(event, destination);
- return;
- }
- final StringBuilder text = toText((Serializer2) eventSerializer,
event, getStringBuilder());
+ final StringBuilder text = toText(eventSerializer, event,
getStringBuilder());
final Encoder<StringBuilder> encoder = getStringBuilderEncoder();
encoder.encode(text, destination);
trimToMaxSize(text);
@@ -317,14 +314,59 @@ public final class PatternLayout extends
AbstractStringLayout {
.build();
}
- private static class PatternSerializer implements Serializer, Serializer2,
LocationAware {
+ private interface PatternSerializer extends Serializer, Serializer2,
LocationAware {}
+
+ private static final class NoFormatPatternSerializer implements
PatternSerializer {
+
+ private final LogEventPatternConverter[] converters;
+
+ private NoFormatPatternSerializer(final PatternFormatter[] formatters)
{
+ this.converters = new LogEventPatternConverter[formatters.length];
+ for (int i = 0; i < formatters.length; i++) {
+ converters[i] = formatters[i].getConverter();
+ }
+ }
+
+ @Override
+ public String toSerializable(final LogEvent event) {
+ final StringBuilder sb = getStringBuilder();
+ try {
+ return toSerializable(event, sb).toString();
+ } finally {
+ trimToMaxSize(sb);
+ }
+ }
+
+ @Override
+ public StringBuilder toSerializable(final LogEvent event, final
StringBuilder buffer) {
+ for (LogEventPatternConverter converter : converters) {
+ converter.format(event, buffer);
+ }
+ return buffer;
+ }
+
+ @Override
+ public boolean requiresLocation() {
+ for (LogEventPatternConverter converter : converters) {
+ if (converter instanceof LocationAware && ((LocationAware)
converter).requiresLocation()) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public String toString() {
+ return super.toString() + "[converters=" +
Arrays.toString(converters) + "]";
+ }
+ }
+
+ private static final class PatternFormatterPatternSerializer implements
PatternSerializer {
private final PatternFormatter[] formatters;
- private final RegexReplacement replace;
- private PatternSerializer(final PatternFormatter[] formatters, final
RegexReplacement replace) {
+ private PatternFormatterPatternSerializer(final PatternFormatter[]
formatters) {
this.formatters = formatters;
- this.replace = replace;
}
@Override
@@ -339,15 +381,8 @@ public final class PatternLayout extends
AbstractStringLayout {
@Override
public StringBuilder toSerializable(final LogEvent event, final
StringBuilder buffer) {
- final int len = formatters.length;
- for (int i = 0; i < len; i++) {
- formatters[i].format(event, buffer);
- }
- if (replace != null) { // creates temporary objects
- String str = buffer.toString();
- str = replace.format(str);
- buffer.setLength(0);
- buffer.append(str);
+ for (PatternFormatter formatter : formatters) {
+ formatter.format(event, buffer);
}
return buffer;
}
@@ -364,14 +399,56 @@ public final class PatternLayout extends
AbstractStringLayout {
@Override
public String toString() {
- final StringBuilder builder = new StringBuilder();
- builder.append(super.toString());
- builder.append("[formatters=");
- builder.append(Arrays.toString(formatters));
- builder.append(", replace=");
- builder.append(replace);
- builder.append("]");
- return builder.toString();
+ return super.toString() +
+ "[formatters=" +
+ Arrays.toString(formatters) +
+ "]";
+ }
+ }
+
+ private static final class PatternSerializerWithReplacement implements
Serializer, Serializer2, LocationAware {
+
+ private final PatternSerializer delegate;
+ private final RegexReplacement replace;
+
+ private PatternSerializerWithReplacement(final PatternSerializer
delegate, final RegexReplacement replace) {
+ this.delegate = delegate;
+ this.replace = replace;
+ }
+
+ @Override
+ public String toSerializable(final LogEvent event) {
+ final StringBuilder sb = getStringBuilder();
+ try {
+ return toSerializable(event, sb).toString();
+ } finally {
+ trimToMaxSize(sb);
+ }
+ }
+
+ @Override
+ public StringBuilder toSerializable(final LogEvent event, final
StringBuilder buf) {
+ StringBuilder buffer = delegate.toSerializable(event, buf);
+ String str = buffer.toString();
+ str = replace.format(str);
+ buffer.setLength(0);
+ buffer.append(str);
+ return buffer;
+ }
+
+ @Override
+ public boolean requiresLocation() {
+ return delegate.requiresLocation();
+ }
+
+ @Override
+ public String toString() {
+ return super.toString() +
+ "[delegate=" +
+ delegate +
+ ", replace=" +
+ replace +
+ "]";
}
}
@@ -397,7 +474,18 @@ public final class PatternLayout extends
AbstractStringLayout {
final List<PatternFormatter> list = parser.parse(pattern
== null ? defaultPattern : pattern,
alwaysWriteExceptions, disableAnsi,
noConsoleNoAnsi);
final PatternFormatter[] formatters =
list.toArray(PatternFormatter.EMPTY_ARRAY);
- return new PatternSerializer(formatters, replace);
+ boolean hasFormattingInfo = false;
+ for (PatternFormatter formatter : formatters) {
+ FormattingInfo info = formatter.getFormattingInfo();
+ if (info != null && info !=
FormattingInfo.getDefault()) {
+ hasFormattingInfo = true;
+ break;
+ }
+ }
+ PatternSerializer serializer = hasFormattingInfo
+ ? new PatternFormatterPatternSerializer(formatters)
+ : new NoFormatPatternSerializer(formatters);
+ return replace == null ? serializer : new
PatternSerializerWithReplacement(serializer, replace);
} catch (final RuntimeException ex) {
throw new IllegalArgumentException("Cannot parse pattern
'" + pattern + "'", ex);
}
@@ -447,7 +535,7 @@ public final class PatternLayout extends
AbstractStringLayout {
}
- private static class PatternSelectorSerializer implements Serializer,
Serializer2, LocationAware {
+ private static final class PatternSelectorSerializer implements
Serializer, Serializer2, LocationAware {
private final PatternSelector patternSelector;
private final RegexReplacement replace;
@@ -469,10 +557,8 @@ public final class PatternLayout extends
AbstractStringLayout {
@Override
public StringBuilder toSerializable(final LogEvent event, final
StringBuilder buffer) {
- final PatternFormatter[] formatters =
patternSelector.getFormatters(event);
- final int len = formatters.length;
- for (int i = 0; i < len; i++) {
- formatters[i].format(event, buffer);
+ for (PatternFormatter formatter :
patternSelector.getFormatters(event)) {
+ formatter.format(event, buffer);
}
if (replace != null) { // creates temporary objects
String str = buffer.toString();
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LevelPatternConverter.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LevelPatternConverter.java
index debee5a..88dca4f 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LevelPatternConverter.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LevelPatternConverter.java
@@ -32,23 +32,20 @@ import org.apache.logging.log4j.util.PerformanceSensitive;
@Plugin(name = "LevelPatternConverter", category = PatternConverter.CATEGORY)
@ConverterKeys({ "p", "level" })
@PerformanceSensitive("allocation")
-public final class LevelPatternConverter extends LogEventPatternConverter {
+public class LevelPatternConverter extends LogEventPatternConverter {
private static final String OPTION_LENGTH = "length";
private static final String OPTION_LOWER = "lowerCase";
/**
* Singleton.
*/
- private static final LevelPatternConverter INSTANCE = new
LevelPatternConverter(null);
-
- private final Map<Level, String> levelMap;
+ private static final LevelPatternConverter INSTANCE = new
SimpleLevelPatternConverter();
/**
* Private constructor.
*/
- private LevelPatternConverter(final Map<Level, String> map) {
+ private LevelPatternConverter() {
super("Level", "level");
- this.levelMap = map;
}
/**
@@ -97,7 +94,7 @@ public final class LevelPatternConverter extends
LogEventPatternConverter {
levelMap.put(level, lowerCase ? left.toLowerCase(Locale.US) :
left);
}
}
- return new LevelPatternConverter(levelMap);
+ return new LevelMapLevelPatternConverter(levelMap);
}
/**
@@ -123,7 +120,7 @@ public final class LevelPatternConverter extends
LogEventPatternConverter {
*/
@Override
public void format(final LogEvent event, final StringBuilder output) {
- output.append(levelMap == null ? event.getLevel().toString() :
levelMap.get(event.getLevel()));
+ throw new UnsupportedOperationException("Overridden by subclasses");
}
/**
@@ -137,4 +134,32 @@ public final class LevelPatternConverter extends
LogEventPatternConverter {
return "level";
}
+
+ private static final class SimpleLevelPatternConverter extends
LevelPatternConverter {
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final LogEvent event, final StringBuilder output) {
+ output.append(event.getLevel());
+ }
+ }
+
+ private static final class LevelMapLevelPatternConverter extends
LevelPatternConverter {
+
+ private final Map<Level, String> levelMap;
+
+ private LevelMapLevelPatternConverter(final Map<Level, String>
levelMap) {
+ this.levelMap = levelMap;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final LogEvent event, final StringBuilder output) {
+ output.append(levelMap.get(event.getLevel()));
+ }
+ }
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LineSeparatorPatternConverter.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LineSeparatorPatternConverter.java
index 1d7b022..d11890e 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LineSeparatorPatternConverter.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LineSeparatorPatternConverter.java
@@ -35,16 +35,10 @@ public final class LineSeparatorPatternConverter extends
LogEventPatternConverte
private static final LineSeparatorPatternConverter INSTANCE = new
LineSeparatorPatternConverter();
/**
- * Line separator.
- */
- private final String lineSep;
-
- /**
* Private constructor.
*/
private LineSeparatorPatternConverter() {
super("Line Sep", "lineSep");
- lineSep = Strings.LINE_SEPARATOR;
}
/**
@@ -62,7 +56,20 @@ public final class LineSeparatorPatternConverter extends
LogEventPatternConverte
* {@inheritDoc}
*/
@Override
- public void format(final LogEvent event, final StringBuilder toAppendTo) {
- toAppendTo.append(lineSep);
+ public void format(final LogEvent ignored, final StringBuilder toAppendTo)
{
+ toAppendTo.append(Strings.LINE_SEPARATOR);
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final Object ignored, final StringBuilder output) {
+ output.append(Strings.LINE_SEPARATOR);
+ }
+
+ @Override
+ public boolean isVariable() {
+ return false;
}
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LiteralPatternConverter.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LiteralPatternConverter.java
index 2e24706..ba0d9ec 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LiteralPatternConverter.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LiteralPatternConverter.java
@@ -49,7 +49,11 @@ public final class LiteralPatternConverter extends
LogEventPatternConverter impl
super("Literal", "literal");
this.literal = convertBackslashes ?
OptionConverter.convertSpecialChars(literal) : literal; // LOG4J2-829
this.config = config;
- substitute = config != null && literal.contains("${");
+ substitute = config != null && containsSubstitutionSequence(literal);
+ }
+
+ static boolean containsSubstitutionSequence(final String literal) {
+ return literal != null && literal.contains("${");
}
/**
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MessagePatternConverter.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MessagePatternConverter.java
index 92d75a2..5c4c6ef 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MessagePatternConverter.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MessagePatternConverter.java
@@ -37,31 +37,15 @@ import
org.apache.logging.log4j.util.StringBuilderFormattable;
@Plugin(name = "MessagePatternConverter", category = PatternConverter.CATEGORY)
@ConverterKeys({ "m", "msg", "message" })
@PerformanceSensitive("allocation")
-public final class MessagePatternConverter extends LogEventPatternConverter {
+public class MessagePatternConverter extends LogEventPatternConverter {
private static final String NOLOOKUPS = "nolookups";
- private final String[] formats;
- private final Configuration config;
- private final TextRenderer textRenderer;
- private final boolean noLookups;
-
- /**
- * Private constructor.
- *
- * @param options
- * options, may be null.
- */
- private MessagePatternConverter(final Configuration config, final String[]
options) {
+ private MessagePatternConverter() {
super("Message", "message");
- this.formats = options;
- this.config = config;
- final int noLookupsIdx = loadNoLookups(options);
- this.noLookups = Constants.FORMAT_MESSAGES_PATTERN_DISABLE_LOOKUPS ||
noLookupsIdx >= 0;
- this.textRenderer = loadMessageRenderer(noLookupsIdx >= 0 ?
ArrayUtils.remove(options, noLookupsIdx) : options);
}
- private int loadNoLookups(final String[] options) {
+ private static int loadNoLookups(final String[] options) {
if (options != null) {
for (int i = 0; i < options.length; i++) {
final String option = options[i];
@@ -73,7 +57,7 @@ public final class MessagePatternConverter extends
LogEventPatternConverter {
return -1;
}
- private TextRenderer loadMessageRenderer(final String[] options) {
+ private static TextRenderer loadMessageRenderer(final String[] options) {
if (options != null) {
for (final String option : options) {
switch (option.toUpperCase(Locale.ROOT)) {
@@ -102,55 +86,115 @@ public final class MessagePatternConverter extends
LogEventPatternConverter {
* @return instance of pattern converter.
*/
public static MessagePatternConverter newInstance(final Configuration
config, final String[] options) {
- return new MessagePatternConverter(config, options);
+ int noLookupsIdx = loadNoLookups(options);
+ boolean noLookups = Constants.FORMAT_MESSAGES_PATTERN_DISABLE_LOOKUPS
|| noLookupsIdx >= 0;
+ String[] formats = noLookupsIdx >= 0 ? ArrayUtils.remove(options,
noLookupsIdx) : options;
+ TextRenderer textRenderer = loadMessageRenderer(noLookupsIdx >= 0 ?
ArrayUtils.remove(options, noLookupsIdx) : options);
+ MessagePatternConverter result = formats == null || formats.length == 0
+ ? SimpleMessagePatternConverter.INSTANCE
+ : new FormattedMessagePatternConverter(formats);
+ if (!noLookups && config != null) {
+ result = new LookupMessagePatternConverter(result, config);
+ }
+ if (textRenderer != null) {
+ result = new RenderingPatternConverter(result, textRenderer);
+ }
+ return result;
}
- /**
- * {@inheritDoc}
- */
@Override
public void format(final LogEvent event, final StringBuilder toAppendTo) {
- final Message msg = event.getMessage();
- if (msg instanceof StringBuilderFormattable) {
+ throw new UnsupportedOperationException();
+ }
- final boolean doRender = textRenderer != null;
- final StringBuilder workingBuilder = doRender ? new
StringBuilder(80) : toAppendTo;
+ private static final class SimpleMessagePatternConverter extends
MessagePatternConverter {
+ private static final MessagePatternConverter INSTANCE = new
SimpleMessagePatternConverter();
- final int offset = workingBuilder.length();
- if (msg instanceof MultiFormatStringBuilderFormattable) {
- ((MultiFormatStringBuilderFormattable) msg).formatTo(formats,
workingBuilder);
- } else {
- ((StringBuilderFormattable) msg).formatTo(workingBuilder);
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final LogEvent event, final StringBuilder
toAppendTo) {
+ Message msg = event.getMessage();
+ if (msg instanceof StringBuilderFormattable) {
+ ((StringBuilderFormattable) msg).formatTo(toAppendTo);
+ } else if (msg != null) {
+ toAppendTo.append(msg.getFormattedMessage());
}
+ }
+ }
- // TODO can we optimize this?
- if (config != null && !noLookups) {
- for (int i = offset; i < workingBuilder.length() - 1; i++) {
- if (workingBuilder.charAt(i) == '$' &&
workingBuilder.charAt(i + 1) == '{') {
- final String value = workingBuilder.substring(offset,
workingBuilder.length());
- workingBuilder.setLength(offset);
-
workingBuilder.append(config.getStrSubstitutor().replace(event, value));
- }
+ private static final class FormattedMessagePatternConverter extends
MessagePatternConverter {
+
+ private final String[] formats;
+
+ FormattedMessagePatternConverter(final String[] formats) {
+ this.formats = formats;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final LogEvent event, final StringBuilder
toAppendTo) {
+ Message msg = event.getMessage();
+ if (msg instanceof StringBuilderFormattable) {
+ if (msg instanceof MultiFormatStringBuilderFormattable) {
+ ((MultiFormatStringBuilderFormattable)
msg).formatTo(formats, toAppendTo);
+ } else {
+ ((StringBuilderFormattable) msg).formatTo(toAppendTo);
}
+ } else if (msg != null) {
+ toAppendTo.append(msg instanceof MultiformatMessage
+ ? ((MultiformatMessage)
msg).getFormattedMessage(formats)
+ : msg.getFormattedMessage());
}
- if (doRender) {
- textRenderer.render(workingBuilder, toAppendTo);
- }
- return;
}
- if (msg != null) {
- String result;
- if (msg instanceof MultiformatMessage) {
- result = ((MultiformatMessage)
msg).getFormattedMessage(formats);
- } else {
- result = msg.getFormattedMessage();
- }
- if (result != null) {
- toAppendTo.append(config != null && result.contains("${")
- ? config.getStrSubstitutor().replace(event, result) :
result);
- } else {
- toAppendTo.append("null");
+ }
+
+ private static final class LookupMessagePatternConverter extends
MessagePatternConverter {
+ private final MessagePatternConverter delegate;
+ private final Configuration config;
+
+ LookupMessagePatternConverter(final MessagePatternConverter delegate,
final Configuration config) {
+ this.delegate = delegate;
+ this.config = config;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final LogEvent event, final StringBuilder
toAppendTo) {
+ int start = toAppendTo.length();
+ delegate.format(event, toAppendTo);
+ int indexOfSubstitution = toAppendTo.indexOf("${", start);
+ if (indexOfSubstitution >= 0) {
+ config.getStrSubstitutor()
+ .replaceIn(event, toAppendTo, indexOfSubstitution,
toAppendTo.length() - indexOfSubstitution);
}
}
}
+
+ private static final class RenderingPatternConverter extends
MessagePatternConverter {
+
+ private final MessagePatternConverter delegate;
+ private final TextRenderer textRenderer;
+
+ RenderingPatternConverter(final MessagePatternConverter delegate,
final TextRenderer textRenderer) {
+ this.delegate = delegate;
+ this.textRenderer = textRenderer;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void format(final LogEvent event, final StringBuilder
toAppendTo) {
+ StringBuilder workingBuilder = new StringBuilder(80);
+ delegate.format(event, workingBuilder);
+ textRenderer.render(workingBuilder, toAppendTo);
+ }
+
+ }
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java
index 96aa819..9e66441 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java
@@ -192,7 +192,7 @@ public final class PatternParser {
pc = (LogEventPatternConverter) converter;
handlesThrowable |= pc.handlesThrowable();
} else {
- pc = new LiteralPatternConverter(config, Strings.EMPTY, true);
+ pc = SimpleLiteralPatternConverter.of(Strings.EMPTY);
}
FormattingInfo field;
@@ -376,8 +376,7 @@ public final class PatternParser {
default:
if (currentLiteral.length() != 0) {
- patternConverters.add(new
LiteralPatternConverter(config, currentLiteral.toString(),
- convertBackslashes));
+
patternConverters.add(literalPattern(currentLiteral.toString(),
convertBackslashes));
formattingInfos.add(FormattingInfo.getDefault());
}
@@ -494,7 +493,7 @@ public final class PatternParser {
// while
if (currentLiteral.length() != 0) {
- patternConverters.add(new LiteralPatternConverter(config,
currentLiteral.toString(), convertBackslashes));
+ patternConverters.add(literalPattern(currentLiteral.toString(),
convertBackslashes));
formattingInfos.add(FormattingInfo.getDefault());
}
}
@@ -669,12 +668,12 @@ public final class PatternParser {
msg.append("] starting at position ");
}
- msg.append(Integer.toString(i));
+ msg.append(i);
msg.append(" in conversion pattern.");
LOGGER.error(msg.toString());
- patternConverters.add(new LiteralPatternConverter(config,
currentLiteral.toString(), convertBackslashes));
+ patternConverters.add(literalPattern(currentLiteral.toString(),
convertBackslashes));
formattingInfos.add(FormattingInfo.getDefault());
} else {
patternConverters.add(pc);
@@ -682,7 +681,7 @@ public final class PatternParser {
if (currentLiteral.length() > 0) {
patternConverters
- .add(new LiteralPatternConverter(config,
currentLiteral.toString(), convertBackslashes));
+ .add(literalPattern(currentLiteral.toString(),
convertBackslashes));
formattingInfos.add(FormattingInfo.getDefault());
}
}
@@ -691,4 +690,12 @@ public final class PatternParser {
return i;
}
+
+ // Create a literal pattern converter with support for substitutions if
necessary
+ private LogEventPatternConverter literalPattern(String literal, boolean
convertBackslashes) {
+ if (config != null &&
LiteralPatternConverter.containsSubstitutionSequence(literal)) {
+ return new LiteralPatternConverter(config, literal,
convertBackslashes);
+ }
+ return SimpleLiteralPatternConverter.of(literal, convertBackslashes);
+ }
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/SimpleLiteralPatternConverter.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/SimpleLiteralPatternConverter.java
new file mode 100644
index 0000000..d3abe55
--- /dev/null
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/SimpleLiteralPatternConverter.java
@@ -0,0 +1,118 @@
+/*
+ * 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.core.pattern;
+
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.util.OptionConverter;
+import org.apache.logging.log4j.util.PerformanceSensitive;
+
+
+/**
+ * Formats a string literal without substitution.
+ *
+ * This is an effectively-sealed internal type.
+ */
+@PerformanceSensitive("allocation")
+abstract class SimpleLiteralPatternConverter extends LogEventPatternConverter
implements ArrayPatternConverter {
+
+ private SimpleLiteralPatternConverter() {
+ super("SimpleLiteral", "literal");
+ }
+
+ static LogEventPatternConverter of(final String literal, final boolean
convertBackslashes) {
+ String value = convertBackslashes ?
OptionConverter.convertSpecialChars(literal) : literal;
+ return of(value);
+ }
+
+ static LogEventPatternConverter of(final String literal) {
+ if (literal == null || literal.isEmpty()) {
+ return Noop.INSTANCE;
+ }
+ if (" ".equals(literal)) {
+ return Space.INSTANCE;
+ }
+ return new StringValue(literal);
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public final void format(final LogEvent ignored, final StringBuilder
output) {
+ format(output);
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public final void format(final Object ignored, final StringBuilder output)
{
+ format(output);
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public final void format(final StringBuilder output, final Object... args)
{
+ format(output);
+ }
+
+ abstract void format(final StringBuilder output);
+
+ @Override
+ public final boolean isVariable() {
+ return false;
+ }
+
+ @Override
+ public final boolean handlesThrowable() {
+ return false;
+ }
+
+ private static final class Noop extends SimpleLiteralPatternConverter {
+ private static final Noop INSTANCE = new Noop();
+
+ @Override
+ void format(final StringBuilder output) {
+ // no-op
+ }
+ }
+
+ private static final class Space extends SimpleLiteralPatternConverter {
+ private static final Space INSTANCE = new Space();
+
+ @Override
+ void format(final StringBuilder output) {
+ output.append(' ');
+ }
+ }
+
+ private static final class StringValue extends
SimpleLiteralPatternConverter {
+
+ private final String literal;
+
+ StringValue(final String literal) {
+ this.literal = literal;
+ }
+
+ @Override
+ void format(final StringBuilder output) {
+ output.append(literal);
+ }
+ }
+}
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/PatternParserTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/PatternParserTest.java
index b1dd31f..617587f 100644
---
a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/PatternParserTest.java
+++
b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/PatternParserTest.java
@@ -362,7 +362,7 @@ public class PatternParserTest {
assertNotNull(formatters);
assertEquals(2, formatters.size());
- validateConverter(formatters, 0, "Literal");
+ validateConverter(formatters, 0, "SimpleLiteral");
validateConverter(formatters, 1, "Date");
}
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/SimpleLiteralPatternConverterTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/SimpleLiteralPatternConverterTest.java
new file mode 100644
index 0000000..ba6b402
--- /dev/null
+++
b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/SimpleLiteralPatternConverterTest.java
@@ -0,0 +1,47 @@
+/*
+ * 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.core.pattern;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class SimpleLiteralPatternConverterTest {
+
+ @Test
+ public void testConvertBackslashes() {
+ String literal = "ABC\\tDEF\\nGHI\\rJKL\\'MNO\\f \\b \\\\DROPPED:\\x";
+ LogEventPatternConverter converter =
SimpleLiteralPatternConverter.of(literal, true);
+ String actual = literal(converter);
+ assertEquals("ABC\tDEF\nGHI\rJKL\'MNO\f \b \\DROPPED:x", actual);
+ }
+
+ @Test
+ public void testDontConvertBackslashes() {
+ String literal = "ABC\\tDEF\\nGHI\\rJKL\\'MNO\\f \\b \\\\DROPPED:\\x";
+ LogEventPatternConverter converter =
SimpleLiteralPatternConverter.of(literal, false);
+ String actual = literal(converter);
+ assertEquals(literal, actual);
+ }
+
+ private static String literal(LogEventPatternConverter converter) {
+ StringBuilder buffer = new StringBuilder();
+ converter.format(null, buffer);
+ return buffer.toString();
+ }
+}
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 564d671..b0a3558 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -97,6 +97,9 @@
Prefer string.getBytes(Charset) over string.getBytes(String)
based on performance improvements in modern Java releases.
</action>
+ <action issue="LOG4J2-3171" dev="ckozak" type="add">
+ Improve PatternLayout performance by reducing unnecessary indirection
and branching.
+ </action>
<!-- FIXES -->
<action issue="LOG4J2-3160" dev="vy" type="fix" due-to="Lars Bohl">
Fix documentation on how to toggle log4j2.debug system property.
