Skip to content

Commit c27faa0

Browse files
committed
Removed lastValue field
1 parent 8d101fb commit c27faa0

File tree

2 files changed

+17
-57
lines changed

2 files changed

+17
-57
lines changed

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/LongTopNBlockHash.java

Lines changed: 6 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,7 @@ public final class LongTopNBlockHash extends BlockHash {
3838
private final boolean nullsFirst;
3939
private final int limit;
4040
private final LongHash hash;
41-
private LongTopNUniqueSort topValues;
42-
private final LongHash seenUniqueValues;
43-
/**
44-
* Helper field to keep track of the last top value, and avoid expensive accesses.
45-
*/
46-
private long lastTopValue;
41+
private final LongTopNUniqueSort topValues;
4742

4843
/**
4944
* Have we seen any {@code null} values?
@@ -63,8 +58,6 @@ public LongTopNBlockHash(int channel, boolean asc, boolean nullsFirst, int limit
6358
this.limit = limit;
6459
this.hash = new LongHash(1, blockFactory.bigArrays());
6560
this.topValues = new LongTopNUniqueSort(blockFactory.bigArrays(), asc ? SortOrder.ASC : SortOrder.DESC, limit);
66-
this.seenUniqueValues = new LongHash(1, blockFactory.bigArrays());
67-
this.lastTopValue = asc ? Long.MAX_VALUE : Long.MIN_VALUE;
6861

6962
assert limit > 0 : "LongTopNBlockHash requires a limit greater than 0";
7063
}
@@ -105,13 +98,6 @@ private boolean acceptNull() {
10598
if (nullsFirst) {
10699
hasNull = true;
107100
migrateToSmallTop();
108-
if (topValues.getCount() == limit - 1) {
109-
if (topValues.getCount() == 0) {
110-
lastTopValue = asc ? Long.MAX_VALUE : Long.MIN_VALUE;
111-
} else {
112-
lastTopValue = topValues.getWorstValue();
113-
}
114-
}
115101
return true;
116102
}
117103

@@ -127,35 +113,13 @@ private boolean acceptNull() {
127113
* Tries to add the value to the top values, and returns true if it was successful.
128114
*/
129115
private boolean acceptValue(long value) {
130-
// Ignore values that are worse than the current worst value
131-
if (isAcceptable(value) == false) {
116+
if (topValues.collect(value) == false) {
132117
return false;
133118
}
134119

135-
// O(1) operation to check if the value is already in the hash, and avoid adding it again
136-
if (seenUniqueValues.add(value) < 0) {
137-
return true;
138-
}
139-
140-
topValues.collect(value);
141-
142-
if (topValues.getCount() == limit) {
143-
lastTopValue = topValues.getWorstValue();
144-
} else if (topValues.getCount() == 1 || isBetterThan(lastTopValue, value)) {
145-
lastTopValue = value;
146-
}
147-
148120
// Full top and null, there's an extra value/null we must remove
149-
if (topValues.getCount() == limit && hasNull) {
150-
if (nullsFirst) {
151-
if (topValues.getCount() == 1) {
152-
lastTopValue = asc ? Long.MAX_VALUE : Long.MIN_VALUE;
153-
} else {
154-
lastTopValue = topValues.getWorstValue();
155-
}
156-
} else {
157-
hasNull = false;
158-
}
121+
if (topValues.getCount() == limit && hasNull && nullsFirst == false) {
122+
hasNull = false;
159123
}
160124

161125
return true;
@@ -171,10 +135,6 @@ private void migrateToSmallTop() {
171135
topValues.reduceLimitByOne();
172136
}
173137

174-
private boolean isBetterThan(long value, long other) {
175-
return asc ? value < other : value > other;
176-
}
177-
178138
/**
179139
* Returns true if the value is in, or can be added to the top; false otherwise.
180140
*/
@@ -189,7 +149,7 @@ private boolean isAcceptable(long value) {
189149
* </p>
190150
*/
191151
private boolean isInTop(long value) {
192-
return asc ? value <= lastTopValue : value >= lastTopValue;
152+
return asc ? value <= topValues.getWorstValue() : value >= topValues.getWorstValue();
193153
}
194154

195155
/**
@@ -349,7 +309,7 @@ public BitArray seenGroupIds(BigArrays bigArrays) {
349309

350310
@Override
351311
public void close() {
352-
Releasables.close(hash, topValues, seenUniqueValues);
312+
Releasables.close(hash, topValues);
353313
}
354314

355315
@Override

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/sort/LongTopNUniqueSort.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
*/
2323
public class LongTopNUniqueSort implements Releasable {
2424

25-
private final BigArrays bigArrays;
2625
private final SortOrder order;
2726
private int limit;
2827

@@ -32,44 +31,43 @@ public class LongTopNUniqueSort implements Releasable {
3231
private int count;
3332

3433
public LongTopNUniqueSort(BigArrays bigArrays, SortOrder order, int limit) {
35-
this.bigArrays = bigArrays;
3634
this.order = order;
3735
this.limit = limit;
36+
this.count = 0;
3837
this.values = bigArrays.newLongArray(limit, false);
3938
this.searcher = new LongBinarySearcher(values, order);
40-
this.count = 0;
4139
}
4240

43-
public void collect(long value) {
41+
public boolean collect(long value) {
4442
if (limit == 0) {
45-
return;
43+
return false;
4644
}
4745

4846
// Short-circuit if the value is worse than the worst value on the top.
4947
// This avoids a O(log(n)) check in the binary search
50-
if (count == limit && betterThan(values.get(count - 1), value)) {
51-
return;
48+
if (count == limit && betterThan(getWorstValue(), value)) {
49+
return false;
5250
}
5351

5452
if (count == 0) {
5553
values.set(0, value);
5654
count++;
57-
return;
55+
return true;
5856
}
5957

6058
int insertionIndex = this.searcher.search(0, count - 1, value);
6159

6260
if (insertionIndex == count - 1) {
63-
if (betterThan(values.get(count - 1), value)) {
61+
if (betterThan(getWorstValue(), value)) {
6462
values.set(count, value);
6563
count++;
66-
return;
64+
return true;
6765
}
6866
}
6967

7068
if (values.get(insertionIndex) == value) {
7169
// Only unique values are stored here
72-
return;
70+
return true;
7371
}
7472

7573
// The searcher returns the upper bound, so we move right the elements from there
@@ -79,6 +77,8 @@ public void collect(long value) {
7977

8078
values.set(insertionIndex, value);
8179
count = Math.min(count + 1, limit);
80+
81+
return true;
8282
}
8383

8484
/**

0 commit comments

Comments
 (0)