Skip to content

Commit c70c01d

Browse files
jkotasjkotas
committed
Revert "Change BulkMoveWithWriteBarrier to be GC suspension friendly (dotnet#27642)"
This reverts commit 5e1ef69.
1 parent 2362662 commit c70c01d

File tree

6 files changed

+29
-103
lines changed

6 files changed

+29
-103
lines changed

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

Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@
1010
#pragma warning disable SA1121 // explicitly using type aliases instead of built-in types
1111
#if BIT64
1212
using nuint = System.UInt64;
13-
using nint = System.UInt64;
1413
#else
1514
using nuint = System.UInt32;
16-
using nint = System.UInt32;
1715
#endif
1816

1917
namespace System
@@ -39,53 +37,8 @@ internal static unsafe void _ZeroMemory(ref byte b, nuint byteLength)
3937
[DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)]
4038
private static extern unsafe void __ZeroMemory(void* b, nuint byteLength);
4139

42-
// The maximum block size to for __BulkMoveWithWriteBarrier FCall. This is required to avoid GC starvation.
43-
private const uint BulkMoveWithWriteBarrierChunk = 0x4000;
44-
45-
internal static void BulkMoveWithWriteBarrier(ref byte destination, ref byte source, nuint byteCount)
46-
{
47-
if (byteCount <= BulkMoveWithWriteBarrierChunk)
48-
__BulkMoveWithWriteBarrier(ref destination, ref source, byteCount);
49-
else
50-
_BulkMoveWithWriteBarrier(ref destination, ref source, byteCount);
51-
}
52-
53-
// Non-inlinable wrapper around the loop for copying large blocks in chunks
54-
[MethodImpl(MethodImplOptions.NoInlining)]
55-
private static void _BulkMoveWithWriteBarrier(ref byte destination, ref byte source, nuint byteCount)
56-
{
57-
Debug.Assert(byteCount > BulkMoveWithWriteBarrierChunk);
58-
59-
if (Unsafe.AreSame(ref destination, ref source))
60-
return;
61-
62-
if ((nuint)(nint)Unsafe.ByteOffset(ref destination, ref source) >= byteCount)
63-
{
64-
// Copy forwards
65-
do
66-
{
67-
byteCount -= BulkMoveWithWriteBarrierChunk;
68-
__BulkMoveWithWriteBarrier(ref destination, ref source, BulkMoveWithWriteBarrierChunk);
69-
destination = ref Unsafe.AddByteOffset(ref destination, BulkMoveWithWriteBarrierChunk);
70-
source = ref Unsafe.AddByteOffset(ref source, BulkMoveWithWriteBarrierChunk);
71-
}
72-
while (byteCount > BulkMoveWithWriteBarrierChunk);
73-
}
74-
else
75-
{
76-
// Copy backwards
77-
do
78-
{
79-
byteCount -= BulkMoveWithWriteBarrierChunk;
80-
__BulkMoveWithWriteBarrier(ref Unsafe.AddByteOffset(ref destination, byteCount), ref Unsafe.AddByteOffset(ref source, byteCount), BulkMoveWithWriteBarrierChunk);
81-
}
82-
while (byteCount > BulkMoveWithWriteBarrierChunk);
83-
}
84-
__BulkMoveWithWriteBarrier(ref destination, ref source, byteCount);
85-
}
86-
8740
[MethodImpl(MethodImplOptions.InternalCall)]
88-
private static extern void __BulkMoveWithWriteBarrier(ref byte destination, ref byte source, nuint byteCount);
41+
internal static extern void BulkMoveWithWriteBarrier(ref byte destination, ref byte source, nuint byteCount);
8942

9043
[DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)]
9144
private static extern unsafe void __Memmove(byte* dest, byte* src, nuint len);

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

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,6 @@
44

55
using System.Runtime.CompilerServices;
66

