Optimization: Reduce many array allocations in GetCustomAtttributes()'s inherited...

* Optimization: avoid 2 array allocations in inherited attribute misses

This is a follow-up to many allocations recognized in dotnet/coreclr#20448.

A lot of methods ultimately call through CustomAttributes.GetCustomAttributes(). In the inherited search path (default for most searches above), the inheritance path of a class is traversed, resulting in an array allocation per crawled type, a copy to the overall List<object> and then after that - in current code - an array allocation from that list and a typed array allocation it's copied to.

However, in the common miss case as simple as:
typeof(T).GetCustomAttributes(typeof(TAttr), true);
...and many other overloads, all the same path underneath...

We can avoid the last 2 arrays in the miss case. We have the List<object> to go off of. If that's a zero-entry list, we can return an Array.Empty<TAttr>(). That's effectively what this change does.

While converting the entire attribute pipeline to generics is problematic and has issue since some object[] return abstracts aren't sealed, we can at least somewhat trivially cache an array per attribute type (only one static, ultimately from Array.Empty<T> underneath) and return that for the miss case.

There are far more wins to be had here, but they require more changes.

* Move RuntimeType empty array cache generation to Attribute.CreateAttributeArrayHelper

This exposes Attribute.CreateAttributeArrayHelper to internal and uses it directly on the RuntimeType caching for empty arrays. Though this allocated 1 additional array overall, it's simpler, faster to init, and still is an infinite win over the per-call allocations before this overall changesets.

* CustomAttributes: remove needless array copy in the inherited hit case

This removes a .ToArray() for the sake of Array.Copy() where a simple for loop suffices and removes the allocation. Reversing the "empty" result checks is also just a bit cleaner here.

This also expands the same fix to the MemberInfo path.

Note: should DRY these up too (longstanding issue) - but let's do that in a separate commit for clarity.

* GetCusomAttributes: use ListBuilder<object> for inheritance crawls

This exposes RuntimeType.ListBuilder<T> for internal usage and replaces the List<T> allocation in GetCustomAttributes() paths to reduce allocations and increase performance in the inherited crawl paths (which is the default for many optional-parameter methods in layers above).

Note: there is a subtle behavior depending on previous-null (not possible with a struct now) in AttributeUsageCheck() that I believe still behaves correctly, but could use another set of eyes and a full test suite run to confirm. object[] attributes was removed there simply because it wasn't used before - only cleaning up.

* Attribute caching: use Array.CreateInstance() directly on RuntimeType

This also reverts the CreateAttributeArrayHelper => internal change, since it's no longer needed.

* Ref passing for RuntimeType.ListBuilder<object> & CustomAttribute simplification

This fixes the struct passing duplication and tweaks how we're creating arrays a bit, centralizing the zero-element checks to cover all cases as well as simplify the per-method code to rely on the fact this is happening underneath.

Commit migrated from https://github.com/dotnet/coreclr/commit/6d9fc60df3b721eab98ca08386a1f01358f93839

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs
index 6166461..6044256 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs
@@ -62,7 +62,7 @@ namespace System.Reflection
 
             IList<CustomAttributeData> cad = GetCustomAttributes(target.GetRuntimeModule(), target.MetadataToken);
 
-            Attribute[] a = PseudoCustomAttribute.GetCustomAttributes((RuntimeType)target, typeof(object) as RuntimeType, out int pcaCount);
+            Attribute[] a = PseudoCustomAttribute.GetCustomAttributes((RuntimeType)target, (RuntimeType)typeof(object), out int pcaCount);
 
             if (pcaCount == 0)
                 return cad;
@@ -83,7 +83,7 @@ namespace System.Reflection
 
             IList<CustomAttributeData> cad = GetCustomAttributes(target.GetRuntimeModule(), target.MetadataToken);
 
