Skip to content

Commit 3f793b6

Browse files
Prevent unnecessary heap sort when buckets needs to be ordered by key in NumericTermsAggregation (opensearch-project#17252)
Signed-off-by: Rishabh Maurya <[email protected]>
1 parent bbdf73a commit 3f793b6

File tree

2 files changed

+15
-3
lines changed

2 files changed

+15
-3
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
4141
- Propagate the sourceIncludes and excludes fields from fetchSourceContext to FieldsVisitor. ([#17080](https://github.com/opensearch-project/OpenSearch/pull/17080))
4242
- [Star Tree] [Search] Resolving Date histogram with metric aggregation using star-tree ([#16674](https://github.com/opensearch-project/OpenSearch/pull/16674))
4343
- [Star Tree] [Search] Extensible design to support different query and field types ([#17137](https://github.com/opensearch-project/OpenSearch/pull/17137))
44+
- Improve performace of NumericTermAggregation by avoiding unnecessary sorting([#17252](https://github.com/opensearch-project/OpenSearch/pull/17252))
4445

4546
### Dependencies
4647
- Bump `com.google.cloud:google-cloud-core-http` from 2.23.0 to 2.47.0 ([#16504](https://github.com/opensearch-project/OpenSearch/pull/16504))

server/src/main/java/org/opensearch/search/aggregations/bucket/terms/NumericTermsAggregator.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
import java.io.IOException;
6565
import java.math.BigInteger;
6666
import java.util.Arrays;
67+
import java.util.Iterator;
6768
import java.util.List;
6869
import java.util.Map;
6970
import java.util.function.BiConsumer;
@@ -202,9 +203,19 @@ private InternalAggregation[] buildAggregations(long[] owningBucketOrds) throws
202203
// Get the top buckets
203204
B[] bucketsForOrd = buildBuckets(ordered.size());
204205
topBucketsPerOrd[ordIdx] = bucketsForOrd;
205-
for (int b = ordered.size() - 1; b >= 0; --b) {
206-
topBucketsPerOrd[ordIdx][b] = ordered.pop();
207-
otherDocCounts[ordIdx] -= topBucketsPerOrd[ordIdx][b].getDocCount();
206+
if (isKeyOrder(order)) {
207+
for (int b = ordered.size() - 1; b >= 0; --b) {
208+
topBucketsPerOrd[ordIdx][b] = ordered.pop();
209+
otherDocCounts[ordIdx] -= topBucketsPerOrd[ordIdx][b].getDocCount();
210+
}
211+
} else {
212+
// sorted buckets not needed as they will be sorted by key in buildResult() which is different from
213+
// order in priority queue ordered
214+
Iterator<B> itr = ordered.iterator();
215+
for (int b = ordered.size() - 1; b >= 0; --b) {
216+
topBucketsPerOrd[ordIdx][b] = itr.next();
217+
otherDocCounts[ordIdx] -= topBucketsPerOrd[ordIdx][b].getDocCount();
218+
}
208219
}
209220
}
210221

0 commit comments

Comments
 (0)