Reflection performance tweaks & cleanup (#21737)
authorJan Kotas <jkotas@microsoft.com>
Fri, 4 Jan 2019 09:05:49 +0000 (23:05 -1000)
committerGitHub <noreply@github.com>
Fri, 4 Jan 2019 09:05:49 +0000 (23:05 -1000)
- Delete internal "Unsafe" field getters/setters. They performance was very close (within 10%) of the regular getters/setters.
- Made a few performance tweaks in reflection to make reflection faster overall. The regular field getters/setters are faster than what the unsafe ones used to be with this change.

src/System.Private.CoreLib/System.Private.CoreLib.csproj
src/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs [moved from src/System.Private.CoreLib/src/System/Attribute.cs with 100% similarity]
src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs
src/System.Private.CoreLib/src/System/Reflection/RtFieldInfo.cs
src/System.Private.CoreLib/src/System/RtType.cs
src/System.Private.CoreLib/src/System/Runtime/InteropServices/GcHandle.cs
src/System.Private.CoreLib/src/System/ValueType.cs

index 255d8c1..7528d12 100644 (file)
     <Compile Include="$(BclSourcesRoot)\System\AppContext.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\ArgIterator.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Array.cs" />
-    <Compile Include="$(BclSourcesRoot)\System\Attribute.cs" />
+    <Compile Include="$(BclSourcesRoot)\System\Attribute.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\BadImageFormatException.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Buffer.cs" />
     <Compile Include="$(BclSourcesRoot)\System\CLRConfig.cs" />
index 0ebedf0..a11d923 100644 (file)
@@ -1524,12 +1524,12 @@ namespace System.Reflection
                                     type = Type_Type;
                             }
 
-                            RuntimePropertyInfo property = null;
+                            PropertyInfo property = null;
 
                             if (type == null)
-                                property = attributeType.GetProperty(name) as RuntimePropertyInfo;
+                                property = attributeType.GetProperty(name);
                             else
-                                property = attributeType.GetProperty(name, type, Type.EmptyTypes) as RuntimePropertyInfo;
+                                property = attributeType.GetProperty(name, type, Type.EmptyTypes);
 
                             // Did we get a valid property reference?
                             if (property == null)
@@ -1539,7 +1539,7 @@ namespace System.Reflection
                                         isProperty ? SR.RFLCT_InvalidPropFail : SR.RFLCT_InvalidFieldFail, name));
                             }
 
-                            RuntimeMethodInfo setMethod = property.GetSetMethod(true) as RuntimeMethodInfo;
+                            MethodInfo setMethod = property.GetSetMethod(true);
 
                             // Public properties may have non-public setter methods
                             if (!setMethod.IsPublic)
@@ -1550,10 +1550,8 @@ namespace System.Reflection
                         }
                         else
                         {
-                            RtFieldInfo field = attributeType.GetField(name) as RtFieldInfo;
-
-                            field.CheckConsistency(attribute);
-                            field.UnsafeSetValue(attribute, value, BindingFlags.Default, Type.DefaultBinder, null);
+                            FieldInfo field = attributeType.GetField(name);
+                            field.SetValue(attribute, value, BindingFlags.Default, Type.DefaultBinder, null);
                         }
                     }
                     catch (Exception e)
