Skip to content

Commit 049d2d0

Browse files
Small cleanups and performance improvements in o.e.c.u.concurrent.AtomicArray
Some small obvious cleanups in AtomicArray, drying up a volatile hack, removing some redundant and potentially buggy volatile access and simplifying (and for large arrays also speeding up) the API for copying the contents to an array.
1 parent 1148861 commit 049d2d0

File tree

7 files changed

+26
-28
lines changed

7 files changed

+26
-28
lines changed

server/src/main/java/org/elasticsearch/action/bulk/BulkOperation.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,7 @@ private void redirectFailuresOrCompleteBulkOperation() {
323323
}
324324

325325
private void completeBulkOperation() {
326-
listener.onResponse(
327-
new BulkResponse(responses.toArray(new BulkItemResponse[responses.length()]), buildTookInMillis(startTimeNanos))
328-
);
326+
listener.onResponse(new BulkResponse(responses.toArray(BulkItemResponse.class), buildTookInMillis(startTimeNanos)));
329327
// Allow memory for bulk shard request items to be reclaimed before all items have been completed
330328
bulkRequest = null;
331329
}

server/src/main/java/org/elasticsearch/action/bulk/TransportSimulateBulkAction.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ protected void createMissingIndicesAndIndexData(
9999
)
100100
);
101101
}
102-
listener.onResponse(new BulkResponse(responses.toArray(new BulkItemResponse[responses.length()]), buildTookInMillis(startTime)));
102+
listener.onResponse(new BulkResponse(responses.toArray(BulkItemResponse.class), buildTookInMillis(startTime)));
103103
}
104104

105105
/*

server/src/main/java/org/elasticsearch/action/get/TransportMultiGetAction.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ protected void doExecute(Task task, final MultiGetRequest request, final ActionL
9999

100100
if (shardRequests.isEmpty()) {
101101
// only failures..
102-
listener.onResponse(new MultiGetResponse(responses.toArray(new MultiGetItemResponse[responses.length()])));
102+
listener.onResponse(new MultiGetResponse(responses.toArray(MultiGetItemResponse.class)));
103103
}
104104

105105
executeShardAction(listener, responses, shardRequests);
@@ -138,7 +138,7 @@ public void onFailure(Exception e) {
138138
}
139139

140140
private void finishHim() {
141-
delegate.onResponse(new MultiGetResponse(responses.toArray(new MultiGetItemResponse[responses.length()])));
141+
delegate.onResponse(new MultiGetResponse(responses.toArray(MultiGetItemResponse.class)));
142142
}
143143
});
144144
}

server/src/main/java/org/elasticsearch/action/search/TransportMultiSearchAction.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ private void handleResponse(final int responseSlot, final MultiSearchResponse.It
189189
private void finish() {
190190
ActionListener.respondAndRelease(
191191
listener,
192-
new MultiSearchResponse(responses.toArray(new MultiSearchResponse.Item[responses.length()]), buildTookInMillis())
192+
new MultiSearchResponse(responses.toArray(MultiSearchResponse.Item.class), buildTookInMillis())
193193
);
194194
}
195195

server/src/main/java/org/elasticsearch/action/termvectors/TransportMultiTermVectorsAction.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ protected void doExecute(Task task, final MultiTermVectorsRequest request, final
101101

102102
if (shardRequests.size() == 0) {
103103
// only failures..
104-
listener.onResponse(new MultiTermVectorsResponse(responses.toArray(new MultiTermVectorsItemResponse[responses.length()])));
104+
listener.onResponse(new MultiTermVectorsResponse(responses.toArray(MultiTermVectorsItemResponse.class)));
105105
}
106106

107107
executeShardAction(listener, responses, shardRequests);
@@ -148,9 +148,7 @@ public void onFailure(Exception e) {
148148
}
149149

150150
private void finishHim() {
151-
listener.onResponse(
152-
new MultiTermVectorsResponse(responses.toArray(new MultiTermVectorsItemResponse[responses.length()]))
153-
);
151+
listener.onResponse(new MultiTermVectorsResponse(responses.toArray(MultiTermVectorsItemResponse.class)));
154152
}
155153
});
156154
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ public void onFailure(Exception e) {
638638

639639
private void processIfFinished() {
640640
if (countDown.countDown()) {
641-
onResponse.accept(new IndexResult(index, results.toArray(new ShardResult[results.length()])));
641+
onResponse.accept(new IndexResult(index, results.toArray(ShardResult.class)));
642642
}
643643
}
644644
}));
@@ -768,7 +768,7 @@ public void onFailure(Exception e) {
768768

769769
private void processIfFinished() {
770770
if (countDown.countDown()) {
771-
AddBlockResult result = new AddBlockResult(index, results.toArray(new AddBlockShardResult[results.length()]));
771+
AddBlockResult result = new AddBlockResult(index, results.toArray(AddBlockShardResult.class));
772772
logger.debug("result of applying block to index {}: {}", index, result);
773773
onResponse.accept(result);
774774
}

server/src/main/java/org/elasticsearch/common/util/concurrent/AtomicArray.java

+17-15
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@
88

99
package org.elasticsearch.common.util.concurrent;
1010

11-
import org.elasticsearch.ElasticsearchGenerationException;
12-
11+
import java.lang.reflect.Array;
1312
import java.util.ArrayList;
1413
import java.util.Collections;
1514
import java.util.List;
@@ -59,15 +58,17 @@ public int nonNullLength() {
5958
*/
6059
public void set(int i, E value) {
6160
array.set(i, value);
62-
if (nonNullList != null) { // read first, lighter, and most times it will be null...
63-
nonNullList = null;
64-
}
61+
resetCachedList();
6562
}
6663

6764
public final void setOnce(int i, E value) {
6865
if (array.compareAndSet(i, null, value) == false) {
6966
throw new IllegalStateException("index [" + i + "] has already been set");
7067
}
68+
resetCachedList();
69+
}
70+
71+
private void resetCachedList() {
7172
if (nonNullList != null) { // read first, lighter, and most times it will be null...
7273
nonNullList = null;
7374
}
@@ -87,9 +88,10 @@ public E get(int i) {
8788
* Returns the it as a non null list.
8889
*/
8990
public List<E> asList() {
90-
if (nonNullList == null) {
91-
if (array == null || array.length() == 0) {
92-
nonNullList = Collections.emptyList();
91+
var res = nonNullList;
92+
if (res == null) {
93+
if (array.length() == 0) {
94+
res = Collections.emptyList();
9395
} else {
9496
List<E> list = new ArrayList<>(array.length());
9597
for (int i = 0; i < array.length(); i++) {
@@ -98,20 +100,20 @@ public List<E> asList() {
98100
list.add(e);
99101
}
100102
}
101-
nonNullList = list;
103+
res = list;
102104
}
105+
nonNullList = res;
103106
}
104-
return nonNullList;
107+
return res;
105108
}
106109

107110
/**
108111
* Copies the content of the underlying atomic array to a normal one.
109112
*/
110-
public E[] toArray(E[] a) {
111-
if (a.length != array.length()) {
112-
throw new ElasticsearchGenerationException("AtomicArrays can only be copied to arrays of the same size");
113-
}
114-
for (int i = 0; i < array.length(); i++) {
113+
public E[] toArray(Class<E> clazz) {
114+
@SuppressWarnings("unchecked")
115+
E[] a = (E[]) Array.newInstance(clazz, array.length());
116+
for (int i = 0; i < a.length; i++) {
115117
a[i] = array.get(i);
116118
}
117119
return a;

0 commit comments

Comments
 (0)