ESQL: replace ExponentialHistogramBlock accessor with scratch (#137368)

JonasKunz · web-flow · commit e522df6b16dd · 2025-10-30T14:00:42.000+01:00
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockUtils.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockUtils.java
@@ -312,7 +312,7 @@ yield new AggregateMetricDoubleLiteral(
             }
             case EXPONENTIAL_HISTOGRAM -> {
                 ExponentialHistogramBlock histoBlock = (ExponentialHistogramBlock) block;
-                ExponentialHistogram histogram = new ExponentialHistogramBlockAccessor(histoBlock).get(offset);
+                ExponentialHistogram histogram = histoBlock.getExponentialHistogram(offset, new ExponentialHistogramScratch());
                 // return a copy so that the returned value is not bound to the lifetime of the block
                 yield ExponentialHistogram.builder(histogram, ExponentialHistogramCircuitBreaker.noop()).build();
             }
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ConstantNullBlock.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ConstantNullBlock.java
@@ -12,6 +12,7 @@
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.core.ReleasableIterator;
+import org.elasticsearch.exponentialhistogram.ExponentialHistogram;
 
 import java.io.IOException;
 
@@ -293,6 +294,12 @@ public long getLong(int valueIndex) {
         throw new UnsupportedOperationException("null block");
     }
 
+    @Override
+    public ExponentialHistogram getExponentialHistogram(int valueIndex, ExponentialHistogramScratch scratch) {
+        assert false : "null block";
+        throw new UnsupportedOperationException("null block");
+    }
+
     @Override
     public int getTotalValueCount() {
         return 0;
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ExponentialHistogramArrayBlock.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ExponentialHistogramArrayBlock.java
@@ -12,7 +12,7 @@
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.core.ReleasableIterator;
 import org.elasticsearch.core.Releasables;
-import org.elasticsearch.exponentialhistogram.CompressedExponentialHistogram;
+import org.elasticsearch.exponentialhistogram.ExponentialHistogram;
 
 import java.io.IOException;
 import java.util.List;
@@ -78,15 +78,17 @@ private List<Block> getSubBlocks() {
         return List.of(sums, valueCounts, zeroThresholds, encodedHistograms, minima, maxima);
     }
 
-    void loadValue(int valueIndex, CompressedExponentialHistogram resultHistogram, BytesRef tempBytesRef) {
-        BytesRef bytes = encodedHistograms.getBytesRef(encodedHistograms.getFirstValueIndex(valueIndex), tempBytesRef);
+    @Override
+    public ExponentialHistogram getExponentialHistogram(int valueIndex, ExponentialHistogramScratch scratch) {
+        BytesRef bytes = encodedHistograms.getBytesRef(encodedHistograms.getFirstValueIndex(valueIndex), scratch.bytesRefScratch);
         double zeroThreshold = zeroThresholds.getDouble(zeroThresholds.getFirstValueIndex(valueIndex));
         long valueCount = valueCounts.getLong(valueCounts.getFirstValueIndex(valueIndex));
         double sum = sums.getDouble(sums.getFirstValueIndex(valueIndex));
         double min = valueCount == 0 ? Double.NaN : minima.getDouble(minima.getFirstValueIndex(valueIndex));
         double max = valueCount == 0 ? Double.NaN : maxima.getDouble(maxima.getFirstValueIndex(valueIndex));
         try {
-            resultHistogram.reset(zeroThreshold, valueCount, sum, min, max, bytes);
+            scratch.reusedHistogram.reset(zeroThreshold, valueCount, sum, min, max, bytes);
+            return scratch.reusedHistogram;
         } catch (IOException e) {
             throw new IllegalStateException("error loading histogram", e);
         }
@@ -374,4 +376,5 @@ public int hashCode() {
         // this ensures proper equality with null blocks and should be unique enough for practical purposes
         return encodedHistograms.hashCode();
     }
+
 }
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ExponentialHistogramBlock.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ExponentialHistogramBlock.java
@@ -11,10 +11,22 @@
 
 /**
  * A block that holds {@link ExponentialHistogram} values.
- * Position access is done through {@link ExponentialHistogramBlockAccessor}.
  */
 public sealed interface ExponentialHistogramBlock extends Block permits ConstantNullBlock, ExponentialHistogramArrayBlock {
 
+    /**
+     * Returns the {@link ExponentialHistogram} value at the given index.
+     * In order to be allocation free, this method requires a scratch object to be passed in,
+     * whose memory will be used to hold the state of the returned histogram.
+     * Therefore, the return value of this method is only valid until either the block is closed
+     * or the same scratch instance is passed to another call to this method on any block.
+     *
+     * @param valueIndex the index of the histogram to get
+     * @param scratch the scratch to use as storage for the returned histogram
+     * @return the exponential histogram at the given index
+     */
+    ExponentialHistogram getExponentialHistogram(int valueIndex, ExponentialHistogramScratch scratch);
+
     static boolean equals(ExponentialHistogramBlock blockA, ExponentialHistogramBlock blockB) {
         if (blockA == blockB) {
             return true;
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ExponentialHistogramBlockAccessor.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ExponentialHistogramBlockAccessor.java
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ExponentialHistogramScratch.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ExponentialHistogramScratch.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.compute.data;
+
+import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.exponentialhistogram.CompressedExponentialHistogram;
+
+/**
+ * Reusable storage to be passed to {@link ExponentialHistogramBlock#getExponentialHistogram(int, ExponentialHistogramScratch)}.
+ */
+public class ExponentialHistogramScratch {
+
+    final BytesRef bytesRefScratch = new BytesRef();
+    final CompressedExponentialHistogram reusedHistogram = new CompressedExponentialHistogram();
+
+}
diff --git a/x-pack/plugin/esql/compute/test/src/main/java/org/elasticsearch/compute/test/BlockTestUtils.java b/x-pack/plugin/esql/compute/test/src/main/java/org/elasticsearch/compute/test/BlockTestUtils.java
@@ -20,8 +20,8 @@
 import org.elasticsearch.compute.data.DoubleBlock;
 import org.elasticsearch.compute.data.ElementType;
 import org.elasticsearch.compute.data.ExponentialHistogramBlock;
-import org.elasticsearch.compute.data.ExponentialHistogramBlockAccessor;
 import org.elasticsearch.compute.data.ExponentialHistogramBlockBuilder;
+import org.elasticsearch.compute.data.ExponentialHistogramScratch;
 import org.elasticsearch.compute.data.FloatBlock;
 import org.elasticsearch.compute.data.IntBlock;
 import org.elasticsearch.compute.data.LongBlock;
@@ -313,7 +313,10 @@ public static List<List<Object>> valuesAtPositions(Block block, int from, int to
                         yield literal;
 
                     }
-                    case EXPONENTIAL_HISTOGRAM -> new ExponentialHistogramBlockAccessor((ExponentialHistogramBlock) block).get(i);
+                    case EXPONENTIAL_HISTOGRAM -> ((ExponentialHistogramBlock) block).getExponentialHistogram(
+                        i++,
+                        new ExponentialHistogramScratch()
+                    );
                     default -> throw new IllegalArgumentException("unsupported element type [" + block.elementType() + "]");
                 });
             }
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PositionToXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PositionToXContent.java
@@ -16,7 +16,7 @@
 import org.elasticsearch.compute.data.BytesRefBlock;
 import org.elasticsearch.compute.data.DoubleBlock;
 import org.elasticsearch.compute.data.ExponentialHistogramBlock;
-import org.elasticsearch.compute.data.ExponentialHistogramBlockAccessor;
+import org.elasticsearch.compute.data.ExponentialHistogramScratch;
 import org.elasticsearch.compute.data.FloatBlock;
 import org.elasticsearch.compute.data.IntBlock;
 import org.elasticsearch.compute.data.LongBlock;
@@ -174,12 +174,12 @@ protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Pa
             };
             case EXPONENTIAL_HISTOGRAM -> new PositionToXContent(block) {
 
-                ExponentialHistogramBlockAccessor accessor = new ExponentialHistogramBlockAccessor((ExponentialHistogramBlock) block);
+                ExponentialHistogramScratch scratch = new ExponentialHistogramScratch();
 
                 @Override
                 protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Params params, int valueIndex)
                     throws IOException {
-                    ExponentialHistogram histogram = accessor.get(valueIndex);
+                    ExponentialHistogram histogram = ((ExponentialHistogramBlock) block).getExponentialHistogram(valueIndex, scratch);
                     ExponentialHistogramXContent.serialize(builder, histogram);
                     return builder;
                 }
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java
@@ -21,7 +21,7 @@
 import org.elasticsearch.compute.data.AggregateMetricDoubleBlockBuilder.Metric;
 import org.elasticsearch.compute.data.DoubleBlock;
 import org.elasticsearch.compute.data.ExponentialHistogramBlock;
-import org.elasticsearch.compute.data.ExponentialHistogramBlockAccessor;
+import org.elasticsearch.compute.data.ExponentialHistogramScratch;
 import org.elasticsearch.compute.data.IntBlock;
 import org.elasticsearch.core.Booleans;
 import org.elasticsearch.exponentialhistogram.ExponentialHistogram;
@@ -799,7 +799,7 @@ public static String exponentialHistogramToString(ExponentialHistogram histo) {
     }
 
     public static String exponentialHistogramBlockToString(ExponentialHistogramBlock histoBlock, int index) {
-        ExponentialHistogram histo = new ExponentialHistogramBlockAccessor(histoBlock).get(index);
+        ExponentialHistogram histo = histoBlock.getExponentialHistogram(index, new ExponentialHistogramScratch());
         return exponentialHistogramToString(histo);
     }
 

Original file line number	Diff line number	Diff line change
`@@ -312,7 +312,7 @@ yield new AggregateMetricDoubleLiteral(`
`312`	`312`	`}`
`313`	`313`	`case EXPONENTIAL_HISTOGRAM -> {`
`314`	`314`	`ExponentialHistogramBlock histoBlock = (ExponentialHistogramBlock) block;`
`315`		`- ExponentialHistogram histogram = new ExponentialHistogramBlockAccessor(histoBlock).get(offset);`
	`315`	`+ ExponentialHistogram histogram = histoBlock.getExponentialHistogram(offset, new ExponentialHistogramScratch());`
`316`	`316`	`// return a copy so that the returned value is not bound to the lifetime of the block`
`317`	`317`	`yield ExponentialHistogram.builder(histogram, ExponentialHistogramCircuitBreaker.noop()).build();`
`318`	`318`	`}`