index 3bbaf86..47b5194 100644 (file)
@@ -23,42 +23,45 @@ namespace System.Reflection
 
         internal INVOCATION_FLAGS InvocationFlags
         {
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
             get
             {
-                if ((m_invocationFlags & INVOCATION_FLAGS.INVOCATION_FLAGS_INITIALIZED) == 0)
-                {
-                    Type declaringType = DeclaringType;
-
-                    INVOCATION_FLAGS invocationFlags = 0;
+                return (m_invocationFlags & INVOCATION_FLAGS.INVOCATION_FLAGS_INITIALIZED) != 0 ?
+                    m_invocationFlags : InitializeInvocationFlags();
+            }
+        }
 
-                    // first take care of all the NO_INVOKE cases
-                    if (declaringType != null && declaringType.ContainsGenericParameters)
-                    {
-                        invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_NO_INVOKE;
-                    }
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private INVOCATION_FLAGS InitializeInvocationFlags()
+        {
+            Type declaringType = DeclaringType;
 
-                    // If the invocationFlags are still 0, then
-                    // this should be an usable field, determine the other flags
-                    if (invocationFlags == 0)
-                    {
-                        if ((m_fieldAttributes & FieldAttributes.InitOnly) != (FieldAttributes)0)
-                            invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_SPECIAL_FIELD;
+            INVOCATION_FLAGS invocationFlags = 0;
 
-                        if ((m_fieldAttributes & FieldAttributes.HasFieldRVA) != (FieldAttributes)0)
-                            invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_SPECIAL_FIELD;
+            // first take care of all the NO_INVOKE cases
+            if (declaringType != null && declaringType.ContainsGenericParameters)
+            {
+                invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_NO_INVOKE;
+            }
 
-                        // find out if the field type is one of the following: Primitive, Enum or Pointer
-                        Type fieldType = FieldType;
-                        if (fieldType.IsPointer || fieldType.IsEnum || fieldType.IsPrimitive)
-                            invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_FIELD_SPECIAL_CAST;
-                    }
+            // If the invocationFlags are still 0, then
+            // this should be an usable field, determine the other flags
+            if (invocationFlags == 0)
+            {
+                if ((m_fieldAttributes & FieldAttributes.InitOnly) != (FieldAttributes)0)
+                    invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_SPECIAL_FIELD;
 
-                    // must be last to avoid threading problems
-                    m_invocationFlags = invocationFlags | INVOCATION_FLAGS.INVOCATION_FLAGS_INITIALIZED;
-                }
+                if ((m_fieldAttributes & FieldAttributes.HasFieldRVA) != (FieldAttributes)0)
+                    invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_SPECIAL_FIELD;
 
-                return m_invocationFlags;
+                // find out if the field type is one of the following: Primitive, Enum or Pointer
+                Type fieldType = FieldType;
+                if (fieldType.IsPointer || fieldType.IsEnum || fieldType.IsPrimitive)
+                    invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_FIELD_SPECIAL_CAST;
             }
+
+            // must be last to avoid threading problems
+            return (m_invocationFlags = invocationFlags | INVOCATION_FLAGS.INVOCATION_FLAGS_INITIALIZED);
         }
         #endregion
 
@@ -84,6 +87,7 @@ namespace System.Reflection
         #endregion
 
         #region Internal Members
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal void CheckConsistency(object target)
         {
             // only test instance fields
@@ -115,57 +119,6 @@ namespace System.Reflection
             return m.m_fieldHandle == m_fieldHandle;
         }
 
-        // UnsafeSetValue doesn't perform any consistency or visibility check.
-        // It is the caller's responsibility to ensure the operation is safe.
-        // When the caller needs to perform consistency checks they should 
-        // call CheckConsistency() before calling this method.
-        [DebuggerStepThroughAttribute]
-        [Diagnostics.DebuggerHidden]
-        internal void UnsafeSetValue(object obj, object value, BindingFlags invokeAttr, Binder binder, CultureInfo culture)
-        {
-            RuntimeType declaringType = DeclaringType as RuntimeType;
-            RuntimeType fieldType = (RuntimeType)FieldType;
-            value = fieldType.CheckValue(value, binder, culture, invokeAttr);
-
-            bool domainInitialized = false;
-            if (declaringType == null)
-            {
-                RuntimeFieldHandle.SetValue(this, obj, value, fieldType, m_fieldAttributes, null, ref domainInitialized);
-            }
-            else
-            {
-                domainInitialized = declaringType.DomainInitialized;
-                RuntimeFieldHandle.SetValue(this, obj, value, fieldType, m_fieldAttributes, declaringType, ref domainInitialized);
-                declaringType.DomainInitialized = domainInitialized;
-            }
-        }
-
-        // UnsafeGetValue doesn't perform any consistency or visibility check.
-        // It is the caller's responsibility to ensure the operation is safe.
-        // When the caller needs to perform consistency checks they should 
-        // call CheckConsistency() before calling this method.
-        [DebuggerStepThroughAttribute]
-        [Diagnostics.DebuggerHidden]
-        internal object UnsafeGetValue(object obj)
-        {
-            RuntimeType declaringType = DeclaringType as RuntimeType;
-
-            RuntimeType fieldType = (RuntimeType)FieldType;
-
-            bool domainInitialized = false;
-            if (declaringType == null)
-            {
-                return RuntimeFieldHandle.GetValue(this, obj, fieldType, null, ref domainInitialized);
-            }
-            else
-            {
-                domainInitialized = declaringType.DomainInitialized;
-                object retVal = RuntimeFieldHandle.GetValue(this, obj, fieldType, declaringType, ref domainInitialized);
-                declaringType.DomainInitialized = domainInitialized;
-                return retVal;
-            }
-        }
-
         #endregion
 
         #region MemberInfo Overrides
