Repository: logging-log4j2
Updated Branches:
  refs/heads/master 3dd15a231 -> f4525d859


LOG4J2-1683 (GC) Avoid allocating temporary objects in MapMessage.


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

Branch: refs/heads/master
Commit: f4525d8592fda96d6fc7c25d053f1ccefadef448
Parents: 3dd15a2
Author: rpopma <[email protected]>
Authored: Sat Nov 19 23:26:57 2016 +0900
Committer: rpopma <[email protected]>
Committed: Sat Nov 19 23:26:57 2016 +0900

----------------------------------------------------------------------
 .../logging/log4j/message/MapMessage.java       | 85 +++++++++++---------
 .../log4j/util/SortedArrayStringMap.java        |  7 ++
 src/changes/changes.xml                         |  3 +
 3 files changed, 59 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/f4525d85/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
----------------------------------------------------------------------
diff --git 
a/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java 
b/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
index b113ed5..43ecffd 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
@@ -18,10 +18,12 @@ package org.apache.logging.log4j.message;
 
 import java.util.Collections;
 import java.util.Map;
-import java.util.SortedMap;
 import java.util.TreeMap;
 
 import org.apache.logging.log4j.util.EnglishEnums;
+import org.apache.logging.log4j.util.IndexedStringMap;
+import org.apache.logging.log4j.util.SortedArrayStringMap;
+import org.apache.logging.log4j.util.StringBuilderFormattable;
 import org.apache.logging.log4j.util.StringBuilders;
 import org.apache.logging.log4j.util.Strings;
 
@@ -33,7 +35,8 @@ import org.apache.logging.log4j.util.Strings;
  * logged, because it is undefined whether the logged message string will 
contain the old values or the modified
  * values.
  */