-            Attribute[] a = PseudoCustomAttribute.GetCustomAttributes((RuntimeFieldInfo)target, typeof(object) as RuntimeType, out int pcaCount);
+            Attribute[] a = PseudoCustomAttribute.GetCustomAttributes((RuntimeFieldInfo)target, (RuntimeType)typeof(object), out int pcaCount);
 
             if (pcaCount == 0)
                 return cad;
@@ -104,7 +104,7 @@ namespace System.Reflection
 
             IList<CustomAttributeData> cad = GetCustomAttributes(target.GetRuntimeModule(), target.MetadataToken);
 
-            Attribute[] a = PseudoCustomAttribute.GetCustomAttributes((RuntimeMethodInfo)target, typeof(object) as RuntimeType, out int pcaCount);
+            Attribute[] a = PseudoCustomAttribute.GetCustomAttributes((RuntimeMethodInfo)target, (RuntimeType)typeof(object), out int pcaCount);
 
             if (pcaCount == 0)
                 return cad;
@@ -156,7 +156,7 @@ namespace System.Reflection
 
             IList<CustomAttributeData> cad = GetCustomAttributes((RuntimeModule)target.ManifestModule, RuntimeAssembly.GetToken(target.GetNativeHandle()));
 
-            Attribute[] a = PseudoCustomAttribute.GetCustomAttributes(target, typeof(object) as RuntimeType, out int pcaCount);
+            Attribute[] a = PseudoCustomAttribute.GetCustomAttributes(target, (RuntimeType)typeof(object), out int pcaCount);
 
             if (pcaCount == 0)
                 return cad;
@@ -177,7 +177,7 @@ namespace System.Reflection
 
             IList<CustomAttributeData> cad = GetCustomAttributes(target.GetRuntimeModule(), target.MetadataToken);
 
-            Attribute[] a = PseudoCustomAttribute.GetCustomAttributes(target, typeof(object) as RuntimeType, out int pcaCount);
+            Attribute[] a = PseudoCustomAttribute.GetCustomAttributes(target, (RuntimeType)typeof(object), out int pcaCount);
 
             if (pcaCount == 0)
                 return cad;
@@ -1276,17 +1276,17 @@ namespace System.Reflection
                 return attributes;
             }
 
-            List<object> result = new List<object>();
+            RuntimeType.ListBuilder<object> result = new RuntimeType.ListBuilder<object>();
             bool mustBeInheritable = false;
             bool useObjectArray = (caType == null || caType.IsValueType || caType.ContainsGenericParameters);
-            Type arrayType = useObjectArray ? typeof(object) : caType;
+            RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : caType;
 
             while (pcaCount > 0)
                 result.Add(pca[--pcaCount]);
 
             while (type != (RuntimeType)typeof(object) && type != null)
             {
-                object[] attributes = GetCustomAttributes(type.GetRuntimeModule(), type.MetadataToken, 0, caType, mustBeInheritable, result);
+                object[] attributes = GetCustomAttributes(type.GetRuntimeModule(), type.MetadataToken, 0, caType, mustBeInheritable, ref result);
                 mustBeInheritable = true;
                 for (int i = 0; i < attributes.Length; i++)
                     result.Add(attributes[i]);
@@ -1295,7 +1295,10 @@ namespace System.Reflection
             }
 
             object[] typedResult = CreateAttributeArrayHelper(arrayType, result.Count);
-            Array.Copy(result.ToArray(), 0, typedResult, 0, result.Count);
+            for (var i = 0; i < result.Count; i++)
+            {
+                typedResult[i] = result[i];
+            }
             return typedResult;
         }
 
@@ -1319,17 +1322,17 @@ namespace System.Reflection
                 return attributes;
             }
 
-            List<object> result = new List<object>();
+            RuntimeType.ListBuilder<object> result = new RuntimeType.ListBuilder<object>();
             bool mustBeInheritable = false;
             bool useObjectArray = (caType == null || caType.IsValueType || caType.ContainsGenericParameters);
