Merge pull request #13191 from linasm/reduceResolution-inplace

Reuse receiver slices in [Float]Histogram.ReduceResolution

model/histogram/float_histogram.go

@@ -94,8 +94,8 @@ func (h *FloatHistogram) CopyToSchema(targetSchema int32) *FloatHistogram {
 		Sum:           h.Sum,
 	}
 
-	c.PositiveSpans, c.PositiveBuckets = reduceResolution(h.PositiveSpans, h.PositiveBuckets, h.Schema, targetSchema, false)
+	c.PositiveSpans, c.PositiveBuckets = reduceResolution(h.PositiveSpans, h.PositiveBuckets, h.Schema, targetSchema, false, false)
-	c.NegativeSpans, c.NegativeBuckets = reduceResolution(h.NegativeSpans, h.NegativeBuckets, h.Schema, targetSchema, false)
+	c.NegativeSpans, c.NegativeBuckets = reduceResolution(h.NegativeSpans, h.NegativeBuckets, h.Schema, targetSchema, false, false)
 
 	return &c
 }
@@ -278,8 +278,8 @@ func (h *FloatHistogram) Add(other *FloatHistogram) *FloatHistogram {
 	if other.Schema < h.Schema {
 		panic(fmt.Errorf("cannot add histogram with schema %d to %d", other.Schema, h.Schema))
 	} else if other.Schema > h.Schema {
-		otherPositiveSpans, otherPositiveBuckets = reduceResolution(otherPositiveSpans, otherPositiveBuckets, other.Schema, h.Schema, false)
+		otherPositiveSpans, otherPositiveBuckets = reduceResolution(otherPositiveSpans, otherPositiveBuckets, other.Schema, h.Schema, false, false)
-		otherNegativeSpans, otherNegativeBuckets = reduceResolution(otherNegativeSpans, otherNegativeBuckets, other.Schema, h.Schema, false)
+		otherNegativeSpans, otherNegativeBuckets = reduceResolution(otherNegativeSpans, otherNegativeBuckets, other.Schema, h.Schema, false, false)
 	}
 
 	h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, h.PositiveSpans, h.PositiveBuckets, otherPositiveSpans, otherPositiveBuckets)
@@ -305,8 +305,8 @@ func (h *FloatHistogram) Sub(other *FloatHistogram) *FloatHistogram {
 	if other.Schema < h.Schema {
 		panic(fmt.Errorf("cannot subtract histigram with schema %d to %d", other.Schema, h.Schema))
 	} else if other.Schema > h.Schema {
-		otherPositiveSpans, otherPositiveBuckets = reduceResolution(otherPositiveSpans, otherPositiveBuckets, other.Schema, h.Schema, false)
+		otherPositiveSpans, otherPositiveBuckets = reduceResolution(otherPositiveSpans, otherPositiveBuckets, other.Schema, h.Schema, false, false)
-		otherNegativeSpans, otherNegativeBuckets = reduceResolution(otherNegativeSpans, otherNegativeBuckets, other.Schema, h.Schema, false)
+		otherNegativeSpans, otherNegativeBuckets = reduceResolution(otherNegativeSpans, otherNegativeBuckets, other.Schema, h.Schema, false, false)
 	}
 
 	h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, h.PositiveSpans, h.PositiveBuckets, otherPositiveSpans, otherPositiveBuckets)
@@ -1136,8 +1136,9 @@ func (h *FloatHistogram) ReduceResolution(targetSchema int32) *FloatHistogram {
 		panic(fmt.Errorf("cannot reduce resolution from schema %d to %d", h.Schema, targetSchema))
 	}
 
-	h.PositiveSpans, h.PositiveBuckets = reduceResolution(h.PositiveSpans, h.PositiveBuckets, h.Schema, targetSchema, false)
+	h.PositiveSpans, h.PositiveBuckets = reduceResolution(h.PositiveSpans, h.PositiveBuckets, h.Schema, targetSchema, false, true)
-	h.NegativeSpans, h.NegativeBuckets = reduceResolution(h.NegativeSpans, h.NegativeBuckets, h.Schema, targetSchema, false)
+	h.NegativeSpans, h.NegativeBuckets = reduceResolution(h.NegativeSpans, h.NegativeBuckets, h.Schema, targetSchema, false, true)
+
 	h.Schema = targetSchema
 	return h
 }

model/histogram/float_histogram_test.go

@@ -1734,7 +1734,10 @@ func TestFloatHistogramCopyToSchema(t *testing.T) {
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
+			inCopy := c.in.Copy()
 			require.Equal(t, c.expected, c.in.CopyToSchema(c.targetSchema))
+			// Check that the receiver histogram was not mutated:
+			require.Equal(t, inCopy, c.in)
 		})
 	}
 }
@@ -2489,5 +2492,7 @@ func TestFloatHistogramReduceResolution(t *testing.T) {
 	for _, tc := range tcs {
 		target := tc.origin.ReduceResolution(tc.target.Schema)
 		require.Equal(t, tc.target, target)
+		// Check that receiver histogram was mutated:
+		require.Equal(t, tc.target, tc.origin)
 	}
 }

model/histogram/generic.go

