Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Metrics Perf Improvement- Use KeyValuePair<string, object>[] to store dimensions #4059

Merged
merged 11 commits into from
Jan 7, 2023
Merged

Metrics Perf Improvement- Use KeyValuePair<string, object>[] to store dimensions #4059

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c719031
Update Aggregator store use a KeyValuePair array
utpilla Dec 30, 2022
8a3abbc
Update Benchmarks
utpilla Jan 6, 2023
1bb346b
Rename comparison delegate
utpilla Jan 6, 2023
986b56c
Remove unnecessary code
utpilla Jan 6, 2023
6a34047
Address PR comments
utpilla Jan 6, 2023
c71765f
Merge branch 'main' into utpilla/Update-AggregatorStore
utpilla Jan 6, 2023
17a758b
Use Array.Copy instead of Array.CopyTo
utpilla Jan 6, 2023
ed9f946
Merge branch 'utpilla/Update-AggregatorStore' of https://github.com/u…
utpilla Jan 6, 2023
18898ce
Merge branch 'main' into utpilla/Update-AggregatorStore
utpilla Jan 6, 2023
d3e3237
Update CHANGELOG
utpilla Jan 6, 2023
8480145
Merge branch 'utpilla/Update-AggregatorStore' of https://github.com/u…
utpilla Jan 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 27 additions & 29 deletions src/OpenTelemetry/Metrics/AggregatorStore.cs
View file
Open in desktop
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 = DimensionComparison;
utpilla marked this conversation as resolved.
Show resolved Hide resolved
private readonly object lockZeroTags = new();
private readonly HashSet<string> tagKeysInteresting;
private readonly int tagsKeysInterestingCount;
Expand Down Expand Up @@ -152,6 +153,11 @@ internal void SnapshotCumulative(int indexSnapshot)
internal MetricPointsAccessor GetMetricPoints()
=> new(this.metricPoints, this.currentMetricPointBatch, this.batchSize);

private static int DimensionComparison(KeyValuePair<string, object> x, KeyValuePair<string, object> y)
{
return x.Key.CompareTo(y.Key);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void InitializeZeroTagPointIfNotInitialized()
{
Expand All @@ -161,17 +167,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 +186,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 +208,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 givenValues = new object[length];
tagValues.CopyTo(givenValues, 0);

var sortedTagKeys = new string[length];
tempSortedTagKeys.CopyTo(sortedTagKeys, 0);
var givenTagKeysAndValues = new KeyValuePair<string, object>[length];
tagKeysAndValues.CopyTo(givenTagKeysAndValues.AsSpan());

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

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 +234,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 +261,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 +283,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 +402,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 +421,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 +432,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
Open in desktop
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
47 changes: 16 additions & 31 deletions src/OpenTelemetry/Metrics/Tags.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,23 @@ 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++)
unchecked
{
hash = (hash * 31) + this.Keys[i].GetHashCode() + this.Values[i]?.GetHashCode() ?? 0;
hash = (hash * 31) + this.KeyValuePairs[i].Key.GetHashCode() + this.KeyValuePairs[i].Value?.GetHashCode() ?? 0;
utpilla marked this conversation as resolved.
Show resolved Hide resolved
}

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 +49,23 @@ 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))
// Equality check for Keys
if (!this.KeyValuePairs[i].Key.Equals(other.KeyValuePairs[i].Key, StringComparison.Ordinal))
utpilla marked this conversation as resolved.
Show resolved Hide resolved
{
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 (!this.KeyValuePairs[i].Value?.Equals(other.KeyValuePairs[i].Value) ?? other.KeyValuePairs[i].Value != null)
{
return false;
}
Expand Down
Loading