Optimize DefaultThreadContextMap.getCopy() performance (#3939)

Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>

Author: anuragdy

log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextMapTest.java

@@ -16,8 +16,10 @@
  */
 package org.apache.logging.log4j.spi;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.HashMap;
@@ -130,4 +132,124 @@ void threadLocalNotInheritableByDefault() {
     void threadLocalInheritableIfConfigured() {
         threadLocalInheritableIfConfigured(createInheritableThreadContextMap());
     }
+
+    @Test
+    void testGetCopyWithEmptyMap() {
+        final DefaultThreadContextMap contextMap = new DefaultThreadContextMap();
+
+        // Verify map is empty
+        assertTrue(contextMap.isEmpty());
+        assertEquals(0, contextMap.size());
+
+        // Get copy of empty map
+        final Map<String, String> copy = contextMap.getCopy();
+
+        // Verify copy is empty HashMap
+        assertThat(copy).isInstanceOf(HashMap.class);
+        assertTrue(copy.isEmpty());
+        assertEquals(0, copy.size());
+
+        // Verify copy is independent
+        copy.put("test", "value");
+        assertTrue(contextMap.isEmpty());
+    }
+
+    @Test
+    void testGetCopyWithSingleElement() {
+        final DefaultThreadContextMap contextMap = new DefaultThreadContextMap();
+
+        // Add single element
+        contextMap.put("key1", "value1");
+        assertEquals(1, contextMap.size());
+        assertEquals("value1", contextMap.get("key1"));
+
+        // Get copy
+        final Map<String, String> copy = contextMap.getCopy();
+
+        // Verify copy contains identical data
+        assertThat(copy).isInstanceOf(HashMap.class);
+        assertEquals(1, copy.size());
+        assertEquals("value1", copy.get("key1"));
+        assertTrue(copy.containsKey("key1"));
+
+        // Verify copy is independent
+        assertNotSame(copy, contextMap.toMap());
+        copy.put("key2", "value2");
+        assertEquals(1, contextMap.size());
+        assertFalse(contextMap.containsKey("key2"));
+    }
+
+    @Test
+    void testGetCopyWithMultipleElements() {
+        final DefaultThreadContextMap contextMap = new DefaultThreadContextMap();
+
+        // Add multiple elements
+        final Map<String, String> testData = new HashMap<>();
+        testData.put("key1", "value1");
+        testData.put("key2", "value2");
+        testData.put("key3", "value3");
+        testData.put("key4", "value4");
+        testData.put("key5", "value5");
+
+        contextMap.putAll(testData);
+        assertEquals(5, contextMap.size());
+
+        // Get copy
+        final Map<String, String> copy = contextMap.getCopy();
+
+        // Verify copy contains identical data
+        assertThat(copy).isInstanceOf(HashMap.class);
+        assertEquals(5, copy.size());
+
+        // Verify all entries match
+        assertEquals(testData, copy);
+
+        // Verify copy is independent
+        copy.clear();
+        assertEquals(5, contextMap.size());
+    }
+
+    @Test
+    void testGetCopyReturnsHashMap() {
+        final DefaultThreadContextMap contextMap = new DefaultThreadContextMap();
+
+        // Test with empty map
+        Map<String, String> copy = contextMap.getCopy();
+        assertThat(copy).isInstanceOf(HashMap.class);
+
+        // Test with populated map
+        contextMap.put("key", "value");
+        copy = contextMap.getCopy();
+        assertThat(copy).isInstanceOf(HashMap.class);
+    }
+
+    @Test
+    void testGetCopyIndependence() {
+        final DefaultThreadContextMap contextMap = new DefaultThreadContextMap();
+
+        // Setup initial data
+        contextMap.put("key1", "value1");
+        contextMap.put("key2", "value2");
+
+        final Map<String, String> copy1 = contextMap.getCopy();
+        final Map<String, String> copy2 = contextMap.getCopy();
+
+        // Verify copies are independent of each other
+        assertNotSame(copy1, copy2);
+        assertEquals(copy1, copy2);
+
+        // Modify first copy
+        copy1.put("key3", "value3");
+        copy1.remove("key1");
+
+        // Verify second copy is unaffected
+        assertEquals(2, copy2.size());
+        assertTrue(copy2.containsKey("key1"));
+        assertFalse(copy2.containsKey("key3"));
+
+        // Verify original map is unaffected
+        assertEquals(2, contextMap.size());
+        assertTrue(contextMap.containsKey("key1"));
+        assertFalse(contextMap.containsKey("key3"));
+    }
 }

log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java

@@ -147,13 +147,37 @@ public <V> V getValue(final String key) {
         return (V) get(key);
     }
 
+    /**
+     * {@return a mutable copy of the current thread context map}
+     */
     @Override
     public Map<String, String> getCopy() {
         final Object[] state = localState.get();
         if (state == null) {
             return new HashMap<>(0);
         }
-        return new HashMap<>(getMap(state));
+
+        final Map<String, String> map = getMap(state);
+
+        // Handle empty map case efficiently - constructor is faster for empty maps
+        if (map.isEmpty()) {
+            return new HashMap<>();
+        }
+
+        // Pre-size HashMap to minimize rehashing operations
+        // Factor 1.35 accounts for HashMap's 0.75 load factor (1/0.75 ≈ 1.33)
+        final HashMap<String, String> copy = new HashMap<>((int) (map.size() * 1.35));
+
+        // Manual iteration avoids megamorphic virtual calls that prevent JIT optimization.
+        // The HashMap(Map) constructor requires (3 + 4n) virtual method calls that become
+        // megamorphic when used with different map types, leading to 24-136% performance
+        // degradation. Manual iteration creates monomorphic call sites that JIT can optimize.
+        // See https://bugs.openjdk.org/browse/JDK-8368292
+        for (final Map.Entry<String, String> entry : map.entrySet()) {
+            copy.put(entry.getKey(), entry.getValue());
+        }
+
+        return copy;
     }
 
     @Override

log4j-perf-test/src/main/java/org/apache/logging/log4j/perf/jmh/ThreadContextBenchmark.java

@@ -167,6 +167,11 @@ public Map<String, String> legacyInjectWithProperties() {
         return createMap(propertyList);
     }
 
+    @Benchmark
+    public Map<String, String> getCopy() {
+        return ThreadContext.getContext();
+    }
+
     // from Log4jLogEvent::createMap
     static Map<String, String> createMap(final List<Property> properties) {
         final Map<String, String> contextMap = ThreadContext.getImmutableContext();

src/changelog/.2.x.x/3935_optimize_DefaultThreadContextMap_getCopy.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:schemaLocation="
+           https://logging.apache.org/xml/ns
+           https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="changed">
+    <issue id="3935" link="https://github.com/apache/logging-log4j2/issues/3935"/>
+    <issue id="3939" link="https://github.com/apache/logging-log4j2/pull/3939"/>
+    <description format="asciidoc">
+        Optimize `DefaultThreadContextMap.getCopy()` performance by avoiding megamorphic calls in `HashMap` constructor
+    </description>
+</entry>