Repository: logging-log4j2 Updated Branches: refs/heads/master c3a81c74b -> 69e7981c5
LOG4J2-1670 make EqualsReplacementConverter garbage-free Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/69e7981c Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/69e7981c Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/69e7981c Branch: refs/heads/master Commit: 69e7981c5f4338e789fe306fce5b87bb17079657 Parents: c3a81c7 Author: rpopma <rpo...@apache.org> Authored: Sun Nov 6 22:51:44 2016 +0900 Committer: rpopma <rpo...@apache.org> Committed: Sun Nov 6 22:51:44 2016 +0900 ---------------------------------------------------------------------- .../pattern/EqualsReplacementConverter.java | 50 ++++++++++++-------- .../log4j/core/GcFreeLoggingTestUtil.java | 8 ++-- .../pattern/EqualsReplacementConverterTest.java | 5 +- log4j-core/src/test/resources/gcFreeLogging.xml | 2 +- .../resources/gcFreeMixedSyncAsyncLogging.xml | 2 +- src/changes/changes.xml | 3 ++ 6 files changed, 42 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/69e7981c/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EqualsReplacementConverter.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EqualsReplacementConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EqualsReplacementConverter.java index d4a2310..7b50d25 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EqualsReplacementConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EqualsReplacementConverter.java @@ -61,13 +61,10 @@ public final class EqualsReplacementConverter extends LogEventPatternConverter { } private final List<PatternFormatter> formatters; - + private final List<PatternFormatter> substitutionFormatters; private final String substitution; - private final String testString; - private final PatternParser parser; - /** * Construct the converter. * @@ -82,7 +79,9 @@ public final class EqualsReplacementConverter extends LogEventPatternConverter { this.testString = testString; this.substitution = substitution; this.formatters = formatters; - this.parser = parser; + + // check if substitution needs to be parsed + substitutionFormatters = substitution.contains("%") ? parser.parse(substitution) : null; } /** @@ -90,32 +89,43 @@ public final class EqualsReplacementConverter extends LogEventPatternConverter { */ @Override public void format(final LogEvent event, final StringBuilder toAppendTo) { - final StringBuilder buf = new StringBuilder(); - for (final PatternFormatter formatter : formatters) { - formatter.format(event, buf); + final int initialSize = toAppendTo.length(); + for (int i = 0; i < formatters.size(); i++) { + final PatternFormatter formatter = formatters.get(i); + formatter.format(event, toAppendTo); + } + if (equals(testString, toAppendTo, initialSize, toAppendTo.length() - initialSize)) { + toAppendTo.setLength(initialSize); + parseSubstitution(event, toAppendTo); } - final String string = buf.toString(); - toAppendTo.append(testString.equals(string) ? parseSubstitution(event) : string); + } + + private static boolean equals(String str, StringBuilder buff, int from, int len) { + if (str.length() == len) { + for (int i = 0; i < len; i++) { + if (str.charAt(i) != buff.charAt(i + from)) { + return false; + } + } + return true; + } + return false; } /** - * Returns the parsed substitution text. + * Adds the parsed substitution text to the specified buffer. * * @param event the current log event - * @return the parsed substitution text + * @param substitutionBuffer the StringBuilder to append the parsed substitution text to */ - String parseSubstitution(final LogEvent event) { - final StringBuilder substitutionBuffer = new StringBuilder(); - // check if substitution needs to be parsed - if (substitution.contains("%")) { - // parse substitution pattern - final List<PatternFormatter> substitutionFormatters = parser.parse(substitution); - for (final PatternFormatter formatter : substitutionFormatters) { + void parseSubstitution(final LogEvent event, final StringBuilder substitutionBuffer) { + if (substitutionFormatters != null) { + for (int i = 0; i < substitutionFormatters.size(); i++) { + final PatternFormatter formatter = substitutionFormatters.get(i); formatter.format(event, substitutionBuffer); } } else { substitutionBuffer.append(substitution); } - return substitutionBuffer.toString(); } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/69e7981c/log4j-core/src/test/java/org/apache/logging/log4j/core/GcFreeLoggingTestUtil.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/GcFreeLoggingTestUtil.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/GcFreeLoggingTestUtil.java index 2136fc4..4604cd9 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/GcFreeLoggingTestUtil.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/GcFreeLoggingTestUtil.java @@ -55,11 +55,9 @@ public class GcFreeLoggingTestUtil { assertFalse("Constants.IS_WEB_APP", Constants.IS_WEB_APP); final MyCharSeq myCharSeq = new MyCharSeq(); - final Marker test = MarkerManager.getMarker("test"); // initial creation, value is cached - final Marker testParent = MarkerManager.getMarker("testParent"); final Marker testGrandParent = MarkerManager.getMarker("testGrandParent"); - testParent.addParents(testGrandParent); - test.addParents(testParent); + final Marker testParent = MarkerManager.getMarker("testParent").setParents(testGrandParent); + final Marker test = MarkerManager.getMarker("test").setParents(testParent); // initial creation, value is cached // initialize LoggerContext etc. // This is not steady-state logging and will allocate objects. @@ -68,7 +66,7 @@ public class GcFreeLoggingTestUtil { final org.apache.logging.log4j.Logger logger = LogManager.getLogger(testClass.getName()); logger.debug("debug not set"); - logger.fatal("This message is logged to the console"); + logger.fatal(test, "This message is logged to the console"); logger.error("Sample error message"); logger.error("Test parameterized message {}", "param"); for (int i = 0; i < 256; i++) { http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/69e7981c/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/EqualsReplacementConverterTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/EqualsReplacementConverterTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/EqualsReplacementConverterTest.java index ec5106f..2b7984a 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/EqualsReplacementConverterTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/EqualsReplacementConverterTest.java @@ -102,7 +102,10 @@ public class EqualsReplacementConverterTest { final LoggerContext ctx = LoggerContext.getContext(); final EqualsReplacementConverter converter = EqualsReplacementConverter.newInstance(ctx.getConfiguration(), new String[]{"[%marker]", "[]", substitution}); - final String actual = converter.parseSubstitution(event); + + final StringBuilder sb = new StringBuilder(); + converter.parseSubstitution(event, sb); + final String actual = sb.toString(); assertEquals(expected, actual); } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/69e7981c/log4j-core/src/test/resources/gcFreeLogging.xml ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/resources/gcFreeLogging.xml b/log4j-core/src/test/resources/gcFreeLogging.xml index 50476b0..c0598ce 100644 --- a/log4j-core/src/test/resources/gcFreeLogging.xml +++ b/log4j-core/src/test/resources/gcFreeLogging.xml @@ -6,7 +6,7 @@ </Console> <File name="File" fileName="target/gcfreefile.log" bufferedIO="false"> <PatternLayout> - <Pattern>%d{DEFAULT}{UTC} %r %sn %markerSimpleName %maxLen{%marker}{10} %p %c{1.} [%t] %m%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> + <Pattern>%d{DEFAULT}{UTC} %r %sn %markerSimpleName %maxLen{%marker}{10} %equals{%markerSimpleName}{test}{substitute} %p %c{1.} [%t] %m%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> </PatternLayout> </File> <RollingFile name="RollingFile" fileName="target/gcfreeRollingFile.log" http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/69e7981c/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml b/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml index 92653ac..b6ce873 100644 --- a/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml +++ b/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml @@ -6,7 +6,7 @@ </Console> <File name="File" fileName="target/gcfreefileMixed.log" bufferedIO="false"> <PatternLayout> - <Pattern>%d{yyyy-MM-dd'T'HH:mm:ss,SSS}{UTC} %r %sn %markerSimpleName %maxLen{%marker}{10} %p %c{1.} [%t] %m%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> + <Pattern>%d{yyyy-MM-dd'T'HH:mm:ss,SSS}{UTC} %r %sn %markerSimpleName %maxLen{%marker}{10} %equals{%markerSimpleName}{test}{substitute} %p %c{1.} [%t] %m%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> </PatternLayout> </File> <RollingFile name="RollingFile" fileName="target/gcfreeRollingFileMixed.log" http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/69e7981c/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 85d095d..e38dba1 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -24,6 +24,9 @@ </properties> <body> <release version="2.8" date="2016-MM-DD" description="GA Release 2.8"> + <action issue="LOG4J2-1670" dev="rpopma" type="fix"> + (GC) Avoid allocating temporary objects in EqualsReplacementConverter. + </action> <action issue="LOG4J2-1669" dev="rpopma" type="fix"> (GC) Avoid allocating temporary objects in MaxLengthConverter. </action>