Skip to content

Commit

Permalink
Metrics Perf Improvement- Use KeyValuePair<string, object>[] to store…
Browse files Browse the repository at this point in the history 
… dimensions (#4059)
  • Loading branch information
@utpilla
utpilla committed Jan 7, 2023
1 parent 9708349 commit 1a02683
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 198 deletions.
37 changes: 22 additions & 15 deletions src/OpenTelemetry/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

* Performance Improvement: Update the internal structure used to store metric
dimensions from a combination of `string[]` and `object[]` to a
`KeyValuePair<string, object>[]`. This results in faster copying of the metric
dimensions required for `MetricPoint` lookup on the hot path.
([#4059](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4059))

## 1.4.0-rc.1

Released 2022-Dec-12
Expand Down Expand Up @@ -146,26 +152,27 @@ Released 2022-June-1
`PeriodicExportingMetricReader`.
([#3291](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3291))
* Add `ConfigureResource` which can replace SetResourceBuilder more succinctly
in most cases and has greater flexibility (applies to
TracerProviderBuilder, MeterProviderBuilder, OpenTelemetryLoggingOptions).
in most cases and has greater flexibility (applies to TracerProviderBuilder,
MeterProviderBuilder, OpenTelemetryLoggingOptions).
([#3307](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3307))

## 1.3.0-beta.2

Released 2022-May-16

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

## 1.3.0-beta.1

Released 2022-Apr-15

* Removes .NET Framework 4.6.1. The minimum .NET Framework
version supported is .NET 4.6.2. ([#3190](https://github.com/open-telemetry/opentelemetry-dotnet/issues/3190))
* Bumped minimum required version of `Microsoft.Extensions.Logging`
and `Microsoft.Extensions.Logging.Configuration` to 3.1.0
* Removes .NET Framework 4.6.1. The minimum .NET Framework version supported is
.NET 4.6.2.
([#3190](https://github.com/open-telemetry/opentelemetry-dotnet/issues/3190))
* Bumped minimum required version of `Microsoft.Extensions.Logging` and
`Microsoft.Extensions.Logging.Configuration` to 3.1.0
([#2582](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3196))

## 1.2.0
Expand All @@ -181,15 +188,15 @@ Released 2022-Apr-15
Released 2022-Apr-12

* Removed the `Temporality` setting on `MetricReader` and replaced it with
`TemporalityPreference`. This is a breaking change.
`TemporalityPreference` is used to determine the `AggregationTemporality`
used on a per-instrument kind basis. Currently, there are two preferences:
`TemporalityPreference`. This is a breaking change. `TemporalityPreference` is
used to determine the `AggregationTemporality` used on a per-instrument kind
basis. Currently, there are two preferences:
* `Cumulative`: Measurements from all instrument kinds are aggregated using
`AggregationTemporality.Cumulative`.
* `Delta`: Measurements from `Counter`, `ObservableCounter`, and `Histogram`
instruments are aggregated using `AggregationTemporality.Delta`. When
UpDownCounters are supported with
[DiagnosticSource version 7.0 onwards](https://www.nuget.org/packages/System.Diagnostics.DiagnosticSource/7.0.0-preview.2.22152.2),
UpDownCounters are supported with [DiagnosticSource version 7.0
onwards](https://www.nuget.org/packages/System.Diagnostics.DiagnosticSource/7.0.0-preview.2.22152.2),
they will be aggregated using `AggregationTemporality.Cumulative`.
([#3153](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3153))
* Fix issue where `ExplicitBucketHistogramConfiguration` could be used to
Expand Down Expand Up @@ -243,8 +250,8 @@ Released 2022-Mar-04
differ in some respect - different type, description, or unit - will result in
a separate metric stream for each distinct instrument.
([#2916](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2916))
* The `Meter` property on `OpenTelemetry.Metrics.Metric` has been removed.
It now has `MeterName` and `MeterVersion` properties.
* The `Meter` property on `OpenTelemetry.Metrics.Metric` has been removed. It
now has `MeterName` and `MeterVersion` properties.
([#2916](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2916))
* Added support for implementing custom `ResourceDetector`.
([#2949](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2949/)
Expand Down
51 changes: 22 additions & 29 deletions src/OpenTelemetry/Metrics/AggregatorStore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ namespace OpenTelemetry.Metrics
internal sealed class AggregatorStore
{
private static readonly string MetricPointCapHitFixMessage = "Modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit.";
private static readonly Comparison<KeyValuePair<string, object>> DimensionComparisonDelegate = (x, y) => x.Key.CompareTo(y.Key);
private readonly object lockZeroTags = new();
private readonly HashSet<string> tagKeysInteresting;
private readonly int tagsKeysInterestingCount;
Expand Down Expand Up @@ -161,17 +162,17 @@ private void InitializeZeroTagPointIfNotInitialized()
{
if (!this.zeroTagMetricPointInitialized)
{
this.metricPoints[0] = new MetricPoint(this, this.aggType, null, null, this.histogramBounds);
this.metricPoints[0] = new MetricPoint(this, this.aggType, null, this.histogramBounds);
this.zeroTagMetricPointInitialized = true;
}
}
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int length)
private int LookupAggregatorStore(KeyValuePair<string, object>[] tagKeysAndValues, int length)
{
var givenTags = new Tags(tagKeys, tagValues);
var givenTags = new Tags(tagKeysAndValues);

if (!this.tagsToMetricPointIndexDictionary.TryGetValue(givenTags, out var aggregatorIndex))
{
Expand All @@ -180,11 +181,11 @@ private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int leng
// Note: We are using storage from ThreadStatic, so need to make a deep copy for Dictionary storage.
// Create or obtain new arrays to temporarily hold the sorted tag Keys and Values
var storage = ThreadStaticStorage.GetStorage();
storage.CloneKeysAndValues(tagKeys, tagValues, length, out var tempSortedTagKeys, out var tempSortedTagValues);
storage.CloneKeysAndValues(tagKeysAndValues, length, out var tempSortedTagKeysAndValues);

Array.Sort(tempSortedTagKeys, tempSortedTagValues);
Array.Sort(tempSortedTagKeysAndValues, DimensionComparisonDelegate);

var sortedTags = new Tags(tempSortedTagKeys, tempSortedTagValues);
var sortedTags = new Tags(tempSortedTagKeysAndValues);

if (!this.tagsToMetricPointIndexDictionary.TryGetValue(sortedTags, out aggregatorIndex))
{
Expand All @@ -202,20 +203,14 @@ private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int leng
// so we need to make a deep copy for Dictionary storage.
if (length <= ThreadStaticStorage.MaxTagCacheSize)
{
var givenKeys = new string[length];
tagKeys.CopyTo(givenKeys, 0);
var givenTagKeysAndValues = new KeyValuePair<string, object>[length];
tagKeysAndValues.CopyTo(givenTagKeysAndValues.AsSpan());

var givenValues = new object[length];
tagValues.CopyTo(givenValues, 0);
var sortedTagKeysAndValues = new KeyValuePair<string, object>[length];
tempSortedTagKeysAndValues.CopyTo(sortedTagKeysAndValues.AsSpan());

var sortedTagKeys = new string[length];
tempSortedTagKeys.CopyTo(sortedTagKeys, 0);

var sortedTagValues = new object[length];
tempSortedTagValues.CopyTo(sortedTagValues, 0);

givenTags = new Tags(givenKeys, givenValues);
sortedTags = new Tags(sortedTagKeys, sortedTagValues);
givenTags = new Tags(givenTagKeysAndValues);
sortedTags = new Tags(sortedTagKeysAndValues);
}

lock (this.tagsToMetricPointIndexDictionary)
Expand All @@ -234,7 +229,7 @@ private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int leng
}

ref var metricPoint = ref this.metricPoints[aggregatorIndex];
metricPoint = new MetricPoint(this, this.aggType, sortedTags.Keys, sortedTags.Values, this.histogramBounds);
metricPoint = new MetricPoint(this, this.aggType, sortedTags.KeyValuePairs, this.histogramBounds);

// Add to dictionary *after* initializing MetricPoint
// as other threads can start writing to the
Expand All @@ -261,13 +256,11 @@ private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int leng
}

// Note: We are using storage from ThreadStatic, so need to make a deep copy for Dictionary storage.
var givenKeys = new string[length];
var givenValues = new object[length];
var givenTagKeysAndValues = new KeyValuePair<string, object>[length];

tagKeys.CopyTo(givenKeys, 0);
tagValues.CopyTo(givenValues, 0);
tagKeysAndValues.CopyTo(givenTagKeysAndValues.AsSpan());

givenTags = new Tags(givenKeys, givenValues);
givenTags = new Tags(givenTagKeysAndValues);

lock (this.tagsToMetricPointIndexDictionary)
{
Expand All @@ -285,7 +278,7 @@ private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int leng
}

ref var metricPoint = ref this.metricPoints[aggregatorIndex];
metricPoint = new MetricPoint(this, this.aggType, givenTags.Keys, givenTags.Values, this.histogramBounds);
metricPoint = new MetricPoint(this, this.aggType, givenTags.KeyValuePairs, this.histogramBounds);

// Add to dictionary *after* initializing MetricPoint
// as other threads can start writing to the
Expand Down Expand Up @@ -404,9 +397,9 @@ private int FindMetricAggregatorsDefault(ReadOnlySpan<KeyValuePair<string, objec

var storage = ThreadStaticStorage.GetStorage();

storage.SplitToKeysAndValues(tags, tagLength, out var tagKeys, out var tagValues);
storage.SplitToKeysAndValues(tags, tagLength, out var tagKeysAndValues);

return this.LookupAggregatorStore(tagKeys, tagValues, tagLength);
return this.LookupAggregatorStore(tagKeysAndValues, tagLength);
}

private int FindMetricAggregatorsCustomTag(ReadOnlySpan<KeyValuePair<string, object>> tags)
Expand All @@ -423,7 +416,7 @@ private int FindMetricAggregatorsCustomTag(ReadOnlySpan<KeyValuePair<string, obj

var storage = ThreadStaticStorage.GetStorage();

storage.SplitToKeysAndValues(tags, tagLength, this.tagKeysInteresting, out var tagKeys, out var tagValues, out var actualLength);
storage.SplitToKeysAndValues(tags, tagLength, this.tagKeysInteresting, out var tagKeysAndValues, out var actualLength);

// Actual number of tags depend on how many
// of the incoming tags has user opted to
Expand All @@ -434,7 +427,7 @@ private int FindMetricAggregatorsCustomTag(ReadOnlySpan<KeyValuePair<string, obj
return 0;
}

return this.LookupAggregatorStore(tagKeys, tagValues, actualLength);
return this.LookupAggregatorStore(tagKeysAndValues, actualLength);
}
}
}
6 changes: 2 additions & 4 deletions src/OpenTelemetry/Metrics/MetricPoint.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,14 @@ public struct MetricPoint
internal MetricPoint(
AggregatorStore aggregatorStore,
AggregationType aggType,
string[] keys,
object[] values,
KeyValuePair<string, object>[] tagKeysAndValues,
double[] histogramExplicitBounds)
{
Debug.Assert(aggregatorStore != null, "AggregatorStore was null.");
Debug.Assert((keys?.Length ?? 0) == (values?.Length ?? 0), "Key and value array lengths did not match.");
Debug.Assert(histogramExplicitBounds != null, "Histogram explicit Bounds was null.");

this.aggType = aggType;
this.Tags = new ReadOnlyTagCollection(keys, values);
this.Tags = new ReadOnlyTagCollection(tagKeysAndValues);
this.runningValue = default;
this.snapshotValue = default;
this.deltaLastValue = default;
Expand Down
53 changes: 22 additions & 31 deletions src/OpenTelemetry/Metrics/Tags.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,26 @@ namespace OpenTelemetry.Metrics
{
private readonly int hashCode;

public Tags(string[] keys, object[] values)
public Tags(KeyValuePair<string, object>[] keyValuePairs)
{
this.Keys = keys;
this.Values = values;
this.KeyValuePairs = keyValuePairs;

unchecked
var hash = 17;
for (int i = 0; i < this.KeyValuePairs.Length; i++)
{
var hash = 17;
for (int i = 0; i < this.Keys.Length; i++)
ref var item = ref this.KeyValuePairs[i];

unchecked
{
hash = (hash * 31) + this.Keys[i].GetHashCode() + this.Values[i]?.GetHashCode() ?? 0;
hash = (hash * 31) + item.Key.GetHashCode();
hash = (hash * 31) + item.Value?.GetHashCode() ?? 0;
}

this.hashCode = hash;
}
}

public readonly string[] Keys { get; }
this.hashCode = hash;
}

public readonly object[] Values { get; }
public readonly KeyValuePair<string, object>[] KeyValuePairs { get; }

public static bool operator ==(Tags tag1, Tags tag2) => tag1.Equals(tag2);

Expand All @@ -52,35 +52,26 @@ public override readonly bool Equals(object obj)

public readonly bool Equals(Tags other)
{
// Equality check for Keys
// Check if the two string[] are equal
var keysLength = this.Keys.Length;
var length = this.KeyValuePairs.Length;

if (keysLength != other.Keys.Length)
if (length != other.KeyValuePairs.Length)
{
return false;
}

for (int i = 0; i < keysLength; i++)
for (int i = 0; i < length; i++)
{
if (!this.Keys[i].Equals(other.Keys[i], StringComparison.Ordinal))
ref var left = ref this.KeyValuePairs[i];
ref var right = ref other.KeyValuePairs[i];

// Equality check for Keys
if (!left.Key.Equals(right.Key, StringComparison.Ordinal))
{
return false;
}
}

// Equality check for Values
// Check if the two object[] are equal
var valuesLength = this.Values.Length;

if (valuesLength != other.Values.Length)
{
return false;
}

for (int i = 0; i < valuesLength; i++)
{
if (!this.Values[i].Equals(other.Values[i]))
// Equality check for Values
if (!left.Value?.Equals(other.KeyValuePairs[i].Value) ?? right.Value != null)
{
return false;
}
Expand Down
Loading

0 comments on commit 1a02683

Please sign in to comment.