Skip to content

Commit 92abe73

Browse files
Speed up Maps.copyMapWithAddedEntry to Speed up ITs (#73308) (#74046)
This method is taking about 4% of CPU time with internal cluster tests for me. 80% of that were coming from the slow immutability assertion, the rest was due to the slow way we were building up the new map. The CPU time slowness likely translates into outright test slowness, because this was mainly hit through adding transport handlers when starting nodes (which happens on the main test thread). Fixed both to save a few % of test runtime.
1 parent b2c17c6 commit 92abe73

File tree

1 file changed

+22
-13
lines changed
  • server/src/main/java/org/elasticsearch/common/util

1 file changed

+22
-13
lines changed

server/src/main/java/org/elasticsearch/common/util/Maps.java

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88

99
package org.elasticsearch.common.util;
1010

11-
import org.elasticsearch.Assertions;
12-
1311
import java.util.Collections;
12+
import java.util.HashMap;
1413
import java.util.Map;
1514
import java.util.Objects;
15+
import java.util.Set;
1616
import java.util.stream.Collectors;
1717

1818
public class Maps {
@@ -49,22 +49,31 @@ public static <K, V> boolean deepEquals(Map<K, V> left, Map<K, V> right) {
4949
public static <K, V> Map<K, V> copyMapWithRemovedEntry(final Map<K, V> map, final K key) {
5050
Objects.requireNonNull(map);
5151
Objects.requireNonNull(key);
52-
assertImmutableMap(map, key, map.get(key));
52+
assert checkIsImmutableMap(map, key, map.get(key));
5353
return map.entrySet().stream().filter(k -> key.equals(k.getKey()) == false)
5454
.collect(Collectors.collectingAndThen(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue),
5555
Collections::<K, V>unmodifiableMap));
5656
}
5757

58-
private static <K, V> void assertImmutableMap(final Map<K, V> map, final K key, final V value) {
59-
if (Assertions.ENABLED) {
60-
boolean immutable;
61-
try {
62-
map.put(key, value);
63-
immutable = false;
64-
} catch (final UnsupportedOperationException e) {
65-
immutable = true;
66-
}
67-
assert immutable : "expected an immutable map but was [" + map.getClass() + "]";
58+
// map classes that are known to be immutable, used to speed up immutability check in #assertImmutableMap
59+
private static final Set<Class<?>> IMMUTABLE_MAP_CLASSES = org.elasticsearch.core.Set.of(
60+
Collections.emptyMap().getClass(),
61+
Collections.unmodifiableMap(new HashMap<>()).getClass(),
62+
org.elasticsearch.core.Map.of().getClass(),
63+
org.elasticsearch.core.Map.of("a", "b").getClass()
64+
);
65+
66+
private static <K, V> boolean checkIsImmutableMap(final Map<K, V> map, final K key, final V value) {
67+
// check in the known immutable classes map first, most of the time we don't need to actually do the put and throw which is slow to
68+
// the point of visibly slowing down internal cluster tests without this short-cut
69+
if (IMMUTABLE_MAP_CLASSES.contains(map.getClass())) {
70+
return true;
71+
}
72+
try {
73+
map.put(key, value);
74+
return false;
75+
} catch (final UnsupportedOperationException ignored) {
6876
}
77+
return true;
6978
}
7079
}

0 commit comments

Comments
 (0)