7-
using Internal.Runtime.CompilerServices;
8-
9-
#pragma warning disable SA1121 // explicitly using type aliases instead of built-in types
10-
#if BIT64
11-
using nuint = System.UInt64;
12-
#else
13-
using nuint = System.UInt32;
14-
#endif
15-
167
namespace System
178
{
189
public partial class Object
@@ -25,22 +16,7 @@ public partial class Object
2516
// object. This is always a shallow copy of the instance. The method is protected
2617
// so that other object may only call this method on themselves. It is intended to
2718
// support the ICloneable interface.
28-
protected unsafe object MemberwiseClone()
29-
{
30-
object clone = RuntimeHelpers.AllocateUninitializedClone(this);
31-
32-
// copy contents of "this" to the clone
33-
34-
nuint byteCount = RuntimeHelpers.GetRawObjectDataSize(clone);
35-
ref byte src = ref this.GetRawData();
36-
ref byte dst = ref clone.GetRawData();
37-
38-
if (RuntimeHelpers.GetMethodTable(clone)->ContainsGCPointers)
39-
Buffer.BulkMoveWithWriteBarrier(ref dst, ref src, byteCount);
40-
else
41-
Buffer.Memmove(ref dst, ref src, byteCount);
42-
43-
return clone;
44-
}
19+
[MethodImpl(MethodImplOptions.InternalCall)]
20+
protected extern object MemberwiseClone();
4521
}
4622
}

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

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,6 @@ public static int OffsetToStringData
149149
[MethodImpl(MethodImplOptions.InternalCall)]
150150
private static extern object GetUninitializedObjectInternal(Type type);
151151

152-
[MethodImpl(MethodImplOptions.InternalCall)]
153-
internal static extern object AllocateUninitializedClone(object obj);
154-
155152
/// <returns>true if given type is reference type or value type that contains references</returns>
156153
[Intrinsic]
157154
public static bool IsReferenceOrContainsReferences<T>()
@@ -192,21 +189,6 @@ internal static int EnumCompareTo<T>(T x, T y) where T : struct, Enum
192189
internal static ref byte GetRawData(this object obj) =>
193190
ref Unsafe.As<RawData>(obj).Data;
194191

195-
[MethodImpl(MethodImplOptions.AggressiveInlining)]
196-
internal static unsafe nuint GetRawObjectDataSize(object obj)
197-
{
198-
MethodTable* pMT = GetMethodTable(obj);
199-
200-
// See comment on RawArrayData for details
201-
nuint rawSize = pMT->BaseSize - (nuint)(2 * sizeof(IntPtr));
202-
if (pMT->HasComponentSize)
203-
rawSize += (uint)Unsafe.As<RawArrayData>(obj).Length * (nuint)pMT->ComponentSize;
204-
205-
GC.KeepAlive(obj); // Keep MethodTable alive
206-
207-
return rawSize;
208-
}
209-
210192
[Intrinsic]
211193
internal static ref byte GetRawSzArrayData(this Array array) =>
212194
ref Unsafe.As<RawArrayData>(array).Data;

‎src/classlibnative/bcltype/objectnative.cpp‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -221,19 +221,22 @@ FCIMPL1(Object*, ObjectNative::GetClass, Object* pThis)
221221
}
222222
FCIMPLEND
223223

