Skip to content

Commit aba4ca9

Browse files
committed
perf: optimize bucket counting and tracking in LongKeyedBucketOrds by maintaining per-bucket counts and max ordinal metadata.
1 parent 27e5d58 commit aba4ca9

2 files changed

Lines changed: 37 additions & 15 deletions

File tree

benchmarks/src/main/java/org/opensearch/benchmark/search/aggregations/bucket/terms/LongKeyedBucketOrdsBenchmark.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,4 +182,17 @@ public void multiBucket(Blackhole bh) {
182182
bh.consume(ords);
183183
}
184184
}
185+
186+
@Benchmark
187+
public void benchmarkBucketsInOrdAndMaxOwning(Blackhole bh) {
188+
try (LongKeyedBucketOrds ords = LongKeyedBucketOrds.build(bigArrays, CardinalityUpperBound.MANY)) {
189+
for (long i = 0; i < 50_000; i++) {
190+
ords.add(i % 100, i % DISTINCT_VALUES);
191+
}
192+
for (long j = 0; j < 10_000; j++) {
193+
bh.consume(ords.bucketsInOrd(j % 100));
194+
bh.consume(ords.maxOwningBucketOrd());
195+
}
196+
}
197+
}
185198
}

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

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
import org.opensearch.common.lease.Releasable;
3636
import org.opensearch.common.util.BigArrays;
37+
import org.opensearch.common.util.LongArray;
3738
import org.opensearch.common.util.LongLongHash;
3839
import org.opensearch.common.util.ReorganizingLongHash;
3940
import org.opensearch.search.aggregations.CardinalityUpperBound;
@@ -229,16 +230,27 @@ public void close() {
229230
* @opensearch.internal
230231
*/
231232
public static class FromMany extends LongKeyedBucketOrds {
233+
private final BigArrays bigArrays;
232234
private final LongLongHash ords;
235+
private long maxOwningBucketOrd = -1;
236+
private LongArray bucketOrdsCounts;
233237

234238
public FromMany(BigArrays bigArrays) {
239+
this.bigArrays = bigArrays;
235240
ords = new LongLongHash(2, bigArrays);
241+
bucketOrdsCounts = bigArrays.newLongArray(1, true);
236242
}
237243

238244
@Override
239245
public long add(long owningBucketOrd, long value) {
240246
// This is in the critical path for collecting most aggs. Be careful of performance.
241-
return ords.add(owningBucketOrd, value);
247+
long ord = ords.add(owningBucketOrd, value);
248+
if (ord >= 0) {
249+
maxOwningBucketOrd = Math.max(maxOwningBucketOrd, owningBucketOrd);
250+
bucketOrdsCounts = bigArrays.grow(bucketOrdsCounts, owningBucketOrd + 1);
251+
bucketOrdsCounts.set(owningBucketOrd, bucketOrdsCounts.get(owningBucketOrd) + 1);
252+
}
253+
return ord;
242254
}
243255

244256
@Override
@@ -253,14 +265,10 @@ public long get(long ordinal) {
253265

254266
@Override
255267
public long bucketsInOrd(long owningBucketOrd) {
256-
// TODO it'd be faster to count the number of buckets in a list of these ords rather than one at a time
257-
long count = 0;
258-
for (long i = 0; i < ords.size(); i++) {
259-
if (ords.getKey1(i) == owningBucketOrd) {
260-
count++;
261-
}
268+
if (owningBucketOrd >= bucketOrdsCounts.size()) {
269+
return 0;
262270
}
263-
return count;
271+
return bucketOrdsCounts.get(owningBucketOrd);
264272
}
265273

266274
@Override
@@ -270,12 +278,7 @@ public long size() {
270278

271279
@Override
272280
public long maxOwningBucketOrd() {
273-
// TODO this is fairly expensive to compute. Can we avoid needing it?
274-
long max = -1;
275-
for (long i = 0; i < ords.size(); i++) {
276-
max = Math.max(max, ords.getKey1(i));
277-
}
278-
return max;
281+
return maxOwningBucketOrd;
279282
}
280283

281284
@Override
@@ -313,7 +316,13 @@ public long ord() {
313316

314317
@Override
315318
public void close() {
316-
ords.close();
319+
try {
320+
ords.close();
321+
} finally {
322+
if (bucketOrdsCounts != null) {
323+
bucketOrdsCounts.close();
324+
}
325+
}
317326
}
318327
}
319328
}

0 commit comments

Comments
 (0)