Skip to content

Commit

Permalink
Binary search for large bucket count histograms (#3252)
Browse files Browse the repository at this point in the history 
* Binary search for large bucket count histograms

* Update CHANGELOG.md

* netcoreapp3.1 was complaining

* ci rerun

* ci rerun

* Update MetricTestData.cs

* use 400 buckets

* Update ExplicitBucketHistogramConfiguration.cs

Binary search for large bucket count histograms

Update CHANGELOG.md

netcoreapp3.1 was complaining

ci rerun

ci rerun

Update MetricTestData.cs

use 400 buckets

* separate nan part

* Address PR comments

- Remove conversion to float from `FindHistogramBucketIndexBinary`
- Make `DefaultHistogramCountForBinarySearch` a `const` instead of a `static readonly`

* update to 140

* remove double.nan from invalid hist bounds

* Refactor and perf to histogram bucket index find

Thanks to @CodeBlanch !!

* fine tune bound limit to switch to binary search

* included benchmark results in comment

* new bench result

* histogram stress test update

* fix changelog

* spacing fix

* sealed bucket class

* ci

* double.negative infinity

* update order of operations

when testing with large MaxValue this would end up having same value bounds

* remove stress test change

* ci

* Add histogram binary mode tests

* ci

* pr review changes

* allocated column - hist benchmark

* CI

Co-authored-by: Utkarsh Umesan Pillai <utpilla@microsoft.com>
Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
  • Loading branch information
@mic-max @utpilla @cijothomas
3 people authored Sep 2, 2022
1 parent db0918e commit 9c3d1b1
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 41 deletions.
5 changes: 4 additions & 1 deletion src/OpenTelemetry/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Use binary search for histograms with 50 or more supplied boundaries.
([#3252](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3252))

* Allows samplers the ability to modify tracestate if desired.
([#3610](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3610))

Expand Down Expand Up @@ -73,7 +76,7 @@ Released 2022-May-16

* Exposed public setters for `LogRecord.State`, `LogRecord.StateValues`,
and `LogRecord.FormattedMessage`.
([#3217](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3217))
([#3217](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3217))

## 1.3.0-beta.1

Expand Down
95 changes: 95 additions & 0 deletions src/OpenTelemetry/Metrics/HistogramBuckets.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
// </copyright>

using System;
using System.Diagnostics;
using System.Runtime.CompilerServices;

namespace OpenTelemetry.Metrics
{
Expand All @@ -24,6 +26,8 @@ namespace OpenTelemetry.Metrics
// Note: Does not implement IEnumerable<> to prevent accidental boxing.
public class HistogramBuckets
{
internal const int DefaultBoundaryCountForBinarySearch = 50;

internal readonly double[] ExplicitBounds;

internal readonly long[] RunningBucketCounts;
Expand All @@ -36,9 +40,38 @@ public class HistogramBuckets

internal int IsCriticalSectionOccupied = 0;

private readonly BucketLookupNode bucketLookupTreeRoot;

private readonly Func<double, int> findHistogramBucketIndex;

internal HistogramBuckets(double[] explicitBounds)
{
this.ExplicitBounds = explicitBounds;
this.findHistogramBucketIndex = this.FindBucketIndexLinear;
if (explicitBounds != null && explicitBounds.Length >= DefaultBoundaryCountForBinarySearch)
{
this.bucketLookupTreeRoot = ConstructBalancedBST(explicitBounds, 0, explicitBounds.Length);
this.findHistogramBucketIndex = this.FindBucketIndexBinary;

static BucketLookupNode ConstructBalancedBST(double[] values, int min, int max)
{
if (min == max)
{
return null;
}

int median = min + ((max - min) / 2);
return new BucketLookupNode
{
Index = median,
UpperBoundInclusive = values[median],
LowerBoundExclusive = median > 0 ? values[median - 1] : double.NegativeInfinity,
Left = ConstructBalancedBST(values, min, median),
Right = ConstructBalancedBST(values, median + 1, max),
};
}
}

this.RunningBucketCounts = explicitBounds != null ? new long[explicitBounds.Length + 1] : null;
this.SnapshotBucketCounts = explicitBounds != null ? new long[explicitBounds.Length + 1] : new long[0];
}
Expand All @@ -55,6 +88,55 @@ internal HistogramBuckets Copy()
return copy;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal int FindBucketIndex(double value)
{
return this.findHistogramBucketIndex(value);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal int FindBucketIndexBinary(double value)
{
BucketLookupNode current = this.bucketLookupTreeRoot;

Debug.Assert(current != null, "Bucket root was null.");

do
{
if (value <= current.LowerBoundExclusive)
{
current = current.Left;
}
else if (value > current.UpperBoundInclusive)
{
current = current.Right;
}
else
{
return current.Index;
}
}
while (current != null);

return this.ExplicitBounds.Length;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal int FindBucketIndexLinear(double value)
{
int i;
for (i = 0; i < this.ExplicitBounds.Length; i++)
{
// Upper bound is inclusive
if (value <= this.ExplicitBounds[i])
{
break;
}
}

return i;
}

/// <summary>
/// Enumerates the elements of a <see cref="HistogramBuckets"/>.
/// </summary>
Expand Down Expand Up @@ -102,5 +184,18 @@ public bool MoveNext()
return false;
}
}

private sealed class BucketLookupNode
{
public double UpperBoundInclusive { get; set; }

public double LowerBoundExclusive { get; set; }

public int Index { get; set; }

public BucketLookupNode Left { get; set; }

public BucketLookupNode Right { get; set; }
}
}
}
10 changes: 1 addition & 9 deletions src/OpenTelemetry/Metrics/MetricPoint.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,15 +326,7 @@ internal void Update(double number)

case AggregationType.Histogram:
{
int i;
for (i = 0; i < this.histogramBuckets.ExplicitBounds.Length; i++)
{
// Upper bound is inclusive
if (number <= this.histogramBuckets.ExplicitBounds[i])
{
break;
}
}
int i = this.histogramBuckets.FindBucketIndex(number);

var sw = default(SpinWait);
while (true)
Expand Down
63 changes: 32 additions & 31 deletions test/Benchmarks/Metrics/HistogramBenchmarks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,50 +24,51 @@
using OpenTelemetry.Tests;

/*
BenchmarkDotNet=v0.13.1, OS=Windows 10.0.22000
Intel Core i7-9700 CPU 3.00GHz, 1 CPU, 8 logical and 8 physical cores
.NET SDK=6.0.200
[Host] : .NET 6.0.2 (6.0.222.6406), X64 RyuJIT
DefaultJob : .NET 6.0.2 (6.0.222.6406), X64 RyuJIT
| Method | BoundCount | Mean | Error | StdDev | Allocated |
|---------------------------- |----------- |----------:|---------:|---------:|----------:|
| HistogramHotPath | 10 | 45.27 ns | 0.384 ns | 0.359 ns | - |
| HistogramWith1LabelHotPath | 10 | 89.99 ns | 0.373 ns | 0.312 ns | - |
| HistogramWith3LabelsHotPath | 10 | 185.34 ns | 3.184 ns | 3.667 ns | - |
| HistogramWith5LabelsHotPath | 10 | 266.69 ns | 1.391 ns | 1.301 ns | - |
| HistogramWith7LabelsHotPath | 10 | 323.20 ns | 1.834 ns | 1.531 ns | - |
| HistogramHotPath | 20 | 48.69 ns | 0.347 ns | 0.307 ns | - |
| HistogramWith1LabelHotPath | 20 | 93.84 ns | 0.696 ns | 0.651 ns | - |
| HistogramWith3LabelsHotPath | 20 | 189.82 ns | 1.208 ns | 1.071 ns | - |
| HistogramWith5LabelsHotPath | 20 | 269.23 ns | 2.027 ns | 1.693 ns | - |
| HistogramWith7LabelsHotPath | 20 | 329.92 ns | 1.272 ns | 1.128 ns | - |
| HistogramHotPath | 50 | 55.73 ns | 0.339 ns | 0.317 ns | - |
| HistogramWith1LabelHotPath | 50 | 100.38 ns | 0.455 ns | 0.425 ns | - |
| HistogramWith3LabelsHotPath | 50 | 200.02 ns | 1.011 ns | 0.844 ns | - |
| HistogramWith5LabelsHotPath | 50 | 279.94 ns | 1.595 ns | 1.492 ns | - |
| HistogramWith7LabelsHotPath | 50 | 346.88 ns | 1.064 ns | 0.943 ns | - |
| HistogramHotPath | 100 | 66.39 ns | 0.167 ns | 0.148 ns | - |
| HistogramWith1LabelHotPath | 100 | 114.98 ns | 1.340 ns | 1.253 ns | - |
| HistogramWith3LabelsHotPath | 100 | 220.52 ns | 1.723 ns | 1.528 ns | - |
| HistogramWith5LabelsHotPath | 100 | 299.10 ns | 1.950 ns | 1.629 ns | - |
| HistogramWith7LabelsHotPath | 100 | 356.25 ns | 2.153 ns | 1.798 ns | - |
BenchmarkDotNet=v0.13.1, OS=Windows 10.0.19044.1706 (21H2)
AMD Ryzen 9 3900X, 1 CPU, 24 logical and 12 physical cores
.NET SDK=6.0.203
[Host] : .NET 6.0.5 (6.0.522.21309), X64 RyuJIT
DefaultJob : .NET 6.0.5 (6.0.522.21309), X64 RyuJIT
| Method | BoundCount | Mean | Error | StdDev |
|---------------------------- |----------- |----------:|----------:|----------:|
| HistogramHotPath | 10 | 45.19 ns | 0.321 ns | 0.285 ns |
| HistogramWith1LabelHotPath | 10 | 97.21 ns | 0.129 ns | 0.114 ns |
| HistogramWith3LabelsHotPath | 10 | 179.77 ns | 0.270 ns | 0.239 ns |
| HistogramWith5LabelsHotPath | 10 | 263.30 ns | 2.423 ns | 2.267 ns |
| HistogramWith7LabelsHotPath | 10 | 338.42 ns | 3.121 ns | 2.919 ns |
| HistogramHotPath | 49 | 56.18 ns | 0.593 ns | 0.554 ns |
| HistogramWith1LabelHotPath | 49 | 110.60 ns | 0.815 ns | 0.762 ns |
| HistogramWith3LabelsHotPath | 49 | 193.30 ns | 1.048 ns | 0.980 ns |
| HistogramWith5LabelsHotPath | 49 | 281.55 ns | 1.638 ns | 1.532 ns |
| HistogramWith7LabelsHotPath | 49 | 343.88 ns | 2.148 ns | 2.010 ns |
| HistogramHotPath | 50 | 57.46 ns | 0.264 ns | 0.234 ns |
| HistogramWith1LabelHotPath | 50 | 121.73 ns | 0.372 ns | 0.348 ns |
| HistogramWith3LabelsHotPath | 50 | 227.95 ns | 1.074 ns | 1.004 ns |
| HistogramWith5LabelsHotPath | 50 | 313.15 ns | 1.068 ns | 0.999 ns |
| HistogramWith7LabelsHotPath | 50 | 377.04 ns | 1.191 ns | 0.930 ns |
| HistogramHotPath | 1000 | 78.33 ns | 0.441 ns | 0.391 ns |
| HistogramWith1LabelHotPath | 1000 | 127.57 ns | 0.457 ns | 0.428 ns |
| HistogramWith3LabelsHotPath | 1000 | 494.19 ns | 4.490 ns | 3.980 ns |
| HistogramWith5LabelsHotPath | 1000 | 608.75 ns | 11.306 ns | 10.576 ns |
| HistogramWith7LabelsHotPath | 1000 | 649.16 ns | 3.273 ns | 2.555 ns |
*/

namespace Benchmarks.Metrics
{
public class HistogramBenchmarks
{
private const int MaxValue = 1000;
private const int MaxValue = 10000;
private readonly Random random = new();
private readonly string[] dimensionValues = new string[] { "DimVal1", "DimVal2", "DimVal3", "DimVal4", "DimVal5", "DimVal6", "DimVal7", "DimVal8", "DimVal9", "DimVal10" };
private Histogram<long> histogram;
private MeterProvider provider;
private Meter meter;
private double[] bounds;

[Params(10, 20, 50, 100)]
// Note: Values related to `HistogramBuckets.DefaultHistogramCountForBinarySearch`
[Params(10, 49, 50, 1000)]
public int BoundCount { get; set; }

[GlobalSetup]
Expand Down
40 changes: 40 additions & 0 deletions test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,46 @@ public void HistogramDistributeToAllBucketsCustom()
Assert.Equal(boundaries.Length + 1, actualCount);
}

[Fact]
public void HistogramBinaryBucketTest()
{
// Arrange
// Bounds = (-Inf, 0] (0, 1], ... (49, +Inf)
var boundaries = new double[HistogramBuckets.DefaultBoundaryCountForBinarySearch];
for (var i = 0; i < boundaries.Length; i++)
{
boundaries[i] = i;
}

var histogramPoint = new MetricPoint(this.aggregatorStore, AggregationType.Histogram, null, null, boundaries);

// Act
histogramPoint.Update(-1);
histogramPoint.Update(boundaries[0]);
histogramPoint.Update(boundaries[boundaries.Length - 1]);
for (var i = 0.5; i < boundaries.Length; i++)
{
histogramPoint.Update(i);
}

histogramPoint.TakeSnapshot(true);

// Assert
var index = 0;
foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets())
{
var expectedCount = 1;

if (index == 0 || index == boundaries.Length - 1)
{
expectedCount = 2;
}

Assert.Equal(expectedCount, histogramMeasurement.BucketCount);
index++;
}
}

[Fact]
public void HistogramWithOnlySumCount()
{
Expand Down

0 comments on commit 9c3d1b1

Please sign in to comment.