From 6ecf4dbde93cb42be184920b72d5bf0c51d90f55 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 5 Nov 2019 08:10:37 -0800 Subject: [PATCH] Change BulkMoveWithWriteBarrier to be GC suspension friendly (dotnet/coreclr#27642) * Change BulkMoveWithWriteBarrier to be GC suspension friendly * Rename RuntimeHelpers.cs Commit migrated from https://github.com/dotnet/coreclr/commit/5e1ef698774c433f70795e194e2554ad6f5b7d6f --- .../System.Private.CoreLib.csproj | 10 ++--- .../src/System/Buffer.CoreCLR.cs | 49 +++++++++++++++++++++- .../src/System/Object.CoreCLR.cs | 28 ++++++++++++- ...RuntimeHelpers.cs => RuntimeHelpers.CoreCLR.cs} | 18 ++++++++ .../src/classlibnative/bcltype/objectnative.cpp | 31 ++++---------- .../src/classlibnative/bcltype/objectnative.h | 2 +- src/coreclr/src/vm/ecalllist.h | 4 +- 7 files changed, 108 insertions(+), 34 deletions(-) rename src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/{RuntimeHelpers.cs => RuntimeHelpers.CoreCLR.cs} (95%) diff --git a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj index a49345f..ac5cba7 100644 --- a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -141,8 +141,8 @@ - + @@ -161,13 +161,13 @@ - + @@ -246,14 +246,13 @@ - - + @@ -266,6 +265,7 @@ + @@ -290,8 +290,8 @@ - + diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Buffer.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Buffer.CoreCLR.cs index 0dc6a57..527b92c 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Buffer.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Buffer.CoreCLR.cs @@ -10,8 +10,10 @@ using Internal.Runtime.CompilerServices; #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 @@ namespace System [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); diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Object.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Object.CoreCLR.cs index aceff3c..db3d693 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Object.CoreCLR.cs +++ b/src/coreclr/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 @@ namespace System // 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; + } } } diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs similarity index 95% rename from src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs rename to src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index f59905b..f11a7d8 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -149,6 +149,9 @@ namespace System.Runtime.CompilerServices [MethodImpl(MethodImplOptions.InternalCall)] private static extern object GetUninitializedObjectInternal(Type type); + [MethodImpl(MethodImplOptions.InternalCall)] + internal static extern object AllocateUninitializedClone(object obj); + /// true if given type is reference type or value type that contains references [Intrinsic] public static bool IsReferenceOrContainsReferences() @@ -189,6 +192,21 @@ namespace System.Runtime.CompilerServices internal static ref byte GetRawData(this object obj) => ref Unsafe.As(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(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(array).Data; diff --git a/src/coreclr/src/classlibnative/bcltype/objectnative.cpp b/src/coreclr/src/classlibnative/bcltype/objectnative.cpp index 45fbfe0..e285eeb 100644 --- a/src/coreclr/src/classlibnative/bcltype/objectnative.cpp +++ b/src/coreclr/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 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); diff --git a/src/coreclr/src/classlibnative/bcltype/objectnative.h b/src/coreclr/src/classlibnative/bcltype/objectnative.h index 2cccb39..0762298 100644 --- a/src/coreclr/src/classlibnative/bcltype/objectnative.h +++ b/src/coreclr/src/classlibnative/bcltype/objectnative.h @@ -32,7 +32,7 @@ public: 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); diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h index 78d2f4a..4818343 100644 --- a/src/coreclr/src/vm/ecalllist.h +++ b/src/coreclr/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) @@ -729,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() @@ -913,6 +912,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) -- 2.7.4