Skip to content

Commit 16a15af

Browse files
committed
Attempt to make some operations cheaper
1 parent 8a741bf commit 16a15af

File tree

2 files changed

+15
-6
lines changed
  • x-pack/plugin
    • esql-core/src/main/java/org/elasticsearch/xpack/esql/core/tree
    • esql/src/main/java/org/elasticsearch/xpack/esql/plan

2 files changed

+15
-6
lines changed

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/tree/Node.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,11 @@ public List<T> children() {
6767
@SuppressWarnings("unchecked")
6868
public void forEachDown(Consumer<? super T> action) {
6969
action.accept((T) this);
70-
children().forEach(c -> c.forEachDown(action));
70+
// please do not refactor it to a for-each loop
71+
// to avoid allocating iterator that performs concurrent modification checks
72+
for (int c = 0; c < children.size(); c++) {
73+
children.get(c).forEachDown(action);
74+
}
7175
}
7276

7377
@SuppressWarnings("unchecked")
@@ -81,7 +85,11 @@ public <E extends T> void forEachDown(Class<E> typeToken, Consumer<? super E> ac
8185

8286
@SuppressWarnings("unchecked")
8387
public void forEachUp(Consumer<? super T> action) {
84-
children().forEach(c -> c.forEachUp(action));
88+
// please do not refactor it to a for-each loop
89+
// to avoid allocating iterator that performs concurrent modification checks
90+
for (int c = 0; c < children.size(); c++) {
91+
children.get(c).forEachUp(action);
92+
}
8593
action.accept((T) this);
8694
}
8795

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/QueryPlan.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,13 @@ private static Object doTransformExpression(Object arg, Function<Expression, ? e
140140
// preserving the type information is hacky and weird (a lot of context needs to be passed around and the lambda itself
141141
// has no type info so it's difficult to have automatic checking without having base classes).
142142

143-
if (arg instanceof Collection<?> c) {
143+
if (arg instanceof List<?> c) {
144144
List<Object> transformed = null;
145145
boolean hasChanged = false;
146-
int i = 0;
147-
for (Object e : c) {
146+
// please do not refactor it to a for-each loop
147+
// to avoid allocating iterator that performs concurrent modification checks
148+
for (int i = 0; i < c.size(); i++) {
149+
var e = c.get(i);
148150
Object next = doTransformExpression(e, traversal);
149151
if (e.equals(next) == false) {
150152
if (hasChanged == false) {
@@ -153,7 +155,6 @@ private static Object doTransformExpression(Object arg, Function<Expression, ? e
153155
}
154156
transformed.set(i, next);
155157
}
156-
i++;
157158
}
158159

159160
return hasChanged ? transformed : arg;

0 commit comments

Comments
 (0)