This is an automated email from the ASF dual-hosted git repository. vy pushed a commit to branch release/2.25.3 in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit c09b01236c4b31f59c3cd1c272d30f58cc8f6f5f Author: Anurag Dubey <[email protected]> AuthorDate: Fri Oct 10 19:11:27 2025 +0530 Optimize `DefaultThreadContextMap.getCopy()` performance (#3939) Co-authored-by: Volkan Yazıcı <[email protected]> --- .../log4j/spi/DefaultThreadContextMapTest.java | 122 +++++++++++++++++++++ .../logging/log4j/spi/DefaultThreadContextMap.java | 26 ++++- .../log4j/perf/jmh/ThreadContextBenchmark.java | 5 + ...35_optimize_DefaultThreadContextMap_getCopy.xml | 13 +++ 4 files changed, 165 insertions(+), 1 deletion(-) 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 4e3356aad3..1d8c3fa517 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 @@ class DefaultThreadContextMapTest extends ThreadContextMapSuite { 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")); + } } 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 62752428f5..ccbd0a220f 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 class DefaultThreadContextMap implements ThreadContextMap, ReadOnlyString 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 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 653028ccee..f3e00c6b7f 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 class ThreadContextBenchmark { 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(); 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 0000000000..35e1cf8b44 --- /dev/null +++ b/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> \ No newline at end of file
