Add fast path for single value in VALUES aggregator #130510

dnhatn · 2025-07-02T22:56:31Z

This change introduces a fast path for the VALUES aggregator in the single-value case. For the first value seen in each group, we add it the new big array without touching the hash. For subsequent values, if they are the same as the current value, we skip them; if they differ, we trigger the slow path and add them to the hash. This optimization speeds up VALUES when the number of groups is large and most groups have only one value.

Before:

Benchmark (dataType) (groups) Mode Cnt Score Error Units ValuesAggregatorBenchmark.run BytesRef 1 avgt 3 177.756 ± 2.111 ms/op ValuesAggregatorBenchmark.run BytesRef 1000 avgt 3 126.174 ± 0.431 ms/op ValuesAggregatorBenchmark.run BytesRef 1000000 avgt 3 66920.144 ± 53588.490 ms/op

After:

Benchmark (dataType) (groups) Mode Cnt Score Error Units ValuesAggregatorBenchmark.run BytesRef 1 avgt 3 180.269 ± 4.019 ms/op ValuesAggregatorBenchmark.run BytesRef 1000 avgt 3 107.051 ± 3.149 ms/op ValuesAggregatorBenchmark.run BytesRef 1000000 avgt 3 26277.863 ± 7214.319 ms/op

elasticsearchmachine · 2025-07-03T04:51:35Z

Hi @dnhatn, I've created a changelog YAML for you.

elasticsearchmachine · 2025-07-03T04:54:12Z

Pinging @elastic/es-analytical-engine (Team:Analytics)

nik9000

I like the idea of collecting the first values into only the firstValues array. So we only go to the hash if there's more than one value. I'll have another look in the morning. I think it's correct, but I'll need another scan to make sure.

nik9000 · 2025-07-07T20:43:23Z

