You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by mi...@apache.org on 2016/08/03 11:34:07 UTC

[17/28] logging-log4j2 git commit: LOG4J2-1489 (GC) Fixed %date conversion patterns with a timezone parameter are now garbage free.

LOG4J2-1489 (GC) Fixed %date conversion patterns with a timezone parameter are now garbage free.

This closes #36 (https://github.com/apache/logging-log4j2/pull/36).


Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/0a85a774
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/0a85a774
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/0a85a774

Branch: refs/heads/LOG4J-1181
Commit: 0a85a77481e8ac289d36befb16828a99dd47cb96
Parents: 2bca903
Author: rpopma <rp...@apache.org>
Authored: Sun Jul 31 15:39:43 2016 +0900
Committer: rpopma <rp...@apache.org>
Committed: Sun Jul 31 15:39:43 2016 +0900

----------------------------------------------------------------------
 .../core/pattern/DatePatternConverter.java      |  2 +-
 .../core/util/datetime/FixedDateFormat.java     | 65 +++++++++++++++++---
 .../core/util/datetime/FixedDateFormatTest.java | 35 ++++++++---
 log4j-core/src/test/resources/gcFreeLogging.xml |  9 +--
 .../resources/gcFreeMixedSyncAsyncLogging.xml   | 10 +--
 src/changes/changes.xml                         |  3 +
 6 files changed, 94 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/DatePatternConverter.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/DatePatternConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/DatePatternConverter.java
index 499f0d9..19edde5 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/DatePatternConverter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/DatePatternConverter.java
@@ -224,7 +224,7 @@ public final class DatePatternConverter extends LogEventPatternConverter impleme
             LOGGER.warn("Could not instantiate FastDateFormat with pattern " + pattern, e);
 
             // default to the DEFAULT format
-            return createFixedFormatter(FixedDateFormat.create(FixedFormat.DEFAULT));
+            return createFixedFormatter(FixedDateFormat.create(FixedFormat.DEFAULT, tz));
         }
     }
 

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/log4j-core/src/main/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormat.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormat.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormat.java
index 25a5127..3b1f102 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormat.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormat.java
@@ -19,6 +19,7 @@ package org.apache.logging.log4j.core.util.datetime;
 
 import java.util.Calendar;
 import java.util.Objects;
+import java.util.TimeZone;
 
 /**
  * Custom time formatter that trades flexibility for performance. This formatter only supports the date patterns defined
@@ -156,11 +157,23 @@ public class FixedDateFormat {
          * @return the {@code FastDateFormat} object for formatting the date part of the pattern or {@code null}
          */
         public FastDateFormat getFastDateFormat() {
-            return getDatePattern() == null ? null : FastDateFormat.getInstance(getDatePattern());
+            return getFastDateFormat(null);
+        }
+
+        /**
+         * Returns the {@code FastDateFormat} object for formatting the date part of the pattern or {@code null} if the
+         * pattern does not have a date part.
+         *
+         * @param tz the time zone to use
+         * @return the {@code FastDateFormat} object for formatting the date part of the pattern or {@code null}
+         */
+        public FastDateFormat getFastDateFormat(TimeZone tz) {
+            return getDatePattern() == null ? null : FastDateFormat.getInstance(getDatePattern(), tz);
         }
     }
 
     private final FixedFormat fixedFormat;
+    private final TimeZone timeZone;
     private final int length;
     private final FastDateFormat fastDateFormat; // may be null
     private final char timeSeparatorChar;
