Revert "Revert "Change BulkMoveWithWriteBarrier to be GC suspension f…

…riendly (dotnet#27642)" (dotnet#27758)"

This reverts commit b06f8a7.

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

@@ -10,8 +10,10 @@
 #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
@@ -37,8 +39,53 @@ 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)]
-        internal static extern void BulkMoveWithWriteBarrier(ref byte destination, ref byte source, nuint byteCount);
+        private 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,6 +4,15 @@
 
 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
@@ -16,7 +25,22 @@ 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.
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        protected extern object MemberwiseClone();
+        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;
+        }
     }
 }

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

@@ -149,6 +149,9 @@ 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>()
@@ -189,6 +192,21 @@ 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,22 +221,19 @@ FCIMPL1(Object*, ObjectNative::GetClass, Object* pThis)
 }
 FCIMPLEND
 
-FCIMPL1(Object*, ObjectNative::Clone, Object* pThisUNSAFE)
+FCIMPL1(Object*, ObjectNative::AllocateUninitializedClone, Object* pObjUNSAFE)
 {
     FCALL_CONTRACT;
 
-    OBJECTREF refClone = NULL;
-    OBJECTREF refThis  = ObjectToOBJECTREF(pThisUNSAFE);
+    // Delegate error handling to managed side (it will throw NullRefenceException)
+    if (pObjUNSAFE == NULL)
+        return NULL;
 
-    if (refThis == NULL)
-        FCThrow(kNullReferenceException);
-
-    HELPER_METHOD_FRAME_BEGIN_RET_2(refClone, refThis);
+    OBJECTREF refClone  = ObjectToOBJECTREF(pObjUNSAFE);
 
-    // ObjectNative::Clone() ensures that the source and destination are always in
-    // the same context.
+    HELPER_METHOD_FRAME_BEGIN_RET_1(refClone);
 
-    MethodTable* pMT = refThis->GetMethodTable();
+    MethodTable* pMT = refClone->GetMethodTable();
 
     // assert that String has overloaded the Clone() method
     _ASSERTE(pMT != g_pStringClass);
@@ -245,25 +242,13 @@ FCIMPL1(Object*, ObjectNative::Clone, Object* pThisUNSAFE)
 #endif // FEATURE_UTF8STRING
 
     if (pMT->IsArray()) {
-        refClone = DupArrayForCloning((BASEARRAYREF)refThis);
+        refClone = DupArrayForCloning((BASEARRAYREF)refClone);
     } 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*, Clone, Object* pThis);
+    static FCDECL1(Object*, AllocateUninitializedClone, Object* pObjUNSAFE);
     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,7 +92,6 @@ FCFuncEnd()
 
 FCFuncStart(gObjectFuncs)
     FCIntrinsic("GetType", ObjectNative::GetClass, CORINFO_INTRINSIC_Object_GetType)
-    FCFuncElement("MemberwiseClone", ObjectNative::Clone)
 FCFuncEnd()
 
 FCFuncStart(gStringFuncs)
@@ -728,7 +727,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()
 
@@ -912,6 +911,7 @@ 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)