Merge pull request #16565 from prometheus/krajo/intern-custom-values

perf(chunkenc): intern custom values for native histograms

model/histogram/float_histogram.go

@@ -73,10 +73,8 @@ func (h *FloatHistogram) Copy() *FloatHistogram {
 	}
 
 	if h.UsesCustomBuckets() {
-		if len(h.CustomValues) != 0 {
-			c.CustomValues = make([]float64, len(h.CustomValues))
-			copy(c.CustomValues, h.CustomValues)
-		}
+		// Custom values are interned, so no need to copy them.
+		c.CustomValues = h.CustomValues
 	} else {
 		c.ZeroThreshold = h.ZeroThreshold
 		c.ZeroCount = h.ZeroCount
@@ -117,9 +115,8 @@ func (h *FloatHistogram) CopyTo(to *FloatHistogram) {
 
 		to.NegativeSpans = clearIfNotNil(to.NegativeSpans)
 		to.NegativeBuckets = clearIfNotNil(to.NegativeBuckets)
-
-		to.CustomValues = resize(to.CustomValues, len(h.CustomValues))
-		copy(to.CustomValues, h.CustomValues)
+		// Custom values are interned, so no need to copy them.
+		to.CustomValues = h.CustomValues
 	} else {
 		to.ZeroThreshold = h.ZeroThreshold
 		to.ZeroCount = h.ZeroCount
@@ -130,7 +127,8 @@ func (h *FloatHistogram) CopyTo(to *FloatHistogram) {
 		to.NegativeBuckets = resize(to.NegativeBuckets, len(h.NegativeBuckets))
 		copy(to.NegativeBuckets, h.NegativeBuckets)
 
-		to.CustomValues = clearIfNotNil(to.CustomValues)
+		// Custom values are interned, so no need to reset them.
+		to.CustomValues = nil
 	}
 
 	to.PositiveSpans = resize(to.PositiveSpans, len(h.PositiveSpans))

model/histogram/histogram.go

@@ -102,10 +102,8 @@ func (h *Histogram) Copy() *Histogram {
 	}
 
 	if h.UsesCustomBuckets() {
-		if len(h.CustomValues) != 0 {
-			c.CustomValues = make([]float64, len(h.CustomValues))
-			copy(c.CustomValues, h.CustomValues)
-		}
+		// Custom values are interned, it's ok to copy by reference.
+		c.CustomValues = h.CustomValues
 	} else {
 		c.ZeroThreshold = h.ZeroThreshold
 		c.ZeroCount = h.ZeroCount
@@ -146,9 +144,8 @@ func (h *Histogram) CopyTo(to *Histogram) {
 
 		to.NegativeSpans = clearIfNotNil(to.NegativeSpans)
 		to.NegativeBuckets = clearIfNotNil(to.NegativeBuckets)
-
-		to.CustomValues = resize(to.CustomValues, len(h.CustomValues))
-		copy(to.CustomValues, h.CustomValues)
+		// Custom values are interned, it's ok to copy by reference.
+		to.CustomValues = h.CustomValues
 	} else {
 		to.ZeroThreshold = h.ZeroThreshold
 		to.ZeroCount = h.ZeroCount
@@ -158,8 +155,8 @@ func (h *Histogram) CopyTo(to *Histogram) {
 
 		to.NegativeBuckets = resize(to.NegativeBuckets, len(h.NegativeBuckets))
 		copy(to.NegativeBuckets, h.NegativeBuckets)
-
-		to.CustomValues = clearIfNotNil(to.CustomValues)
+		// Custom values are interned, no need to reset.
+		to.CustomValues = nil
 	}
 
 	to.PositiveSpans = resize(to.PositiveSpans, len(h.PositiveSpans))
@@ -379,9 +376,8 @@ func (h *Histogram) ToFloat(fh *FloatHistogram) *FloatHistogram {
 		fh.ZeroCount = 0
 		fh.NegativeSpans = clearIfNotNil(fh.NegativeSpans)
 		fh.NegativeBuckets = clearIfNotNil(fh.NegativeBuckets)
-
-		fh.CustomValues = resize(fh.CustomValues, len(h.CustomValues))
-		copy(fh.CustomValues, h.CustomValues)
+		// Custom values are interned, it's ok to copy by reference.
+		fh.CustomValues = h.CustomValues
 	} else {
 		fh.ZeroThreshold = h.ZeroThreshold
 		fh.ZeroCount = float64(h.ZeroCount)
@@ -395,7 +391,8 @@ func (h *Histogram) ToFloat(fh *FloatHistogram) *FloatHistogram {
 			currentNegative += float64(b)
 			fh.NegativeBuckets[i] = currentNegative
 		}
-		fh.CustomValues = clearIfNotNil(fh.CustomValues)
+		// Custom values are interned, no need to reset.
+		fh.CustomValues = nil
 	}
 
 	fh.PositiveSpans = resize(fh.PositiveSpans, len(h.PositiveSpans))

promql/bench_test.go

@@ -382,6 +382,76 @@ func BenchmarkNativeHistograms(b *testing.B) {
 	}
 }
 
+func BenchmarkNativeHistogramsCustomBuckets(b *testing.B) {
+	testStorage := teststorage.New(b)
+	defer testStorage.Close()
+
+	app := testStorage.Appender(context.TODO())
+	if err := generateNativeHistogramCustomBucketsSeries(app, 3000); err != nil {
+		b.Fatal(err)
+	}
+	if err := app.Commit(); err != nil {
+		b.Fatal(err)
+	}
+
+	start := time.Unix(0, 0)
+	end := start.Add(2 * time.Hour)
+	step := time.Second * 30
+
+	cases := []struct {
+		name  string
+		query string
+	}{
+		{
+			name:  "sum",
+			query: "sum(native_histogram_custom_bucket_series)",
+		},
+		{
+			name:  "sum rate with short rate interval",
+			query: "sum(rate(native_histogram_custom_bucket_series[2m]))",
+		},
+		{
+			name:  "sum rate with long rate interval",
+			query: "sum(rate(native_histogram_custom_bucket_series[20m]))",
+		},
+		{
+			name:  "histogram_count with short rate interval",
+			query: "histogram_count(sum(rate(native_histogram_custom_bucket_series[2m])))",
+		},
+		{
+			name:  "histogram_count with long rate interval",
+			query: "histogram_count(sum(rate(native_histogram_custom_bucket_series[20m])))",
+		},
+	}
+
+	opts := promql.EngineOpts{
+		Logger:               nil,
+		Reg:                  nil,
+		MaxSamples:           50000000,
+		Timeout:              100 * time.Second,
+		EnableAtModifier:     true,
+		EnableNegativeOffset: true,
+	}
+
+	b.ResetTimer()
+	b.ReportAllocs()
+
+	for _, tc := range cases {
+		b.Run(tc.name, func(b *testing.B) {
+			ng := promqltest.NewTestEngineWithOpts(b, opts)
+			for i := 0; i < b.N; i++ {
+				qry, err := ng.NewRangeQuery(context.Background(), testStorage, nil, tc.query, start, end, step)
+				if err != nil {
+					b.Fatal(err)
+				}
+				if result := qry.Exec(context.Background()); result.Err != nil {
+					b.Fatal(result.Err)
+				}
+			}
+		})
+	}
+}
+
 func BenchmarkInfoFunction(b *testing.B) {
 	// Initialize test storage and generate test series data.
 	testStorage := teststorage.New(b)
@@ -537,6 +607,26 @@ func generateNativeHistogramSeries(app storage.Appender, numSeries int) error {
 	return nil
 }
 
+func generateNativeHistogramCustomBucketsSeries(app storage.Appender, numSeries int) error {
+	commonLabels := []string{labels.MetricName, "native_histogram_custom_bucket_series", "foo", "bar"}
+	series := make([][]*histogram.Histogram, numSeries)
+	for i := range series {
+		series[i] = tsdbutil.GenerateTestCustomBucketsHistograms(2000)
+	}
+
+	for sid, histograms := range series {
+		seriesLabels := labels.FromStrings(append(commonLabels, "h", strconv.Itoa(sid))...)
+		for i := range histograms {
+			ts := time.Unix(int64(i*15), 0).UnixMilli()
+			if _, err := app.AppendHistogram(0, seriesLabels, ts, histograms[i], nil); err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 func BenchmarkParser(b *testing.B) {
 	cases := []string{
 		"a",

tsdb/chunkenc/float_histogram.go

@@ -946,8 +946,8 @@ func (it *floatHistogramIterator) AtFloatHistogram(fh *histogram.FloatHistogram)
 	fh.NegativeBuckets = resize(fh.NegativeBuckets, len(it.nBuckets))
 	copy(fh.NegativeBuckets, it.nBuckets)
 
-	fh.CustomValues = resize(fh.CustomValues, len(it.customValues))
-	copy(fh.CustomValues, it.customValues)
+	// Custom values are interned. The single copy is in this iterator.
+	fh.CustomValues = it.customValues
 
 	return it.t, fh
 }

tsdb/chunkenc/histogram.go

@@ -201,7 +201,8 @@ type HistogramAppender struct {
 	schema         int32
 	zThreshold     float64
 	pSpans, nSpans []histogram.Span
-	customValues   []float64
+	// customValues is read only after the first sample is appended.
+	customValues []float64
 
 	// Although we intend to start new chunks on counter resets, we still
 	// have to handle negative deltas for gauge histograms. Therefore, even
@@ -995,8 +996,8 @@ func (it *histogramIterator) AtHistogram(h *histogram.Histogram) (int64, *histog
 	h.NegativeBuckets = resize(h.NegativeBuckets, len(it.nBuckets))
 	copy(h.NegativeBuckets, it.nBuckets)
 
-	h.CustomValues = resize(h.CustomValues, len(it.customValues))
-	copy(h.CustomValues, it.customValues)
+	// Custom values are interned. The single copy is here in the iterator.
+	h.CustomValues = it.customValues
 
 	return it.t, h
 }
@@ -1049,8 +1050,8 @@ func (it *histogramIterator) AtFloatHistogram(fh *histogram.FloatHistogram) (int
 		fh.NegativeBuckets[i] = currentNegative
 	}
 
-	fh.CustomValues = resize(fh.CustomValues, len(it.customValues))
-	copy(fh.CustomValues, it.customValues)
+	// Custom values are interned. The single copy is here in the iterator.
+	fh.CustomValues = it.customValues
 
 	return it.t, fh
 }
@@ -1081,7 +1082,6 @@ func (it *histogramIterator) Reset(b []byte) {
 		it.atHistogramCalled = false
 		it.pBuckets, it.nBuckets = nil, nil
 		it.pSpans, it.nSpans = nil, nil
-		it.customValues = nil
 	} else {
 		it.pBuckets = it.pBuckets[:0]
 		it.nBuckets = it.nBuckets[:0]
@@ -1089,7 +1089,6 @@ func (it *histogramIterator) Reset(b []byte) {
 	if it.atFloatHistogramCalled {
 		it.atFloatHistogramCalled = false
 		it.pFloatBuckets, it.nFloatBuckets = nil, nil
-		it.customValues = nil
 	} else {
 		it.pFloatBuckets = it.pFloatBuckets[:0]
 		it.nFloatBuckets = it.nFloatBuckets[:0]
@@ -1102,6 +1101,7 @@ func (it *histogramIterator) Reset(b []byte) {
 	it.leading = 0
 	it.trailing = 0
 	it.err = nil
+	it.customValues = nil
 }
 
 func (it *histogramIterator) Next() ValueType {
@@ -1211,13 +1211,7 @@ func (it *histogramIterator) Next() ValueType {
 		} else {
 			it.nSpans = nil
 		}
-		if len(it.customValues) > 0 {
-			newCustomValues := make([]float64, len(it.customValues))
-			copy(newCustomValues, it.customValues)
-			it.customValues = newCustomValues
-		} else {
-			it.customValues = nil
-		}
+		// it.CustomValues are interned, so we don't need to copy them.
 	}
 
 	if it.atHistogramCalled {

tsdb/chunkenc/histogram_test.go

@@ -1631,7 +1631,7 @@ func TestHistogramUniqueSpansAfterNextWithAtFloatHistogram(t *testing.T) {
 	require.NotSame(t, &rh1.NegativeSpans[0], &rh2.NegativeSpans[0], "NegativeSpans should be unique between histograms")
 }
 
-func TestHistogramUniqueCustomValuesAfterNextWithAtHistogram(t *testing.T) {
+func TestHistogramCustomValuesInternedAfterNextWithAtHistogram(t *testing.T) {
 	// Create two histograms with the same schema and custom values.
 	h1 := &histogram.Histogram{
 		Schema: -53,
@@ -1674,10 +1674,10 @@ func TestHistogramUniqueCustomValuesAfterNextWithAtHistogram(t *testing.T) {
 
 	// Check that the spans and custom values for h1 and h2 are unique slices.
 	require.NotSame(t, &rh1.PositiveSpans[0], &rh2.PositiveSpans[0], "PositiveSpans should be unique between histograms")
-	require.NotSame(t, &rh1.CustomValues[0], &rh2.CustomValues[0], "CustomValues should be unique between histograms")
+	require.Same(t, &rh1.CustomValues[0], &rh2.CustomValues[0], "CustomValues should be unique between histograms")
 }
 
-func TestHistogramUniqueCustomValuesAfterNextWithAtFloatHistogram(t *testing.T) {
+func TestHistogramCustomValuesInternedAfterNextWithAtFloatHistogram(t *testing.T) {
 	// Create two histograms with the same schema and custom values.
 	h1 := &histogram.Histogram{
 		Schema: -53,
@@ -1720,7 +1720,7 @@ func TestHistogramUniqueCustomValuesAfterNextWithAtFloatHistogram(t *testing.T)
 
 	// Check that the spans and custom values for h1 and h2 are unique slices.
 	require.NotSame(t, &rh1.PositiveSpans[0], &rh2.PositiveSpans[0], "PositiveSpans should be unique between histograms")
-	require.NotSame(t, &rh1.CustomValues[0], &rh2.CustomValues[0], "CustomValues should be unique between histograms")
+	require.Same(t, &rh1.CustomValues[0], &rh2.CustomValues[0], "CustomValues should be unique between histograms")
 }
 
 func BenchmarkAppendable(b *testing.B) {