Skip to content

Commit 8b937fb

Browse files
committed
Fix sizing of Maps
Fix several instances where Maps were sized incorrectly when the number of elements to insert is known; specifying the capacity without taking into account the load factor meant it would always result in a rehash / resize operation. To this end, add a simple Maps utility class that handles the capacity calculation, much like the static HashMap.newHashMap helper that was added in JDK19.
1 parent 286191e commit 8b937fb

File tree

22 files changed

+157
-47
lines changed

22 files changed

+157
-47
lines changed

log4j-api-test/src/test/java/org/apache/logging/log4j/ThreadContextTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@
2222
import static org.junit.jupiter.api.Assertions.assertTrue;
2323

2424
import java.util.Arrays;
25-
import java.util.HashMap;
2625
import java.util.Map;
2726
import org.apache.logging.log4j.test.ThreadContextUtilityClass;
2827
import org.apache.logging.log4j.test.junit.UsingAnyThreadContext;
28+
import org.apache.logging.log4j.util.internal.Maps;
2929
import org.junit.jupiter.api.Tag;
3030
import org.junit.jupiter.api.Test;
3131

@@ -115,7 +115,7 @@ void testPutAll() {
115115
assertTrue(ThreadContext.isEmpty());
116116
assertFalse(ThreadContext.containsKey("key"));
117117
final int mapSize = 10;
118-
final Map<String, String> newMap = new HashMap<>(mapSize);
118+
final Map<String, String> newMap = Maps.newHashMap(mapSize);
119119
for (int i = 1; i <= mapSize; i++) {
120120
newMap.put("key" + i, "value" + i);
121121
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@
2020
import static org.junit.jupiter.api.Assertions.assertFalse;
2121
import static org.junit.jupiter.api.Assertions.assertTrue;
2222

23-
import java.util.HashMap;
2423
import java.util.Map;
2524
import java.util.Properties;
2625
import org.apache.logging.log4j.test.junit.UsingThreadContextMap;
2726
import org.apache.logging.log4j.test.spi.ThreadContextMapSuite;
27+
import org.apache.logging.log4j.util.internal.Maps;
2828
import org.apache.logging.log4j.util.PropertiesUtil;
2929
import org.junit.jupiter.api.Test;
3030

@@ -56,7 +56,7 @@ void testPutAll() {
5656
assertTrue(map.isEmpty());
5757
assertFalse(map.containsKey("key"));
5858
final int mapSize = 10;
59-
final Map<String, String> newMap = new HashMap<>(mapSize);
59+
final Map<String, String> newMap = Maps.newHashMap(mapSize);
6060
for (int i = 1; i <= mapSize; i++) {
6161
newMap.put("key" + i, "value" + i);
6262
}

log4j-api/src/main/java/org/apache/logging/log4j/CloseableThreadContext.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.HashMap;
2121
import java.util.List;
2222
import java.util.Map;
23+
import org.apache.logging.log4j.util.internal.Maps;
2324

2425
/**
2526
* Adds entries to the {@link ThreadContext stack or map} and them removes them when the object is closed, e.g. as part
@@ -206,7 +207,7 @@ public void close() {
206207
}
207208

208209
private void closeMap() {
209-
final Map<String, String> valuesToReplace = new HashMap<>(originalValues.size());
210+
final Map<String, String> valuesToReplace = Maps.newHashMap(originalValues.size());
210211
final List<String> keysToRemove = new ArrayList<>(originalValues.size());
211212
for (final Map.Entry<String, String> entry : originalValues.entrySet()) {
212213
final String key = entry.getKey();

log4j-api/src/main/java/org/apache/logging/log4j/message/ThreadDumpMessage.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@
2424
import java.io.InvalidObjectException;
2525
import java.io.ObjectInputStream;
2626
import java.io.Serializable;
27-
import java.util.HashMap;
2827
import java.util.Map;
2928
import java.util.ServiceLoader;
3029
import org.apache.logging.log4j.message.ThreadDumpMessage.ThreadInfoFactory;
3130
import org.apache.logging.log4j.status.StatusLogger;
3231
import org.apache.logging.log4j.util.Lazy;
32+
import org.apache.logging.log4j.util.internal.Maps;
3333
import org.apache.logging.log4j.util.ServiceLoaderUtil;
3434
import org.apache.logging.log4j.util.StringBuilderFormattable;
3535
import org.apache.logging.log4j.util.Strings;
@@ -176,7 +176,7 @@ private static class BasicThreadInfoFactory implements ThreadInfoFactory {
176176
@Override
177177
public Map<ThreadInformation, StackTraceElement[]> createThreadInfo() {
178178
final Map<Thread, StackTraceElement[]> map = Thread.getAllStackTraces();
179-
final Map<ThreadInformation, StackTraceElement[]> threads = new HashMap<>(map.size());
179+
final Map<ThreadInformation, StackTraceElement[]> threads = Maps.newHashMap(map.size());
180180
for (final Map.Entry<Thread, StackTraceElement[]> entry : map.entrySet()) {
181181
threads.put(new BasicThreadInformation(entry.getKey()), entry.getValue());
182182
}

log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@
2121
import java.io.Serializable;
2222
import java.util.Arrays;
2323
import java.util.ConcurrentModificationException;
24-
import java.util.HashMap;
2524
import java.util.Map;
2625
import java.util.Objects;
26+
27+
import org.apache.logging.log4j.util.internal.Maps;
2728
import org.apache.logging.log4j.util.internal.SerializationUtil;
2829

2930
/**
@@ -152,7 +153,7 @@ public boolean containsKey(final String key) {
152153

153154
@Override
154155
public Map<String, String> toMap() {
155-
final Map<String, String> result = new HashMap<>(size());
156+
final Map<String, String> result = Maps.newHashMap(size());
156157
for (int i = 0; i < size(); i++) {
157158
final Object value = getValueAt(i);
158159
result.put(getKeyAt(i), value == null ? null : String.valueOf(value));
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.util.internal;
18+
19+
import java.util.HashMap;
20+
import java.util.LinkedHashMap;
21+
22+
public final class Maps {
23+
private Maps() {}
24+
25+
/**
26+
* Calculate initial capacity from expected size and default load factor (0.75).
27+
*/
28+
private static int calculateCapacity(int numMappings) {
29+
return (int) Math.ceil(numMappings / 0.75d);
30+
}
31+
32+
/**
33+
* Creates a new, empty HashMap suitable for the expected number of mappings.
34+
* The returned map is large enough so that the expected number of mappings can be
35+
* added without resizing the map.
36+
*
37+
* This is essentially a backport of HashMap.newHashMap which was added in JDK19.
38+
*/
39+
public static <K, V> HashMap<K, V> newHashMap(int numMappings) {
40+
return new HashMap<>(calculateCapacity(numMappings));
41+
}
42+
43+
/**
44+
* Creates a new, empty LinkedHashMap suitable for the expected number of mappings.
45+
* The returned map is large enough so that the expected number of mappings can be
46+
* added without resizing the map.
47+
*
48+
* This is essentially a backport of LinkedHashMap.newLinkedHashMap which was added in JDK19.
49+
*/
50+
public static <K, V> LinkedHashMap<K, V> newLinkedHashMap(int numMappings) {
51+
return new LinkedHashMap<>(calculateCapacity(numMappings));
52+
}
53+
}

log4j-core-test/src/test/java/org/apache/logging/log4j/core/context/internal/GarbageFreeSortedArrayThreadContextMapTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@
2020
import static org.junit.jupiter.api.Assertions.assertFalse;
2121
import static org.junit.jupiter.api.Assertions.assertTrue;
2222

23-
import java.util.HashMap;
2423
import java.util.Map;
2524
import java.util.Properties;
25+
26+
import org.apache.logging.log4j.core.util.internal.Maps;
2627
import org.apache.logging.log4j.spi.ThreadContextMap;
2728
import org.apache.logging.log4j.test.spi.ThreadContextMapSuite;
2829
import org.apache.logging.log4j.util.PropertiesUtil;
@@ -52,7 +53,7 @@ void testPutAll() {
5253
assertTrue(map.isEmpty());
5354
assertFalse(map.containsKey("key"));
5455
final int mapSize = 10;
55-
final Map<String, String> newMap = new HashMap<>(mapSize);
56+
final Map<String, String> newMap = Maps.newHashMap(mapSize);
5657
for (int i = 1; i <= mapSize; i++) {
5758
newMap.put("key" + i, "value" + i);
5859
}

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AppenderSet.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
*/
1717
package org.apache.logging.log4j.core.appender;
1818

19-
import java.util.HashMap;
2019
import java.util.List;
2120
import java.util.Map;
2221
import org.apache.logging.log4j.core.Appender;
@@ -28,6 +27,7 @@
2827
import org.apache.logging.log4j.core.config.plugins.PluginConfiguration;
2928
import org.apache.logging.log4j.core.config.plugins.PluginNode;
3029
import org.apache.logging.log4j.core.config.plugins.validation.constraints.Required;
30+
import org.apache.logging.log4j.core.util.internal.Maps;
3131
import org.apache.logging.log4j.status.StatusLogger;
3232

3333
/**
@@ -60,7 +60,7 @@ public AppenderSet build() {
6060
LOGGER.error("No children node in AppenderSet {}", this);
6161
return null;
6262
}
63-
final Map<String, Node> map = new HashMap<>(children.size());
63+
final Map<String, Node> map = Maps.newHashMap(children.size());
6464
for (final Node childNode : children) {
6565
final String key = childNode.getAttributes().get("name");
6666
if (key == null) {

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rewrite/LoggerNameLevelRewritePolicy.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import static org.apache.logging.log4j.util.Strings.toRootUpperCase;
2020

21-
import java.util.HashMap;
2221
import java.util.Map;
2322
import org.apache.logging.log4j.Level;
2423
import org.apache.logging.log4j.core.Core;
@@ -29,6 +28,7 @@
2928
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
3029
import org.apache.logging.log4j.core.impl.Log4jLogEvent;
3130
import org.apache.logging.log4j.core.util.KeyValuePair;
31+
import org.apache.logging.log4j.core.util.internal.Maps;
3232

3333
/**
3434
* Rewrites log event levels for a given logger name.
@@ -45,11 +45,9 @@ public class LoggerNameLevelRewritePolicy implements RewritePolicy {
4545
/**
4646
* Creates a policy to rewrite levels for a given logger name.
4747
*
48-
* @param loggerNamePrefix
49-
* The logger name prefix for events to rewrite; all event logger names that start with this string will be
50-
* rewritten.
51-
* @param levelPairs
52-
* The levels to rewrite, the key is the source level, the value the target level.
48+
* @param loggerNamePrefix The logger name prefix for events to rewrite; all event logger names that start with this string will be
49+
* rewritten.
50+
* @param levelPairs The levels to rewrite, the key is the source level, the value the target level.
5351
* @return a new LoggerNameLevelRewritePolicy
5452
*/
5553
@PluginFactory
@@ -58,7 +56,7 @@ public static LoggerNameLevelRewritePolicy createPolicy(
5856
@PluginAttribute("logger") final String loggerNamePrefix,
5957
@PluginElement("KeyValuePair") final KeyValuePair[] levelPairs) {
6058
// @formatter:on
61-
final Map<Level, Level> newMap = new HashMap<>(levelPairs.length);
59+
final Map<Level, Level> newMap = Maps.newHashMap(levelPairs.length);
6260
for (final KeyValuePair keyValuePair : levelPairs) {
6361
newMap.put(getLevel(keyValuePair.getKey()), getLevel(keyValuePair.getValue()));
6462
}

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rewrite/PropertiesRewritePolicy.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.apache.logging.log4j.core.appender.rewrite;
1818

1919
import java.util.Arrays;
20-
import java.util.HashMap;
2120
import java.util.List;
2221
import java.util.Map;
2322
import org.apache.logging.log4j.Logger;
@@ -31,6 +30,7 @@
3130
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
3231
import org.apache.logging.log4j.core.impl.ContextDataFactory;
3332
import org.apache.logging.log4j.core.impl.Log4jLogEvent;
33+
import org.apache.logging.log4j.core.util.internal.Maps;
3434
import org.apache.logging.log4j.status.StatusLogger;
3535
import org.apache.logging.log4j.util.StringMap;
3636

@@ -55,7 +55,7 @@ public final class PropertiesRewritePolicy implements RewritePolicy {
5555

5656
private PropertiesRewritePolicy(final Configuration config, final List<Property> props) {
5757
this.config = config;
58-
this.properties = new HashMap<>(props.size());
58+
this.properties = Maps.newHashMap(props.size());
5959
for (final Property property : props) {
6060
final Boolean interpolate = Boolean.valueOf(property.getValue().contains("${"));
6161
properties.put(property, interpolate);

0 commit comments

Comments
 (0)