...e/src/main/generated-src/org/elasticsearch/compute/aggregation/ValuesBytesRefAggregator.java

 }

 public static void combineStates(GroupingState current, int currentGroupId, GroupingState state, int statePosition) {
 BytesRef scratch = new BytesRef();


Scratch can go below the quick bail outs.

nik9000 · 2025-07-07T21:06:46Z

...e/src/main/generated-src/org/elasticsearch/compute/aggregation/ValuesBytesRefAggregator.java

+ for (int s = 0; s < selected.getPositionCount(); s++) {
+ int group = selected.getInt(s);
+ int count = -selectedCounts[group];
+ selectedCounts[group] = total;


This isn't the counts any more - it's the counts in values, which is one less than the actual number of values. That's tricky. I'm not entirely sure what's correct here.

…r-values

Similar to #127849, this change adds an optimized path for leveraging ordinal blocks of intermediate input pages in the Values aggregator. Below are the micro-benchmark results. Before: ``` // 1 raw input page + 1000 intermediate input pages Benchmark (dataType) (groups) Mode Cnt Score Error Units ValuesAggregatorBenchmark.run BytesRef 1 avgt 2 0.382 ms/op ValuesAggregatorBenchmark.run BytesRef 1000 avgt 2 112.293 ms/op ValuesAggregatorBenchmark.run BytesRef 1000000 avgt 2 113182.908 ms/op ``` ``` After: // 1 raw input page + 1000 intermediate input pages Benchmark (dataType) (groups) Mode Cnt Score Error Units ValuesAggregatorBenchmark.run BytesRef 1 avgt 2 0.378 ms/op ValuesAggregatorBenchmark.run BytesRef 1000 avgt 2 34.410 ms/op ValuesAggregatorBenchmark.run BytesRef 1000000 avgt 2 64654.830 ms/op ``` 1K groups: 112 ms -> 34.4ms 1M groups: 113s -> 64s More to come with #130510 Relates #127849

…r-values

nik9000 · 2025-07-23T22:03:21Z

...ute/src/main/generated-src/org/elasticsearch/compute/aggregation/ValuesDoubleAggregator.java

- * @param ids positions of the {@link GroupingState#values} to read.
+ * Values after the first in each group are collected in a hash, keyed by the pair of groupId and value.
+ * When emitting the output, we need to iterate the hash one group at a time to build the output block,
+ * which can require O(N^2). To avoid this, we compute the counts for each group and remap the hash id


s/can/would/

nik9000 · 2025-07-23T22:04:55Z

...ute/src/main/generated-src/org/elasticsearch/compute/aggregation/ValuesDoubleAggregator.java

+ DoubleArray _firstValues = null;
+ NextValues _nextValues = null;
+ try {
+ _firstValues = driverContext.bigArrays().newDoubleArray(1, false);


I tend not to do the _ thing in the ctor because this shadowing thing is so so common.

Fixed in e390d42

nik9000 · 2025-07-23T22:07:09Z

...ute/src/main/generated-src/org/elasticsearch/compute/aggregation/ValuesDoubleAggregator.java

 private final BlockFactory blockFactory;
- private final LongLongHash values;
+ DoubleArray firstValues;
+ private BitArray seen;


It's worth commenting that a false seen means the firstValues unset.

nik9000 · 2025-07-23T22:09:27Z

...ute/src/main/generated-src/org/elasticsearch/compute/aggregation/ValuesDoubleAggregator.java

+ if (groupId > maxGroupId) {
+ firstValues = blockFactory.bigArrays().grow(firstValues, groupId + 1);
+ firstValues.set(groupId, v);
+ if (seen == null && groupId > maxGroupId + 1) {


Maybe a comment that we don't build seen if the array is dense.

It should be super common that we're dense.

++ added in 239c90d

…r-values

dnhatn · 2025-07-24T19:22:25Z

Thanks Nik!

dnhatn · 2025-07-24T19:40:41Z

I also ran time-series aggregation queries with this change.

// before: "took": 470, // after: "took": 377 POST /_query { "profile": true, "query": "TS metrics-hostmetricsreceiver.otel-default | STATS avg(avg_over_time(`metrics.system.cpu.load_average.1m`)) BY host.name, BUCKET(@timestamp, 5 minute)" }

// before: "took": 1094, // after: "took": 786, POST /_query { "profile": true, "query": "TS metrics-hostmetricsreceiver.otel-default | STATS avg(rate(`metrics.process.cpu.time`)) BY host.name, BUCKET(@timestamp, 5 minute)" }

…king * upstream/main: (90 commits) Register a blob cache long counter metric for total evicted regions (elastic#131862) Move plan attribute resolution to its own component (elastic#131830) Make restore support multi-project (elastic#131661) Use logically more correct expression (elastic#131869) [ES|QL] Change equals and hashcode for ConstantNullBlock (elastic#131817) Update `TransportVersion` to support a new model (elastic#131488) Correct slow log user for RCS 2.0 (elastic#130140) Revert "Remove 8.17 from dev branches" Mute org.elasticsearch.compute.aggregation.ValuesBytesRefGroupingAggregatorFunctionTests testSomeFiltered elastic#131878 Remove 8.17 from dev branches Revert "CompressorFactory.compressor (elastic#131655)" (elastic#131866) Add fast path for single value in VALUES aggregator (elastic#130510) Resolve inference release tests failing due to missing feature flag (elastic#131841) [Docs] Replace placeholder URLs (elastic#131309) CompressorFactory.compressor (elastic#131655) add availability info for speed loading setting (elastic#131714) [Logstash] Move `elastic_integration` plugin usage to ES logstash-bridge. (elastic#131486) Migrate x-pack-enrich legacy rest tests to new test framework (elastic#131743) Fix plugin example test failures due to deprecation warning (elastic#131819) Remove deprecated function isNotNullAndFoldable (elastic#130944) ...

There are two bugs introduced in #130510 and #131390 affecting the VALUES aggregator. The random tests do not cover these edge cases: 1. The check should be firstValues.size() <= group instead of firstValues.size() < group when reading values from the firstValues array. We need to inject nulls with repeated values (to simulate ordinals) to trigger this case. 2. We incorrectly added positionOffset when reading the group ID. We need to generate more groups to trigger chunking. Relates #130510 Relates #131390 Closes #131878

elasticsearchmachine added the v9.2.0 label Jul 2, 2025

dnhatn force-pushed the single-value-path-for-values branch 2 times, most recently from 61f3ad6 to c1d8eb7 Compare July 3, 2025 04:21

Add fast path for single value in VALUES

5860c6e

dnhatn force-pushed the single-value-path-for-values branch from c1d8eb7 to 5860c6e Compare July 3, 2025 04:29

dnhatn added >enhancement :Analytics/ES|QL AKA ESQL labels Jul 3, 2025

Update docs/changelog/130510.yaml

90c3463

naming

5a7a5c7

dnhatn requested a review from nik9000 July 3, 2025 04:53

dnhatn marked this pull request as ready for review July 3, 2025 04:53

elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 3, 2025

nik9000 reviewed Jul 7, 2025

View reviewed changes

dnhatn mentioned this pull request Jul 17, 2025

Add optimized path for intermediate values aggregator #131390

Merged

Merge remote-tracking branch 'elastic/main' into single-value-path-fo…

9812508

…r-values

dnhatn added 3 commits July 21, 2025 13:23

Merge remote-tracking branch 'elastic/main' into single-value-path-fo…

73c26b4

…r-values

single-value

d3f9027

Merge remote-tracking branch 'elastic/main' into single-value-path-fo…

c4e0669

…r-values

dnhatn requested review from ivancea and nik9000 July 23, 2025 16:01

dnhatn added 2 commits July 23, 2025 14:57

fix seen

bab4094

Merge remote-tracking branch 'elastic/main' into single-value-path-fo…

a15d000

…r-values

nik9000 approved these changes Jul 23, 2025

View reviewed changes

dnhatn added 3 commits July 24, 2025 10:19

javadocs

239c90d

ctor

e390d42

Merge remote-tracking branch 'elastic/main' into single-value-path-fo…

61bed08

…r-values

dnhatn merged commit 71957ca into elastic:main Jul 24, 2025
33 checks passed

dnhatn deleted the single-value-path-for-values branch July 24, 2025 19:23

dnhatn mentioned this pull request Jul 28, 2025

Fix off by one in ValuesBytesRefAggregator #132032

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add fast path for single value in VALUES aggregator #130510

Add fast path for single value in VALUES aggregator #130510

Uh oh!

dnhatn commented Jul 2, 2025 •

edited

Loading

elasticsearchmachine commented Jul 3, 2025

elasticsearchmachine commented Jul 3, 2025

nik9000 left a comment

nik9000 Jul 7, 2025

nik9000 Jul 7, 2025

nik9000 Jul 23, 2025

nik9000 Jul 23, 2025

dnhatn Jul 24, 2025

nik9000 Jul 23, 2025

nik9000 Jul 23, 2025

dnhatn Jul 24, 2025

dnhatn commented Jul 24, 2025

Uh oh!

dnhatn commented Jul 24, 2025

Labels

3 participants

Add fast path for single value in VALUES aggregator #130510

Add fast path for single value in VALUES aggregator #130510

Uh oh!

Conversation

dnhatn commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

elasticsearchmachine commented Jul 3, 2025

elasticsearchmachine commented Jul 3, 2025

nik9000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnhatn commented Jul 24, 2025

Uh oh!

dnhatn commented Jul 24, 2025

Labels

3 participants

dnhatn commented Jul 2, 2025 •

edited

Loading