Skip to content

Commit 592bf66

Browse files
authored
Avoid needless map copy during disk usage simulation (elastic#91654)
1 parent e053f21 commit 592bf66

File tree

4 files changed

+157
-4
lines changed

4 files changed

+157
-4
lines changed

server/src/main/java/org/elasticsearch/cluster/ClusterInfo.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ public ClusterInfo(
7676
Map<NodeAndPath, ReservedSpace> reservedSpace
7777
) {
7878
this.leastAvailableSpaceUsage = Map.copyOf(leastAvailableSpaceUsage);
79+
this.mostAvailableSpaceUsage = Map.copyOf(mostAvailableSpaceUsage);
7980
this.shardSizes = Map.copyOf(shardSizes);
8081
this.shardDataSetSizes = Map.copyOf(shardDataSetSizes);
81-
this.mostAvailableSpaceUsage = Map.copyOf(mostAvailableSpaceUsage);
8282
this.dataPath = Map.copyOf(dataPath);
8383
this.reservedSpace = Map.copyOf(reservedSpace);
8484
}

server/src/main/java/org/elasticsearch/cluster/ClusterInfoSimulator.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package org.elasticsearch.cluster;
1010

1111
import org.elasticsearch.cluster.routing.ShardRouting;
12+
import org.elasticsearch.common.util.CopyOnFirstWriteMap;
1213
import org.elasticsearch.index.shard.ShardId;
1314

1415
import java.util.HashMap;
@@ -19,14 +20,14 @@ public class ClusterInfoSimulator {
1920

2021
private final Map<String, DiskUsage> leastAvailableSpaceUsage;
2122
private final Map<String, DiskUsage> mostAvailableSpaceUsage;
22-
private final Map<String, Long> shardSizes;
23+
private final CopyOnFirstWriteMap<String, Long> shardSizes;
2324
private final Map<ShardId, Long> shardDataSetSizes;
2425
private final Map<ClusterInfo.NodeAndShard, String> dataPath;
2526

2627
public ClusterInfoSimulator(ClusterInfo clusterInfo) {
2728
this.leastAvailableSpaceUsage = new HashMap<>(clusterInfo.getNodeLeastAvailableDiskUsages());
2829
this.mostAvailableSpaceUsage = new HashMap<>(clusterInfo.getNodeMostAvailableDiskUsages());
29-
this.shardSizes = new HashMap<>(clusterInfo.shardSizes);
30+
this.shardSizes = new CopyOnFirstWriteMap<>(clusterInfo.shardSizes);
3031
this.shardDataSetSizes = Map.copyOf(clusterInfo.shardDataSetSizes);
3132
this.dataPath = Map.copyOf(clusterInfo.dataPath);
3233
}
@@ -101,6 +102,13 @@ private static long withinRange(long min, long max, long value) {
101102
}
102103

103104
public ClusterInfo getClusterInfo() {
104-
return new ClusterInfo(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, shardDataSetSizes, dataPath, Map.of());
105+
return new ClusterInfo(
106+
leastAvailableSpaceUsage,
107+
mostAvailableSpaceUsage,
108+
shardSizes.toImmutableMap(),
109+
shardDataSetSizes,
110+
dataPath,
111+
Map.of()
112+
);
105113
}
106114
}
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
package org.elasticsearch.common.util;
10+
11+
import java.util.Collection;
12+
import java.util.Collections;
13+
import java.util.HashMap;
14+
import java.util.Map;
15+
import java.util.Set;
16+
17+
/**
18+
* This map is designed to be constructed from an immutable map and be copied only if a (rare) mutation operation occurs.
19+
* It should be converted back to an immutable map using `org.elasticsearch.common.util.LazyCopyOnWriteMap#toImmutableMap()`.
20+
*/
21+
public class CopyOnFirstWriteMap<K, V> implements Map<K, V> {
22+
23+
private Map<K, V> source;
24+
private boolean wasCopied = false;
25+
26+
public CopyOnFirstWriteMap(Map<K, V> source) {
27+
this.source = source;
28+
}
29+
30+
private Map<K, V> getForUpdate() {
31+
if (wasCopied == false) {
32+
source = new HashMap<>(source);
33+
wasCopied = true;
34+
}
35+
return source;
36+
}
37+
38+
private Map<K, V> getForRead() {
39+
return source;
40+
}
41+
42+
public Map<K, V> toImmutableMap() {
43+
return Map.copyOf(getForRead());
44+
}
45+
46+
@Override
47+
public int size() {
48+
return getForRead().size();
49+
}
50+
51+
@Override
52+
public boolean isEmpty() {
53+
return getForRead().isEmpty();
54+
}
55+
56+
@Override
57+
public boolean containsKey(Object key) {
58+
return getForRead().containsKey(key);
59+
}
60+
61+
@Override
62+
public boolean containsValue(Object value) {
63+
return getForRead().containsValue(value);
64+
}
65+
66+
@Override
67+
public V get(Object key) {
68+
return getForRead().get(key);
69+
}
70+
71+
@Override
72+
public V put(K key, V value) {
73+
return getForUpdate().put(key, value);
74+
}
75+
76+
@Override
77+
public V remove(Object key) {
78+
return getForUpdate().remove(key);
79+
}
80+
81+
@Override
82+
public void putAll(Map<? extends K, ? extends V> m) {
83+
getForUpdate().putAll(m);
84+
}
85+
86+
@Override
87+
public void clear() {
88+
getForUpdate().clear();
89+
}
90+
91+
@Override
92+
public Set<K> keySet() {
93+
return Collections.unmodifiableSet(getForRead().keySet());
94+
}
95+
96+
@Override
97+
public Collection<V> values() {
98+
return Collections.unmodifiableCollection(getForRead().values());
99+
}
100+
101+
@Override
102+
public Set<Entry<K, V>> entrySet() {
103+
return Collections.unmodifiableSet(getForRead().entrySet());
104+
}
105+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
package org.elasticsearch.common.util;
10+
11+
import org.elasticsearch.test.ESTestCase;
12+
13+
import java.util.Map;
14+
15+
import static org.hamcrest.Matchers.equalTo;
16+
import static org.hamcrest.Matchers.not;
17+
import static org.hamcrest.Matchers.sameInstance;
18+
19+
public class CopyOnFirstWriteMapTests extends ESTestCase {
20+
21+
public void testShouldNotCopyIfThereWereNoUpdates() {
22+
var source = Map.of("key", "value");
23+
var copyOnFirstWrite = new CopyOnFirstWriteMap<>(source);
24+
source.get("key");
25+
var copy = copyOnFirstWrite.toImmutableMap();
26+
27+
assertThat(copy, sameInstance(source));
28+
assertThat(copy, equalTo(source));
29+
}
30+
31+
public void testShouldBeUpdatable() {
32+
var source = Map.of("key", "value");
33+
var copyOnFirstWrite = new CopyOnFirstWriteMap<>(source);
34+
copyOnFirstWrite.put("key", "new_value");
35+
var copy = copyOnFirstWrite.toImmutableMap();
36+
37+
assertThat(copy, not(sameInstance(source)));
38+
assertThat(copy, equalTo(Map.of("key", "new_value")));
39+
}
40+
}

0 commit comments

Comments
 (0)