Metric AggregatorStore optimizations for sorting tags (#2805)

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

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<string> tagKeysInteresting;
         private readonly int tagsKeysInterestingCount;
 
-        // Two-Level lookup. TagKeys x [ TagValues x Metrics ]
+        private readonly ConcurrentDictionary<Tags, int> tagsToMetricPointIndexDictionary =
-        private readonly ConcurrentDictionary<string[], ConcurrentDictionary<object[], int>> keyValue2MetricAggs =
+            new ConcurrentDictionary<Tags, int>();
-            new ConcurrentDictionary<string[], ConcurrentDictionary<object[], int>>(new StringArrayEqualityComparer());
 
         private readonly AggregationTemporality temporality;
         private readonly string name;
@@ -178,44 +176,37 @@ namespace OpenTelemetry.Metrics
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int length)
         {
-            int aggregatorIndex;
+            var givenTags = new Tags(tagKeys, tagValues);
-            string[] seqKey = null;
 
-            // GetOrAdd by TagKeys at 1st Level of 2-level dictionary structure.
+            if (!this.tagsToMetricPointIndexDictionary.TryGetValue(givenTags, out var aggregatorIndex))
-            // Get back a Dictionary of [ Values x Metrics[] ].
-            if (!this.keyValue2MetricAggs.TryGetValue(tagKeys, out var value2metrics))
             {
-                // Note: We are using storage from ThreadStatic, so need to make a deep copy for Dictionary storage.
+                if (length > 1)
-                seqKey = new string[length];
-                tagKeys.CopyTo(seqKey, 0);
-
-                value2metrics = new ConcurrentDictionary<object[], int>(ObjectArrayComparer);
-                if (!this.keyValue2MetricAggs.TryAdd(seqKey, value2metrics))
                 {
-                    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.
+                    // Create a new array for the sorted Tag values.
-            // Get back Metrics[].
+                    var sortedTagValues = new object[length];
-            if (!value2metrics.TryGetValue(tagValues, out aggregatorIndex))
+                    tagValues.CopyTo(sortedTagValues, 0);
-            {
-                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 (value2metrics)
+                    Array.Sort(sortedTagKeys, sortedTagValues);
-                {
+
-                    // check again after acquiring lock.
+                    var sortedTags = new Tags(sortedTagKeys, sortedTagValues);
-                    if (!value2metrics.TryGetValue(tagValues, out aggregatorIndex))
+
+                    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 @@ namespace OpenTelemetry.Metrics
                             return -1;
                         }
 
-                        // Note: We are using storage from ThreadStatic, so need to make a deep copy for Dictionary storage.
+                        lock (this.tagsToMetricPointIndexDictionary)
-                        if (seqKey == null)
                         {
-                            seqKey = new string[length];
+                            // check again after acquiring lock.
-                            tagKeys.CopyTo(seqKey, 0);
+                            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];
+                    tagKeys.CopyTo(givenKeys, 0);
-                        tagValues.CopyTo(seqVal, 0);
+                    tagValues.CopyTo(givenValues, 0);
 
-                        ref var metricPoint = ref this.metricPoints[aggregatorIndex];
+                    givenTags = new Tags(givenKeys, givenValues);
-                        var dt = DateTimeOffset.UtcNow;
-                        metricPoint = new MetricPoint(this.aggType, dt, seqKey, seqVal, this.histogramBounds);
 
-                        // Add to dictionary *after* initializing MetricPoint
+                    aggregatorIndex = this.metricPointIndex;
-                        // as other threads can start writing to the
+                    if (aggregatorIndex >= this.maxMetricPoints)
-                        // MetricPoint, if dictionary entry found.
+                    {
-                        value2metrics.TryAdd(seqVal, aggregatorIndex);
+                        // 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 @@ namespace OpenTelemetry.Metrics
 
             storage.SplitToKeysAndValues(tags, tagLength, out var tagKeys, out var tagValues);
 
-            if (tagLength > 1)
-            {
-                Array.Sort(tagKeys, tagValues);
-            }
-
             return this.LookupAggregatorStore(tagKeys, tagValues, tagLength);
         }
 
@@ -388,11 +433,6 @@ namespace OpenTelemetry.Metrics
                 return 0;
             }
 
-            if (actualLength > 1)
-            {
-                Array.Sort(tagKeys, tagValues);
-            }
-
             return this.LookupAggregatorStore(tagKeys, tagValues, actualLength);
         }
     }

src/OpenTelemetry/Metrics/ObjectArrayEqualityComparer.cs

@@ -1,68 +0,0 @@
-// <copyright file="ObjectArrayEqualityComparer.cs" company="OpenTelemetry Authors">
-// 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.
-// </copyright>
-
-using System.Collections.Generic;
-
-namespace OpenTelemetry.Metrics
-{
-    internal class ObjectArrayEqualityComparer : IEqualityComparer<object[]>
-    {
-        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;
-        }
-    }
-}

src/OpenTelemetry/Metrics/StringArrayEqualityComparer.cs

@@ -1,69 +0,0 @@
-// <copyright file="StringArrayEqualityComparer.cs" company="OpenTelemetry Authors">
-// 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.
-// </copyright>
-
-using System;
-using System.Collections.Generic;
-
-namespace OpenTelemetry.Metrics
-{
-    internal class StringArrayEqualityComparer : IEqualityComparer<string[]>
-    {
-        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;
-        }
-    }
-}

src/OpenTelemetry/Metrics/Tags.cs

@@ -0,0 +1,96 @@
+// <copyright file="Tags.cs" company="OpenTelemetry Authors">
+// 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.
+// </copyright>
+
+using System;
+
+namespace OpenTelemetry.Metrics
+{
+    internal readonly struct Tags : IEquatable<Tags>
+    {
+        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;
+        }
+    }
+}