ESQL: Add support for exponential_histogram in code generation #137459
Conversation
elasticsearchmachine
added
v9.3.0
external-contributor
JonasKunz
added
>non-issue
and removed
external-contributor
| if (intermediateState.stream().map(IntermediateStateDesc::elementType).anyMatch(n -> n.equals("BYTES_REF"))) { | ||
| builder.addStatement("$T scratch = new $T()", BYTES_REF, BYTES_REF); | ||
| for (IntermediateStateDesc interState : intermediateState) { | ||
| interState.addScratchDeclaration(builder); |
There was a problem hiding this comment.
I think this was buggy prior to my change, but without causing a defect?
If there were multiple BYTES_REF state-members, they would have shared the same scratch, which seems incorrect?
There was a problem hiding this comment.
That seems possible. I imagine we don't have any intermediate states with two strings.
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
That's fine. Maybe they will one day we'll do it then. |
|
|
||
| this.hasOnlyBlockArguments = this.aggParams.stream().allMatch(a -> a instanceof BlockArgument); | ||
| this.tryToUseVectors = aggParams.stream().anyMatch(a -> (a instanceof BlockArgument) == false) | ||
| && aggParams.stream().noneMatch(a -> a instanceof StandardArgument && a.dataType(false) == null); |
There was a problem hiding this comment.
I'd prefer making a method in Argument that's, like, hasVector or something.
| if (intermediateState.stream().map(IntermediateStateDesc::elementType).anyMatch(n -> n.equals("BYTES_REF"))) { | ||
| builder.addStatement("$T scratch = new $T()", BYTES_REF, BYTES_REF); | ||
| for (IntermediateStateDesc interState : intermediateState) { | ||
| interState.addScratchDeclaration(builder); |
There was a problem hiding this comment.
That seems possible. I imagine we don't have any intermediate states with two strings.
...l/compute/gen/src/main/java/org/elasticsearch/compute/gen/GroupingAggregatorImplementer.java
Outdated
Show resolved
Hide resolved
3 participants
Adds support for the
exponential_histogramtype in theEvaluator,AggregatorandGrouping Aggregatorcode generation.MvEvaluatorandConvertEvaluatorhave not been touched yet, we'll get to those when we actually need them for exponential histograms.The code generation is changed as follows:
BytesRefhas been generalized to also work with exponential histograms, as there the getter also requires a scratchIn my understanding,
ExponentialHistogramBlocks will never have a correspondingVector, as they don't have a memory layout which directly benefits from e.g. SIMD. Vectorization still works by first "extracting" the sub-blocks, which might be vector-backed.For example
MIN(histogram)will be implemented as a surrogateMIN(HISTOGRAM_MIN(histogram)), whereHISTOGRAM_MINis a function which directly returns the min-subblock, which in turn might be a vector.To see these code-generation changes in action, please have a look at my PoC PR:
Note that I'm not planning on exposing those two functions to end-users for now. Instead, they will be just used as surrogate for the existing
PERCENTILEagg.