Optimize Activator.CreateInstance (#25145)

* Optimize Activator.CreateInstance
- Short-circuit Activator.CreateInstance<T>() for value types without default constructor
- Cache default constructor delegate on RuntimeType instead of fixed-size singleton cache

8 files changed, 104 insertions, 227 deletions

diff --git a/src/System.Private.CoreLib/shared/System/Activator.RuntimeType.cs b/src/System.Private.CoreLib/shared/System/Activator.RuntimeType.cs
index 72e312aae7..4f35d53ca3 100644
--- a/src/System.Private.CoreLib/shared/System/Activator.RuntimeType.cs
+++ b/src/System.Private.CoreLib/shared/System/Activator.RuntimeType.cs
@@ -127,26 +127,20 @@ namespace System
             }
 
             if (type == null)
-            {                
+            {
                 type = assembly!.GetType(typeName, throwOnError: true, ignoreCase);
             }
 
             object? o = CreateInstance(type!, bindingAttr, binder, args, culture, activationAttributes);
 
-            return o != null ? new ObjectHandle(o) : null;          
+            return o != null ? new ObjectHandle(o) : null;
         }
 
         public static T CreateInstance<T>()
         {
-            var rt = (RuntimeType)typeof(T);
-
-            // This is a workaround to maintain compatibility with V2. Without this we would throw a NotSupportedException for void[].
-            // Array, Ref, and Pointer types don't have default constructors.
-            if (rt.HasElementType)
-                throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, rt));
-
-            // Skip the CreateInstanceCheckThis call to avoid perf cost and to maintain compatibility with V2 (throwing the same exceptions).
-            return (T)rt.CreateInstanceDefaultCtor(publicOnly: true, skipCheckThis: true, fillCache: true, wrapExceptions: true);
+            return (T)((RuntimeType)typeof(T)).CreateInstanceDefaultCtor(publicOnly: true, skipCheckThis: true, fillCache: true, wrapExceptions: true);
         }
+
+        private static T CreateDefaultInstance<T>() where T: struct => default;
     }
 }
diff --git a/src/System.Private.CoreLib/src/System/RtType.cs b/src/System.Private.CoreLib/src/System/RtType.cs
index 4277b79b2e..d163c744e9 100644
--- a/src/System.Private.CoreLib/src/System/RtType.cs
+++ b/src/System.Private.CoreLib/src/System/RtType.cs
@@ -1862,7 +1862,7 @@ namespace System
 
         internal object? GenericCache
         {
-            get => Cache.GenericCache;
+            get => CacheIfExists?.GenericCache;
             set => Cache.GenericCache = value;
         }
 
@@ -2376,6 +2376,21 @@ namespace System
             return (o is RuntimeType t) && (t.m_handle == m_handle);
         }
 
+        private RuntimeTypeCache? CacheIfExists
+        {
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            get
+            {
+                if (m_cache != IntPtr.Zero)
+                {
+                    object cache = GCHandle.InternalGet(m_cache);
+                    Debug.Assert(cache == null || cache is RuntimeTypeCache);
+                    return Unsafe.As<RuntimeTypeCache>(cache);
+                }
+                return null;
+            }
+        }
+
         private RuntimeTypeCache Cache
         {
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -4439,84 +4454,45 @@ namespace System
         }
 
         // the cache entry
-        private class ActivatorCacheEntry
+        private sealed class ActivatorCache
         {
-            // the type to cache
-            internal readonly RuntimeType _type;
-            // the delegate containing the call to the ctor, will be replaced by an IntPtr to feed a calli with
-            internal volatile CtorDelegate? _ctor;
+            // the delegate containing the call to the ctor
             internal readonly RuntimeMethodHandleInternal _hCtorMethodHandle;
-            internal readonly MethodAttributes _ctorAttributes;
+            internal MethodAttributes _ctorAttributes;
+            internal CtorDelegate? _ctor;
+
             // Lazy initialization was performed
             internal volatile bool _isFullyInitialized;
 
-            internal ActivatorCacheEntry(RuntimeType t, RuntimeMethodHandleInternal rmh)
-            {
-                _type = t;
-                _hCtorMethodHandle = rmh;
-                if (!_hCtorMethodHandle.IsNullHandle())
-                    _ctorAttributes = RuntimeMethodHandle.GetAttributes(_hCtorMethodHandle);
-            }
-        }
+            private static ConstructorInfo? s_delegateCtorInfo;
 
