Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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>