System.Diagnostics.Activity Perf Improvement Part 2 (#40544)

dotnet · Aug 13, 2020 · 6dad3b4 · 6dad3b4
1 parent 80962c8
commit 6dad3b4
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 10 deletions.
diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
@@ -255,9 +255,6 @@ public string? RootId
         /// </summary>
         public IEnumerable<KeyValuePair<string, object?>> TagObjects
         {
-#if ALLOW_PARTIALLY_TRUSTED_CALLERS
-        [System.Security.SecuritySafeCriticalAttribute]
-#endif
             get => _tags ?? s_emptyTagObjects;
         }
 
@@ -1284,9 +1281,10 @@ public void Add(T value)
                 }
             }
 
+            // Note: Some customers use this GetEnumerator dynamically to avoid allocations.
             public Enumerator<T> GetEnumerator() => new Enumerator<T>(_first);
-            IEnumerator<T> IEnumerable<T>.GetEnumerator() => new Enumerator<T>(_first);
-            IEnumerator IEnumerable.GetEnumerator() => new Enumerator<T>(_first);
+            IEnumerator<T> IEnumerable<T>.GetEnumerator() => GetEnumerator();
+            IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
         }
 
         private class TagsLinkedList : IEnumerable<KeyValuePair<string, object?>>
@@ -1419,9 +1417,10 @@ public void Set(KeyValuePair<string, object?> value)
                 }
             }
 
+            // Note: Some customers use this GetEnumerator dynamically to avoid allocations.
             public Enumerator<KeyValuePair<string, object?>> GetEnumerator() => new Enumerator<KeyValuePair<string, object?>>(_first);
-            IEnumerator<KeyValuePair<string, object?>> IEnumerable<KeyValuePair<string, object?>>.GetEnumerator() => new Enumerator<KeyValuePair<string, object?>>(_first);
-            IEnumerator IEnumerable.GetEnumerator() => new Enumerator<KeyValuePair<string, object?>>(_first);
+            IEnumerator<KeyValuePair<string, object?>> IEnumerable<KeyValuePair<string, object?>>.GetEnumerator() => GetEnumerator();
+            IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
 
             public IEnumerable<KeyValuePair<string, string?>> EnumerateStringValues()
             {
@@ -1439,15 +1438,15 @@ public void Set(KeyValuePair<string, object?> value)
             }
         }
 
+        // Note: Some customers use this Enumerator dynamically to avoid allocations.
         private struct Enumerator<T> : IEnumerator<T>
         {
-            private readonly LinkedListNode<T>? _head;
             private LinkedListNode<T>? _nextNode;
             [AllowNull, MaybeNull] private T _currentItem;
 
             public Enumerator(LinkedListNode<T>? head)
             {
-                _nextNode = _head = head;
+                _nextNode = head;
                 _currentItem = default;
             }
 
@@ -1468,7 +1467,7 @@ public bool MoveNext()
                 return true;
             }
 
-            public void Reset() => _nextNode = _head;
+            public void Reset() => throw new NotSupportedException();
 
             public void Dispose()
             {

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
@@ -1508,6 +1508,40 @@ public void TestInsertingFirstTag(string key, object value, bool add, int result
             Assert.Equal(resultCount, a.TagObjects.Count());
         }
 
+        [Fact]
+        public void StructEnumerator_TagsLinkedList()
+        {
+            // Note: This test verifies the presence of the struct Enumerator on TagsLinkedList used by customers dynamically to avoid allocations.
+
+            Activity a = new Activity("TestActivity");
+            a.AddTag("Tag1", true);
+
+            IEnumerable<KeyValuePair<string, object>> enumerable = a.TagObjects;
+
+            MethodInfo method = enumerable.GetType().GetMethod("GetEnumerator", BindingFlags.Instance | BindingFlags.Public);
+
+            Assert.NotNull(method);
+            Assert.False(method.ReturnType.IsInterface);
+            Assert.True(method.ReturnType.IsValueType);
+        }
+
+        [Fact]
+        public void StructEnumerator_GenericLinkedList()
+        {
+            // Note: This test verifies the presence of the struct Enumerator on LinkedList<T> used by customers dynamically to avoid allocations.
+
+            Activity a = new Activity("TestActivity");
+            a.AddEvent(new ActivityEvent());
+
+            IEnumerable<ActivityEvent> enumerable = a.Events;
+
+            MethodInfo method = enumerable.GetType().GetMethod("GetEnumerator", BindingFlags.Instance | BindingFlags.Public);
+
+            Assert.NotNull(method);
+            Assert.False(method.ReturnType.IsInterface);
+            Assert.True(method.ReturnType.IsValueType);
+        }
+
         public void Dispose()
         {
             Activity.Current = null;