Skip to content

Commit 0e3a6dd

Browse files
Speed up iteration over ImmutableOpenMap on hot paths (#87600)
The adjusted spots can see very large maps of O(index_count) getting iterated. Moving them to the efficient `forEach` that iterates the action in a tight loop over an array instead of using the indirection of the iterator loop gives a measurable speedup in many-shards benchmark bootstrapping.
1 parent 336df7a commit 0e3a6dd

File tree

3 files changed

+16
-35
lines changed

3 files changed

+16
-35
lines changed

server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,7 +1845,7 @@ private static void ensureNoNameCollisions(
18451845
final ArrayList<String> duplicates = new ArrayList<>();
18461846
final Set<String> aliasDuplicatesWithIndices = new HashSet<>();
18471847
final Set<String> aliasDuplicatesWithDataStreams = new HashSet<>();
1848-
final Set<String> allDataStreams = dataStreamMetadata.dataStreams().keySet();
1848+
final var allDataStreams = dataStreamMetadata.dataStreams();
18491849
// Adding data stream aliases:
18501850
for (String dataStreamAlias : dataStreamMetadata.getDataStreamAliases().keySet()) {
18511851
if (indexAliases.contains(dataStreamAlias)) {
@@ -1854,23 +1854,23 @@ private static void ensureNoNameCollisions(
18541854
if (indicesMap.containsKey(dataStreamAlias)) {
18551855
aliasDuplicatesWithIndices.add(dataStreamAlias);
18561856
}
1857-
if (allDataStreams.contains(dataStreamAlias)) {
1857+
if (allDataStreams.containsKey(dataStreamAlias)) {
18581858
aliasDuplicatesWithDataStreams.add(dataStreamAlias);
18591859
}
18601860
}
18611861
for (String alias : indexAliases) {
1862-
if (allDataStreams.contains(alias)) {
1862+
if (allDataStreams.containsKey(alias)) {
18631863
aliasDuplicatesWithDataStreams.add(alias);
18641864
}
18651865
if (indicesMap.containsKey(alias)) {
18661866
aliasDuplicatesWithIndices.add(alias);
18671867
}
18681868
}
1869-
for (String ds : allDataStreams) {
1870-
if (indicesMap.containsKey(ds)) {
1871-
duplicates.add("data stream [" + ds + "] conflicts with index");
1869+
allDataStreams.forEach((key, value) -> {
1870+
if (indicesMap.containsKey(key)) {
1871+
duplicates.add("data stream [" + key + "] conflicts with index");
18721872
}
1873-
}
1873+
});
18741874
if (aliasDuplicatesWithIndices.isEmpty() == false) {
18751875
collectAliasDuplicates(indicesMap, aliasDuplicatesWithIndices, duplicates);
18761876
}

server/src/main/java/org/elasticsearch/common/collect/ImmutableOpenMap.java

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import com.carrotsearch.hppc.ObjectObjectHashMap;
1313
import com.carrotsearch.hppc.cursors.ObjectCursor;
1414
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
15+
import com.carrotsearch.hppc.procedures.ObjectObjectProcedure;
1516

1617
import java.util.AbstractCollection;
1718
import java.util.AbstractMap;
@@ -23,6 +24,7 @@
2324
import java.util.Set;
2425
import java.util.Spliterator;
2526
import java.util.Spliterators;
27+
import java.util.function.BiConsumer;
2628
import java.util.function.BiPredicate;
2729
import java.util.function.Consumer;
2830
import java.util.function.Predicate;
@@ -310,6 +312,11 @@ public int size() {
310312
};
311313
}
312314

315+
@Override
316+
public void forEach(BiConsumer<? super KType, ? super VType> action) {
317+
map.forEach((ObjectObjectProcedure<KType, VType>) action::accept);
318+
}
319+
313320
static <T> Iterator<T> iterator(ObjectCollection<T> collection) {
314321
final Iterator<ObjectCursor<T>> iterator = collection.iterator();
315322
return new Iterator<>() {
@@ -492,13 +499,6 @@ public int removeAll(Predicate<? super KType> predicate) {
492499
return mutableMap.removeAll(predicate::test);
493500
}
494501

495-
public void removeAllFromCollection(Collection<KType> collection) {
496-
maybeCloneMap();
497-
for (var k : collection) {
498-
mutableMap.remove(k);
499-
}
500-
}
501-
502502
public void clear() {
503503
maybeCloneMap();
504504
mutableMap.clear();
@@ -529,29 +529,10 @@ public boolean indexExists(int index) {
529529
return mutableMap.indexExists(index);
530530
}
531531

532-
public VType indexGet(int index) {
533-
maybeCloneMap();
534-
return mutableMap.indexGet(index);
535-
}
536-
537-
public VType indexReplace(int index, VType newValue) {
538-
maybeCloneMap();
539-
return mutableMap.indexReplace(index, newValue);
540-
}
541-
542-
public void indexInsert(int index, KType key, VType value) {
543-
maybeCloneMap();
544-
mutableMap.indexInsert(index, key, value);
545-
}
546-
547532
public void release() {
548533
maybeCloneMap();
549534
mutableMap.release();
550535
}
551536

552-
public String visualizeKeyDistribution(int characters) {
553-
maybeCloneMap();
554-
return mutableMap.visualizeKeyDistribution(characters);
555-
}
556537
}
557538
}

server/src/main/java/org/elasticsearch/gateway/PersistedClusterStateService.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -907,10 +907,10 @@ private WriterStats updateMetadata(Metadata previouslyWrittenMetadata, Metadata
907907
}
908908

909909
final Map<String, Long> indexMetadataVersionByUUID = Maps.newMapWithExpectedSize(previouslyWrittenMetadata.indices().size());
910-
for (IndexMetadata indexMetadata : previouslyWrittenMetadata.indices().values()) {
910+
previouslyWrittenMetadata.indices().forEach((name, indexMetadata) -> {
911911
final Long previousValue = indexMetadataVersionByUUID.putIfAbsent(indexMetadata.getIndexUUID(), indexMetadata.getVersion());
912912
assert previousValue == null : indexMetadata.getIndexUUID() + " already mapped to " + previousValue;
913-
}
913+
});
914914

915915
int numIndicesAdded = 0;
916916
int numIndicesUpdated = 0;

0 commit comments

Comments
 (0)