@@ -605,7 +605,16 @@ var exponentialBounds = [][]float64{
 // The target schema must be smaller than the original schema.
 // Set deltaBuckets to true if the provided buckets are
 // deltas. Set it to false if the buckets contain absolute counts.
-func reduceResolution[IBC InternalBucketCount](originSpans []Span, originBuckets []IBC, originSchema, targetSchema int32, deltaBuckets bool) ([]Span, []IBC) {
+// Set inplace to true to reuse input slices and avoid allocations (otherwise
+// new slices will be allocated for result).
+func reduceResolution[IBC InternalBucketCount](
+	originSpans []Span,
+	originBuckets []IBC,
+	originSchema,
+	targetSchema int32,
+	deltaBuckets bool,
+	inplace bool,
+) ([]Span, []IBC) {
 	var (
 		targetSpans           []Span // The spans in the target schema.
 		targetBuckets         []IBC  // The bucket counts in the target schema.
@@ -617,6 +626,13 @@ func reduceResolution[IBC InternalBucketCount](originSpans []Span, originBuckets
 		lastTargetBucketCount IBC
 	)
 
+	if inplace {
+		// Slice reuse is safe because when reducing the resolution,
+		// target slices don't grow faster than origin slices are being read.
+		targetSpans = originSpans[:0]
+		targetBuckets = originBuckets[:0]
+	}
+
 	for _, span := range originSpans {
 		// Determine the index of the first bucket in this span.
 		bucketIdx += span.Offset

model/histogram/generic_test.go

@@ -15,6 +15,7 @@ package histogram
 
 import (
 	"math"
+	"slices"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -140,9 +141,22 @@ func TestReduceResolutionHistogram(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		spans, buckets := reduceResolution(tc.spans, tc.buckets, tc.schema, tc.targetSchema, true)
+		spansCopy, bucketsCopy := slices.Clone(tc.spans), slices.Clone(tc.buckets)
+		spans, buckets := reduceResolution(tc.spans, tc.buckets, tc.schema, tc.targetSchema, true, false)
 		require.Equal(t, tc.expectedSpans, spans)
 		require.Equal(t, tc.expectedBuckets, buckets)
+		// Verify inputs were not mutated:
+		require.Equal(t, spansCopy, tc.spans)
+		require.Equal(t, bucketsCopy, tc.buckets)
+
+		// Output slices reuse input slices:
+		const inplace = true
+		spans, buckets = reduceResolution(tc.spans, tc.buckets, tc.schema, tc.targetSchema, true, inplace)
+		require.Equal(t, tc.expectedSpans, spans)
+		require.Equal(t, tc.expectedBuckets, buckets)
+		// Verify inputs were mutated which is now expected:
+		require.Equal(t, spans, tc.spans[:len(spans)])
+		require.Equal(t, buckets, tc.buckets[:len(buckets)])
 	}
 }
 
@@ -175,8 +189,21 @@ func TestReduceResolutionFloatHistogram(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		spans, buckets := reduceResolution(tc.spans, tc.buckets, tc.schema, tc.targetSchema, false)
+		spansCopy, bucketsCopy := slices.Clone(tc.spans), slices.Clone(tc.buckets)
+		spans, buckets := reduceResolution(tc.spans, tc.buckets, tc.schema, tc.targetSchema, false, false)
 		require.Equal(t, tc.expectedSpans, spans)
 		require.Equal(t, tc.expectedBuckets, buckets)
+		// Verify inputs were not mutated:
+		require.Equal(t, spansCopy, tc.spans)
+		require.Equal(t, bucketsCopy, tc.buckets)
+
+		// Output slices reuse input slices:
+		const inplace = true
+		spans, buckets = reduceResolution(tc.spans, tc.buckets, tc.schema, tc.targetSchema, false, inplace)
+		require.Equal(t, tc.expectedSpans, spans)
+		require.Equal(t, tc.expectedBuckets, buckets)
+		// Verify inputs were mutated which is now expected:
+		require.Equal(t, spans, tc.spans[:len(spans)])
+		require.Equal(t, buckets, tc.buckets[:len(buckets)])
 	}
 }

model/histogram/histogram.go

@@ -502,10 +502,10 @@ func (h *Histogram) ReduceResolution(targetSchema int32) *Histogram {
 	}
 
 	h.PositiveSpans, h.PositiveBuckets = reduceResolution(
-		h.PositiveSpans, h.PositiveBuckets, h.Schema, targetSchema, true,
+		h.PositiveSpans, h.PositiveBuckets, h.Schema, targetSchema, true, true,
 	)
 	h.NegativeSpans, h.NegativeBuckets = reduceResolution(
-		h.NegativeSpans, h.NegativeBuckets, h.Schema, targetSchema, true,
+		h.NegativeSpans, h.NegativeBuckets, h.Schema, targetSchema, true, true,
 	)
 	h.Schema = targetSchema
 	return h

model/histogram/histogram_test.go

@@ -1008,5 +1008,7 @@ func TestHistogramReduceResolution(t *testing.T) {
 	for _, tc := range tcs {
 		target := tc.origin.ReduceResolution(tc.target.Schema)
 		require.Equal(t, tc.target, target)
+		// Check that receiver histogram was mutated:
+		require.Equal(t, tc.target, tc.origin)
 	}
 }