From 5343de76aab63e56f6a7b877728e94b4c98d07a3 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 12 Feb 2025 03:35:05 +0100 Subject: [PATCH] Simplify comparators in InternalOrder Random find :) We can build slightly more compact (and likely also fasster) iterators while using less code for these. Also, no need to create method references as a way of casting. --- .../search/aggregations/InternalOrder.java | 55 +++++++------------ 1 file changed, 20 insertions(+), 35 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/InternalOrder.java b/server/src/main/java/org/elasticsearch/search/aggregations/InternalOrder.java index afeeaa9bd6752..ead1374fb9b4f 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/InternalOrder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/InternalOrder.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Objects; @@ -188,33 +189,22 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public Comparator> partiallyBuiltBucketComparator(Aggregator aggregator) { - List>> comparators = new ArrayList<>(orderElements.size()); - for (BucketOrder order : orderElements) { - comparators.add(order.partiallyBuiltBucketComparator(aggregator)); + Iterator iterator = orderElements.iterator(); + Comparator> comparator = iterator.next().partiallyBuiltBucketComparator(aggregator); + while (iterator.hasNext()) { + comparator = comparator.thenComparing(iterator.next().partiallyBuiltBucketComparator(aggregator)); } - return (lhs, rhs) -> { - for (Comparator> c : comparators) { - int result = c.compare(lhs, rhs); - if (result != 0) { - return result; - } - } - return 0; - }; + return comparator; } @Override public Comparator comparator() { - List> comparators = orderElements.stream().map(BucketOrder::comparator).toList(); - return (lhs, rhs) -> { - for (Comparator c : comparators) { - int result = c.compare(lhs, rhs); - if (result != 0) { - return result; - } - } - return 0; - }; + Iterator iterator = orderElements.iterator(); + Comparator comparator = iterator.next().comparator(); + while (iterator.hasNext()) { + comparator = comparator.thenComparing(iterator.next().comparator()); + } + return comparator; } @Override @@ -222,18 +212,12 @@ Comparator, AggregationReduceContext, B> reduce, AggregationReduceContext reduceContext ) { - List>> comparators = orderElements.stream() - .map(b -> b.delayedBucketComparator(reduce, reduceContext)) - .toList(); - return (lhs, rhs) -> { - for (Comparator> c : comparators) { - int result = c.compare(lhs, rhs); - if (result != 0) { - return result; - } - } - return 0; - }; + Iterator iterator = orderElements.iterator(); + Comparator> comparator = iterator.next().delayedBucketComparator(reduce, reduceContext); + while (iterator.hasNext()) { + comparator = comparator.thenComparing(iterator.next().delayedBucketComparator(reduce, reduceContext)); + } + return comparator; } @Override @@ -285,12 +269,13 @@ public Comparator comparator() { return comparator; } + @SuppressWarnings({ "rawtypes", "unchecked" }) @Override Comparator> delayedBucketComparator( BiFunction, AggregationReduceContext, B> reduce, AggregationReduceContext reduceContext ) { - return delayedBucketCompator::compare; + return (Comparator) delayedBucketCompator; } @Override