Add additional ThreadStatic storage to avoid allocation while sorting…

… tags (#2865) Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
open-telemetry · Apr 15, 2022 · 1d16e0b · 1d16e0b
1 parent edede3a
commit 1d16e0b
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 16 deletions.
diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs
@@ -181,28 +181,32 @@ private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int leng
                 if (length > 1)
                 {
                     // 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);
+                    // 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);
 
-                    // Create a new array for the sorted Tag values.
-                    var sortedTagValues = new object[length];
-                    tagValues.CopyTo(sortedTagValues, 0);
+                    Array.Sort(tempSortedTagKeys, tempSortedTagValues);
 
-                    Array.Sort(sortedTagKeys, sortedTagValues);
-
-                    var sortedTags = new Tags(sortedTagKeys, sortedTagValues);
+                    var sortedTags = new Tags(tempSortedTagKeys, tempSortedTagValues);
 
                     if (!this.tagsToMetricPointIndexDictionary.TryGetValue(sortedTags, out aggregatorIndex))
                     {
-                        // Note: We are using storage from ThreadStatic, so need to make a deep copy for Dictionary storage.
+                        // Note: We are using storage from ThreadStatic for both the input order of tags and the sorted order of tags,
+                        // so we 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);
 
+                        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);
 
                         aggregatorIndex = this.metricPointIndex;
                         if (aggregatorIndex >= this.maxMetricPoints)

diff --git a/src/OpenTelemetry/Metrics/ThreadStaticStorage.cs b/src/OpenTelemetry/Metrics/ThreadStaticStorage.cs
@@ -28,13 +28,15 @@ internal class ThreadStaticStorage
         [ThreadStatic]
         private static ThreadStaticStorage storage;
 
-        private readonly TagStorage[] tagStorage = new TagStorage[MaxTagCacheSize];
+        private readonly TagStorage[] primaryTagStorage = new TagStorage[MaxTagCacheSize];
+        private readonly TagStorage[] secondaryTagStorage = new TagStorage[MaxTagCacheSize];
 
         private ThreadStaticStorage()
         {
             for (int i = 0; i < MaxTagCacheSize; i++)
             {
-                this.tagStorage[i] = new TagStorage(i + 1);
+                this.primaryTagStorage[i] = new TagStorage(i + 1);
+                this.secondaryTagStorage[i] = new TagStorage(i + 1);
             }
         }
 
@@ -56,8 +58,8 @@ internal void SplitToKeysAndValues(ReadOnlySpan<KeyValuePair<string, object>> ta
 
             if (tagLength <= MaxTagCacheSize)
             {
-                tagKeys = this.tagStorage[tagLength - 1].TagKeys;
-                tagValues = this.tagStorage[tagLength - 1].TagValues;
+                tagKeys = this.primaryTagStorage[tagLength - 1].TagKeys;
+                tagValues = this.primaryTagStorage[tagLength - 1].TagValues;
             }
             else
             {
@@ -94,8 +96,8 @@ internal void SplitToKeysAndValues(ReadOnlySpan<KeyValuePair<string, object>> ta
             }
             else if (actualLength <= MaxTagCacheSize)
             {
-                tagKeys = this.tagStorage[actualLength - 1].TagKeys;
-                tagValues = this.tagStorage[actualLength - 1].TagValues;
+                tagKeys = this.primaryTagStorage[actualLength - 1].TagKeys;
+                tagValues = this.primaryTagStorage[actualLength - 1].TagValues;
             }
             else
             {
@@ -125,6 +127,29 @@ internal void SplitToKeysAndValues(ReadOnlySpan<KeyValuePair<string, object>> ta
             }
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal void CloneKeysAndValues(string[] inputTagKeys, object[] inputTagValues, int tagLength, out string[] clonedTagKeys, out object[] clonedTagValues)
+        {
+            Guard.ThrowIfZero(tagLength, $"There must be at least one tag to use {nameof(ThreadStaticStorage)}", $"{nameof(tagLength)}");
+
+            if (tagLength <= MaxTagCacheSize)
+            {
+                clonedTagKeys = this.secondaryTagStorage[tagLength - 1].TagKeys;
+                clonedTagValues = this.secondaryTagStorage[tagLength - 1].TagValues;
+            }
+            else
+            {
+                clonedTagKeys = new string[tagLength];
+                clonedTagValues = new object[tagLength];
+            }
+
+            for (int i = 0; i < tagLength; i++)
+            {
+                clonedTagKeys[i] = inputTagKeys[i];
+                clonedTagValues[i] = inputTagValues[i];
+            }
+        }
+
         internal class TagStorage
         {
             // Used to split into Key sequence, Value sequence.