224-
FCIMPL1(Object*, ObjectNative::AllocateUninitializedClone, Object* pObjUNSAFE)
224+
FCIMPL1(Object*, ObjectNative::Clone, Object* pThisUNSAFE)
225225
{
226226
FCALL_CONTRACT;
227227

228-
// Delegate error handling to managed side (it will throw NullRefenceException)
229-
if (pObjUNSAFE == NULL)
230-
return NULL;
228+
OBJECTREF refClone = NULL;
229+
OBJECTREF refThis = ObjectToOBJECTREF(pThisUNSAFE);
231230

232-
OBJECTREF refClone = ObjectToOBJECTREF(pObjUNSAFE);
231+
if (refThis == NULL)
232+
FCThrow(kNullReferenceException);
233+
234+
HELPER_METHOD_FRAME_BEGIN_RET_2(refClone, refThis);
233235

234-
HELPER_METHOD_FRAME_BEGIN_RET_1(refClone);
236+
// ObjectNative::Clone() ensures that the source and destination are always in
237+
// the same context.
235238

236-
MethodTable* pMT = refClone->GetMethodTable();
239+
MethodTable* pMT = refThis->GetMethodTable();
237240

238241
// assert that String has overloaded the Clone() method
239242
_ASSERTE(pMT != g_pStringClass);
@@ -242,13 +245,25 @@ FCIMPL1(Object*, ObjectNative::AllocateUninitializedClone, Object* pObjUNSAFE)
242245
#endif // FEATURE_UTF8STRING
243246

244247
if (pMT->IsArray()) {
245-
refClone = DupArrayForCloning((BASEARRAYREF)refClone);
248+
refClone = DupArrayForCloning((BASEARRAYREF)refThis);
246249
} else {
247250
// We don't need to call the <cinit> because we know
248251
// that it has been called....(It was called before this was created)
249252
refClone = AllocateObject(pMT);
250253
}
251254

255+
SIZE_T cb = refThis->GetSize() - sizeof(ObjHeader);
256+
257+
// copy contents of "this" to the clone
258+
if (pMT->ContainsPointers())
259+
{
260+
memmoveGCRefs(OBJECTREFToObject(refClone), OBJECTREFToObject(refThis), cb);
261+
}
262+
else
263+
{
264+
memcpyNoGCRefs(OBJECTREFToObject(refClone), OBJECTREFToObject(refThis), cb);
265+
}
266+
252267
HELPER_METHOD_FRAME_END();
253268

254269
return OBJECTREFToObject(refClone);

‎src/classlibnative/bcltype/objectnative.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class ObjectNative
3232
static FCDECL1(Object*, GetObjectValue, Object* vThisRef);
3333
static FCDECL1(INT32, GetHashCode, Object* vThisRef);
3434
static FCDECL2(FC_BOOL_RET, Equals, Object *pThisRef, Object *pCompareRef);
35-
static FCDECL1(Object*, AllocateUninitializedClone, Object* pObjUNSAFE);
35+
static FCDECL1(Object*, Clone, Object* pThis);
3636
static FCDECL1(Object*, GetClass, Object* pThis);
3737
static FCDECL3(FC_BOOL_RET, WaitTimeout, CLR_BOOL exitContext, INT32 Timeout, Object* pThisUNSAFE);
3838
static FCDECL1(void, Pulse, Object* pThisUNSAFE);

‎src/vm/ecalllist.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ FCFuncEnd()
9292

9393
FCFuncStart(gObjectFuncs)
9494
FCIntrinsic("GetType", ObjectNative::GetClass, CORINFO_INTRINSIC_Object_GetType)
95+
FCFuncElement("MemberwiseClone", ObjectNative::Clone)
9596
FCFuncEnd()
9697

9798
FCFuncStart(gStringFuncs)
@@ -727,7 +728,7 @@ FCFuncEnd()
727728
FCFuncStart(gBufferFuncs)
728729
FCFuncElement("IsPrimitiveTypeArray", Buffer::IsPrimitiveTypeArray)
729730
QCFuncElement("__ZeroMemory", Buffer::Clear)
730-
FCFuncElement("__BulkMoveWithWriteBarrier", Buffer::BulkMoveWithWriteBarrier)
731+
FCFuncElement("BulkMoveWithWriteBarrier", Buffer::BulkMoveWithWriteBarrier)
731732
QCFuncElement("__Memmove", Buffer::MemMove)
732733
FCFuncEnd()
733734

@@ -911,7 +912,6 @@ FCFuncStart(gRuntimeHelpers)
911912
FCFuncElement("PrepareDelegate", ReflectionInvocation::PrepareDelegate)
912913
FCFuncElement("GetHashCode", ObjectNative::GetHashCode)
913914
FCFuncElement("Equals", ObjectNative::Equals)
914-
FCFuncElement("AllocateUninitializedClone", ObjectNative::AllocateUninitializedClone)
915915
FCFuncElement("EnsureSufficientExecutionStack", ReflectionInvocation::EnsureSufficientExecutionStack)
916916
FCFuncElement("TryEnsureSufficientExecutionStack", ReflectionInvocation::TryEnsureSufficientExecutionStack)
917917
FCFuncElement("GetUninitializedObjectInternal", ReflectionSerialization::GetUninitializedObject)

0 commit comments

Comments
 (0)