From bf880a6e77326ea1863d8ddc17f1e360bed88718 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Wed, 16 Aug 2023 22:26:31 +0800 Subject: [PATCH 1/7] enhance floathistogram add and sub method Signed-off-by: Ziqi Zhao --- model/histogram/float_histogram.go | 158 ++++++++ model/histogram/float_histogram_test.go | 517 ++++++++++++++++++++++++ 2 files changed, 675 insertions(+) diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index d3f013935c..0a65dafbb4 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -238,6 +238,52 @@ func (h *FloatHistogram) Add(other *FloatHistogram) *FloatHistogram { return h } +func (h *FloatHistogram) AddNew(other *FloatHistogram) *FloatHistogram { + switch { + case other.CounterResetHint == h.CounterResetHint: + // Adding apples to apples, all good. No need to change anything. + case h.CounterResetHint == GaugeType: + // Adding something else to a gauge. That's probably OK. Outcome is a gauge. + // Nothing to do since the receiver is already marked as gauge. + case other.CounterResetHint == GaugeType: + // Similar to before, but this time the receiver is "something else" and we have to change it to gauge. + h.CounterResetHint = GaugeType + case h.CounterResetHint == UnknownCounterReset: + // With the receiver's CounterResetHint being "unknown", this could still be legitimate + // if the caller knows what they are doing. Outcome is then again "unknown". + // No need to do anything since the receiver's CounterResetHint is already "unknown". + case other.CounterResetHint == UnknownCounterReset: + // Similar to before, but now we have to set the receiver's CounterResetHint to "unknown". + h.CounterResetHint = UnknownCounterReset + default: + // All other cases shouldn't actually happen. + // They are a direct collision of CounterReset and NotCounterReset. + // Conservatively set the CounterResetHint to "unknown" and isse a warning. + h.CounterResetHint = UnknownCounterReset + // TODO(trevorwhitney): Actually issue the warning as soon as the plumbing for it is in place + } + + otherZeroCount := h.reconcileZeroBuckets(other) + h.ZeroCount += otherZeroCount + h.Count += other.Count + h.Sum += other.Sum + + otherPositiveSpans := other.PositiveSpans + otherPositiveBuckets := other.PositiveBuckets + otherNegativeSpans := other.NegativeSpans + otherNegativeBuckets := other.NegativeBuckets + if other.Schema != h.Schema { + otherPositiveSpans, otherPositiveBuckets = mergeToSchema(other.PositiveSpans, other.PositiveBuckets, other.Schema, h.Schema) + otherNegativeSpans, otherNegativeBuckets = mergeToSchema(other.NegativeSpans, other.NegativeBuckets, other.Schema, h.Schema) + } + + // TODO(beorn7): If needed, this can be optimized by inspecting the + // spans in other and create missing buckets in h in batches. + h.PositiveSpans, h.PositiveBuckets = mergeTwoSpans(h.Schema, h.ZeroThreshold, h.PositiveSpans, h.PositiveBuckets, otherPositiveSpans, otherPositiveBuckets) + h.NegativeSpans, h.NegativeBuckets = mergeTwoSpans(h.Schema, h.ZeroThreshold, h.NegativeSpans, h.NegativeBuckets, otherNegativeSpans, otherNegativeBuckets) + return h +} + // Sub works like Add but subtracts the other histogram. func (h *FloatHistogram) Sub(other *FloatHistogram) *FloatHistogram { otherZeroCount := h.reconcileZeroBuckets(other) @@ -1033,3 +1079,115 @@ func mergeToSchema(originSpans []Span, originBuckets []float64, originSchema, ta return targetSpans, targetBuckets } + +func mergeTwoSpans(schema int32, threshold float64, spansA []Span, bucketsA []float64, spansB []Span, bucketsB []float64) ([]Span, []float64) { + var ( + iSpan int = -1 + iBucket int = -1 + iInSpan int32 + indexA int32 + indexB int32 = 0 + bIdxB int = 0 + lowerThanThreshold = true + deltaIndex int32 + ) + + for _, spanB := range spansB { + indexB += spanB.Offset + for j := 0; j < int(spanB.Length); j++ { + if lowerThanThreshold && getBound(indexB, schema) <= threshold { + goto nextLoop + } + lowerThanThreshold = false + + if iSpan == -1 { + if len(spansA) == 0 || spansA[0].Offset > indexB { + // Add bucket before all others. + bucketsA = append(bucketsA, 0) + copy(bucketsA[1:], bucketsA) + bucketsA[0] = bucketsB[bIdxB] + if len(spansA) > 0 && spansA[0].Offset == indexB+1 { // bIndex just preceed spansA[0] by one step + spansA[0].Length++ + spansA[0].Offset-- + goto nextLoop + } else { // if not create new span + spansA = append(spansA, Span{}) + copy(spansA[1:], spansA) + spansA[0] = Span{Offset: indexB, Length: 1} + if len(spansA) > 1 { + // Convert the absolute offset in the formerly + // first span to a relative offset. + spansA[1].Offset -= indexB + 1 + } + goto nextLoop + } + } else if spansA[0].Offset == indexB { + // Just add to first bucket. + bucketsA[0] += bucketsB[bIdxB] + goto nextLoop + } + iSpan, iBucket, iInSpan = 0, 0, 0 + indexA = spansA[0].Offset + } + deltaIndex = indexB - indexA + for { + remainingInSpan := int32(spansA[iSpan].Length) - iInSpan + if deltaIndex < remainingInSpan { + // Bucket is in current span. + iBucket += int(deltaIndex) + iInSpan += deltaIndex + bucketsA[iBucket] += bucketsB[bIdxB] + break + } else { + deltaIndex -= remainingInSpan + iBucket += int(remainingInSpan) + iSpan++ + if iSpan == len(spansA) || deltaIndex < spansA[iSpan].Offset { + // Bucket is in gap behind previous span (or there are no further spans). + bucketsA = append(bucketsA, 0) + copy(bucketsA[iBucket+1:], bucketsA[iBucket:]) + bucketsA[iBucket] = bucketsB[bIdxB] + if deltaIndex == 0 { + // Directly after previous span, extend previous span. + if iSpan < len(spansA) { + spansA[iSpan].Offset-- + } + iSpan-- + iInSpan = int32(spansA[iSpan].Length) + spansA[iSpan].Length++ + break + } else if iSpan < len(spansA) && deltaIndex == spansA[iSpan].Offset-1 { + // Directly before next span, extend next span. + iInSpan = 0 + spansA[iSpan].Offset-- + spansA[iSpan].Length++ + break + } else { + // No next span, or next span is not directly adjacent to new bucket. + // Add new span. + iInSpan = 0 + if iSpan < len(spansA) { + spansA[iSpan].Offset -= deltaIndex + 1 + } + spansA = append(spansA, Span{}) + copy(spansA[iSpan+1:], spansA[iSpan:]) + spansA[iSpan] = Span{Length: 1, Offset: deltaIndex} + break + } + } else { + // Try start of next span. + deltaIndex -= spansA[iSpan].Offset + iInSpan = 0 + } + } + } + + nextLoop: + indexA = indexB + indexB++ + bIdxB++ + } + } + + return spansA, bucketsA +} diff --git a/model/histogram/float_histogram_test.go b/model/histogram/float_histogram_test.go index dd3e30427e..5d1e969314 100644 --- a/model/histogram/float_histogram_test.go +++ b/model/histogram/float_histogram_test.go @@ -2252,3 +2252,520 @@ func TestFloatBucketIteratorTargetSchema(t *testing.T) { } require.False(t, it.Next(), "negative iterator not exhausted") } + +func TestFloatHistogramAddNew(t *testing.T) { + cases := []struct { + name string + in1, in2, expected *FloatHistogram + }{ + { + "same bucket layout", + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + PositiveSpans: []Span{{-2, 2}, {1, 3}}, + PositiveBuckets: []float64{1, 0, 3, 4, 7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{3, 1, 5, 6}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + PositiveSpans: []Span{{-2, 2}, {1, 3}}, + PositiveBuckets: []float64{0, 0, 2, 3, 6}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 19, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{-2, 2}, {1, 3}}, + PositiveBuckets: []float64{1, 0, 5, 7, 13}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{4, 2, 9, 10}, + }, + }, + { + "same bucket layout, defined differently", + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + PositiveSpans: []Span{{-2, 2}, {1, 1}, {0, 2}}, + PositiveBuckets: []float64{1, 0, 3, 4, 7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{3, 1, 5, 6}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + PositiveSpans: []Span{{-2, 2}, {1, 2}, {0, 1}}, + PositiveBuckets: []float64{0, 0, 2, 3, 6}, + NegativeSpans: []Span{{3, 7}}, + NegativeBuckets: []float64{1, 1, 0, 0, 0, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 19, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{-2, 2}, {1, 1}, {0, 2}}, + PositiveBuckets: []float64{1, 0, 5, 7, 13}, + NegativeSpans: []Span{{3, 5}, {0, 2}}, + NegativeBuckets: []float64{4, 2, 0, 0, 0, 9, 10}, + }, + }, + { + "non-overlapping spans", + &FloatHistogram{ + ZeroThreshold: 0.001, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + PositiveSpans: []Span{{-2, 2}, {2, 3}}, + PositiveBuckets: []float64{1, 0, 3, 4, 7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{3, 1, 5, 6}, + }, + &FloatHistogram{ + ZeroThreshold: 0.001, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + PositiveSpans: []Span{{0, 2}, {3, 3}}, + PositiveBuckets: []float64{5, 4, 2, 3, 6}, + NegativeSpans: []Span{{-9, 2}, {3, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.001, + ZeroCount: 19, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{-2, 4}, {0, 6}}, + PositiveBuckets: []float64{1, 0, 5, 4, 3, 4, 7, 2, 3, 6}, + NegativeSpans: []Span{{-9, 2}, {3, 2}, {5, 2}, {3, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4, 3, 1, 5, 6}, + }, + }, + { + "non-overlapping inverted order", + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + PositiveSpans: []Span{{0, 2}, {3, 3}}, + PositiveBuckets: []float64{5, 4, 2, 3, 6}, + NegativeSpans: []Span{{-9, 2}, {3, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + PositiveSpans: []Span{{-2, 2}, {2, 3}}, + PositiveBuckets: []float64{1, 0, 3, 4, 7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{3, 1, 5, 6}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 19, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{-2, 2}, {0, 5}, {0, 3}}, + PositiveBuckets: []float64{1, 0, 5, 4, 3, 4, 7, 2, 3, 6}, + NegativeSpans: []Span{{-9, 2}, {3, 2}, {5, 2}, {3, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4, 3, 1, 5, 6}, + }, + }, + { + "overlapping spans", + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + PositiveSpans: []Span{{-2, 2}, {2, 3}}, + PositiveBuckets: []float64{1, 0, 3, 4, 7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{3, 1, 5, 6}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + PositiveSpans: []Span{{-1, 4}, {0, 3}}, + PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 19, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{-2, 4}, {0, 4}}, + PositiveBuckets: []float64{1, 5, 4, 2, 6, 10, 9, 5}, + NegativeSpans: []Span{{3, 3}, {1, 3}}, + NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, + }, + }, + { + "overlapping spans inverted order", + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + PositiveSpans: []Span{{-1, 4}, {0, 3}}, + PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + PositiveSpans: []Span{{-2, 2}, {2, 3}}, + PositiveBuckets: []float64{1, 0, 3, 4, 7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{3, 1, 5, 6}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 19, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{-2, 5}, {0, 3}}, + PositiveBuckets: []float64{1, 5, 4, 2, 6, 10, 9, 5}, + NegativeSpans: []Span{{3, 3}, {1, 3}}, + NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, + }, + }, + { + "schema change", + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + Schema: 0, + PositiveSpans: []Span{{-1, 4}, {0, 3}}, + PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + Schema: 1, + PositiveSpans: []Span{{-4, 3}, {5, 5}}, + PositiveBuckets: []float64{1, 0, 0, 3, 2, 2, 3, 4}, + NegativeSpans: []Span{{6, 3}, {6, 4}}, + NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 19, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{-2, 5}, {0, 3}}, + PositiveBuckets: []float64{1, 5, 4, 2, 6, 10, 9, 5}, + NegativeSpans: []Span{{3, 3}, {1, 3}}, + NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, + }, + }, + { + "larger zero bucket in first histogram", + &FloatHistogram{ + ZeroThreshold: 1, + ZeroCount: 17, + Count: 21, + Sum: 1.234, + PositiveSpans: []Span{{1, 2}, {0, 3}}, + PositiveBuckets: []float64{2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + PositiveSpans: []Span{{-2, 2}, {2, 3}}, + PositiveBuckets: []float64{1, 0, 3, 4, 7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{3, 1, 5, 6}, + }, + &FloatHistogram{ + ZeroThreshold: 1, + ZeroCount: 29, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{1, 2}, {0, 3}}, + PositiveBuckets: []float64{2, 6, 10, 9, 5}, + NegativeSpans: []Span{{3, 3}, {1, 3}}, + NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, + }, + }, + { + "larger zero bucket in second histogram", + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + PositiveSpans: []Span{{-2, 2}, {2, 3}}, + PositiveBuckets: []float64{1, 0, 3, 4, 7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{3, 1, 5, 6}, + }, + &FloatHistogram{ + ZeroThreshold: 1, + ZeroCount: 17, + Count: 21, + Sum: 1.234, + PositiveSpans: []Span{{1, 2}, {0, 3}}, + PositiveBuckets: []float64{2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 1, + ZeroCount: 29, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{1, 5}}, + PositiveBuckets: []float64{2, 6, 10, 9, 5}, + NegativeSpans: []Span{{3, 3}, {1, 3}}, + NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, + }, + }, + { + "larger zero threshold in first histogram ends up inside a populated bucket of second histogram", + &FloatHistogram{ + ZeroThreshold: 0.2, + ZeroCount: 17, + Count: 21, + Sum: 1.234, + PositiveSpans: []Span{{1, 2}, {0, 3}}, + PositiveBuckets: []float64{2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + PositiveSpans: []Span{{-2, 2}, {2, 3}}, + PositiveBuckets: []float64{1, 0, 3, 4, 7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{3, 1, 5, 6}, + }, + &FloatHistogram{ + ZeroThreshold: 0.25, + ZeroCount: 29, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{-1, 1}, {1, 5}}, + PositiveBuckets: []float64{0, 2, 6, 10, 9, 5}, + NegativeSpans: []Span{{3, 3}, {1, 3}}, + NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, + }, + }, + { + "larger zero threshold in second histogram ends up inside a populated bucket of first histogram", + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + PositiveSpans: []Span{{-2, 2}, {2, 3}}, + PositiveBuckets: []float64{1, 0, 3, 4, 7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{3, 1, 5, 6}, + }, + &FloatHistogram{ + ZeroThreshold: 0.2, + ZeroCount: 17, + Count: 21, + Sum: 1.234, + PositiveSpans: []Span{{1, 2}, {0, 3}}, + PositiveBuckets: []float64{2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.25, + ZeroCount: 29, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{1, 5}}, + PositiveBuckets: []float64{2, 6, 10, 9, 5}, + NegativeSpans: []Span{{3, 3}, {1, 3}}, + NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, + }, + }, + { + "schema change combined with larger zero bucket in second histogram", + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + Schema: 0, + PositiveSpans: []Span{{-2, 5}, {0, 3}}, + PositiveBuckets: []float64{2, 5, 4, 2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.25, + ZeroCount: 12, + Count: 30, + Sum: 2.345, + Schema: 1, + PositiveSpans: []Span{{-3, 2}, {5, 5}}, + PositiveBuckets: []float64{1, 0, 3, 2, 2, 3, 4}, + NegativeSpans: []Span{{6, 3}, {6, 4}}, + NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.25, + ZeroCount: 22, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{-1, 7}}, + PositiveBuckets: []float64{6, 4, 2, 6, 10, 9, 5}, + NegativeSpans: []Span{{3, 3}, {1, 3}}, + NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, + }, + }, + { + "schema change combined with larger zero bucket in first histogram", + &FloatHistogram{ + ZeroThreshold: 0.25, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + Schema: 0, + PositiveSpans: []Span{{-1, 4}, {0, 3}}, + PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + Schema: 1, + PositiveSpans: []Span{{-4, 3}, {5, 5}}, + PositiveBuckets: []float64{1, 0, 0, 3, 2, 2, 3, 4}, + NegativeSpans: []Span{{6, 3}, {6, 4}}, + NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, + }, + &FloatHistogram{ + ZeroThreshold: 0.25, + ZeroCount: 20, + Count: 51, + Sum: 3.579, + PositiveSpans: []Span{{-1, 4}, {0, 3}}, + PositiveBuckets: []float64{5, 4, 2, 6, 10, 9, 5}, + NegativeSpans: []Span{{3, 3}, {1, 3}}, + NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + require.Equal(t, c.expected, c.in1.AddNew(c.in2)) + // Has it also happened in-place? + require.Equal(t, c.expected, c.in1) + }) + } +} + +func BenchmarkAddOld(b *testing.B) { + // run the Fib function b.N times + + f1 := &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + Schema: 0, + PositiveSpans: []Span{{-1, 4}, {0, 3}}, + PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + } + + f2 := &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + Schema: 1, + PositiveSpans: []Span{{-4, 3}, {5, 5}}, + PositiveBuckets: []float64{1, 0, 0, 3, 2, 2, 3, 4}, + NegativeSpans: []Span{{6, 3}, {6, 4}}, + NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, + } + + for n := 0; n < b.N; n++ { + f1.Add(f2) + } + +} + +func BenchmarkAddNew(b *testing.B) { + // run the Fib function b.N times + + f1 := &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 8, + Count: 21, + Sum: 1.234, + Schema: 0, + PositiveSpans: []Span{{-1, 4}, {0, 3}}, + PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, + NegativeSpans: []Span{{4, 2}, {1, 2}}, + NegativeBuckets: []float64{1, 1, 4, 4}, + } + + f2 := &FloatHistogram{ + ZeroThreshold: 0.01, + ZeroCount: 11, + Count: 30, + Sum: 2.345, + Schema: 1, + PositiveSpans: []Span{{-4, 3}, {5, 5}}, + PositiveBuckets: []float64{1, 0, 0, 3, 2, 2, 3, 4}, + NegativeSpans: []Span{{6, 3}, {6, 4}}, + NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, + } + + for n := 0; n < b.N; n++ { + f1.AddNew(f2) + } + +} From 4787c879bce2b44a8a5634d94f1c8b847dc07875 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Mon, 21 Aug 2023 13:28:06 +0800 Subject: [PATCH 2/7] add more elaborate benchmark test Signed-off-by: Ziqi Zhao --- model/histogram/float_histogram.go | 6 +- model/histogram/float_histogram_test.go | 82 +++++++++---------------- 2 files changed, 32 insertions(+), 56 deletions(-) diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 0a65dafbb4..ffd991d86b 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -279,8 +279,8 @@ func (h *FloatHistogram) AddNew(other *FloatHistogram) *FloatHistogram { // TODO(beorn7): If needed, this can be optimized by inspecting the // spans in other and create missing buckets in h in batches. - h.PositiveSpans, h.PositiveBuckets = mergeTwoSpans(h.Schema, h.ZeroThreshold, h.PositiveSpans, h.PositiveBuckets, otherPositiveSpans, otherPositiveBuckets) - h.NegativeSpans, h.NegativeBuckets = mergeTwoSpans(h.Schema, h.ZeroThreshold, h.NegativeSpans, h.NegativeBuckets, otherNegativeSpans, otherNegativeBuckets) + h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, h.PositiveSpans, h.PositiveBuckets, otherPositiveSpans, otherPositiveBuckets) + h.NegativeSpans, h.NegativeBuckets = addBuckets(h.Schema, h.ZeroThreshold, h.NegativeSpans, h.NegativeBuckets, otherNegativeSpans, otherNegativeBuckets) return h } @@ -1080,7 +1080,7 @@ func mergeToSchema(originSpans []Span, originBuckets []float64, originSchema, ta return targetSpans, targetBuckets } -func mergeTwoSpans(schema int32, threshold float64, spansA []Span, bucketsA []float64, spansB []Span, bucketsB []float64) ([]Span, []float64) { +func addBuckets(schema int32, threshold float64, spansA []Span, bucketsA []float64, spansB []Span, bucketsB []float64) ([]Span, []float64) { var ( iSpan int = -1 iBucket int = -1 diff --git a/model/histogram/float_histogram_test.go b/model/histogram/float_histogram_test.go index 5d1e969314..bef9a1d6c6 100644 --- a/model/histogram/float_histogram_test.go +++ b/model/histogram/float_histogram_test.go @@ -16,6 +16,7 @@ package histogram import ( "fmt" "math" + "math/rand" "testing" "github.com/stretchr/testify/require" @@ -2705,67 +2706,42 @@ func TestFloatHistogramAddNew(t *testing.T) { } func BenchmarkAddOld(b *testing.B) { - // run the Fib function b.N times - - f1 := &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - Schema: 0, - PositiveSpans: []Span{{-1, 4}, {0, 3}}, - PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - } - - f2 := &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - Schema: 1, - PositiveSpans: []Span{{-4, 3}, {5, 5}}, - PositiveBuckets: []float64{1, 0, 0, 3, 2, 2, 3, 4}, - NegativeSpans: []Span{{6, 3}, {6, 4}}, - NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, - } - for n := 0; n < b.N; n++ { + b.StopTimer() + f1 := createRandomFloatHistogram(50) + f2 := createRandomFloatHistogram(50) + b.StartTimer() f1.Add(f2) } } func BenchmarkAddNew(b *testing.B) { - // run the Fib function b.N times - - f1 := &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - Schema: 0, - PositiveSpans: []Span{{-1, 4}, {0, 3}}, - PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - } - - f2 := &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - Schema: 1, - PositiveSpans: []Span{{-4, 3}, {5, 5}}, - PositiveBuckets: []float64{1, 0, 0, 3, 2, 2, 3, 4}, - NegativeSpans: []Span{{6, 3}, {6, 4}}, - NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, - } - for n := 0; n < b.N; n++ { + b.StopTimer() + f1 := createRandomFloatHistogram(50) + f2 := createRandomFloatHistogram(50) + b.StartTimer() f1.AddNew(f2) } - +} + +func createRandomFloatHistogram(spanNum int32) *FloatHistogram { + f := &FloatHistogram{} + f.PositiveSpans, f.PositiveBuckets = createRandomSpans(spanNum) + f.NegativeSpans, f.NegativeBuckets = createRandomSpans(spanNum) + return f +} + +func createRandomSpans(spanNum int32) ([]Span, []float64) { + Spans := make([]Span, spanNum) + Buckets := make([]float64, 0) + for i := 0; i < int(spanNum); i++ { + Spans[i].Offset = rand.Int31n(spanNum) + 1 + Spans[i].Length = uint32(rand.Int31n(spanNum) + 1) + for j := 0; j < int(Spans[i].Length); j++ { + Buckets = append(Buckets, float64(rand.Int31n(spanNum)+1)) + } + } + return Spans, Buckets } From eab3c93e80e4d098b4478d4d2687c5ca593b70b7 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Wed, 23 Aug 2023 12:52:24 +0800 Subject: [PATCH 3/7] make code ready for review Signed-off-by: Ziqi Zhao --- model/histogram/float_histogram.go | 112 ++---- model/histogram/float_histogram_test.go | 493 ------------------------ 2 files changed, 30 insertions(+), 575 deletions(-) diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index ffd991d86b..7c0ea86726 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -218,56 +218,6 @@ func (h *FloatHistogram) Add(other *FloatHistogram) *FloatHistogram { h.Count += other.Count h.Sum += other.Sum - // TODO(beorn7): If needed, this can be optimized by inspecting the - // spans in other and create missing buckets in h in batches. - var iInSpan, index int32 - for iSpan, iBucket, it := -1, -1, other.floatBucketIterator(true, h.ZeroThreshold, h.Schema); it.Next(); { - b := it.At() - h.PositiveSpans, h.PositiveBuckets, iSpan, iBucket, iInSpan = addBucket( - b, h.PositiveSpans, h.PositiveBuckets, iSpan, iBucket, iInSpan, index, - ) - index = b.Index - } - for iSpan, iBucket, it := -1, -1, other.floatBucketIterator(false, h.ZeroThreshold, h.Schema); it.Next(); { - b := it.At() - h.NegativeSpans, h.NegativeBuckets, iSpan, iBucket, iInSpan = addBucket( - b, h.NegativeSpans, h.NegativeBuckets, iSpan, iBucket, iInSpan, index, - ) - index = b.Index - } - return h -} - -func (h *FloatHistogram) AddNew(other *FloatHistogram) *FloatHistogram { - switch { - case other.CounterResetHint == h.CounterResetHint: - // Adding apples to apples, all good. No need to change anything. - case h.CounterResetHint == GaugeType: - // Adding something else to a gauge. That's probably OK. Outcome is a gauge. - // Nothing to do since the receiver is already marked as gauge. - case other.CounterResetHint == GaugeType: - // Similar to before, but this time the receiver is "something else" and we have to change it to gauge. - h.CounterResetHint = GaugeType - case h.CounterResetHint == UnknownCounterReset: - // With the receiver's CounterResetHint being "unknown", this could still be legitimate - // if the caller knows what they are doing. Outcome is then again "unknown". - // No need to do anything since the receiver's CounterResetHint is already "unknown". - case other.CounterResetHint == UnknownCounterReset: - // Similar to before, but now we have to set the receiver's CounterResetHint to "unknown". - h.CounterResetHint = UnknownCounterReset - default: - // All other cases shouldn't actually happen. - // They are a direct collision of CounterReset and NotCounterReset. - // Conservatively set the CounterResetHint to "unknown" and isse a warning. - h.CounterResetHint = UnknownCounterReset - // TODO(trevorwhitney): Actually issue the warning as soon as the plumbing for it is in place - } - - otherZeroCount := h.reconcileZeroBuckets(other) - h.ZeroCount += otherZeroCount - h.Count += other.Count - h.Sum += other.Sum - otherPositiveSpans := other.PositiveSpans otherPositiveBuckets := other.PositiveBuckets otherNegativeSpans := other.NegativeSpans @@ -277,10 +227,8 @@ func (h *FloatHistogram) AddNew(other *FloatHistogram) *FloatHistogram { otherNegativeSpans, otherNegativeBuckets = mergeToSchema(other.NegativeSpans, other.NegativeBuckets, other.Schema, h.Schema) } - // TODO(beorn7): If needed, this can be optimized by inspecting the - // spans in other and create missing buckets in h in batches. - h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, h.PositiveSpans, h.PositiveBuckets, otherPositiveSpans, otherPositiveBuckets) - h.NegativeSpans, h.NegativeBuckets = addBuckets(h.Schema, h.ZeroThreshold, h.NegativeSpans, h.NegativeBuckets, otherNegativeSpans, otherNegativeBuckets) + h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, h.PositiveSpans, h.PositiveBuckets, otherPositiveSpans, otherPositiveBuckets) + h.NegativeSpans, h.NegativeBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, h.NegativeSpans, h.NegativeBuckets, otherNegativeSpans, otherNegativeBuckets) return h } @@ -291,25 +239,17 @@ func (h *FloatHistogram) Sub(other *FloatHistogram) *FloatHistogram { h.Count -= other.Count h.Sum -= other.Sum - // TODO(beorn7): If needed, this can be optimized by inspecting the - // spans in other and create missing buckets in h in batches. - var iInSpan, index int32 - for iSpan, iBucket, it := -1, -1, other.floatBucketIterator(true, h.ZeroThreshold, h.Schema); it.Next(); { - b := it.At() - b.Count *= -1 - h.PositiveSpans, h.PositiveBuckets, iSpan, iBucket, iInSpan = addBucket( - b, h.PositiveSpans, h.PositiveBuckets, iSpan, iBucket, iInSpan, index, - ) - index = b.Index - } - for iSpan, iBucket, it := -1, -1, other.floatBucketIterator(false, h.ZeroThreshold, h.Schema); it.Next(); { - b := it.At() - b.Count *= -1 - h.NegativeSpans, h.NegativeBuckets, iSpan, iBucket, iInSpan = addBucket( - b, h.NegativeSpans, h.NegativeBuckets, iSpan, iBucket, iInSpan, index, - ) - index = b.Index + otherPositiveSpans := other.PositiveSpans + otherPositiveBuckets := other.PositiveBuckets + otherNegativeSpans := other.NegativeSpans + otherNegativeBuckets := other.NegativeBuckets + if other.Schema != h.Schema { + otherPositiveSpans, otherPositiveBuckets = mergeToSchema(other.PositiveSpans, other.PositiveBuckets, other.Schema, h.Schema) + otherNegativeSpans, otherNegativeBuckets = mergeToSchema(other.NegativeSpans, other.NegativeBuckets, other.Schema, h.Schema) } + + h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, h.PositiveSpans, h.PositiveBuckets, otherPositiveSpans, otherPositiveBuckets) + h.NegativeSpans, h.NegativeBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, h.NegativeSpans, h.NegativeBuckets, otherNegativeSpans, otherNegativeBuckets) return h } @@ -1080,16 +1020,19 @@ func mergeToSchema(originSpans []Span, originBuckets []float64, originSchema, ta return targetSpans, targetBuckets } -func addBuckets(schema int32, threshold float64, spansA []Span, bucketsA []float64, spansB []Span, bucketsB []float64) ([]Span, []float64) { +// addBuckets add two groups of Spans by inspecting the +// spans in other and create missing buckets in origin in batches. +func addBuckets(schema int32, threshold float64, negative bool, spansA []Span, bucketsA []float64, spansB []Span, bucketsB []float64) ([]Span, []float64) { var ( iSpan int = -1 iBucket int = -1 iInSpan int32 indexA int32 - indexB int32 = 0 - bIdxB int = 0 - lowerThanThreshold = true + indexB int32 + bIdxB int + bucketB float64 deltaIndex int32 + lowerThanThreshold = true ) for _, spanB := range spansB { @@ -1100,17 +1043,22 @@ func addBuckets(schema int32, threshold float64, spansA []Span, bucketsA []float } lowerThanThreshold = false + bucketB = bucketsB[bIdxB] + if negative { + bucketB *= -1 + } + if iSpan == -1 { if len(spansA) == 0 || spansA[0].Offset > indexB { // Add bucket before all others. bucketsA = append(bucketsA, 0) copy(bucketsA[1:], bucketsA) - bucketsA[0] = bucketsB[bIdxB] - if len(spansA) > 0 && spansA[0].Offset == indexB+1 { // bIndex just preceed spansA[0] by one step + bucketsA[0] = bucketB + if len(spansA) > 0 && spansA[0].Offset == indexB+1 { spansA[0].Length++ spansA[0].Offset-- goto nextLoop - } else { // if not create new span + } else { spansA = append(spansA, Span{}) copy(spansA[1:], spansA) spansA[0] = Span{Offset: indexB, Length: 1} @@ -1123,7 +1071,7 @@ func addBuckets(schema int32, threshold float64, spansA []Span, bucketsA []float } } else if spansA[0].Offset == indexB { // Just add to first bucket. - bucketsA[0] += bucketsB[bIdxB] + bucketsA[0] += bucketB goto nextLoop } iSpan, iBucket, iInSpan = 0, 0, 0 @@ -1136,7 +1084,7 @@ func addBuckets(schema int32, threshold float64, spansA []Span, bucketsA []float // Bucket is in current span. iBucket += int(deltaIndex) iInSpan += deltaIndex - bucketsA[iBucket] += bucketsB[bIdxB] + bucketsA[iBucket] += bucketB break } else { deltaIndex -= remainingInSpan @@ -1146,7 +1094,7 @@ func addBuckets(schema int32, threshold float64, spansA []Span, bucketsA []float // Bucket is in gap behind previous span (or there are no further spans). bucketsA = append(bucketsA, 0) copy(bucketsA[iBucket+1:], bucketsA[iBucket:]) - bucketsA[iBucket] = bucketsB[bIdxB] + bucketsA[iBucket] = bucketB if deltaIndex == 0 { // Directly after previous span, extend previous span. if iSpan < len(spansA) { diff --git a/model/histogram/float_histogram_test.go b/model/histogram/float_histogram_test.go index bef9a1d6c6..dd3e30427e 100644 --- a/model/histogram/float_histogram_test.go +++ b/model/histogram/float_histogram_test.go @@ -16,7 +16,6 @@ package histogram import ( "fmt" "math" - "math/rand" "testing" "github.com/stretchr/testify/require" @@ -2253,495 +2252,3 @@ func TestFloatBucketIteratorTargetSchema(t *testing.T) { } require.False(t, it.Next(), "negative iterator not exhausted") } - -func TestFloatHistogramAddNew(t *testing.T) { - cases := []struct { - name string - in1, in2, expected *FloatHistogram - }{ - { - "same bucket layout", - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - PositiveSpans: []Span{{-2, 2}, {1, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{3, 1, 5, 6}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - PositiveSpans: []Span{{-2, 2}, {1, 3}}, - PositiveBuckets: []float64{0, 0, 2, 3, 6}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 19, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{-2, 2}, {1, 3}}, - PositiveBuckets: []float64{1, 0, 5, 7, 13}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{4, 2, 9, 10}, - }, - }, - { - "same bucket layout, defined differently", - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - PositiveSpans: []Span{{-2, 2}, {1, 1}, {0, 2}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{3, 1, 5, 6}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - PositiveSpans: []Span{{-2, 2}, {1, 2}, {0, 1}}, - PositiveBuckets: []float64{0, 0, 2, 3, 6}, - NegativeSpans: []Span{{3, 7}}, - NegativeBuckets: []float64{1, 1, 0, 0, 0, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 19, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{-2, 2}, {1, 1}, {0, 2}}, - PositiveBuckets: []float64{1, 0, 5, 7, 13}, - NegativeSpans: []Span{{3, 5}, {0, 2}}, - NegativeBuckets: []float64{4, 2, 0, 0, 0, 9, 10}, - }, - }, - { - "non-overlapping spans", - &FloatHistogram{ - ZeroThreshold: 0.001, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - PositiveSpans: []Span{{-2, 2}, {2, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{3, 1, 5, 6}, - }, - &FloatHistogram{ - ZeroThreshold: 0.001, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - PositiveSpans: []Span{{0, 2}, {3, 3}}, - PositiveBuckets: []float64{5, 4, 2, 3, 6}, - NegativeSpans: []Span{{-9, 2}, {3, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.001, - ZeroCount: 19, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{-2, 4}, {0, 6}}, - PositiveBuckets: []float64{1, 0, 5, 4, 3, 4, 7, 2, 3, 6}, - NegativeSpans: []Span{{-9, 2}, {3, 2}, {5, 2}, {3, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4, 3, 1, 5, 6}, - }, - }, - { - "non-overlapping inverted order", - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - PositiveSpans: []Span{{0, 2}, {3, 3}}, - PositiveBuckets: []float64{5, 4, 2, 3, 6}, - NegativeSpans: []Span{{-9, 2}, {3, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - PositiveSpans: []Span{{-2, 2}, {2, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{3, 1, 5, 6}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 19, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{-2, 2}, {0, 5}, {0, 3}}, - PositiveBuckets: []float64{1, 0, 5, 4, 3, 4, 7, 2, 3, 6}, - NegativeSpans: []Span{{-9, 2}, {3, 2}, {5, 2}, {3, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4, 3, 1, 5, 6}, - }, - }, - { - "overlapping spans", - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - PositiveSpans: []Span{{-2, 2}, {2, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{3, 1, 5, 6}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - PositiveSpans: []Span{{-1, 4}, {0, 3}}, - PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 19, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{-2, 4}, {0, 4}}, - PositiveBuckets: []float64{1, 5, 4, 2, 6, 10, 9, 5}, - NegativeSpans: []Span{{3, 3}, {1, 3}}, - NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, - }, - }, - { - "overlapping spans inverted order", - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - PositiveSpans: []Span{{-1, 4}, {0, 3}}, - PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - PositiveSpans: []Span{{-2, 2}, {2, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{3, 1, 5, 6}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 19, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{-2, 5}, {0, 3}}, - PositiveBuckets: []float64{1, 5, 4, 2, 6, 10, 9, 5}, - NegativeSpans: []Span{{3, 3}, {1, 3}}, - NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, - }, - }, - { - "schema change", - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - Schema: 0, - PositiveSpans: []Span{{-1, 4}, {0, 3}}, - PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - Schema: 1, - PositiveSpans: []Span{{-4, 3}, {5, 5}}, - PositiveBuckets: []float64{1, 0, 0, 3, 2, 2, 3, 4}, - NegativeSpans: []Span{{6, 3}, {6, 4}}, - NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 19, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{-2, 5}, {0, 3}}, - PositiveBuckets: []float64{1, 5, 4, 2, 6, 10, 9, 5}, - NegativeSpans: []Span{{3, 3}, {1, 3}}, - NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, - }, - }, - { - "larger zero bucket in first histogram", - &FloatHistogram{ - ZeroThreshold: 1, - ZeroCount: 17, - Count: 21, - Sum: 1.234, - PositiveSpans: []Span{{1, 2}, {0, 3}}, - PositiveBuckets: []float64{2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - PositiveSpans: []Span{{-2, 2}, {2, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{3, 1, 5, 6}, - }, - &FloatHistogram{ - ZeroThreshold: 1, - ZeroCount: 29, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{1, 2}, {0, 3}}, - PositiveBuckets: []float64{2, 6, 10, 9, 5}, - NegativeSpans: []Span{{3, 3}, {1, 3}}, - NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, - }, - }, - { - "larger zero bucket in second histogram", - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - PositiveSpans: []Span{{-2, 2}, {2, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{3, 1, 5, 6}, - }, - &FloatHistogram{ - ZeroThreshold: 1, - ZeroCount: 17, - Count: 21, - Sum: 1.234, - PositiveSpans: []Span{{1, 2}, {0, 3}}, - PositiveBuckets: []float64{2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 1, - ZeroCount: 29, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{1, 5}}, - PositiveBuckets: []float64{2, 6, 10, 9, 5}, - NegativeSpans: []Span{{3, 3}, {1, 3}}, - NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, - }, - }, - { - "larger zero threshold in first histogram ends up inside a populated bucket of second histogram", - &FloatHistogram{ - ZeroThreshold: 0.2, - ZeroCount: 17, - Count: 21, - Sum: 1.234, - PositiveSpans: []Span{{1, 2}, {0, 3}}, - PositiveBuckets: []float64{2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - PositiveSpans: []Span{{-2, 2}, {2, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{3, 1, 5, 6}, - }, - &FloatHistogram{ - ZeroThreshold: 0.25, - ZeroCount: 29, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{-1, 1}, {1, 5}}, - PositiveBuckets: []float64{0, 2, 6, 10, 9, 5}, - NegativeSpans: []Span{{3, 3}, {1, 3}}, - NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, - }, - }, - { - "larger zero threshold in second histogram ends up inside a populated bucket of first histogram", - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - PositiveSpans: []Span{{-2, 2}, {2, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{3, 1, 5, 6}, - }, - &FloatHistogram{ - ZeroThreshold: 0.2, - ZeroCount: 17, - Count: 21, - Sum: 1.234, - PositiveSpans: []Span{{1, 2}, {0, 3}}, - PositiveBuckets: []float64{2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.25, - ZeroCount: 29, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{1, 5}}, - PositiveBuckets: []float64{2, 6, 10, 9, 5}, - NegativeSpans: []Span{{3, 3}, {1, 3}}, - NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, - }, - }, - { - "schema change combined with larger zero bucket in second histogram", - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - Schema: 0, - PositiveSpans: []Span{{-2, 5}, {0, 3}}, - PositiveBuckets: []float64{2, 5, 4, 2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.25, - ZeroCount: 12, - Count: 30, - Sum: 2.345, - Schema: 1, - PositiveSpans: []Span{{-3, 2}, {5, 5}}, - PositiveBuckets: []float64{1, 0, 3, 2, 2, 3, 4}, - NegativeSpans: []Span{{6, 3}, {6, 4}}, - NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.25, - ZeroCount: 22, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{-1, 7}}, - PositiveBuckets: []float64{6, 4, 2, 6, 10, 9, 5}, - NegativeSpans: []Span{{3, 3}, {1, 3}}, - NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, - }, - }, - { - "schema change combined with larger zero bucket in first histogram", - &FloatHistogram{ - ZeroThreshold: 0.25, - ZeroCount: 8, - Count: 21, - Sum: 1.234, - Schema: 0, - PositiveSpans: []Span{{-1, 4}, {0, 3}}, - PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, - NegativeSpans: []Span{{4, 2}, {1, 2}}, - NegativeBuckets: []float64{1, 1, 4, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 11, - Count: 30, - Sum: 2.345, - Schema: 1, - PositiveSpans: []Span{{-4, 3}, {5, 5}}, - PositiveBuckets: []float64{1, 0, 0, 3, 2, 2, 3, 4}, - NegativeSpans: []Span{{6, 3}, {6, 4}}, - NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, - }, - &FloatHistogram{ - ZeroThreshold: 0.25, - ZeroCount: 20, - Count: 51, - Sum: 3.579, - PositiveSpans: []Span{{-1, 4}, {0, 3}}, - PositiveBuckets: []float64{5, 4, 2, 6, 10, 9, 5}, - NegativeSpans: []Span{{3, 3}, {1, 3}}, - NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, - }, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - require.Equal(t, c.expected, c.in1.AddNew(c.in2)) - // Has it also happened in-place? - require.Equal(t, c.expected, c.in1) - }) - } -} - -func BenchmarkAddOld(b *testing.B) { - for n := 0; n < b.N; n++ { - b.StopTimer() - f1 := createRandomFloatHistogram(50) - f2 := createRandomFloatHistogram(50) - b.StartTimer() - f1.Add(f2) - } - -} - -func BenchmarkAddNew(b *testing.B) { - for n := 0; n < b.N; n++ { - b.StopTimer() - f1 := createRandomFloatHistogram(50) - f2 := createRandomFloatHistogram(50) - b.StartTimer() - f1.AddNew(f2) - } -} - -func createRandomFloatHistogram(spanNum int32) *FloatHistogram { - f := &FloatHistogram{} - f.PositiveSpans, f.PositiveBuckets = createRandomSpans(spanNum) - f.NegativeSpans, f.NegativeBuckets = createRandomSpans(spanNum) - return f -} - -func createRandomSpans(spanNum int32) ([]Span, []float64) { - Spans := make([]Span, spanNum) - Buckets := make([]float64, 0) - for i := 0; i < int(spanNum); i++ { - Spans[i].Offset = rand.Int31n(spanNum) + 1 - Spans[i].Length = uint32(rand.Int31n(spanNum) + 1) - for j := 0; j < int(Spans[i].Length); j++ { - Buckets = append(Buckets, float64(rand.Int31n(spanNum)+1)) - } - } - return Spans, Buckets -} From 788061e50933ea140e84b36b3852acb0dc54c76f Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Wed, 23 Aug 2023 12:55:59 +0800 Subject: [PATCH 4/7] remove unused addBucket function Signed-off-by: Ziqi Zhao --- model/histogram/float_histogram.go | 97 ------------------------------ 1 file changed, 97 deletions(-) diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 7c0ea86726..539ee73b95 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -284,103 +284,6 @@ func (h *FloatHistogram) Equals(h2 *FloatHistogram) bool { return true } -// addBucket takes the "coordinates" of the last bucket that was handled and -// adds the provided bucket after it. If a corresponding bucket exists, the -// count is added. If not, the bucket is inserted. The updated slices and the -// coordinates of the inserted or added-to bucket are returned. -func addBucket( - b Bucket[float64], - spans []Span, buckets []float64, - iSpan, iBucket int, - iInSpan, index int32, -) ( - newSpans []Span, newBuckets []float64, - newISpan, newIBucket int, newIInSpan int32, -) { - if iSpan == -1 { - // First add, check if it is before all spans. - if len(spans) == 0 || spans[0].Offset > b.Index { - // Add bucket before all others. - buckets = append(buckets, 0) - copy(buckets[1:], buckets) - buckets[0] = b.Count - if len(spans) > 0 && spans[0].Offset == b.Index+1 { - spans[0].Length++ - spans[0].Offset-- - return spans, buckets, 0, 0, 0 - } - spans = append(spans, Span{}) - copy(spans[1:], spans) - spans[0] = Span{Offset: b.Index, Length: 1} - if len(spans) > 1 { - // Convert the absolute offset in the formerly - // first span to a relative offset. - spans[1].Offset -= b.Index + 1 - } - return spans, buckets, 0, 0, 0 - } - if spans[0].Offset == b.Index { - // Just add to first bucket. - buckets[0] += b.Count - return spans, buckets, 0, 0, 0 - } - // We are behind the first bucket, so set everything to the - // first bucket and continue normally. - iSpan, iBucket, iInSpan = 0, 0, 0 - index = spans[0].Offset - } - deltaIndex := b.Index - index - for { - remainingInSpan := int32(spans[iSpan].Length) - iInSpan - if deltaIndex < remainingInSpan { - // Bucket is in current span. - iBucket += int(deltaIndex) - iInSpan += deltaIndex - buckets[iBucket] += b.Count - return spans, buckets, iSpan, iBucket, iInSpan - } - deltaIndex -= remainingInSpan - iBucket += int(remainingInSpan) - iSpan++ - if iSpan == len(spans) || deltaIndex < spans[iSpan].Offset { - // Bucket is in gap behind previous span (or there are no further spans). - buckets = append(buckets, 0) - copy(buckets[iBucket+1:], buckets[iBucket:]) - buckets[iBucket] = b.Count - if deltaIndex == 0 { - // Directly after previous span, extend previous span. - if iSpan < len(spans) { - spans[iSpan].Offset-- - } - iSpan-- - iInSpan = int32(spans[iSpan].Length) - spans[iSpan].Length++ - return spans, buckets, iSpan, iBucket, iInSpan - } - if iSpan < len(spans) && deltaIndex == spans[iSpan].Offset-1 { - // Directly before next span, extend next span. - iInSpan = 0 - spans[iSpan].Offset-- - spans[iSpan].Length++ - return spans, buckets, iSpan, iBucket, iInSpan - } - // No next span, or next span is not directly adjacent to new bucket. - // Add new span. - iInSpan = 0 - if iSpan < len(spans) { - spans[iSpan].Offset -= deltaIndex + 1 - } - spans = append(spans, Span{}) - copy(spans[iSpan+1:], spans[iSpan:]) - spans[iSpan] = Span{Length: 1, Offset: deltaIndex} - return spans, buckets, iSpan, iBucket, iInSpan - } - // Try start of next span. - deltaIndex -= spans[iSpan].Offset - iInSpan = 0 - } -} - // Compact eliminates empty buckets at the beginning and end of each span, then // merges spans that are consecutive or at most maxEmptyBuckets apart, and // finally splits spans that contain more consecutive empty buckets than From 893f97556f7648821121180fb8e8fb0f2aa58053 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Wed, 23 Aug 2023 13:13:25 +0800 Subject: [PATCH 5/7] use switch instead of if-else to fix lint error Signed-off-by: Ziqi Zhao --- model/histogram/float_histogram.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 539ee73b95..360fa68234 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -998,7 +998,8 @@ func addBuckets(schema int32, threshold float64, negative bool, spansA []Span, b bucketsA = append(bucketsA, 0) copy(bucketsA[iBucket+1:], bucketsA[iBucket:]) bucketsA[iBucket] = bucketB - if deltaIndex == 0 { + switch { + case deltaIndex == 0: // Directly after previous span, extend previous span. if iSpan < len(spansA) { spansA[iSpan].Offset-- @@ -1006,14 +1007,14 @@ func addBuckets(schema int32, threshold float64, negative bool, spansA []Span, b iSpan-- iInSpan = int32(spansA[iSpan].Length) spansA[iSpan].Length++ - break - } else if iSpan < len(spansA) && deltaIndex == spansA[iSpan].Offset-1 { + goto nextLoop + case iSpan < len(spansA) && deltaIndex == spansA[iSpan].Offset-1: // Directly before next span, extend next span. iInSpan = 0 spansA[iSpan].Offset-- spansA[iSpan].Length++ - break - } else { + goto nextLoop + default: // No next span, or next span is not directly adjacent to new bucket. // Add new span. iInSpan = 0 @@ -1023,7 +1024,7 @@ func addBuckets(schema int32, threshold float64, negative bool, spansA []Span, b spansA = append(spansA, Span{}) copy(spansA[iSpan+1:], spansA[iSpan:]) spansA[iSpan] = Span{Length: 1, Offset: deltaIndex} - break + goto nextLoop } } else { // Try start of next span. From d3633d4e764a71a0aff925c699ba0f30d40fd60d Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Thu, 24 Aug 2023 07:17:23 +0800 Subject: [PATCH 6/7] Update model/histogram/float_histogram.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Björn Rabenstein Signed-off-by: Ziqi Zhao --- model/histogram/float_histogram.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 360fa68234..d7de64abfe 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -923,9 +923,18 @@ func mergeToSchema(originSpans []Span, originBuckets []float64, originSchema, ta return targetSpans, targetBuckets } -// addBuckets add two groups of Spans by inspecting the -// spans in other and create missing buckets in origin in batches. -func addBuckets(schema int32, threshold float64, negative bool, spansA []Span, bucketsA []float64, spansB []Span, bucketsB []float64) ([]Span, []float64) { +// addBuckets adds the buckets described by spansB/bucketsB to the buckets described by spansA/bucketsA, +// creating missing buckets in spansA/bucketsA as needed. +// It returns the resulting spans/buckets (which must be used instead of the original spansA/bucketsA, +// although spansA/bucketsA might get modified by this function). +// All buckets must use the same provided schema. +// Buckets in spansB/bucketsB with an absolute upper limit ≤ threshold are ignored. +// If negative is true, the buckets in spansB/bucketsB are subtracted rather than added. +func addBuckets( + schema int32, threshold float64, negative bool, + spansA []Span, bucketsA []float64, + spansB []Span, bucketsB []float64, +) ([]Span, []float64) { var ( iSpan int = -1 iBucket int = -1 From de172049abc9b795e226c2153fcfaea48804bbf9 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Thu, 24 Aug 2023 07:27:33 +0800 Subject: [PATCH 7/7] fix lint error Signed-off-by: Ziqi Zhao --- model/histogram/float_histogram.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index d7de64abfe..bfb3c3d19f 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -931,8 +931,8 @@ func mergeToSchema(originSpans []Span, originBuckets []float64, originSchema, ta // Buckets in spansB/bucketsB with an absolute upper limit ≤ threshold are ignored. // If negative is true, the buckets in spansB/bucketsB are subtracted rather than added. func addBuckets( - schema int32, threshold float64, negative bool, - spansA []Span, bucketsA []float64, + schema int32, threshold float64, negative bool, + spansA []Span, bucketsA []float64, spansB []Span, bucketsB []float64, ) ([]Span, []float64) { var (