@@ -186,35 +199,57 @@ public class FixedDateFormat {
      *
      * @param fixedFormat the fixed format
      */
-    FixedDateFormat(final FixedFormat fixedFormat) {
+    FixedDateFormat(final FixedFormat fixedFormat, final TimeZone tz) {
         this.fixedFormat = Objects.requireNonNull(fixedFormat);
+        this.timeZone = Objects.requireNonNull(tz);
         this.timeSeparatorChar = fixedFormat.timeSeparatorChar;
         this.timeSeparatorLength = fixedFormat.timeSeparatorLength;
         this.millisSeparatorChar = fixedFormat.millisSeparatorChar;
         this.millisSeparatorLength = fixedFormat.millisSeparatorLength;
         this.length = fixedFormat.getLength();
-        this.fastDateFormat = fixedFormat.getFastDateFormat();
+        this.fastDateFormat = fixedFormat.getFastDateFormat(tz);
     }
 
     public static FixedDateFormat createIfSupported(final String... options) {
         if (options == null || options.length == 0 || options[0] == null) {
-            return new FixedDateFormat(FixedFormat.DEFAULT);
+            return new FixedDateFormat(FixedFormat.DEFAULT, TimeZone.getDefault());
         }
+        final TimeZone tz;
         if (options.length > 1) {
-            return null; // time zone not supported
+            if (options[1] != null){
+                tz = TimeZone.getTimeZone(options[1]);
+            } else {
+                tz = TimeZone.getDefault();
+            }
+        } else if (options.length > 2) {
+            return null;
+        } else {
+            tz = TimeZone.getDefault();
         }
+
         final FixedFormat type = FixedFormat.lookup(options[0]);
-        return type == null ? null : new FixedDateFormat(type);
+        return type == null ? null : new FixedDateFormat(type, tz);
     }
 
     /**
-     * Returns a new {@code FixedDateFormat} object for the specified {@code FixedFormat} and a {@code null} TimeZone.
+     * Returns a new {@code FixedDateFormat} object for the specified {@code FixedFormat} and a {@code TimeZone.getDefault()} TimeZone.
      *
      * @param format the format to use
      * @return a new {@code FixedDateFormat} object
      */
     public static FixedDateFormat create(final FixedFormat format) {
-        return new FixedDateFormat(format);
+        return new FixedDateFormat(format, TimeZone.getDefault());
+    }
+
+    /**
+     * Returns a new {@code FixedDateFormat} object for the specified {@code FixedFormat} and TimeZone.
+     *
+     * @param format the format to use
+     * @param tz the time zone to use
+     * @return a new {@code FixedDateFormat} object
+     */
+    public static FixedDateFormat create(final FixedFormat format, final TimeZone tz) {
+        return new FixedDateFormat(format, tz != null ? tz : TimeZone.getDefault());
     }
 
     /**
@@ -226,6 +261,16 @@ public class FixedDateFormat {
         return fixedFormat.getPattern();
     }
 
+    /**
+     * Returns the time zone.
+     *
+     * @return the time zone
+     */
+
+    public TimeZone getTimeZone() {
+        return timeZone;
+    }
+
     // Profiling showed this method is important to log4j performance. Modify with care!
     // 30 bytes (allows immediate JVM inlining: <= -XX:MaxInlineSize=35 bytes)
     private long millisSinceMidnight(final long now) {
@@ -243,8 +288,8 @@ public class FixedDateFormat {
         midnightTomorrow = calcMidnightMillis(now, 1);
     }
 
-    static long calcMidnightMillis(final long time, final int addDays) {
-        final Calendar cal = Calendar.getInstance();
+    private long calcMidnightMillis(final long time, final int addDays) {
+        final Calendar cal = Calendar.getInstance(timeZone);
         cal.setTimeInMillis(time);
         cal.set(Calendar.HOUR_OF_DAY, 0);
         cal.set(Calendar.MINUTE, 0);

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/log4j-core/src/test/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormatTest.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormatTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormatTest.java
index 0946d73..571e901 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormatTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormatTest.java
@@ -20,6 +20,7 @@ package org.apache.logging.log4j.core.util.datetime;
 import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.Locale;
+import java.util.TimeZone;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.logging.log4j.core.util.datetime.FixedDateFormat.FixedFormat;
@@ -114,22 +115,36 @@ public class FixedDateFormatTest {
     public void testCreateIfSupported_defaultIfOptionsArrayWithSingleNullElement() {
         final FixedDateFormat fmt = FixedDateFormat.createIfSupported(new String[1]);
         assertEquals(FixedFormat.DEFAULT.getPattern(), fmt.getFormat());
+        assertEquals(TimeZone.getDefault(), fmt.getTimeZone());
     }
 
     @Test
-    public void testCreateIfSupported_nullIfOptionsArrayHasTwoElements() {
-        final String[] options = {FixedDateFormat.FixedFormat.ABSOLUTE.getPattern(), "+08:00"};
-        assertNull("timezone", FixedDateFormat.createIfSupported(options));
+    public void testCreateIfSupported_defaultTimeZoneIfOptionsArrayWithSecondNullElement() {
+        final FixedDateFormat fmt = FixedDateFormat.createIfSupported(new String[] {FixedFormat.DEFAULT.getPattern(), null, ""});
+        assertEquals(FixedFormat.DEFAULT.getPattern(), fmt.getFormat());
+        assertEquals(TimeZone.getDefault(), fmt.getTimeZone());
+    }
+
+    @Test
+    public void testCreateIfSupported_customTimeZoneIfOptionsArrayWithTimeZoneElement() {
+        final FixedDateFormat fmt = FixedDateFormat.createIfSupported(new String[] {FixedFormat.DEFAULT.getPattern(), "+08:00", ""});
+        assertEquals(FixedFormat.DEFAULT.getPattern(), fmt.getFormat());
+        assertEquals(TimeZone.getTimeZone("+08:00"), fmt.getTimeZone());
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testConstructorDisallowsNullFormat() {
+        new FixedDateFormat(null, TimeZone.getDefault());
     }
 
     @Test(expected = NullPointerException.class)
-    public void testConstructorDisallowsNull() {
-        new FixedDateFormat(null);
+    public void testConstructorDisallowsNullTimeZone() {
+        new FixedDateFormat(FixedFormat.ABSOLUTE, null);
     }
 
     @Test
     public void testGetFormatReturnsConstructorFixedFormatPattern() {
-        final FixedDateFormat format = new FixedDateFormat(FixedDateFormat.FixedFormat.ABSOLUTE);
+        final FixedDateFormat format = new FixedDateFormat(FixedDateFormat.FixedFormat.ABSOLUTE, TimeZone.getDefault());
         assertSame(FixedDateFormat.FixedFormat.ABSOLUTE.getPattern(), format.getFormat());
     }
 
@@ -140,7 +155,7 @@ public class FixedDateFormatTest {
         final long end = now + TimeUnit.HOURS.toMillis(25);
         for (final FixedFormat format : FixedFormat.values()) {
             final SimpleDateFormat simpleDF = new SimpleDateFormat(format.getPattern(), Locale.getDefault());
-            final FixedDateFormat customTF = new FixedDateFormat(format);
+            final FixedDateFormat customTF = new FixedDateFormat(format, TimeZone.getDefault());
             for (long time = start; time < end; time += 12345) {
                 final String actual = customTF.format(time);
                 final String expected = simpleDF.format(new Date(time));
@@ -156,7 +171,7 @@ public class FixedDateFormatTest {
         final long end = now + TimeUnit.HOURS.toMillis(25);
         for (final FixedFormat format : FixedFormat.values()) {
             final SimpleDateFormat simpleDF = new SimpleDateFormat(format.getPattern(), Locale.getDefault());
-            final FixedDateFormat customTF = new FixedDateFormat(format);
+            final FixedDateFormat customTF = new FixedDateFormat(format, TimeZone.getDefault());
             for (long time = end; time > start; time -= 12345) {
                 final String actual = customTF.format(time);
                 final String expected = simpleDF.format(new Date(time));
@@ -173,7 +188,7 @@ public class FixedDateFormatTest {
         final char[] buffer = new char[128];
         for (final FixedFormat format : FixedFormat.values()) {
             final SimpleDateFormat simpleDF = new SimpleDateFormat(format.getPattern(), Locale.getDefault());
-            final FixedDateFormat customTF = new FixedDateFormat(format);
+            final FixedDateFormat customTF = new FixedDateFormat(format, TimeZone.getDefault());
             for (long time = start; time < end; time += 12345) {
                 final int length = customTF.format(time, buffer, 23);
                 final String actual = new String(buffer, 23, length);
@@ -191,7 +206,7 @@ public class FixedDateFormatTest {
         final char[] buffer = new char[128];
         for (final FixedFormat format : FixedFormat.values()) {
             final SimpleDateFormat simpleDF = new SimpleDateFormat(format.getPattern(), Locale.getDefault());
-            final FixedDateFormat customTF = new FixedDateFormat(format);
+            final FixedDateFormat customTF = new FixedDateFormat(format, TimeZone.getDefault());
             for (long time = end; time > start; time -= 12345) {
                 final int length = customTF.format(time, buffer, 23);
                 final String actual = new String(buffer, 23, length);

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/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 8893466..97fc345 100644
--- a/log4j-core/src/test/resources/gcFreeLogging.xml
+++ b/log4j-core/src/test/resources/gcFreeLogging.xml
@@ -6,13 +6,13 @@
     </Console>
     <File name="File" fileName="target/gcfreefile.log" bufferedIO="false">
       <PatternLayout>
-        <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern>
+        <Pattern>%d{DEFAULT}{UTC} %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"
         filePattern="target/gcfree-%d{MM-dd-yy-HH-mm-ss}.log.gz">
       <PatternLayout>
-        <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern>
+        <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %m%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern>
       </PatternLayout>
       <Policies>
         <SizeBasedTriggeringPolicy size="50M" />
@@ -20,7 +20,7 @@
     </RollingFile>
     <RandomAccessFile name="RandomAccessFile" fileName="target/gcfreeRAF.log" immediateFlush="false" append="false">
       <PatternLayout>
-        <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %ex%n}</Pattern>
+        <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m %ex%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %ex%n}</Pattern>
       </PatternLayout>
     </RandomAccessFile>
     <RollingRandomAccessFile name="RollingRandomAccessFile"
@@ -28,7 +28,7 @@
         filePattern="target/afterRollover-%i.log" append="false"
         immediateFlush="false">
       <PatternLayout>
-        <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %location %ex%n}</Pattern>
+        <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m %location %ex%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %location %ex%n}</Pattern>
       </PatternLayout>
       <Policies>
         <SizeBasedTriggeringPolicy size="50 M"/>
@@ -38,6 +38,7 @@
         fileName="target/gcfreemmap.log"
         immediateFlush="false" append="false">
       <PatternLayout>
+        <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m%ex%n</Pattern>
         <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m%ex%n}</Pattern>
       </PatternLayout>
     </MemoryMappedFile>

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/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 50bff57..6944e36 100644
--- a/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml
+++ b/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml
@@ -6,13 +6,13 @@
     </Console>
     <File name="File" fileName="target/gcfreefileMixed.log" bufferedIO="false">
       <PatternLayout>
-        <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern>
+        <Pattern>%d{DEFAULT}{UTC} %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"
         filePattern="target/gcfree-%d{MM-dd-yy-HH-mm-ss}.log.gz">
       <PatternLayout>
-        <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern>
+        <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %m%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern>
       </PatternLayout>
       <Policies>
         <SizeBasedTriggeringPolicy size="50M" />
@@ -20,7 +20,7 @@
     </RollingFile>
     <RandomAccessFile name="RandomAccessFile" fileName="target/gcfreeRAFMixed.log" immediateFlush="false" append="false">
       <PatternLayout>
-        <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %ex%n}</Pattern>
+        <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m %ex%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %ex%n}</Pattern>
       </PatternLayout>
     </RandomAccessFile>
     <RollingRandomAccessFile name="RollingRandomAccessFile"
@@ -28,7 +28,7 @@
         filePattern="target/afterRollover-%i.log" append="false"
         immediateFlush="false">
       <PatternLayout>
-        <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %location %ex%n}</Pattern>
+        <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m %location %ex%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %location %ex%n}</Pattern>
       </PatternLayout>
       <Policies>
         <SizeBasedTriggeringPolicy size="50 M"/>
@@ -38,7 +38,7 @@
         fileName="target/gcfreemmapMixed.log"
         immediateFlush="false" append="false">
       <PatternLayout>
-        <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m%ex%n}</Pattern>
+        <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m%ex%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m%ex%n}</Pattern>
       </PatternLayout>
     </MemoryMappedFile>
     <RandomAccessFile name="RandomAccessFileGelf" fileName="target/gcfreeMixed.json" immediateFlush="false" append="false">

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index cbb8466..7e44b1b 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -24,6 +24,9 @@
   </properties>
   <body>
     <release version="2.7" date="2016-MM-DD" description="GA Release 2.7">
+      <action issue="LOG4J2-1489" dev="rpopma" type="fix" due-to="Richard Zschech">
+        (GC) Fixed %date conversion patterns with a timezone parameter are now garbage free.
+      </action>
       <action issue="LOG4J2-1279" dev="rpopma" type="fix" due-to="Tony Baines">
         Prevent NullPointerException in FastDateParser$TimeZoneStrategy.
       </action>