From 6c646657d5d48266c4beeaaf709e15737fffcf95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Wed, 7 May 2025 14:29:35 +0200 Subject: [PATCH] perf(chunkenc): intern the custom values for native histograms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The custom values are the "le" bucket boundaries of native histograms with custom buckets. They are never modified. It is ok to not copy them when iterating a chunk, just reference them. If we will ever have a function that modifies the custom values, like 'trim' for example. That function will have to make a copy on write. Signed-off-by: György Krajcsovits --- model/histogram/float_histogram.go | 14 ++++++-------- model/histogram/histogram.go | 23 ++++++++++------------- tsdb/chunkenc/float_histogram.go | 4 ++-- tsdb/chunkenc/histogram.go | 22 ++++++++-------------- tsdb/chunkenc/histogram_test.go | 8 ++++---- 5 files changed, 30 insertions(+), 41 deletions(-) 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/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) {