diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index c938dc3e503..de240ff3347 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -12,6 +12,10 @@ thread. ([2844](https://github.com/open-telemetry/opentelemetry-dotnet/issues/2844)) +* Performance improvement: when emitting metrics, users are strongly advised to + provide tags with same Key order, to achieve maximum performance. + ([2805](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2805/files)) + ## 1.2.0-rc1 Released 2021-Nov-29 diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index a4b3df38aef..c61f1a59273 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -25,14 +25,12 @@ namespace OpenTelemetry.Metrics { internal sealed class AggregatorStore { - private static readonly ObjectArrayEqualityComparer ObjectArrayComparer = new ObjectArrayEqualityComparer(); private readonly object lockZeroTags = new object(); private readonly HashSet tagKeysInteresting; private readonly int tagsKeysInterestingCount; - // Two-Level lookup. TagKeys x [ TagValues x Metrics ] - private readonly ConcurrentDictionary> keyValue2MetricAggs = - new ConcurrentDictionary>(new StringArrayEqualityComparer()); + private readonly ConcurrentDictionary tagsToMetricPointIndexDictionary = + new ConcurrentDictionary(); private readonly AggregationTemporality temporality; private readonly string name; @@ -178,44 +176,37 @@ private void InitializeZeroTagPointIfNotInitialized() [MethodImpl(MethodImplOptions.AggressiveInlining)] private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int length) { - int aggregatorIndex; - string[] seqKey = null; + var givenTags = new Tags(tagKeys, tagValues); - // GetOrAdd by TagKeys at 1st Level of 2-level dictionary structure. - // Get back a Dictionary of [ Values x Metrics[] ]. - if (!this.keyValue2MetricAggs.TryGetValue(tagKeys, out var value2metrics)) + if (!this.tagsToMetricPointIndexDictionary.TryGetValue(givenTags, out var aggregatorIndex)) { - // Note: We are using storage from ThreadStatic, so need to make a deep copy for Dictionary storage. - seqKey = new string[length]; - tagKeys.CopyTo(seqKey, 0); - - value2metrics = new ConcurrentDictionary(ObjectArrayComparer); - if (!this.keyValue2MetricAggs.TryAdd(seqKey, value2metrics)) + if (length > 1) { - this.keyValue2MetricAggs.TryGetValue(seqKey, out value2metrics); - } - } + // Note: We are using storage from ThreadStatic, so need to make a deep copy for Dictionary storage. + // Create a new array for the sorted Tag keys. + var sortedTagKeys = new string[length]; + tagKeys.CopyTo(sortedTagKeys, 0); - // GetOrAdd by TagValues at 2st Level of 2-level dictionary structure. - // Get back Metrics[]. - if (!value2metrics.TryGetValue(tagValues, out aggregatorIndex)) - { - aggregatorIndex = this.metricPointIndex; - if (aggregatorIndex >= this.maxMetricPoints) - { - // sorry! out of data points. - // TODO: Once we support cleanup of - // unused points (typically with delta) - // we can re-claim them here. - return -1; - } + // Create a new array for the sorted Tag values. + var sortedTagValues = new object[length]; + tagValues.CopyTo(sortedTagValues, 0); - lock (value2metrics) - { - // check again after acquiring lock. - if (!value2metrics.TryGetValue(tagValues, out aggregatorIndex)) + Array.Sort(sortedTagKeys, sortedTagValues); + + var sortedTags = new Tags(sortedTagKeys, sortedTagValues); + + if (!this.tagsToMetricPointIndexDictionary.TryGetValue(sortedTags, out aggregatorIndex)) { - aggregatorIndex = Interlocked.Increment(ref this.metricPointIndex); + // Note: We are using storage from ThreadStatic, so need to make a deep copy for Dictionary storage. + var givenKeys = new string[length]; + tagKeys.CopyTo(givenKeys, 0); + + var givenValues = new object[length]; + tagValues.CopyTo(givenValues, 0); + + givenTags = new Tags(givenKeys, givenValues); + + aggregatorIndex = this.metricPointIndex; if (aggregatorIndex >= this.maxMetricPoints) { // sorry! out of data points. @@ -225,24 +216,83 @@ private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int leng return -1; } - // Note: We are using storage from ThreadStatic, so need to make a deep copy for Dictionary storage. - if (seqKey == null) + lock (this.tagsToMetricPointIndexDictionary) { - seqKey = new string[length]; - tagKeys.CopyTo(seqKey, 0); + // check again after acquiring lock. + if (!this.tagsToMetricPointIndexDictionary.TryGetValue(sortedTags, out aggregatorIndex)) + { + aggregatorIndex = ++this.metricPointIndex; + if (aggregatorIndex >= this.maxMetricPoints) + { + // sorry! out of data points. + // TODO: Once we support cleanup of + // unused points (typically with delta) + // we can re-claim them here. + return -1; + } + + ref var metricPoint = ref this.metricPoints[aggregatorIndex]; + var dt = DateTimeOffset.UtcNow; + metricPoint = new MetricPoint(this.aggType, dt, sortedTags.Keys, sortedTags.Values, this.histogramBounds); + + // Add to dictionary *after* initializing MetricPoint + // as other threads can start writing to the + // MetricPoint, if dictionary entry found. + + // Add the sorted order along with the given order of tags + this.tagsToMetricPointIndexDictionary.TryAdd(sortedTags, aggregatorIndex); + this.tagsToMetricPointIndexDictionary.TryAdd(givenTags, aggregatorIndex); + } } + } + } + else + { + // 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 seqVal = new object[length]; - tagValues.CopyTo(seqVal, 0); + tagKeys.CopyTo(givenKeys, 0); + tagValues.CopyTo(givenValues, 0); - ref var metricPoint = ref this.metricPoints[aggregatorIndex]; - var dt = DateTimeOffset.UtcNow; - metricPoint = new MetricPoint(this.aggType, dt, seqKey, seqVal, this.histogramBounds); + givenTags = new Tags(givenKeys, givenValues); - // Add to dictionary *after* initializing MetricPoint - // as other threads can start writing to the - // MetricPoint, if dictionary entry found. - value2metrics.TryAdd(seqVal, aggregatorIndex); + aggregatorIndex = this.metricPointIndex; + if (aggregatorIndex >= this.maxMetricPoints) + { + // sorry! out of data points. + // TODO: Once we support cleanup of + // unused points (typically with delta) + // we can re-claim them here. + return -1; + } + + lock (this.tagsToMetricPointIndexDictionary) + { + // check again after acquiring lock. + if (!this.tagsToMetricPointIndexDictionary.TryGetValue(givenTags, out aggregatorIndex)) + { + aggregatorIndex = ++this.metricPointIndex; + if (aggregatorIndex >= this.maxMetricPoints) + { + // sorry! out of data points. + // TODO: Once we support cleanup of + // unused points (typically with delta) + // we can re-claim them here. + return -1; + } + + ref var metricPoint = ref this.metricPoints[aggregatorIndex]; + var dt = DateTimeOffset.UtcNow; + metricPoint = new MetricPoint(this.aggType, dt, givenTags.Keys, givenTags.Values, this.histogramBounds); + + // Add to dictionary *after* initializing MetricPoint + // as other threads can start writing to the + // MetricPoint, if dictionary entry found. + + // givenTags will always be sorted when tags length == 1 + this.tagsToMetricPointIndexDictionary.TryAdd(givenTags, aggregatorIndex); + } } } } @@ -355,11 +405,6 @@ private int FindMetricAggregatorsDefault(ReadOnlySpan 1) - { - Array.Sort(tagKeys, tagValues); - } - return this.LookupAggregatorStore(tagKeys, tagValues, tagLength); } @@ -388,11 +433,6 @@ private int FindMetricAggregatorsCustomTag(ReadOnlySpan 1) - { - Array.Sort(tagKeys, tagValues); - } - return this.LookupAggregatorStore(tagKeys, tagValues, actualLength); } } diff --git a/src/OpenTelemetry/Metrics/ObjectArrayEqualityComparer.cs b/src/OpenTelemetry/Metrics/ObjectArrayEqualityComparer.cs deleted file mode 100644 index 3d806ac751e..00000000000 --- a/src/OpenTelemetry/Metrics/ObjectArrayEqualityComparer.cs +++ /dev/null @@ -1,68 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System.Collections.Generic; - -namespace OpenTelemetry.Metrics -{ - internal class ObjectArrayEqualityComparer : IEqualityComparer - { - public bool Equals(object[] obj1, object[] obj2) - { - if (ReferenceEquals(obj1, obj2)) - { - return true; - } - - if (ReferenceEquals(obj1, null) || ReferenceEquals(obj2, null)) - { - return false; - } - - var len1 = obj1.Length; - - if (len1 != obj2.Length) - { - return false; - } - - for (int i = 0; i < len1; i++) - { - if (!obj1[i].Equals(obj2[i])) - { - return false; - } - } - - return true; - } - - public int GetHashCode(object[] objs) - { - int hash = 17; - - unchecked - { - for (int i = 0; i < objs.Length; i++) - { - hash = (hash * 31) + objs[i].GetHashCode(); - } - } - - return hash; - } - } -} diff --git a/src/OpenTelemetry/Metrics/StringArrayEqualityComparer.cs b/src/OpenTelemetry/Metrics/StringArrayEqualityComparer.cs deleted file mode 100644 index 6a91f201eae..00000000000 --- a/src/OpenTelemetry/Metrics/StringArrayEqualityComparer.cs +++ /dev/null @@ -1,69 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System; -using System.Collections.Generic; - -namespace OpenTelemetry.Metrics -{ - internal class StringArrayEqualityComparer : IEqualityComparer - { - public bool Equals(string[] strings1, string[] strings2) - { - if (ReferenceEquals(strings1, strings2)) - { - return true; - } - - if (ReferenceEquals(strings1, null) || ReferenceEquals(strings2, null)) - { - return false; - } - - var len1 = strings1.Length; - - if (len1 != strings2.Length) - { - return false; - } - - for (int i = 0; i < len1; i++) - { - if (!strings1[i].Equals(strings2[i], StringComparison.Ordinal)) - { - return false; - } - } - - return true; - } - - public int GetHashCode(string[] strings) - { - int hash = 17; - - unchecked - { - for (int i = 0; i < strings.Length; i++) - { - hash = (hash * 31) + strings[i].GetHashCode(); - } - } - - return hash; - } - } -} diff --git a/src/OpenTelemetry/Metrics/Tags.cs b/src/OpenTelemetry/Metrics/Tags.cs new file mode 100644 index 00000000000..1357a2848d3 --- /dev/null +++ b/src/OpenTelemetry/Metrics/Tags.cs @@ -0,0 +1,96 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; + +namespace OpenTelemetry.Metrics +{ + internal readonly struct Tags : IEquatable + { + public Tags(string[] keys, object[] values) + { + this.Keys = keys; + this.Values = values; + } + + public readonly string[] Keys { get; } + + public readonly object[] Values { get; } + + public static bool operator ==(Tags tag1, Tags tag2) => tag1.Equals(tag2); + + public static bool operator !=(Tags tag1, Tags tag2) => !tag1.Equals(tag2); + + public readonly override bool Equals(object obj) + { + return obj is Tags other && this.Equals(other); + } + + public readonly bool Equals(Tags other) + { + // Equality check for Keys + // Check if the two string[] are equal + var keysLength = this.Keys.Length; + + if (keysLength != other.Keys.Length) + { + return false; + } + + for (int i = 0; i < keysLength; i++) + { + if (!this.Keys[i].Equals(other.Keys[i], 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])) + { + return false; + } + } + + return true; + } + + public readonly override int GetHashCode() + { + int hash = 17; + + unchecked + { + for (int i = 0; i < this.Keys.Length; i++) + { + hash = (hash * 31) + this.Keys[i].GetHashCode() + this.Values[i].GetHashCode(); + } + } + + return hash; + } + } +}