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

logging-log4j2 git commit: LOG4J2-1677 Make MapFilter garbage-free

Repository: logging-log4j2
Updated Branches:
  refs/heads/master 4db2f5ab1 -> ee6d28c23


LOG4J2-1677 Make MapFilter 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/ee6d28c2
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/ee6d28c2
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/ee6d28c2

Branch: refs/heads/master
Commit: ee6d28c23488f7626523b33a9b2def79f3d6574b
Parents: 4db2f5a
Author: rpopma <rp...@apache.org>
Authored: Tue Nov 8 23:00:57 2016 +0900
Committer: rpopma <rp...@apache.org>
Committed: Tue Nov 8 23:00:57 2016 +0900

----------------------------------------------------------------------
 .../logging/log4j/core/filter/MapFilter.java    | 131 ++++++++++++++++---
 .../log4j/core/GcFreeLoggingTestUtil.java       |   3 +
 log4j-core/src/test/resources/gcFreeLogging.xml |  11 ++
 .../resources/gcFreeMixedSyncAsyncLogging.xml   |  11 ++
 src/changes/changes.xml                         |   3 +
 5 files changed, 141 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ee6d28c2/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/MapFilter.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/MapFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/MapFilter.java
index d8b8f00..7379aba 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/MapFilter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/MapFilter.java
@@ -35,24 +35,32 @@ import org.apache.logging.log4j.core.config.plugins.PluginFactory;
 import org.apache.logging.log4j.core.util.KeyValuePair;
 import org.apache.logging.log4j.message.MapMessage;
 import org.apache.logging.log4j.message.Message;
+import org.apache.logging.log4j.util.BiConsumer;
+import org.apache.logging.log4j.util.PerformanceSensitive;
 import org.apache.logging.log4j.util.ReadOnlyStringMap;
+import org.apache.logging.log4j.util.SortedArrayStringMap;
+import org.apache.logging.log4j.util.StringMap;
 
 /**
  * A Filter that operates on a Map.
  */
 @Plugin(name = "MapFilter", category = Node.CATEGORY, elementType = Filter.ELEMENT_TYPE, printObject = true)
