diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextMapTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextMapTest.java index 4e3356aad31..1d8c3fa5172 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextMapTest.java +++ b/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 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 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 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 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 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 copy1 = contextMap.getCopy(); + final Map 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")); + } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java index 62752428f50..ccbd0a220fb 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java @@ -147,13 +147,37 @@ public V getValue(final String key) { return (V) get(key); } + /** + * {@return a mutable copy of the current thread context map} + */ @Override public Map getCopy() { final Object[] state = localState.get(); if (state == null) { return new HashMap<>(0); } - return new HashMap<>(getMap(state)); + + final Map 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 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 entry : map.entrySet()) { + copy.put(entry.getKey(), entry.getValue()); + } + + return copy; } @Override diff --git a/log4j-perf-test/src/main/java/org/apache/logging/log4j/perf/jmh/ThreadContextBenchmark.java b/log4j-perf-test/src/main/java/org/apache/logging/log4j/perf/jmh/ThreadContextBenchmark.java index 653028ccee2..f3e00c6b7f9 100644 --- a/log4j-perf-test/src/main/java/org/apache/logging/log4j/perf/jmh/ThreadContextBenchmark.java +++ b/log4j-perf-test/src/main/java/org/apache/logging/log4j/perf/jmh/ThreadContextBenchmark.java @@ -167,6 +167,11 @@ public Map legacyInjectWithProperties() { return createMap(propertyList); } + @Benchmark + public Map getCopy() { + return ThreadContext.getContext(); + } + // from Log4jLogEvent::createMap static Map createMap(final List properties) { final Map contextMap = ThreadContext.getImmutableContext(); diff --git a/src/changelog/.2.x.x/3935_optimize_DefaultThreadContextMap_getCopy.xml b/src/changelog/.2.x.x/3935_optimize_DefaultThreadContextMap_getCopy.xml new file mode 100644 index 00000000000..35e1cf8b444 --- /dev/null +++ b/src/changelog/.2.x.x/3935_optimize_DefaultThreadContextMap_getCopy.xml @@ -0,0 +1,13 @@ + + + + + + Optimize `DefaultThreadContextMap.getCopy()` performance by avoiding megamorphic calls in `HashMap` constructor + + \ No newline at end of file