From dd877fe179f38c0d16a8e6793d7b8e036b3f19d3 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 16 Feb 2024 11:37:52 -0800 Subject: [PATCH] [sdk-metrics] Improve the perf of histogram snapshots (#5366) --- src/OpenTelemetry/Metrics/HistogramBuckets.cs | 43 ++++++++++++--- src/OpenTelemetry/Metrics/MetricPoint.cs | 52 +++---------------- .../Metrics/MetricApiTestsBase.cs | 2 +- .../Metrics/MetricPointTests.cs | 4 +- 4 files changed, 45 insertions(+), 56 deletions(-) diff --git a/src/OpenTelemetry/Metrics/HistogramBuckets.cs b/src/OpenTelemetry/Metrics/HistogramBuckets.cs index 78b47a35e1c..b6e8d3ae632 100644 --- a/src/OpenTelemetry/Metrics/HistogramBuckets.cs +++ b/src/OpenTelemetry/Metrics/HistogramBuckets.cs @@ -16,8 +16,7 @@ public class HistogramBuckets internal readonly double[]? ExplicitBounds; - internal readonly long[]? RunningBucketCounts; - internal readonly long[] SnapshotBucketCounts; + internal readonly HistogramBucketValues[] BucketCounts; internal double RunningSum; internal double SnapshotSum; @@ -62,8 +61,7 @@ internal HistogramBuckets(double[]? explicitBounds) } } - this.RunningBucketCounts = explicitBounds != null ? new long[explicitBounds.Length + 1] : null; - this.SnapshotBucketCounts = explicitBounds != null ? new long[explicitBounds.Length + 1] : new long[0]; + this.BucketCounts = explicitBounds != null ? new HistogramBucketValues[explicitBounds.Length + 1] : Array.Empty(); } /// @@ -76,7 +74,7 @@ internal HistogramBuckets Copy() { HistogramBuckets copy = new HistogramBuckets(this.ExplicitBounds); - Array.Copy(this.SnapshotBucketCounts, copy.SnapshotBucketCounts, this.SnapshotBucketCounts.Length); + Array.Copy(this.BucketCounts, copy.BucketCounts, this.BucketCounts.Length); copy.SnapshotSum = this.SnapshotSum; copy.SnapshotMin = this.SnapshotMin; copy.SnapshotMax = this.SnapshotMax; @@ -137,6 +135,31 @@ internal int FindBucketIndexLinear(double value) return i; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal void Snapshot(bool outputDelta) + { + var bucketCounts = this.BucketCounts; + + if (outputDelta) + { + for (int i = 0; i < bucketCounts.Length; i++) + { + ref var values = ref bucketCounts[i]; + ref var running = ref values.RunningValue; + values.SnapshotValue = running; + running = 0L; + } + } + else + { + for (int i = 0; i < bucketCounts.Length; i++) + { + ref var values = ref bucketCounts[i]; + values.SnapshotValue = values.RunningValue; + } + } + } + /// /// Enumerates the elements of a . /// @@ -152,7 +175,7 @@ internal Enumerator(HistogramBuckets histogramMeasurements) this.histogramMeasurements = histogramMeasurements; this.index = 0; this.Current = default; - this.numberOfBuckets = histogramMeasurements.SnapshotBucketCounts.Length; + this.numberOfBuckets = histogramMeasurements.BucketCounts.Length; } /// @@ -175,7 +198,7 @@ public bool MoveNext() double explicitBound = this.index < this.numberOfBuckets - 1 ? this.histogramMeasurements.ExplicitBounds![this.index] : double.PositiveInfinity; - long bucketCount = this.histogramMeasurements.SnapshotBucketCounts[this.index]; + long bucketCount = this.histogramMeasurements.BucketCounts[this.index].SnapshotValue; this.Current = new HistogramBucket(explicitBound, bucketCount); this.index++; return true; @@ -185,6 +208,12 @@ public bool MoveNext() } } + internal struct HistogramBucketValues + { + public long RunningValue; + public long SnapshotValue; + } + private sealed class BucketLookupNode { public double UpperBoundInclusive { get; set; } diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index 8931c9692a9..aedf5cf3c5d 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -915,16 +915,7 @@ internal void TakeSnapshot(bool outputDelta) histogramBuckets.RunningSum = 0; } - Debug.Assert(histogramBuckets.RunningBucketCounts != null, "histogramBuckets.RunningBucketCounts was null"); - - for (int i = 0; i < histogramBuckets.RunningBucketCounts!.Length; i++) - { - histogramBuckets.SnapshotBucketCounts[i] = histogramBuckets.RunningBucketCounts[i]; - if (outputDelta) - { - histogramBuckets.RunningBucketCounts[i] = 0; - } - } + histogramBuckets.Snapshot(outputDelta); this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect(this.Tags, outputDelta); @@ -979,16 +970,7 @@ internal void TakeSnapshot(bool outputDelta) histogramBuckets.RunningMax = double.NegativeInfinity; } - Debug.Assert(histogramBuckets.RunningBucketCounts != null, "histogramBuckets.RunningBucketCounts was null"); - - for (int i = 0; i < histogramBuckets.RunningBucketCounts!.Length; i++) - { - histogramBuckets.SnapshotBucketCounts[i] = histogramBuckets.RunningBucketCounts[i]; - if (outputDelta) - { - histogramBuckets.RunningBucketCounts[i] = 0; - } - } + histogramBuckets.Snapshot(outputDelta); this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect(this.Tags, outputDelta); this.MetricPointStatus = MetricPointStatus.NoCollectPending; @@ -1181,16 +1163,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta) histogramBuckets.RunningSum = 0; } - Debug.Assert(histogramBuckets.RunningBucketCounts != null, "histogramBuckets.RunningBucketCounts was null"); - - for (int i = 0; i < histogramBuckets.RunningBucketCounts!.Length; i++) - { - histogramBuckets.SnapshotBucketCounts[i] = histogramBuckets.RunningBucketCounts[i]; - if (outputDelta) - { - histogramBuckets.RunningBucketCounts[i] = 0; - } - } + histogramBuckets.Snapshot(outputDelta); this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect(this.Tags, outputDelta); @@ -1247,16 +1220,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta) histogramBuckets.RunningMax = double.NegativeInfinity; } - Debug.Assert(histogramBuckets.RunningBucketCounts != null, "histogramBuckets.RunningBucketCounts was null"); - - for (int i = 0; i < histogramBuckets.RunningBucketCounts!.Length; i++) - { - histogramBuckets.SnapshotBucketCounts[i] = histogramBuckets.RunningBucketCounts[i]; - if (outputDelta) - { - histogramBuckets.RunningBucketCounts[i] = 0; - } - } + histogramBuckets.Snapshot(outputDelta); this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect(this.Tags, outputDelta); this.MetricPointStatus = MetricPointStatus.NoCollectPending; @@ -1429,15 +1393,13 @@ private void UpdateHistogramWithBuckets(double number, ReadOnlySpan(long[] expected, T[] values) { foreach (var metricPoint in metric.GetMetricPoints()) { - bucketCounts = metricPoint.GetHistogramBuckets().RunningBucketCounts; + bucketCounts = metricPoint.GetHistogramBuckets().BucketCounts.Select(v => v.RunningValue).ToArray(); } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricPointTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricPointTests.cs index 070c7d57754..9e50c5354b5 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricPointTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricPointTests.cs @@ -77,8 +77,8 @@ public void VerifyHistogramBucketsCopy() Assert.NotSame(copy, histogramBuckets); // Verify fields are copied - Assert.NotSame(copy.SnapshotBucketCounts, histogramBuckets.SnapshotBucketCounts); - Assert.Equal(copy.SnapshotBucketCounts, histogramBuckets.SnapshotBucketCounts); + Assert.NotSame(copy.BucketCounts, histogramBuckets.BucketCounts); + Assert.Equal(copy.BucketCounts, histogramBuckets.BucketCounts); Assert.Equal(copy.SnapshotSum, histogramBuckets.SnapshotSum); } }