Revert "Change BulkMoveWithWriteBarrier to be GC suspension friendly (d…

…otnet#27642)" (dotnet#27758)

This reverts commit 5e1ef69.

src/System.Private.CoreLib/src/System/Buffer.CoreCLR.cs

@@ -10,10 +10,8 @@
 #pragma warning disable SA1121 // explicitly using type aliases instead of built-in types
 #if BIT64
 using nuint = System.UInt64;
-using nint = System.UInt64;
 #else
 using nuint = System.UInt32;
-using nint = System.UInt32;
 #endif
 
 namespace System
@@ -39,53 +37,8 @@ internal static unsafe void _ZeroMemory(ref byte b, nuint byteLength)
         [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)]
         private static extern unsafe void __ZeroMemory(void* b, nuint byteLength);
 
-        // The maximum block size to for __BulkMoveWithWriteBarrier FCall. This is required to avoid GC starvation.
-        private const uint BulkMoveWithWriteBarrierChunk = 0x4000;
-
-        internal static void BulkMoveWithWriteBarrier(ref byte destination, ref byte source, nuint byteCount)
-        {
-            if (byteCount <= BulkMoveWithWriteBarrierChunk)
-                __BulkMoveWithWriteBarrier(ref destination, ref source, byteCount);
-            else
-                _BulkMoveWithWriteBarrier(ref destination, ref source, byteCount);
-        }
-
-        // Non-inlinable wrapper around the loop for copying large blocks in chunks
-        [MethodImpl(MethodImplOptions.NoInlining)]
-        private static void _BulkMoveWithWriteBarrier(ref byte destination, ref byte source, nuint byteCount)
-        {
-            Debug.Assert(byteCount > BulkMoveWithWriteBarrierChunk);
-
-            if (Unsafe.AreSame(ref destination, ref source))
-                return;
-
-            if ((nuint)(nint)Unsafe.ByteOffset(ref destination, ref source) >= byteCount)
-            {
-                // Copy forwards
-                do
-                {
-                    byteCount -= BulkMoveWithWriteBarrierChunk;
-                    __BulkMoveWithWriteBarrier(ref destination, ref source, BulkMoveWithWriteBarrierChunk);
-                    destination = ref Unsafe.AddByteOffset(ref destination, BulkMoveWithWriteBarrierChunk);
-                    source = ref Unsafe.AddByteOffset(ref source, BulkMoveWithWriteBarrierChunk);
-                }
-                while (byteCount > BulkMoveWithWriteBarrierChunk);
-            }
-            else
-            {
-                // Copy backwards
-                do
-                {
-                    byteCount -= BulkMoveWithWriteBarrierChunk;
-                    __BulkMoveWithWriteBarrier(ref Unsafe.AddByteOffset(ref destination, byteCount), ref Unsafe.AddByteOffset(ref source, byteCount), BulkMoveWithWriteBarrierChunk);
-                }
-                while (byteCount > BulkMoveWithWriteBarrierChunk);
-            }
-            __BulkMoveWithWriteBarrier(ref destination, ref source, byteCount);
-        }
-
         [MethodImpl(MethodImplOptions.InternalCall)]
-        private static extern void __BulkMoveWithWriteBarrier(ref byte destination, ref byte source, nuint byteCount);
+        internal static extern void BulkMoveWithWriteBarrier(ref byte destination, ref byte source, nuint byteCount);
 
         [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)]
         private static extern unsafe void __Memmove(byte* dest, byte* src, nuint len);

src/System.Private.CoreLib/src/System/Object.CoreCLR.cs

@@ -4,15 +4,6 @@
 
 using System.Runtime.CompilerServices;
 
-using Internal.Runtime.CompilerServices;
-
-#pragma warning disable SA1121 // explicitly using type aliases instead of built-in types
-#if BIT64
-using nuint = System.UInt64;
-#else
-using nuint = System.UInt32;
-#endif
-
 namespace System
 {
     public partial class Object
@@ -25,22 +16,7 @@ public partial class Object
         // object.  This is always a shallow copy of the instance. The method is protected
         // so that other object may only call this method on themselves.  It is intended to
         // support the ICloneable interface.
-        protected unsafe object MemberwiseClone()
-        {
-            object clone = RuntimeHelpers.AllocateUninitializedClone(this);
-
-            // copy contents of "this" to the clone
-
-            nuint byteCount = RuntimeHelpers.GetRawObjectDataSize(clone);
-            ref byte src = ref this.GetRawData();
-            ref byte dst = ref clone.GetRawData();
-
-            if (RuntimeHelpers.GetMethodTable(clone)->ContainsGCPointers)
-                Buffer.BulkMoveWithWriteBarrier(ref dst, ref src, byteCount);
-            else
-                Buffer.Memmove(ref dst, ref src, byteCount);
-
-            return clone;
-        }
+        [MethodImpl(MethodImplOptions.InternalCall)]
+        protected extern object MemberwiseClone();
     }
 }

src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

@@ -149,9 +149,6 @@ public static int OffsetToStringData
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern object GetUninitializedObjectInternal(Type type);
 
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        internal static extern object AllocateUninitializedClone(object obj);
-
         /// <returns>true if given type is reference type or value type that contains references</returns>
         [Intrinsic]
         public static bool IsReferenceOrContainsReferences<T>()