-            Type arrayType = useObjectArray ? typeof(object) : caType;
+            RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : caType;
 
             while (pcaCount > 0)
                 result.Add(pca[--pcaCount]);
 
             while (method != null)
             {
-                object[] attributes = GetCustomAttributes(method.GetRuntimeModule(), method.MetadataToken, 0, caType, mustBeInheritable, result);
+                object[] attributes = GetCustomAttributes(method.GetRuntimeModule(), method.MetadataToken, 0, caType, mustBeInheritable, ref result);
                 mustBeInheritable = true;
                 for (int i = 0; i < attributes.Length; i++)
                     result.Add(attributes[i]);
@@ -1338,7 +1341,10 @@ namespace System.Reflection
             }
 
             object[] typedResult = CreateAttributeArrayHelper(arrayType, result.Count);
-            Array.Copy(result.ToArray(), 0, typedResult, 0, result.Count);
+            for (var i = 0; i < result.Count; i++)
+            {
+                typedResult[i] = result[i];
+            }
             return typedResult;
         }
 
@@ -1445,6 +1451,7 @@ namespace System.Reflection
                 RuntimeType attributeType;
                 IRuntimeMethodInfo ctor;
                 bool ctorHasParameters, isVarArg;
+                RuntimeType.ListBuilder<object> derivedAttributes = default;
 
                 // Optimization for the case where attributes decorate entities in the same assembly in which case 
                 // we can cache the successful APTCA check between the decorated and the declared assembly.
@@ -1455,7 +1462,7 @@ namespace System.Reflection
                     CustomAttributeRecord caRecord = car[i];
 
                     if (FilterCustomAttributeRecord(caRecord, scope, ref lastAptcaOkAssembly,
-                        decoratedModule, decoratedMetadataToken, attributeFilterType, mustBeInheritable, null, null,
+                        decoratedModule, decoratedMetadataToken, attributeFilterType, mustBeInheritable, null, ref derivedAttributes,
                         out attributeType, out ctor, out ctorHasParameters, out isVarArg))
                         return true;
                 }
@@ -1480,18 +1487,19 @@ namespace System.Reflection
         private static unsafe object[] GetCustomAttributes(
             RuntimeModule decoratedModule, int decoratedMetadataToken, int pcaCount, RuntimeType attributeFilterType)
         {
-            return GetCustomAttributes(decoratedModule, decoratedMetadataToken, pcaCount, attributeFilterType, false, null);
+            RuntimeType.ListBuilder<object> _ = default;
+            return GetCustomAttributes(decoratedModule, decoratedMetadataToken, pcaCount, attributeFilterType, false, ref _);
         }
 
         private static unsafe object[] GetCustomAttributes(
             RuntimeModule decoratedModule, int decoratedMetadataToken, int pcaCount,
-            RuntimeType attributeFilterType, bool mustBeInheritable, IList derivedAttributes)
+            RuntimeType attributeFilterType, bool mustBeInheritable, ref RuntimeType.ListBuilder<object> derivedAttributes)
         {
             MetadataImport scope = decoratedModule.MetadataImport;
             CustomAttributeRecord[] car = CustomAttributeData.GetCustomAttributeRecords(decoratedModule, decoratedMetadataToken);
 
             bool useObjectArray = (attributeFilterType == null || attributeFilterType.IsValueType || attributeFilterType.ContainsGenericParameters);
-            Type arrayType = useObjectArray ? typeof(object) : attributeFilterType;
+            RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : attributeFilterType;
 
             if (attributeFilterType == null && car.Length == 0)
                 return CreateAttributeArrayHelper(arrayType, 0);
@@ -1519,7 +1527,7 @@ namespace System.Reflection
 
                 if (!FilterCustomAttributeRecord(caRecord, scope, ref lastAptcaOkAssembly,
                                                  decoratedModule, decoratedMetadataToken, attributeFilterType, mustBeInheritable,
-                                                 attributes, derivedAttributes,
+                                                 attributes, ref derivedAttributes,
                                                  out attributeType, out ctor, out ctorHasParameters, out isVarArg))
                     continue;
 
@@ -1647,7 +1655,7 @@ namespace System.Reflection
             RuntimeType attributeFilterType,
             bool mustBeInheritable,
             object[] attributes,
-            IList derivedAttributes,
+            ref RuntimeType.ListBuilder<object> derivedAttributes,
             out RuntimeType attributeType,
             out IRuntimeMethodInfo ctor,
             out bool ctorHasParameters,
@@ -1671,7 +1679,7 @@ namespace System.Reflection
 
             // Ensure if attribute type must be inheritable that it is inhertiable
             // Ensure that to consider a duplicate attribute type AllowMultiple is true
-            if (!AttributeUsageCheck(attributeType, mustBeInheritable, attributes, derivedAttributes))
+            if (!AttributeUsageCheck(attributeType, mustBeInheritable, ref derivedAttributes))
                 return false;
 
             // Windows Runtime attributes aren't real types - they exist to be read as metadata only, and as such
@@ -1743,7 +1751,7 @@ namespace System.Reflection
 
         #region Private Static Methods
         private static bool AttributeUsageCheck(
-            RuntimeType attributeType, bool mustBeInheritable, object[] attributes, IList derivedAttributes)
+            RuntimeType attributeType, bool mustBeInheritable, ref RuntimeType.ListBuilder<object> derivedAttributes)
         {
             AttributeUsageAttribute attributeUsageAttribute = null;
 
@@ -1756,8 +1764,7 @@ namespace System.Reflection
             }
 
             // Legacy: AllowMultiple ignored for none inheritable attributes