-        private class ActivatorCache
-        {
-            private const int CacheSize = 16;
-            private volatile int hash_counter; //Counter for wrap around
-            private readonly ActivatorCacheEntry[] cache = new ActivatorCacheEntry[CacheSize];
-
-            private volatile ConstructorInfo? delegateCtorInfo;
-
-            private void InitializeDelegateCreator()
+            internal ActivatorCache(RuntimeMethodHandleInternal rmh)
             {
-                ConstructorInfo? ctorInfo = typeof(CtorDelegate).GetConstructor(new Type[] { typeof(object), typeof(IntPtr) });
-                delegateCtorInfo = ctorInfo; // this assignment should be last
+                _hCtorMethodHandle = rmh;
             }
 
-            private void InitializeCacheEntry(ActivatorCacheEntry ace)
+            private void Initialize()
             {
-                if (!ace._type.IsValueType)
+                if (!_hCtorMethodHandle.IsNullHandle())
                 {
-                    Debug.Assert(!ace._hCtorMethodHandle.IsNullHandle(), "Expected the default ctor method handle for a reference type.");
+                    _ctorAttributes = RuntimeMethodHandle.GetAttributes(_hCtorMethodHandle);
 
-                    if (delegateCtorInfo == null)
-                        InitializeDelegateCreator();
+                    // The default ctor path is optimized for reference types only
+                    ConstructorInfo delegateCtorInfo = s_delegateCtorInfo ?? (s_delegateCtorInfo = typeof(CtorDelegate).GetConstructor(new Type[] { typeof(object), typeof(IntPtr) })!);
 
                     // No synchronization needed here. In the worst case we create extra garbage
-                    CtorDelegate ctor = (CtorDelegate)delegateCtorInfo!.Invoke(new object?[] { null, RuntimeMethodHandle.GetFunctionPointer(ace._hCtorMethodHandle) });
-                    ace._ctor = ctor;
+                    _ctor = (CtorDelegate)delegateCtorInfo.Invoke(new object?[] { null, RuntimeMethodHandle.GetFunctionPointer(_hCtorMethodHandle) });
                 }
-                ace._isFullyInitialized = true;
+                _isFullyInitialized = true;
             }
 
-            internal ActivatorCacheEntry? GetEntry(RuntimeType t)
+            public void EnsureInitialized()
             {
-                int index = hash_counter;
-                for (int i = 0; i < CacheSize; i++)
-                {
-                    ActivatorCacheEntry ace = Volatile.Read(ref cache[index]);
-                    if (ace != null && ace._type == t) //check for type match..
-                    {
-                        if (!ace._isFullyInitialized)
-                            InitializeCacheEntry(ace);
-                        return ace;
-                    }
-                    index = (index + 1) & (ActivatorCache.CacheSize - 1);
-                }
-                return null;
-            }
-
-            internal void SetEntry(ActivatorCacheEntry ace)
-            {
-                // fill the array backwards to hit the most recently filled entries first in GetEntry
-                int index = (hash_counter - 1) & (ActivatorCache.CacheSize - 1);
-                hash_counter = index;
-                Volatile.Write(ref cache[index], ace);
+                if (!_isFullyInitialized)
+                    Initialize();
             }
         }
 
-        private static volatile ActivatorCache s_ActivatorCache;
-
         /// <summary>
         /// The slow path of CreateInstanceDefaultCtor
         /// </summary>
@@ -4534,17 +4510,8 @@ namespace System
 
             if (canBeCached && fillCache)
             {
-                ActivatorCache activatorCache = s_ActivatorCache;
-                if (activatorCache == null)
-                {
-                    // No synchronization needed here. In the worst case we create extra garbage
-                    activatorCache = new ActivatorCache();
-                    s_ActivatorCache = activatorCache;
-                }
-
                 // cache the ctor
-                ActivatorCacheEntry ace = new ActivatorCacheEntry(this, runtimeCtor);
-                activatorCache.SetEntry(ace);
+                GenericCache = new ActivatorCache(runtimeCtor);
             }
 
             return instance;
