Fix regression in JdkMapAdapterStringMap performance

We introduce a new constructor for `JdkMapAdapterStringMap` to prevent
the performance penalty of an exception if the map is immutable.

Author: ppkarwasz

log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMapTest.java

@@ -44,6 +44,7 @@ public class JdkMapAdapterStringMapTest {
     @Test
     public void testConstructorDisallowsNull() {
         assertThrows(NullPointerException.class, () -> new JdkMapAdapterStringMap(null));
+        assertThrows(NullPointerException.class, () -> new JdkMapAdapterStringMap(null, false));
     }
 
     @Test

log4j-core/src/main/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMap.java

@@ -47,9 +47,14 @@ public class JdkMapAdapterStringMap implements StringMap {
     private transient String[] sortedKeys;
 
     public JdkMapAdapterStringMap() {
-        this(new HashMap<String, String>());
+        this(new HashMap<String, String>(), false);
     }
 
+    /**
+     * @deprecated for performance reasons since 2.23.
+     *             Use {@link #JdkMapAdapterStringMap(Map, boolean)} instead.
+     */
+    @Deprecated
     public JdkMapAdapterStringMap(final Map<String, String> map) {
         this.map = Objects.requireNonNull(map, "map");
         try {
@@ -59,6 +64,15 @@ public JdkMapAdapterStringMap(final Map<String, String> map) {
         }
     }
 
+    /**
+     * @param map a JDK map,
+     * @param immutable must be {@code true} if the map is immutable or it should not be modified.
+     */
+    public JdkMapAdapterStringMap(final Map<String, String> map, final boolean immutable) {
+        this.map = Objects.requireNonNull(map, "map");
+        this.immutable = immutable;
+    }
+
     @Override
     public Map<String, String> toMap() {
         return map;

log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThreadContextDataInjector.java

@@ -121,7 +121,7 @@ public StringMap injectContextData(final List<Property> props, final StringMap i
             // data. Note that we cannot reuse the specified StringMap: some Loggers may have properties defined
             // and others not, so the LogEvent's context data may have been replaced with an immutable copy from
             // the ThreadContext - this will throw an UnsupportedOperationException if we try to modify it.
-            final StringMap result = new JdkMapAdapterStringMap(new HashMap<>(copy));
+            final StringMap result = new JdkMapAdapterStringMap(new HashMap<>(copy), false);
             for (int i = 0; i < props.size(); i++) {
                 final Property prop = props.get(i);
                 if (!copy.containsKey(prop.getName())) {
@@ -133,20 +133,20 @@ public StringMap injectContextData(final List<Property> props, final StringMap i
         }
 
         private static JdkMapAdapterStringMap frozenStringMap(final Map<String, String> copy) {
-            final JdkMapAdapterStringMap result = new JdkMapAdapterStringMap(copy);
-            result.freeze();
-            return result;
+            return new JdkMapAdapterStringMap(copy, true);
         }
 
         @Override
         public ReadOnlyStringMap rawContextData() {
             final ReadOnlyThreadContextMap map = ThreadContext.getThreadContextMap();
-            if (map instanceof ReadOnlyStringMap) {
-                return (ReadOnlyStringMap) map;
+            if (map != null) {
+                return map.getReadOnlyContextData();
             }
             // note: default ThreadContextMap is null
             final Map<String, String> copy = ThreadContext.getImmutableContext();
-            return copy.isEmpty() ? ContextDataFactory.emptyFrozenContextData() : new JdkMapAdapterStringMap(copy);
+            return copy.isEmpty()
+                    ? ContextDataFactory.emptyFrozenContextData()
+                    : new JdkMapAdapterStringMap(copy, true);
         }
     }
 

log4j-core/src/main/java/org/apache/logging/log4j/core/util/ContextDataProvider.java

@@ -36,6 +36,6 @@ public interface ContextDataProvider {
      * @return the context data in a StringMap.
      */
     default StringMap supplyStringMap() {
-        return new JdkMapAdapterStringMap(supplyContextData());
+        return new JdkMapAdapterStringMap(supplyContextData(), true);
     }
 }

log4j-layout-template-json-test/src/test/java/org/apache/logging/log4j/layout/template/json/util/JsonWriterTest.java

@@ -149,7 +149,7 @@ void test_writeObject_null_StringMap() {
 
     @Test
     void test_writeObject_StringMap() {
-        final StringMap map = new JdkMapAdapterStringMap(Collections.singletonMap("a", "b"));
+        final StringMap map = new JdkMapAdapterStringMap(Collections.singletonMap("a", "b"), true);
         final String expectedJson = "{'a':'b'}".replace('\'', '"');
         final String actualJson = withLockedWriterReturning(writer -> writer.use(() -> writer.writeObject(map)));
         Assertions.assertThat(actualJson).isEqualTo(expectedJson);
@@ -224,7 +224,7 @@ public String toString() {
                                         put("foo", "bar");
                                     }
                                 }),
-                                new JdkMapAdapterStringMap(Collections.singletonMap("a", "b"))));
+                                new JdkMapAdapterStringMap(Collections.singletonMap("a", "b"), true)));
                 put("key7", (StringBuilderFormattable) buffer -> buffer.append(7.7777777777777D));
             }
         };

src/changelog/.3.x.x/2238_fix_jdk_string_map.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="http://logging.apache.org/log4j/changelog"
+       xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.2.xsd"
+       type="fixed">
+  <issue id="2238" link="https://github.com/apache/logging-log4j2/issues/2238"/>
+  <description format="asciidoc">
+    Fix regression in `JdkMapAdapterStringMap` performance.
+  </description>
+</entry>