-
-            if (derivedAttributes == null)
+            if (derivedAttributes.Count == 0)
                 return true;
 
             for (int i = 0; i < derivedAttributes.Count; i++)
@@ -1844,8 +1851,14 @@ namespace System.Reflection
             blobStart = (IntPtr)pBlobStart;
         }
 
-        private static object[] CreateAttributeArrayHelper(Type elementType, int elementCount)
+        private static object[] CreateAttributeArrayHelper(RuntimeType elementType, int elementCount)
         {
+            // If we have 0 elements, don't allocate a new array
+            if (elementCount == 0)
+            {
+                return elementType.GetEmptyArray();
+            }
+
             return (object[])Array.CreateInstance(elementType, elementCount);
         }
         #endregion

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RtType.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RtType.cs
index 59da425..c1be93e 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/RtType.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/RtType.cs
@@ -75,7 +75,7 @@ namespace System
         }
 
         // Helper to build lists of MemberInfos. Special cased to avoid allocations for lists of one element.
-        private struct ListBuilder<T> where T : class
+        internal struct ListBuilder<T> where T : class
         {
             private T[] _items;
             private T _item;
@@ -1453,6 +1453,7 @@ namespace System
             private static object s_methodInstantiationsLock;
             private string m_defaultMemberName;
             private object m_genericCache; // Generic cache for rare scenario specific data. It is used to cache Enum names and values.
+            private object[] _emptyArray; // Object array cache for Attribute.GetCustomAttributes() pathological no-result case.
             #endregion
 
             #region Constructor
@@ -1620,6 +1621,11 @@ namespace System
 
                 return m_defaultMemberName;
             }
+
+            internal object[] GetEmptyArray()
+            {
+                return _emptyArray ?? (_emptyArray = (object[])Array.CreateInstance(m_runtimeType, 0));
+            }
             #endregion
 
             #region Caches Accessors
@@ -3533,6 +3539,11 @@ namespace System
         {
             return RuntimeTypeHandle.GetElementType(this);
         }
+
+        internal object[] GetEmptyArray()
+        {
+            return Cache.GetEmptyArray();
+        }
         #endregion
 
         #region Enums