@@ -4558,9 +4525,11 @@ namespace System
         internal object CreateInstanceDefaultCtor(bool publicOnly, bool skipCheckThis, bool fillCache, bool wrapExceptions)
         {
             // Call the cached 
-            ActivatorCacheEntry? cacheEntry = s_ActivatorCache?.GetEntry(this);
+            ActivatorCache? cacheEntry = GenericCache as ActivatorCache;
             if (cacheEntry != null)
             {
+                cacheEntry.EnsureInitialized();
+
                 if (publicOnly)
                 {
                     if (cacheEntry._ctor != null &&
diff --git a/src/vm/comdelegate.cpp b/src/vm/comdelegate.cpp
index c0392eedf6..a7ed685c3e 100644
--- a/src/vm/comdelegate.cpp
+++ b/src/vm/comdelegate.cpp
@@ -3204,109 +3204,6 @@ BOOL COMDelegate::ValidateCtor(TypeHandle instHnd,
     return IsMethodDescCompatible(instHnd, ftnParentHnd, pFtn, dlgtHnd, pDlgtInvoke, DBF_RelaxedSignature, pfIsOpenDelegate);
 }
 
-BOOL COMDelegate::ValidateBeginInvoke(DelegateEEClass* pClass)
-{
-    CONTRACTL
-    {
-        THROWS;
-        GC_TRIGGERS;
-        MODE_ANY;
-
-        PRECONDITION(CheckPointer(pClass));
-        PRECONDITION(CheckPointer(pClass->GetBeginInvokeMethod()));
-
-        // insert fault. Can the binder throw an OOM?
-    }
-    CONTRACTL_END;
-
-    if (pClass->GetInvokeMethod() == NULL)
-        return FALSE;
-
-    // We check the signatures under the typical instantiation of the possibly generic class 
-    MetaSig beginInvokeSig(pClass->GetBeginInvokeMethod()->LoadTypicalMethodDefinition());
-    MetaSig invokeSig(pClass->GetInvokeMethod()->LoadTypicalMethodDefinition());
-
-    if (beginInvokeSig.GetCallingConventionInfo() != (IMAGE_CEE_CS_CALLCONV_HASTHIS | IMAGE_CEE_CS_CALLCONV_DEFAULT))
-        return FALSE;
-
-    if (beginInvokeSig.NumFixedArgs() != invokeSig.NumFixedArgs() + 2)
-        return FALSE;
-
-    if (beginInvokeSig.GetRetTypeHandleThrowing() != TypeHandle(MscorlibBinder::GetClass(CLASS__IASYNCRESULT)))
-        return FALSE;
-
-    while(invokeSig.NextArg() != ELEMENT_TYPE_END)
-    {
-        beginInvokeSig.NextArg();
-        if (beginInvokeSig.GetLastTypeHandleThrowing() != invokeSig.GetLastTypeHandleThrowing())
-            return FALSE;
-    }
-
-    beginInvokeSig.NextArg();
-    if (beginInvokeSig.GetLastTypeHandleThrowing()!= TypeHandle(MscorlibBinder::GetClass(CLASS__ASYNCCALLBACK)))
-        return FALSE;
-
-    beginInvokeSig.NextArg();
-    if (beginInvokeSig.GetLastTypeHandleThrowing()!= TypeHandle(g_pObjectClass))
-        return FALSE;
-
-    if (beginInvokeSig.NextArg() != ELEMENT_TYPE_END)
-        return FALSE;
-
-    return TRUE;
-}
-
-BOOL COMDelegate::ValidateEndInvoke(DelegateEEClass* pClass)
-{
-    CONTRACTL
-    {
-        THROWS;
-        GC_TRIGGERS;
-        MODE_ANY;
-
-        PRECONDITION(CheckPointer(pClass));
-        PRECONDITION(CheckPointer(pClass->GetEndInvokeMethod()));
-
-        // insert fault. Can the binder throw an OOM?
-    }
-    CONTRACTL_END;
-
-    if (pClass->GetInvokeMethod() == NULL)
-        return FALSE;
-
-    // We check the signatures under the typical instantiation of the possibly generic class 
-    MetaSig endInvokeSig(pClass->GetEndInvokeMethod()->LoadTypicalMethodDefinition());
-    MetaSig invokeSig(pClass->GetInvokeMethod()->LoadTypicalMethodDefinition());
-
-    if (endInvokeSig.GetCallingConventionInfo() != (IMAGE_CEE_CS_CALLCONV_HASTHIS | IMAGE_CEE_CS_CALLCONV_DEFAULT))
-        return FALSE;
-
-    if (endInvokeSig.GetRetTypeHandleThrowing() != invokeSig.GetRetTypeHandleThrowing())
-        return FALSE;
-
-    CorElementType type;
-    while((type = invokeSig.NextArg()) != ELEMENT_TYPE_END)
-    {
-        if (type == ELEMENT_TYPE_BYREF)
-        {
-            endInvokeSig.NextArg();
-            if (endInvokeSig.GetLastTypeHandleThrowing() != invokeSig.GetLastTypeHandleThrowing())
-                return FALSE;
-        }
-    }
-
-    if (endInvokeSig.NextArg() == ELEMENT_TYPE_END)
-        return FALSE;
-
-    if (endInvokeSig.GetLastTypeHandleThrowing() != TypeHandle(MscorlibBinder::GetClass(CLASS__IASYNCRESULT)))
-        return FALSE;
-
-    if (endInvokeSig.NextArg() != ELEMENT_TYPE_END)
-        return FALSE;
-
-    return TRUE;
-}
-
 BOOL COMDelegate::IsSecureDelegate(DELEGATEREF dRef)
 {
     CONTRACTL
diff --git a/src/vm/comdelegate.h b/src/vm/comdelegate.h
index 862f469a04..3d5ec8e30e 100644
--- a/src/vm/comdelegate.h
+++ b/src/vm/comdelegate.h
@@ -138,9 +138,6 @@ public:
     static BOOL ValidateCtor(TypeHandle objHnd, TypeHandle ftnParentHnd, MethodDesc *pFtn, TypeHandle dlgtHnd, BOOL *pfIsOpenDelegate);
 
 private:
-    static BOOL ValidateBeginInvoke(DelegateEEClass* pClass);   // make certain the BeginInvoke method is consistant with the Invoke Method
-    static BOOL ValidateEndInvoke(DelegateEEClass* pClass);     // make certain the EndInvoke method is consistant with the Invoke Method
-
     static void BindToMethod(DELEGATEREF   *pRefThis,
                              OBJECTREF     *pRefFirstArg,
                              MethodDesc    *pTargetMethod,
diff --git a/src/vm/jitinterface.cpp b/src/vm/jitinterface.cpp
index de9538e6c2..d67ad2f251 100644
--- a/src/vm/jitinterface.cpp
+++ b/src/vm/jitinterface.cpp
@@ -7000,8 +7000,7 @@ bool getILIntrinsicImplementation(MethodDesc * ftn,
 {
     STANDARD_VM_CONTRACT;
 
-    // Precondition: ftn is a method in mscorlib 
-    _ASSERTE(ftn->GetModule()->IsSystem());
+    _ASSERTE(MscorlibBinder::IsClass(ftn->GetMethodTable(), CLASS__JIT_HELPERS));
 
     mdMethodDef tk = ftn->GetMemberDef();
 
@@ -7129,8 +7128,7 @@ bool getILIntrinsicImplementationForUnsafe(MethodDesc * ftn,
 {
     STANDARD_VM_CONTRACT;
 
-    // Precondition: ftn is a method in mscorlib 
-    _ASSERTE(ftn->GetModule()->IsSystem());
+    _ASSERTE(MscorlibBinder::IsClass(ftn->GetMethodTable(), CLASS__UNSAFE));
 
     mdMethodDef tk = ftn->GetMemberDef();
 
@@ -7371,10 +7369,7 @@ bool getILIntrinsicImplementationForVolatile(MethodDesc * ftn,
     // we substitute raw IL bodies for these methods that use the correct volatile instructions.
     //
 
-    // Precondition: ftn is a method in mscorlib in the System.Threading.Volatile class
-    _ASSERTE(ftn->GetModule()->IsSystem());
     _ASSERTE(MscorlibBinder::IsClass(ftn->GetMethodTable(), CLASS__VOLATILE));
-    _ASSERTE(strcmp(ftn->GetMethodTable()->GetClass()->GetDebugClassName(), "System.Threading.Volatile") == 0);
 
     const size_t VolatileMethodBodySize = 6;
 
@@ -7453,20 +7448,14 @@ bool getILIntrinsicImplementationForInterlocked(MethodDesc * ftn,
 {
     STANDARD_VM_CONTRACT;
 
-    // Precondition: ftn is a method in mscorlib in the System.Threading.Interlocked class
-    _ASSERTE(ftn->GetModule()->IsSystem());
     _ASSERTE(MscorlibBinder::IsClass(ftn->GetMethodTable(), CLASS__INTERLOCKED));
 
     // We are only interested if ftn's token and CompareExchange<T> token match
     if (ftn->GetMemberDef() != MscorlibBinder::GetMethod(METHOD__INTERLOCKED__COMPARE_EXCHANGE_T)->GetMemberDef())
-        return false;       
-
-    // Get MethodDesc for System.Threading.Interlocked.CompareExchangeFast()
-    MethodDesc* cmpxchgFast = MscorlibBinder::GetMethod(METHOD__INTERLOCKED__COMPARE_EXCHANGE_OBJECT);
+        return false;
 
-    // The MethodDesc lookup must not fail, and it should have the name "CompareExchangeFast"
-    _ASSERTE(cmpxchgFast != NULL);
-    _ASSERTE(strcmp(cmpxchgFast->GetName(), "CompareExchange") == 0);
+    // Get MethodDesc for non-generic System.Threading.Interlocked.CompareExchange()
+    MethodDesc* cmpxchgObject = MscorlibBinder::GetMethod(METHOD__INTERLOCKED__COMPARE_EXCHANGE_OBJECT);
 
     // Setup up the body of the method
     static BYTE il[] = {
@@ -7477,12 +7466,12 @@ bool getILIntrinsicImplementationForInterlocked(MethodDesc * ftn,
                           CEE_RET
                         };
 
-    // Get the token for System.Threading.Interlocked.CompareExchangeFast(), and patch [target]
-    mdMethodDef cmpxchgFastToken = cmpxchgFast->GetMemberDef();
-    il[4] = (BYTE)((int)cmpxchgFastToken >> 0);
-    il[5] = (BYTE)((int)cmpxchgFastToken >> 8);
-    il[6] = (BYTE)((int)cmpxchgFastToken >> 16);
-    il[7] = (BYTE)((int)cmpxchgFastToken >> 24);
+    // Get the token for non-generic System.Threading.Interlocked.CompareExchange(), and patch [target]
+    mdMethodDef cmpxchgObjectToken = cmpxchgObject->GetMemberDef();
+    il[4] = (BYTE)((int)cmpxchgObjectToken >> 0);
+    il[5] = (BYTE)((int)cmpxchgObjectToken >> 8);
+    il[6] = (BYTE)((int)cmpxchgObjectToken >> 16);
+    il[7] = (BYTE)((int)cmpxchgObjectToken >> 24);
 
     // Initialize methInfo
     methInfo->ILCode = const_cast<BYTE*>(il);
@@ -7499,8 +7488,7 @@ bool getILIntrinsicImplementationForRuntimeHelpers(MethodDesc * ftn,
 {
     STANDARD_VM_CONTRACT;
 
-    // Precondition: ftn is a method in mscorlib 
-    _ASSERTE(ftn->GetModule()->IsSystem());
+    _ASSERTE(MscorlibBinder::IsClass(ftn->GetMethodTable(), CLASS__RUNTIME_HELPERS));
 
     mdMethodDef tk = ftn->GetMemberDef();
 
@@ -7585,6 +7573,37 @@ bool getILIntrinsicImplementationForRuntimeHelpers(MethodDesc * ftn,
     return false;
 }
 
+bool getILIntrinsicImplementationForActivator(MethodDesc* ftn,
+    CORINFO_METHOD_INFO* methInfo,
+    SigPointer* pSig)
+{
+    STANDARD_VM_CONTRACT;
+
+    _ASSERTE(MscorlibBinder::IsClass(ftn->GetMethodTable(), CLASS__ACTIVATOR));
+
+    // We are only interested if ftn's token and CreateInstance<T> token match
+    if (ftn->GetMemberDef() != MscorlibBinder::GetMethod(METHOD__ACTIVATOR__CREATE_INSTANCE_OF_T)->GetMemberDef())
+        return false;
+
+    _ASSERTE(ftn->HasMethodInstantiation());
+    Instantiation inst = ftn->GetMethodInstantiation();
+
+    _ASSERTE(ftn->GetNumGenericMethodArgs() == 1);
+    TypeHandle typeHandle = inst[0];
+    MethodTable* methodTable = typeHandle.GetMethodTable();
+
+    if (!methodTable->IsValueType() || methodTable->HasDefaultConstructor())
+        return false;
+
+    // Replace the body with implementation that just returns "default"
+    MethodDesc* createDefaultInstance = MscorlibBinder::GetMethod(METHOD__ACTIVATOR__CREATE_DEFAULT_INSTANCE_OF_T);
+    COR_ILMETHOD_DECODER header(createDefaultInstance->GetILHeader(FALSE), createDefaultInstance->GetMDImport(), NULL);
+    getMethodInfoILMethodHeaderHelper(&header, methInfo);
+    *pSig = SigPointer(header.LocalVarSig, header.cbLocalVarSig);
+
+    return true;
+}
+
 //---------------------------------------------------------------------------------------
 // 
 //static
@@ -7640,6 +7659,15 @@ getMethodInfoHelper(
             {
                 fILIntrinsic = getILIntrinsicImplementationForRuntimeHelpers(ftn, methInfo);
             }
+            else if (MscorlibBinder::IsClass(pMT, CLASS__ACTIVATOR))
+            {
+                SigPointer localSig;
+                fILIntrinsic = getILIntrinsicImplementationForActivator(ftn, methInfo, &localSig);
+                if (fILIntrinsic)
+                {
+                    localSig.GetSignature(&pLocalSig, &cbLocalSig);
+                }
+            }
         }
 
         if (!fILIntrinsic)
@@ -7653,7 +7681,7 @@ getMethodInfoHelper(
     {
         _ASSERTE(ftn->IsDynamicMethod());
 
-        DynamicResolver * pResolver = ftn->AsDynamicMethodDesc()->GetResolver();        
+        DynamicResolver * pResolver = ftn->AsDynamicMethodDesc()->GetResolver();
         unsigned int EHCount;
         methInfo->ILCode = pResolver->GetCodeInfo(&methInfo->ILCodeSize,
                                                   &methInfo->maxStack,
diff --git a/src/vm/metasig.h b/src/vm/metasig.h
index 5e0a821e44..15402805f0 100644
--- a/src/vm/metasig.h
+++ b/src/vm/metasig.h
@@ -601,6 +601,8 @@ DEFINE_METASIG_T(IM(IAsyncResult_RetVoid, C(IASYNCRESULT), v))
 DEFINE_METASIG(IM(Int_RetRefT, i, r(G(0))))
 DEFINE_METASIG_T(IM(Int_RetReadOnlyRefT, i, Q(INATTRIBUTE) r(G(0))))
 
+DEFINE_METASIG(GM(RetT, IMAGE_CEE_CS_CALLCONV_DEFAULT, 1, _, M(0)))
+
 // Undefine macros in case we include the file again in the compilation unit
 
 #undef  DEFINE_METASIG
diff --git a/src/vm/mscorlib.h b/src/vm/mscorlib.h
index c986137b99..403b27bf87 100644
--- a/src/vm/mscorlib.h
+++ b/src/vm/mscorlib.h
@@ -72,6 +72,8 @@
 // NOTE: Make this window really wide if you want to read the table...
 
 DEFINE_CLASS(ACTIVATOR,             System,                 Activator)
+DEFINE_METHOD(ACTIVATOR,            CREATE_INSTANCE_OF_T,   CreateInstance, GM_RetT)
+DEFINE_METHOD(ACTIVATOR,            CREATE_DEFAULT_INSTANCE_OF_T,   CreateDefaultInstance,  GM_RetT)
 
 DEFINE_CLASS(ACCESS_VIOLATION_EXCEPTION, System,            AccessViolationException)
 DEFINE_FIELD(ACCESS_VIOLATION_EXCEPTION, IP,                _ip)
@@ -457,14 +459,8 @@ DEFINE_METHOD(ICUSTOM_QUERYINTERFACE,     GET_INTERFACE,    GetInterface,
 DEFINE_CLASS(CUSTOMQUERYINTERFACERESULT,  Interop,          CustomQueryInterfaceResult)
 #endif //FEATURE_COMINTEROP
 
-
-DEFINE_CLASS(ISERIALIZABLE,         Serialization,          ISerializable)
-DEFINE_CLASS(IOBJECTREFERENCE,      Serialization,          IObjectReference)
-DEFINE_CLASS(IDESERIALIZATIONCB,    Serialization,          IDeserializationCallback)
-DEFINE_CLASS(STREAMING_CONTEXT,     Serialization,          StreamingContext)
-DEFINE_CLASS(SERIALIZATION_INFO,    Serialization,          SerializationInfo)
-DEFINE_CLASS(DESERIALIZATION_TRACKER, Serialization, DeserializationTracker)
-
+DEFINE_CLASS(SERIALIZATION_INFO,        Serialization,      SerializationInfo)
+DEFINE_CLASS(DESERIALIZATION_TRACKER,   Serialization,      DeserializationTracker)
 
 DEFINE_CLASS(IENUMERATOR,           Collections,            IEnumerator)
 
@@ -844,8 +840,6 @@ DEFINE_FIELD_U(_requireWaitNotification, SynchronizationContextObject, _requireW
 DEFINE_CLASS(SYNCHRONIZATION_CONTEXT,    Threading,              SynchronizationContext)
 DEFINE_METHOD(SYNCHRONIZATION_CONTEXT,  INVOKE_WAIT_METHOD_HELPER, InvokeWaitMethodHelper, SM_SyncCtx_ArrIntPtr_Bool_Int_RetInt)
 
-DEFINE_CLASS(CONTEXTCALLBACK,       Threading,       ContextCallback)
-
 #ifdef _DEBUG
 DEFINE_CLASS(STACKCRAWMARK,         Threading,       StackCrawlMark)
 #endif
diff --git a/src/vm/reflectioninvocation.cpp b/src/vm/reflectioninvocation.cpp
index 99879bfad8..932ad11df9 100644
--- a/src/vm/reflectioninvocation.cpp
+++ b/src/vm/reflectioninvocation.cpp
@@ -476,10 +476,7 @@ FCIMPL6(Object*, RuntimeTypeHandle::CreateInstance, ReflectClassBaseObject* refT
             else 
                 rv = pVMT->Allocate();
 
-            if (!pVMT->Collectible())
-            {
-                *pbCanBeCached = true;
-            }
+            *pbCanBeCached = true;
         }
         else // !pVMT->HasDefaultConstructor()
         {
@@ -495,7 +492,6 @@ FCIMPL6(Object*, RuntimeTypeHandle::CreateInstance, ReflectClassBaseObject* refT
 
             // We've got the class, lets allocate it and call the constructor
             OBJECTREF o;
-            bool remoting = false;
 
             o = AllocateObject(pVMT);
             GCPROTECT_BEGIN(o);
@@ -518,7 +514,7 @@ FCIMPL6(Object*, RuntimeTypeHandle::CreateInstance, ReflectClassBaseObject* refT
             // No need to set these if they cannot be cached. In particular, if the type is a value type with a custom
             // parameterless constructor, don't allow caching and have subsequent calls come back here to allocate an object and
             // call the constructor.
-            if (!remoting && !pVMT->Collectible() && !pVMT->IsValueType())
+            if (!pVMT->IsValueType())
             {
                 *pbCanBeCached = true;
                 *pConstructor = pMeth;