+@PerformanceSensitive("allocation")
 public class MapFilter extends AbstractFilter {
 
-    private final Map<String, List<String>> map;
-
+    //private final Map<String, List<String>> map;
+    private final SortedArrayStringMap map;
     private final boolean isAnd;
 
-    protected MapFilter(final Map<String, List<String>> map, final boolean oper, final Result onMatch,
-                        final Result onMismatch) {
+    protected MapFilter(final Map<String, List<String>> map, final boolean oper, final Result onMatch, final Result onMismatch) {
         super(onMatch, onMismatch);
-        Objects.requireNonNull(map, "map cannot be null");
         this.isAnd = oper;
-        this.map = map;
+        Objects.requireNonNull(map, "map cannot be null");
+
+        this.map = new SortedArrayStringMap(map.size());
+        for (final Map.Entry<String, List<String>> entry : map.entrySet()) {
+            this.map.putValue(entry.getKey(), entry.getValue());
+        }
     }
 
     @Override
@@ -75,9 +83,10 @@ public class MapFilter extends AbstractFilter {
 
     protected boolean filter(final Map<String, String> data) {
         boolean match = false;
-        for (final Map.Entry<String, List<String>> entry : map.entrySet()) {
-            final String toMatch = data.get(entry.getKey());
-            match = toMatch != null && entry.getValue().contains(toMatch);
+        for (int i = 0; i < map.size(); i++) {
+            final String toMatch = data.get(map.getKeyAt(i));
+            match = toMatch != null && ((List<String>) map.getValueAt(i)).contains(toMatch);
+
             if ((!isAnd && match) || (isAnd && !match)) {
                 break;
             }
@@ -87,9 +96,10 @@ public class MapFilter extends AbstractFilter {
 
     protected boolean filter(final ReadOnlyStringMap data) {
         boolean match = false;
-        for (final Map.Entry<String, List<String>> entry : map.entrySet()) {
-            final String toMatch = data.getValue(entry.getKey());
-            match = toMatch != null && entry.getValue().contains(toMatch);
+        for (int i = 0; i < map.size(); i++) {
+            final String toMatch = data.getValue(map.getKeyAt(i));
+            match = toMatch != null && ((List<String>) map.getValueAt(i)).contains(toMatch);
+
             if ((!isAnd && match) || (isAnd && !match)) {
                 break;
             }
@@ -98,20 +108,87 @@ public class MapFilter extends AbstractFilter {
     }
 
     @Override
+    public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
+            final Object p0) {
+        return Result.NEUTRAL;
+    }
+
+    @Override
+    public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
+            final Object p0, final Object p1) {
+        return Result.NEUTRAL;
+    }
+
+    @Override
+    public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
+            final Object p0, final Object p1, final Object p2) {
+        return Result.NEUTRAL;
+    }
+
+    @Override
+    public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
+            final Object p0, final Object p1, final Object p2, final Object p3) {
+        return Result.NEUTRAL;
+    }
+
+    @Override
+    public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
+            final Object p0, final Object p1, final Object p2, final Object p3,
+            final Object p4) {
+        return Result.NEUTRAL;
+    }
+
+    @Override
+    public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
+            final Object p0, final Object p1, final Object p2, final Object p3,
+            final Object p4, final Object p5) {
+        return Result.NEUTRAL;
+    }
+
+    @Override
+    public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
+            final Object p0, final Object p1, final Object p2, final Object p3,
+            final Object p4, final Object p5, final Object p6) {
+        return Result.NEUTRAL;
+    }
+
+    @Override
+    public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
+            final Object p0, final Object p1, final Object p2, final Object p3,
+            final Object p4, final Object p5, final Object p6,
+            final Object p7) {
+        return Result.NEUTRAL;
+    }
+
+    @Override
+    public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
+            final Object p0, final Object p1, final Object p2, final Object p3,
+            final Object p4, final Object p5, final Object p6,
+            final Object p7, final Object p8) {
+        return Result.NEUTRAL;
+    }
+
+    @Override
+    public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
+            final Object p0, final Object p1, final Object p2, final Object p3,
+            final Object p4, final Object p5, final Object p6,
+            final Object p7, final Object p8, final Object p9) {
+        return Result.NEUTRAL;
+    }
+
+    @Override
     public String toString() {
         final StringBuilder sb = new StringBuilder();
         sb.append("isAnd=").append(isAnd);
         if (map.size() > 0) {
             sb.append(", {");
-            boolean first = true;
-            for (final Map.Entry<String, List<String>> entry : map.entrySet()) {
-                if (!first) {
+            for (int i = 0; i < map.size(); i++) {
+                if (i > 0) {
                     sb.append(", ");
                 }
-                first = false;
-                final List<String> list = entry.getValue();
+                final List<String> list = map.getValueAt(i);
                 final String value = list.size() > 1 ? list.get(0) : list.toString();
-                sb.append(entry.getKey()).append('=').append(value);
+                sb.append(map.getKeyAt(i)).append('=').append(value);
             }
             sb.append('}');
         }
@@ -122,7 +199,25 @@ public class MapFilter extends AbstractFilter {
         return isAnd;
     }
 
+    /** @deprecated  use {@link #getStringMap()} instead */
+    @Deprecated
     protected Map<String, List<String>> getMap() {
+        final Map<String, List<String>> result = new HashMap<>(map.size());
+        map.forEach(new BiConsumer<String, List<String>>() {
+            @Override
+            public void accept(final String key, final List<String> value) {
+                result.put(key, value);
+            }
+        });
+        return result;
+    }
+
+    /**
+     * Returns the StringMap with {@code List<String>} values that this MapFilter was constructed with.
+     * @return the StringMap with {@code List<String>} values to match against
+     * @since 2.8
+     */
+    protected StringMap getStringMap() {
         return map;
     }
 

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ee6d28c2/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 4604cd9..584ba37 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
@@ -32,6 +32,7 @@ import org.apache.logging.log4j.Marker;
 import org.apache.logging.log4j.MarkerManager;
 import org.apache.logging.log4j.ThreadContext;
 import org.apache.logging.log4j.core.util.Constants;
+import org.apache.logging.log4j.message.MapMessage;
 import org.apache.logging.log4j.util.Strings;
 
 import com.google.monitoring.runtime.instrumentation.AllocationRecorder;
@@ -103,6 +104,7 @@ public class GcFreeLoggingTestUtil {
             }
         };
         Thread.sleep(500);
+        final MapMessage mapMessage = new MapMessage().with("eventId", "Login");
         AllocationRecorder.addSampler(sampler);
 
         // now do some steady-state logging
@@ -118,6 +120,7 @@ public class GcFreeLoggingTestUtil {
             logger.error("Test parameterized message {}", "param");
             logger.error("Test parameterized message {}{}", "param", "param2");
             logger.error("Test parameterized message {}{}{}", "param", "param2", "abc");
+            //logger.error(mapMessage); // TODO LOG4J2-1683
             ThreadContext.remove("aKey");
             ThreadContext.put("aKey", "value1");
         }

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ee6d28c2/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 c0598ce..ca9d4f3 100644
--- a/log4j-core/src/test/resources/gcFreeLogging.xml
+++ b/log4j-core/src/test/resources/gcFreeLogging.xml
@@ -1,5 +1,16 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Configuration status="OFF">
+  <Filters>
+    <DynamicThresholdFilter key="loginId" defaultThreshold="ERROR"
+        onMatch="ACCEPT" onMismatch="NEUTRAL">
+      <KeyValuePair key="User1" value="DEBUG"/>
+    </DynamicThresholdFilter>
+    <MapFilter onMatch="ACCEPT" onMismatch="NEUTRAL" operator="or">
+      <KeyValuePair key="eventId" value="Login"/>
+      <KeyValuePair key="eventId" value="Logout"/>
+    </MapFilter>
+    <Marker marker="EVENT" onMatch="ACCEPT" onMismatch="NEUTRAL"/>
+  </Filters>
   <Appenders>
     <Console name="Console" target="SYSTEM_OUT">
       <PatternLayout pattern="%p %c{1.} [%t] %X{aKey} %X %m%ex%n" />

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ee6d28c2/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 b6ce873..6ab59b6 100644
--- a/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml
+++ b/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml
@@ -1,5 +1,16 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Configuration status="OFF">
+  <Filters>
+    <DynamicThresholdFilter key="loginId" defaultThreshold="ERROR"
+        onMatch="ACCEPT" onMismatch="NEUTRAL">
+      <KeyValuePair key="User1" value="DEBUG"/>
+    </DynamicThresholdFilter>
+    <MapFilter onMatch="ACCEPT" onMismatch="NEUTRAL" operator="or">
+      <KeyValuePair key="eventId" value="Login"/>
+      <KeyValuePair key="eventId" value="Logout"/>
+    </MapFilter>
+    <Marker marker="EVENT" onMatch="ACCEPT" onMismatch="NEUTRAL"/>
+  </Filters>
   <Appenders>
     <Console name="Console" target="SYSTEM_OUT">
       <PatternLayout pattern="%p %c{1.} [%t] %X{aKey} %X %m%ex%n" />

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ee6d28c2/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 4fd61c5..d785f22 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-1677" dev="rpopma" type="fix">
+        (GC) Avoid allocating temporary objects in MapFilter.
+      </action>
       <action issue="LOG4J2-1681" dev="rpopma" type="add">
         Changed visibility of indexed getter methods in SortedArrayStringMap from package-protected to public.
       </action>