@@ -218,7 +171,20 @@ namespace System.Reflection
 
             CheckConsistency(obj);
 
-            return UnsafeGetValue(obj);
+            RuntimeType fieldType = (RuntimeType)FieldType;
+
+            bool domainInitialized = false;
+            if (declaringType == null)
+            {
+                return RuntimeFieldHandle.GetValue(this, obj, fieldType, null, ref domainInitialized);
+            }
+            else
+            {
+                domainInitialized = declaringType.DomainInitialized;
+                object retVal = RuntimeFieldHandle.GetValue(this, obj, fieldType, declaringType, ref domainInitialized);
+                declaringType.DomainInitialized = domainInitialized;
+                return retVal;
+            }
         }
 
         public override object GetRawConstantValue() { throw new InvalidOperationException(); }
@@ -307,15 +273,19 @@ namespace System.Reflection
 
         public override Type FieldType
         {
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
             get
             {
-                if (m_fieldType == null)
-                    m_fieldType = new Signature(this, m_declaringType).FieldType;
-
-                return m_fieldType;
+                return m_fieldType ?? InitializeFieldType();
             }
         }
 
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private RuntimeType InitializeFieldType()
+        {
+            return (m_fieldType = new Signature(this, m_declaringType).FieldType);
+        }
+
         public override Type[] GetRequiredCustomModifiers()
         {
             return new Signature(this, m_declaringType).GetCustomModifiers(1, true);
index 0fcbd3b..dd30ea4 100644 (file)
@@ -14,6 +14,8 @@ using System.Threading;
 using DebuggerStepThroughAttribute = System.Diagnostics.DebuggerStepThroughAttribute;
 using MdToken = System.Reflection.MetadataToken;
 
+using Internal.Runtime.CompilerServices;
+
 namespace System
 {
     // this is a work around to get the concept of a calli. It's not as fast but it would be interesting to 
@@ -2379,30 +2381,44 @@ namespace System
 
         private RuntimeTypeCache Cache
         {
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
             get
             {
-                if (m_cache == IntPtr.Zero)
+                if (m_cache != IntPtr.Zero)
                 {
-                    IntPtr newgcHandle = new RuntimeTypeHandle(this).GetGCHandle(GCHandleType.WeakTrackResurrection);
-                    IntPtr gcHandle = Interlocked.CompareExchange(ref m_cache, newgcHandle, IntPtr.Zero);
-                    // Leak the handle if the type is collectible. It will be reclaimed when
-                    // the type goes away.
-                    if (gcHandle != IntPtr.Zero && !IsCollectible)
-                        GCHandle.InternalFree(newgcHandle);
+                    object cache = GCHandle.InternalGet(m_cache);
+                    if (cache != null)
+                    {
+                        Debug.Assert(cache is RuntimeTypeCache);
+                        return Unsafe.As<RuntimeTypeCache>(cache);
+                    }
                 }
+                return InitializeCache();
+            }
+        }
 
-                RuntimeTypeCache cache = GCHandle.InternalGet(m_cache) as RuntimeTypeCache;
-                if (cache == null)
-                {
-                    cache = new RuntimeTypeCache(this);
-                    RuntimeTypeCache existingCache = GCHandle.InternalCompareExchange(m_cache, cache, null, false) as RuntimeTypeCache;
-                    if (existingCache != null)
-                        cache = existingCache;
-                }
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private RuntimeTypeCache InitializeCache()
+        {
+            if (m_cache == IntPtr.Zero)
+            {
+                IntPtr newgcHandle = new RuntimeTypeHandle(this).GetGCHandle(GCHandleType.WeakTrackResurrection);
+                IntPtr gcHandle = Interlocked.CompareExchange(ref m_cache, newgcHandle, IntPtr.Zero);
+                if (gcHandle != IntPtr.Zero)
+                    GCHandle.InternalFree(newgcHandle);
+            }
 
-                Debug.Assert(cache != null);
-                return cache;
+            RuntimeTypeCache cache = (RuntimeTypeCache)GCHandle.InternalGet(m_cache);
+            if (cache == null)
+            {
+                cache = new RuntimeTypeCache(this);
+                RuntimeTypeCache existingCache = (RuntimeTypeCache)GCHandle.InternalCompareExchange(m_cache, cache, null, false);
+                if (existingCache != null)
+                    cache = existingCache;
             }
+
+            Debug.Assert(cache != null);
+            return cache;
         }
 
         private string GetDefaultMemberName()
index f6daa3f..37e7150 100644 (file)
@@ -2,17 +2,18 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-namespace System.Runtime.InteropServices
-{
-    using System;
-    using System.Runtime.CompilerServices;
-    using System.Threading;
+using System;
+using System.Runtime.CompilerServices;
+using System.Threading;
 #if BIT64
-    using nint = System.Int64;
+using nint = System.Int64;
 #else
-    using nint = System.Int32;
+using nint = System.Int32;
 #endif
+using Internal.Runtime.CompilerServices;
 
+namespace System.Runtime.InteropServices
+{
     // These are the types of handles used by the EE.  
     // IMPORTANT: These must match the definitions in ObjectHandle.h in the EE. 
     // IMPORTANT: If new values are added to the enum the GCHandle::MaxHandleType
@@ -247,14 +248,20 @@ namespace System.Runtime.InteropServices
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
         internal static extern void InternalFree(IntPtr handle);
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
-        internal static extern object InternalGet(IntPtr handle);
-        [MethodImplAttribute(MethodImplOptions.InternalCall)]
         internal static extern void InternalSet(IntPtr handle, object value, bool isPinned);
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
         internal static extern object InternalCompareExchange(IntPtr handle, object value, object oldValue, bool isPinned);
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
         internal static extern IntPtr InternalAddrOfPinnedObject(IntPtr handle);
 
+#if DEBUG
+        // The runtime performs additional checks in debug builds
+        [MethodImplAttribute(MethodImplOptions.InternalCall)]
+        internal static extern object InternalGet(IntPtr handle);
+#else
+        internal static unsafe object InternalGet(IntPtr handle) => Unsafe.As<IntPtr, object>(ref *(IntPtr*)handle);
+#endif
+
         // The actual integer handle value that the EE uses internally.
         private IntPtr m_handle;
 
index 59d8bdd..516014f 100644 (file)
@@ -28,8 +28,8 @@ namespace System
             {
                 return false;
             }
-            RuntimeType thisType = (RuntimeType)this.GetType();
-            RuntimeType thatType = (RuntimeType)obj.GetType();
+            Type thisType = this.GetType();
+            Type thatType = obj.GetType();
 
             if (thatType != thisType)
             {
@@ -48,8 +48,8 @@ namespace System
 
             for (int i = 0; i < thisFields.Length; i++)
             {
-                thisResult = ((RtFieldInfo)thisFields[i]).UnsafeGetValue(thisObj);
-                thatResult = ((RtFieldInfo)thisFields[i]).UnsafeGetValue(obj);
+                thisResult = thisFields[i].GetValue(thisObj);
+                thatResult = thisFields[i].GetValue(obj);
 
                 if (thisResult == null)
                 {