@@ -192,21 +189,6 @@ internal static int EnumCompareTo<T>(T x, T y) where T : struct, Enum
         internal static ref byte GetRawData(this object obj) =>
             ref Unsafe.As<RawData>(obj).Data;
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        internal static unsafe nuint GetRawObjectDataSize(object obj)
-        {
-            MethodTable* pMT = GetMethodTable(obj);
-
-            // See comment on RawArrayData for details
-            nuint rawSize = pMT->BaseSize - (nuint)(2 * sizeof(IntPtr));
-            if (pMT->HasComponentSize)
-                rawSize += (uint)Unsafe.As<RawArrayData>(obj).Length * (nuint)pMT->ComponentSize;
-
-            GC.KeepAlive(obj); // Keep MethodTable alive
-
-            return rawSize;
-        }
-
         [Intrinsic]
         internal static ref byte GetRawSzArrayData(this Array array) =>
             ref Unsafe.As<RawArrayData>(array).Data;

src/classlibnative/bcltype/objectnative.cpp

@@ -221,19 +221,22 @@ FCIMPL1(Object*, ObjectNative::GetClass, Object* pThis)
 }
 FCIMPLEND
 
-FCIMPL1(Object*, ObjectNative::AllocateUninitializedClone, Object* pObjUNSAFE)
+FCIMPL1(Object*, ObjectNative::Clone, Object* pThisUNSAFE)
 {
     FCALL_CONTRACT;
 
-    // Delegate error handling to managed side (it will throw NullRefenceException)
-    if (pObjUNSAFE == NULL)
-        return NULL;
+    OBJECTREF refClone = NULL;
+    OBJECTREF refThis  = ObjectToOBJECTREF(pThisUNSAFE);
 
-    OBJECTREF refClone  = ObjectToOBJECTREF(pObjUNSAFE);
+    if (refThis == NULL)
+        FCThrow(kNullReferenceException);
+
+    HELPER_METHOD_FRAME_BEGIN_RET_2(refClone, refThis);
 
-    HELPER_METHOD_FRAME_BEGIN_RET_1(refClone);
+    // ObjectNative::Clone() ensures that the source and destination are always in
+    // the same context.
 
-    MethodTable* pMT = refClone->GetMethodTable();
+    MethodTable* pMT = refThis->GetMethodTable();
 
     // assert that String has overloaded the Clone() method
     _ASSERTE(pMT != g_pStringClass);
@@ -242,13 +245,25 @@ FCIMPL1(Object*, ObjectNative::AllocateUninitializedClone, Object* pObjUNSAFE)
 #endif // FEATURE_UTF8STRING
 
     if (pMT->IsArray()) {
-        refClone = DupArrayForCloning((BASEARRAYREF)refClone);
+        refClone = DupArrayForCloning((BASEARRAYREF)refThis);
     } else {
         // We don't need to call the <cinit> because we know
         //  that it has been called....(It was called before this was created)
         refClone = AllocateObject(pMT);
     }
 
+    SIZE_T cb = refThis->GetSize() - sizeof(ObjHeader);
+
+    // copy contents of "this" to the clone
+    if (pMT->ContainsPointers())
+    {
+        memmoveGCRefs(OBJECTREFToObject(refClone), OBJECTREFToObject(refThis), cb);
+    }
+    else
+    {
+        memcpyNoGCRefs(OBJECTREFToObject(refClone), OBJECTREFToObject(refThis), cb);
+    }
+
     HELPER_METHOD_FRAME_END();
 
     return OBJECTREFToObject(refClone);

src/classlibnative/bcltype/objectnative.h

@@ -32,7 +32,7 @@ class ObjectNative
     static FCDECL1(Object*, GetObjectValue, Object* vThisRef);
     static FCDECL1(INT32, GetHashCode, Object* vThisRef);
     static FCDECL2(FC_BOOL_RET, Equals, Object *pThisRef, Object *pCompareRef);
-    static FCDECL1(Object*, AllocateUninitializedClone, Object* pObjUNSAFE);
+    static FCDECL1(Object*, Clone, Object* pThis);
     static FCDECL1(Object*, GetClass, Object* pThis);
     static FCDECL3(FC_BOOL_RET, WaitTimeout, CLR_BOOL exitContext, INT32 Timeout, Object* pThisUNSAFE);
     static FCDECL1(void, Pulse, Object* pThisUNSAFE);

src/vm/ecalllist.h

@@ -92,6 +92,7 @@ FCFuncEnd()
 
 FCFuncStart(gObjectFuncs)
     FCIntrinsic("GetType", ObjectNative::GetClass, CORINFO_INTRINSIC_Object_GetType)
+    FCFuncElement("MemberwiseClone", ObjectNative::Clone)
 FCFuncEnd()
 
 FCFuncStart(gStringFuncs)
@@ -727,7 +728,7 @@ FCFuncEnd()
 FCFuncStart(gBufferFuncs)
     FCFuncElement("IsPrimitiveTypeArray", Buffer::IsPrimitiveTypeArray)
     QCFuncElement("__ZeroMemory", Buffer::Clear)
-    FCFuncElement("__BulkMoveWithWriteBarrier", Buffer::BulkMoveWithWriteBarrier)
+    FCFuncElement("BulkMoveWithWriteBarrier", Buffer::BulkMoveWithWriteBarrier)
     QCFuncElement("__Memmove", Buffer::MemMove)
 FCFuncEnd()
 
@@ -911,7 +912,6 @@ FCFuncStart(gRuntimeHelpers)
     FCFuncElement("PrepareDelegate", ReflectionInvocation::PrepareDelegate)
     FCFuncElement("GetHashCode", ObjectNative::GetHashCode)
     FCFuncElement("Equals", ObjectNative::Equals)
-    FCFuncElement("AllocateUninitializedClone", ObjectNative::AllocateUninitializedClone)
     FCFuncElement("EnsureSufficientExecutionStack", ReflectionInvocation::EnsureSufficientExecutionStack)
     FCFuncElement("TryEnsureSufficientExecutionStack", ReflectionInvocation::TryEnsureSufficientExecutionStack)
     FCFuncElement("GetUninitializedObjectInternal", ReflectionSerialization::GetUninitializedObject)