diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 0a2b43951e..92f084bdf6 100644 --- a/model/histogram/float_histogram.go +++ b/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)) diff --git a/model/histogram/histogram.go b/model/histogram/histogram.go index 778aefe282..cfb63e6341 100644 --- a/model/histogram/histogram.go +++ b/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)) diff --git a/promql/bench_test.go b/promql/bench_test.go index 9741a02102..695b2c1eb4 100644 --- a/promql/bench_test.go +++ b/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", diff --git a/tsdb/chunkenc/float_histogram.go b/tsdb/chunkenc/float_histogram.go index 0da00dcda4..e5ad4028bb 100644 --- a/tsdb/chunkenc/float_histogram.go +++ b/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 } diff --git a/tsdb/chunkenc/histogram.go b/tsdb/chunkenc/histogram.go index 7f528df8d5..0f54eb6928 100644 --- a/tsdb/chunkenc/histogram.go +++ b/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 { diff --git a/tsdb/chunkenc/histogram_test.go b/tsdb/chunkenc/histogram_test.go index fb36b232c4..304a9858f1 100644 --- a/tsdb/chunkenc/histogram_test.go +++ b/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) {