-public class MapMessage implements MultiformatMessage {
+public class MapMessage implements MultiformatMessage, 
StringBuilderFormattable {
+
     /**
      * When set as the format specifier causes the Map to be formatted as XML.
      */
@@ -49,13 +52,13 @@ public class MapMessage implements MultiformatMessage {
 
     private static final long serialVersionUID = -5031471831131487120L;
 
-    private final SortedMap<String, String> data;
+    private final IndexedStringMap data;
 
     /**
      * Constructor.
      */
     public MapMessage() {
-        data = new TreeMap<>();
+        data = new SortedArrayStringMap();
     }
 
     /**
@@ -63,7 +66,7 @@ public class MapMessage implements MultiformatMessage {
      * @param map The Map.
      */
     public MapMessage(final Map<String, String> map) {
-        this.data = map instanceof SortedMap ? (SortedMap<String, String>) map 
: new TreeMap<>(map);
+        this.data = new SortedArrayStringMap(map);
     }
 
     @Override
@@ -82,7 +85,11 @@ public class MapMessage implements MultiformatMessage {
      */
     @Override
     public Object[] getParameters() {
-        return data.values().toArray();
+        final Object[] result = new Object[data.size()];
+        for (int i = 0; i < data.size(); i++) {
+            result[i] = data.getValueAt(i);
+        }
+        return result;
     }
 
     /**
@@ -99,7 +106,11 @@ public class MapMessage implements MultiformatMessage {
      * @return the message data as an unmodifiable map.
      */
     public Map<String, String> getData() {
-        return Collections.unmodifiableMap(data);
+        final TreeMap<String, String> result = new TreeMap<>(); // returned 
map must be sorted
+        for (int i = 0; i < data.size(); i++) {
+            result.put(data.getKeyAt(i), (String) data.getValueAt(i));
+        }
+        return Collections.unmodifiableMap(result);
     }
 
     /**
@@ -130,7 +141,7 @@ public class MapMessage implements MultiformatMessage {
             throw new IllegalArgumentException("No value provided for key " + 
key);
         }
         validate(key, value);
-        data.put(key, value);
+        data.putValue(key, value);
     }
 
     protected void validate(final String key, final String value) {
@@ -142,7 +153,9 @@ public class MapMessage implements MultiformatMessage {
      * @param map The Map to add.
      */
     public void putAll(final Map<String, String> map) {
-        data.putAll(map);
+        for (final Map.Entry<String, ?> entry : map.entrySet()) {
+            data.putValue(entry.getKey(), entry.getValue());
+        }
     }
 
     /**
@@ -151,7 +164,7 @@ public class MapMessage implements MultiformatMessage {
      * @return The value of the element or null if the key is not present.
      */
     public String get(final String key) {
-        return data.get(key);
+        return data.getValue(key);
     }
 
     /**
@@ -160,7 +173,9 @@ public class MapMessage implements MultiformatMessage {
      * @return The previous value of the element.
      */
     public String remove(final String key) {
-        return data.remove(key);
+        final String result = data.getValue(key);
+        data.remove(key);
+        return result;
     }
 
     /**
@@ -169,12 +184,12 @@ public class MapMessage implements MultiformatMessage {
      * @return The formatted String.
      */
     public String asString() {
-        return asString((MapFormat) null);
+        return format((MapFormat) null, new StringBuilder()).toString();
     }
 
     public String asString(final String format) {
         try {
-            return asString(EnglishEnums.valueOf(MapFormat.class, format));
+            return format(EnglishEnums.valueOf(MapFormat.class, format), new 
StringBuilder()).toString();
         } catch (final IllegalArgumentException ex) {
             return asString();
         }
@@ -185,8 +200,7 @@ public class MapMessage implements MultiformatMessage {
      * @param format The format identifier. Ignored in this implementation.
      * @return The formatted String.
      */
-    private String asString(final MapFormat format) {
-        final StringBuilder sb = new StringBuilder();
+    private StringBuilder format(final MapFormat format, final StringBuilder 
sb) {
         if (format == null) {
             appendMap(sb);
         } else {
@@ -208,14 +222,14 @@ public class MapMessage implements MultiformatMessage {
                 }
             }
         }
-        return sb.toString();
+        return sb;
     }
 
     public void asXml(final StringBuilder sb) {
         sb.append("<Map>\n");
-        for (final Map.Entry<String, String> entry : data.entrySet()) {
-            sb.append("  <Entry 
key=\"").append(entry.getKey()).append("\">").append(entry.getValue())
-              .append("</Entry>\n");
+        for (int i = 0; i < data.size(); i++) {
+            sb.append("  <Entry 
key=\"").append(data.getKeyAt(i)).append("\">").append(data.getValueAt(i))
+                    .append("</Entry>\n");
         }
         sb.append("</Map>");
     }
@@ -245,7 +259,7 @@ public class MapMessage implements MultiformatMessage {
         for (final String format : formats) {
             for (final MapFormat mapFormat : MapFormat.values()) {
                 if (mapFormat.name().equalsIgnoreCase(format)) {
-                    return asString(mapFormat);
+                    return format(mapFormat, new StringBuilder()).toString();
                 }
             }
         }
@@ -254,40 +268,34 @@ public class MapMessage implements MultiformatMessage {
     }
 
     protected void appendMap(final StringBuilder sb) {
-        boolean first = true;
-        for (final Map.Entry<String, String> entry : data.entrySet()) {
-            if (!first) {
+        for (int i = 0; i < data.size(); i++) {
+            if (i > 0) {
                 sb.append(' ');
             }
-            first = false;
-            StringBuilders.appendKeyDqValue(sb, entry);
+            StringBuilders.appendKeyDqValue(sb, data.getKeyAt(i), 
data.getValueAt(i));
         }
     }
 
     protected void asJson(final StringBuilder sb) {
-        boolean first = true;
         sb.append('{');
-        for (final Map.Entry<String, String> entry : data.entrySet()) {
-            if (!first) {
+        for (int i = 0; i < data.size(); i++) {
+            if (i > 0) {
                 sb.append(", ");
             }
-            first = false;
-            StringBuilders.appendDqValue(sb, entry.getKey()).append(':');
-            StringBuilders.appendDqValue(sb, entry.getValue());
+            StringBuilders.appendDqValue(sb, data.getKeyAt(i)).append(':');
+            StringBuilders.appendDqValue(sb, data.getValueAt(i));
         }
         sb.append('}');
     }
 
 
     protected void asJava(final StringBuilder sb) {
-        boolean first = true;
         sb.append('{');
-        for (final Map.Entry<String, String> entry : data.entrySet()) {
-            if (!first) {
+        for (int i = 0; i < data.size(); i++) {
+            if (i > 0) {
                 sb.append(", ");
             }
-            first = false;
-            StringBuilders.appendKeyDqValue(sb, entry);
+            StringBuilders.appendKeyDqValue(sb, data.getKeyAt(i), 
data.getValueAt(i));
         }
         sb.append('}');
     }
@@ -302,6 +310,11 @@ public class MapMessage implements MultiformatMessage {
     }
 
     @Override
+    public void formatTo(final StringBuilder buffer) {
+        format((MapFormat) null, buffer);
+    }
+
+    @Override
     public boolean equals(final Object o) {
         if (this == o) {
             return true;

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/f4525d85/log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java
----------------------------------------------------------------------
diff --git 
a/log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java
 
b/log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java
index af9ede0..1c1ce90 100644
--- 
a/log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java
+++ 
b/log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java
@@ -110,6 +110,13 @@ public class SortedArrayStringMap implements 
IndexedStringMap {
         }
     }
 
+    public SortedArrayStringMap(final Map<String, ?> map) {
+        resize(ceilingNextPowerOfTwo(map.size()));
+        for (final Map.Entry<String, ?> entry : map.entrySet()) {
+            putValue(entry.getKey(), entry.getValue());
+        }
+    }
+
     private void assertNotFrozen() {
         if (immutable) {
             throw new UnsupportedOperationException(FROZEN);

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/f4525d85/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 11cf886..4c9c815 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -39,6 +39,9 @@
       <action issue="LOG4J2-1706" dev="rpopma" type="fix">
         Make TimeFilter usable as global filter and as logger filter.
       </action>
+      <action issue="LOG4J2-1683" dev="rpopma" type="fix">
+        (GC) Avoid allocating temporary objects in MapMessage.
+      </action>
       <action issue="LOG4J2-1715" dev="rpopma" type="fix">
         (GC) Avoid allocating temporary objects in NdcPatternConverter. (Note 
that use of the ThreadContext stack is not garbage-free.)
       </action>

Reply via email to