Skip to content

Commit d7fb0e2

Browse files
authored
Speedup equals (#126394) (#128949)
(cherry picked from commit e95397c)
1 parent 4e0aaa7 commit d7fb0e2

File tree

11 files changed

+54
-70
lines changed

11 files changed

+54
-70
lines changed

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,18 +112,15 @@ protected Expression canonicalize() {
112112
}
113113

114114
@Override
115+
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
115116
public int hashCode() {
116117
return Objects.hash(super.hashCode(), nullability);
117118
}
118119

119120
@Override
120-
public boolean equals(Object obj) {
121-
if (super.equals(obj)) {
122-
Attribute other = (Attribute) obj;
123-
return Objects.equals(nullability, other.nullability);
124-
}
125-
126-
return false;
121+
protected boolean innerEquals(Object o) {
122+
var other = (Attribute) o;
123+
return super.innerEquals(other) && Objects.equals(nullability, other.nullability);
127124
}
128125

129126
@Override

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/EmptyAttribute.java

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,20 +60,13 @@ protected NodeInfo<? extends Expression> info() {
6060
}
6161

6262
@Override
63+
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
6364
public int hashCode() {
6465
return EmptyAttribute.class.hashCode();
6566
}
6667

6768
@Override
68-
public boolean equals(Object obj) {
69-
if (this == obj) {
70-
return true;
71-
}
72-
73-
if (obj == null || getClass() != obj.getClass()) {
74-
return false;
75-
}
76-
69+
protected boolean innerEquals(Object o) {
7770
return true;
7871
}
7972
}

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/EntryExpression.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ public boolean equals(Object obj) {
9999
if (obj == null || getClass() != obj.getClass()) {
100100
return false;
101101
}
102-
103102
EntryExpression other = (EntryExpression) obj;
104103
return Objects.equals(key, other.key) && Objects.equals(value, other.value);
105104
}

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,15 +223,15 @@ public Attribute withDataType(DataType type) {
223223
}
224224

225225
@Override
226+
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
226227
public int hashCode() {
227228
return Objects.hash(super.hashCode(), parentName, field);
228229
}
229230

230231
@Override
231-
public boolean equals(Object obj) {
232-
return super.equals(obj)
233-
&& Objects.equals(parentName, ((FieldAttribute) obj).parentName)
234-
&& Objects.equals(field, ((FieldAttribute) obj).field);
232+
protected boolean innerEquals(Object o) {
233+
var other = (FieldAttribute) o;
234+
return super.innerEquals(other) && Objects.equals(parentName, other.parentName) && Objects.equals(field, other.field);
235235
}
236236

237237
@Override

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/MetadataAttribute.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -173,16 +173,14 @@ public static boolean isSupported(String name) {
173173
}
174174

175175
@Override
176-
public boolean equals(Object obj) {
177-
if (false == super.equals(obj)) {
178-
return false;
179-
}
180-
MetadataAttribute other = (MetadataAttribute) obj;
181-
return searchable == other.searchable;
176+
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
177+
public int hashCode() {
178+
return Objects.hash(super.hashCode(), searchable);
182179
}
183180

184181
@Override
185-
public int hashCode() {
186-
return Objects.hash(super.hashCode(), searchable);
182+
protected boolean innerEquals(Object o) {
183+
var other = (MetadataAttribute) o;
184+
return super.innerEquals(other) && searchable == other.searchable;
187185
}
188186
}

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/NamedExpression.java

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,17 +61,26 @@ public int hashCode() {
6161
return Objects.hash(super.hashCode(), name, synthetic);
6262
}
6363

64+
/**
65+
* Polymorphic equality is a pain and are likely slower than a regular ones.
66+
* This equals shortcuts `this == o` and type checks (important when we expect only a few non-equal objects).
67+
* Here equals is final to ensure we are not duplicating those checks.
68+
* For actual equality check override `innerEquals` instead.
69+
*/
6470
@Override
65-
public boolean equals(Object obj) {
66-
if (this == obj) {
71+
public final boolean equals(Object o) {
72+
if (this == o) {
6773
return true;
6874
}
69-
if (obj == null || getClass() != obj.getClass()) {
75+
if (o == null || getClass() != o.getClass()) {
7076
return false;
7177
}
78+
return innerEquals(o);
79+
}
7280

73-
NamedExpression other = (NamedExpression) obj;
74-
return Objects.equals(synthetic, other.synthetic)
81+
protected boolean innerEquals(Object o) {
82+
var other = (NamedExpression) o;
83+
return synthetic == other.synthetic
7584
/*
7685
* It is important that the line below be `name`
7786
* and not `name()` because subclasses might override

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/TypedAttribute.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,14 @@ public DataType dataType() {
3434
}
3535

3636
@Override
37+
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
3738
public int hashCode() {
3839
return Objects.hash(super.hashCode(), dataType);
3940
}
4041

4142
@Override
42-
public boolean equals(Object obj) {
43-
return super.equals(obj) && Objects.equals(dataType, ((TypedAttribute) obj).dataType);
43+
protected boolean innerEquals(Object o) {
44+
var other = (TypedAttribute) o;
45+
return super.innerEquals(other) && dataType == other.dataType;
4446
}
4547
}

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedAttribute.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@
2121

2222
// unfortunately we can't use UnresolvedNamedExpression
2323
public class UnresolvedAttribute extends Attribute implements Unresolvable {
24-
private final String unresolvedMsg;
2524
private final boolean customMessage;
25+
private final String unresolvedMsg;
2626
private final Object resolutionMetadata;
2727

2828
public UnresolvedAttribute(Source source, String name) {
@@ -119,16 +119,16 @@ public static String errorMessage(String name, List<String> potentialMatches) {
119119
}
120120

121121
@Override
122+
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
122123
public int hashCode() {
123124
return Objects.hash(super.hashCode(), resolutionMetadata, unresolvedMsg);
124125
}
125126

126127
@Override
127-
public boolean equals(Object obj) {
128-
if (super.equals(obj)) {
129-
UnresolvedAttribute ua = (UnresolvedAttribute) obj;
130-
return Objects.equals(resolutionMetadata, ua.resolutionMetadata) && Objects.equals(unresolvedMsg, ua.unresolvedMsg);
131-
}
132-
return false;
128+
protected boolean innerEquals(Object o) {
129+
var other = (UnresolvedAttribute) o;
130+
return super.innerEquals(other)
131+
&& Objects.equals(resolutionMetadata, other.resolutionMetadata)
132+
&& Objects.equals(unresolvedMsg, other.unresolvedMsg);
133133
}
134134
}

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedStar.java

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,25 +57,15 @@ public UnresolvedAttribute qualifier() {
5757
}
5858

5959
@Override
60+
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
6061
public int hashCode() {
6162
return Objects.hash(qualifier);
6263
}
6364

6465
@Override
65-
public boolean equals(Object obj) {
66-
/*
67-
* Intentionally not calling the superclass
68-
* equals because it uses id which we always
69-
* mutate when we make a clone. So we need
70-
* to ignore it in equals for the transform
71-
* tests to pass.
72-
*/
73-
if (obj == null || obj.getClass() != getClass()) {
74-
return false;
75-
}
76-
77-
UnresolvedStar other = (UnresolvedStar) obj;
78-
return Objects.equals(qualifier, other.qualifier);
66+
protected boolean innerEquals(Object o) {
67+
var other = (UnresolvedStar) o;
68+
return super.innerEquals(other) && Objects.equals(qualifier, other.qualifier);
7969
}
8070

8171
private String message() {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/UnresolvedNamePattern.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,17 +98,15 @@ public Nullability nullable() {
9898
}
9999

100100
@Override
101+
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
101102
public int hashCode() {
102103
return Objects.hash(super.hashCode(), pattern);
103104
}
104105

105106
@Override
106-
public boolean equals(Object obj) {
107-
if (super.equals(obj)) {
108-
UnresolvedNamePattern ua = (UnresolvedNamePattern) obj;
109-
return Objects.equals(pattern, ua.pattern);
110-
}
111-
return false;
107+
protected boolean innerEquals(Object o) {
108+
var other = (UnresolvedNamePattern) o;
109+
return super.innerEquals(other) && Objects.equals(pattern, other.pattern);
112110
}
113111

114112
@Override

0 commit comments

Comments
 (0)