From 9d59204b6d4ed4f0dc0f7cc0dc5b82b588478834 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 25 Oct 2019 10:46:15 -0700 Subject: [PATCH 1/8] Rewrite of Array.Copy fast path in C# Contributes to #27106 --- .../src/System/Array.CoreCLR.cs | 130 ++++++++- .../InteropServices/Marshal.CoreCLR.cs | 1 - .../src/System/Threading/Thread.CoreCLR.cs | 1 - src/classlibnative/bcltype/arraynative.cpp | 257 +++++------------- src/classlibnative/bcltype/arraynative.h | 7 +- src/vm/ecalllist.h | 3 +- 6 files changed, 190 insertions(+), 209 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs index 1a65a8f71129..1a796e702658 100644 --- a/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs +++ b/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs @@ -135,30 +135,132 @@ public static unsafe Array CreateInstance(Type elementType, int[] lengths, int[] // Copies length elements from sourceArray, starting at index 0, to // destinationArray, starting at index 0. // - public static void Copy(Array sourceArray, Array destinationArray, int length) + public static unsafe void Copy(Array sourceArray, Array destinationArray, int length) { if (sourceArray == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.sourceArray); if (destinationArray == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.destinationArray); - Copy(sourceArray, sourceArray.GetLowerBound(0), destinationArray, destinationArray.GetLowerBound(0), length, false); + MethodTable* pMT = RuntimeHelpers.GetMethodTable(sourceArray); + if (pMT == RuntimeHelpers.GetMethodTable(destinationArray) && + !pMT->IsMultiDimensionalArray && + (uint)length < (uint)sourceArray.Length && + (uint)length < (uint)destinationArray.Length) + { + nuint byteCount = (uint)length * (nuint)pMT->ComponentSize; + ref byte src = ref sourceArray.GetRawSzArrayData(); + ref byte dst = ref destinationArray.GetRawSzArrayData(); + + if (pMT->ContainsGCPointers) + Buffer.BulkMoveWithWriteBarrier(ref dst, ref src, byteCount); + else + Buffer.Memmove(ref dst, ref src, byteCount); + + // GC.KeepAlive(sourceArray) not required. pMT kept alive via sourceArray + return; + } + + // Less common + Copy(sourceArray, sourceArray.GetLowerBound(0), destinationArray, destinationArray.GetLowerBound(0), length, reliable: false); } // Copies length elements from sourceArray, starting at sourceIndex, to // destinationArray, starting at destinationIndex. // - public static void Copy(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length) + public static unsafe void Copy(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length) { - Copy(sourceArray, sourceIndex, destinationArray, destinationIndex, length, false); + if (sourceArray != null && destinationArray != null) + { + MethodTable* pMT = RuntimeHelpers.GetMethodTable(sourceArray); + if (pMT == RuntimeHelpers.GetMethodTable(destinationArray) && + !pMT->IsMultiDimensionalArray && + length >= 0 && sourceIndex >= 0 && destinationIndex >= 0 && + (uint)(sourceIndex + length) <= (uint)sourceArray.Length && + (uint)(destinationIndex + length) <= (uint)destinationArray.Length) + { + nuint elementSize = (nuint)pMT->ComponentSize; + nuint byteCount = (uint)length * elementSize; + ref byte src = ref Unsafe.AddByteOffset(ref sourceArray.GetRawSzArrayData(), (uint)sourceIndex * elementSize); + ref byte dst = ref Unsafe.AddByteOffset(ref destinationArray.GetRawSzArrayData(), (uint)destinationIndex * elementSize); + + if (pMT->ContainsGCPointers) + Buffer.BulkMoveWithWriteBarrier(ref dst, ref src, byteCount); + else + Buffer.Memmove(ref dst, ref src, byteCount); + + // GC.KeepAlive(sourceArray) not required. pMT kept alive via sourceArray + return; + } + } + + // Less common + Copy(sourceArray!, sourceIndex, destinationArray!, destinationIndex, length, reliable: false); } + private static unsafe void Copy(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length, bool reliable) + { + if (sourceArray == null) + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.sourceArray); + if (destinationArray == null) + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.destinationArray); + + if (sourceArray.GetType() != destinationArray.GetType() && sourceArray.Rank != destinationArray.Rank) + throw new RankException(SR.Rank_MustMatch); + + if (length < 0) + throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NeedNonNegNum); + + int srcLB = sourceArray.GetLowerBound(0); + if (sourceIndex < srcLB || sourceIndex - srcLB < 0) + throw new ArgumentOutOfRangeException(nameof(sourceIndex), SR.ArgumentOutOfRange_ArrayLB); + sourceIndex -= srcLB; + + int dstLB = destinationArray.GetLowerBound(0); + if (destinationIndex < dstLB || destinationIndex - dstLB < 0) + throw new ArgumentOutOfRangeException(nameof(destinationIndex), SR.ArgumentOutOfRange_ArrayLB); + destinationIndex -= dstLB; + + if ((uint)(sourceIndex + length) > (uint)sourceArray.Length) + throw new ArgumentException(nameof(sourceArray), SR.Arg_LongerThanSrcArray); + if ((uint)(destinationIndex + length) > (uint)destinationArray.Length) + throw new ArgumentException(nameof(destinationArray), SR.Arg_LongerThanDestArray); + + if (sourceArray.GetType() == destinationArray.GetType() || IsSimpleCopy(sourceArray, destinationArray)) + { + MethodTable* pMT = RuntimeHelpers.GetMethodTable(sourceArray); + + nuint elementSize = (nuint)pMT->ComponentSize; + nuint byteCount = (uint)length * elementSize; + ref byte src = ref Unsafe.AddByteOffset(ref sourceArray.GetRawArrayData(), (uint)sourceIndex * elementSize); + ref byte dst = ref Unsafe.AddByteOffset(ref destinationArray.GetRawArrayData(), (uint)destinationIndex * elementSize); + + if (pMT->ContainsGCPointers) + Buffer.BulkMoveWithWriteBarrier(ref dst, ref src, byteCount); + else + Buffer.Memmove(ref dst, ref src, byteCount); + + return; + } + + // If we were called from Array.ConstrainedCopy, ensure that the array copy + // is guaranteed to succeed. + if (reliable) + throw new ArrayTypeMismatchException(SR.ArrayTypeMismatch_ConstrainedCopy); + + // Rare + CopySlow(sourceArray, sourceIndex, destinationArray, destinationIndex, length); + } + + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern bool IsSimpleCopy(Array sourceArray, Array destinationArray); + // Reliability-wise, this method will either possibly corrupt your // instance & might fail when called from within a CER, or if the // reliable flag is true, it will either always succeed or always // throw an exception with no side effects. [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern void Copy(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length, bool reliable); + private static extern void CopySlow(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length); // Provides a strong exception guarantee - either it succeeds, or // it throws an exception with no side effects. The arrays must be @@ -167,7 +269,7 @@ public static void Copy(Array sourceArray, int sourceIndex, Array destinationArr // It will up-cast, assuming the array types are correct. public static void ConstrainedCopy(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length) { - Copy(sourceArray, sourceIndex, destinationArray, destinationIndex, length, true); + Copy(sourceArray, sourceIndex, destinationArray, destinationIndex, length, reliable: true); } // Sets length elements in array to 0 (or null for Object arrays), starting @@ -194,10 +296,10 @@ public static unsafe void Clear(Array array, int index, int length) if (index < lowerBound || offset < 0 || length < 0 || (uint)(offset + length) > (uint)array.Length) ThrowHelper.ThrowIndexOutOfRangeException(); - uint elementSize = pMT->ComponentSize; + nuint elementSize = pMT->ComponentSize; - ref byte ptr = ref Unsafe.AddByteOffset(ref p, (uint)offset * (nuint)elementSize); - nuint byteLength = (uint)length * (nuint)elementSize; + ref byte ptr = ref Unsafe.AddByteOffset(ref p, (uint)offset * elementSize); + nuint byteLength = (uint)length * elementSize; if (pMT->ContainsGCPointers) SpanHelpers.ClearWithReferences(ref Unsafe.As(ref ptr), byteLength / (uint)sizeof(IntPtr)); @@ -398,6 +500,16 @@ public unsafe int GetLowerBound(int dimension) return Unsafe.Add(ref RuntimeHelpers.GetMultiDimensionalArrayBounds(this), rank + dimension); } + internal unsafe int GetLowerBound() + { + // TODO: Optimize? + int rank = RuntimeHelpers.GetMethodTable(this)->MultiDimensionalArrayRank; + if (rank == 0) + return 0; + + return Unsafe.Add(ref RuntimeHelpers.GetMultiDimensionalArrayBounds(this), rank); + } + [MethodImpl(MethodImplOptions.InternalCall)] private static extern bool TrySZBinarySearch(Array sourceArray, int sourceIndex, int count, object? value, out int retVal); diff --git a/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs index 34dd7c9c2531..d83b3c67b88f 100644 --- a/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs +++ b/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs @@ -193,7 +193,6 @@ private static void PrelinkCore(MethodInfo m) /// structure contains pointers to allocated blocks and "fDeleteOld" is /// true, this routine will call DestroyStructure() first. /// - [MethodImpl(MethodImplOptions.InternalCall), ReliabilityContract(Consistency.WillNotCorruptState, Cer.MayFail)] public static extern void StructureToPtr(object structure, IntPtr ptr, bool fDeleteOld); private static object PtrToStructureHelper(IntPtr ptr, Type structureType) diff --git a/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs index 5e3899a92dfb..89e936838a93 100644 --- a/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs +++ b/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs @@ -276,7 +276,6 @@ private void SetCultureOnUnstartedThreadNoCheck(CultureInfo value, bool uiCultur [MethodImpl(MethodImplOptions.NoInlining)] private static Thread InitializeCurrentThread() => t_currentThread = GetCurrentThreadNative(); - [MethodImpl(MethodImplOptions.InternalCall), ReliabilityContract(Consistency.WillNotCorruptState, Cer.MayFail)] private static extern Thread GetCurrentThreadNative(); private void SetStartHelper(Delegate start, int maxStackSize) diff --git a/src/classlibnative/bcltype/arraynative.cpp b/src/classlibnative/bcltype/arraynative.cpp index ac885f69b6d3..c7062972d090 100644 --- a/src/classlibnative/bcltype/arraynative.cpp +++ b/src/classlibnative/bcltype/arraynative.cpp @@ -129,95 +129,71 @@ FCIMPL1(void, ArrayNative::Initialize, ArrayBase* array) FCIMPLEND - - - - - // Returns an enum saying whether you can copy an array of srcType into destType. -ArrayNative::AssignArrayEnum ArrayNative::CanAssignArrayTypeNoGC(const BASEARRAYREF pSrc, const BASEARRAYREF pDest) + // Returns whether you can directly copy an array of srcType into destType. +FCIMPL2(FC_BOOL_RET, ArrayNative::IsSimpleCopy, ArrayBase* pSrc, ArrayBase* pDst) { - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - PRECONDITION(pSrc != NULL); - PRECONDITION(pDest != NULL); - } - CONTRACTL_END; + FCALL_CONTRACT; - // The next 50 lines are a little tricky. Change them with great care. - // + _ASSERTE(pSrc != NULL); + _ASSERTE(pDst != NULL); - // This first bit is a minor optimization: e.g. when copying byte[] to byte[] - // we do not need to call GetArrayElementTypeHandle(). - MethodTable *pSrcMT = pSrc->GetMethodTable(); - MethodTable *pDestMT = pDest->GetMethodTable(); - if (pSrcMT == pDestMT) - return AssignWillWork; + // This case is expected to be handled by the fast path + _ASSERTE(pSrc->GetMethodTable() != pSrc->GetMethodTable()); - TypeHandle srcTH = pSrcMT->GetApproxArrayElementTypeHandle(); - TypeHandle destTH = pDestMT->GetApproxArrayElementTypeHandle(); + TypeHandle srcTH = pSrc->GetMethodTable()->GetApproxArrayElementTypeHandle(); + TypeHandle destTH = pDst->GetMethodTable()->GetApproxArrayElementTypeHandle(); if (srcTH == destTH) // This check kicks for different array kind or dimensions - return AssignWillWork; + FC_RETURN_BOOL(true); - // Value class boxing - if (srcTH.IsValueType() && !destTH.IsValueType()) + if (srcTH.IsValueType()) { - switch (srcTH.CanCastToCached(destTH)) + // Value class boxing + if (!destTH.IsValueType()) + FC_RETURN_BOOL(false); + + const CorElementType srcElType = srcTH.GetVerifierCorElementType(); + const CorElementType destElType = destTH.GetVerifierCorElementType(); + _ASSERTE(srcElType < ELEMENT_TYPE_MAX); + _ASSERTE(destElType < ELEMENT_TYPE_MAX); + + // Copying primitives from one type to another + if (CorTypeInfo::IsPrimitiveType_NoThrow(srcElType) && CorTypeInfo::IsPrimitiveType_NoThrow(destElType)) { - case TypeHandle::CanCast : return AssignBoxValueClassOrPrimitive; - case TypeHandle::CannotCast : return AssignWrongType; - default : return AssignDontKnow; + if (GetNormalizedIntegralArrayElementType(srcElType) == GetNormalizedIntegralArrayElementType(destElType)) + FC_RETURN_BOOL(true); } } - - // Value class unboxing. - if (!srcTH.IsValueType() && destTH.IsValueType()) + else { - if (srcTH.CanCastToCached(destTH) == TypeHandle::CanCast) - return AssignUnboxValueClass; - else if (destTH.CanCastToCached(srcTH) == TypeHandle::CanCast) // V extends IV. Copying from IV to V, or Object to V. - return AssignUnboxValueClass; - else - return AssignDontKnow; + // Value class unboxing + if (destTH.IsValueType()) + FC_RETURN_BOOL(false); } - const CorElementType srcElType = srcTH.GetVerifierCorElementType(); - const CorElementType destElType = destTH.GetVerifierCorElementType(); - _ASSERTE(srcElType < ELEMENT_TYPE_MAX); - _ASSERTE(destElType < ELEMENT_TYPE_MAX); - - // Copying primitives from one type to another - if (CorTypeInfo::IsPrimitiveType_NoThrow(srcElType) && CorTypeInfo::IsPrimitiveType_NoThrow(destElType)) + TypeHandle::CastResult r = srcTH.CanCastToCached(destTH); + if (r != TypeHandle::MaybeCast) { - if (GetNormalizedIntegralArrayElementType(srcElType) == GetNormalizedIntegralArrayElementType(destElType)) - return AssignWillWork; - - if (InvokeUtil::CanPrimitiveWiden(destElType, srcElType)) - return AssignPrimitiveWiden; - else - return AssignWrongType; + FC_RETURN_BOOL(r); } - // dest Object extends src - if (srcTH.CanCastToCached(destTH) == TypeHandle::CanCast) - return AssignWillWork; + struct + { + OBJECTREF src; + OBJECTREF dst; + } gc; - // src Object extends dest - if (destTH.CanCastToCached(srcTH) == TypeHandle::CanCast) - return AssignMustCast; + gc.src = ObjectToOBJECTREF(pSrc); + gc.dst = ObjectToOBJECTREF(pDst); - // class X extends/implements src and implements dest. - if (destTH.IsInterface() && srcElType != ELEMENT_TYPE_VALUETYPE) - return AssignMustCast; + BOOL iRetVal = FALSE; - // class X implements src and extends/implements dest - if (srcTH.IsInterface() && destElType != ELEMENT_TYPE_VALUETYPE) - return AssignMustCast; + HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc); + iRetVal = srcTH.CanCastTo(destTH); + HELPER_METHOD_FRAME_END(); - return AssignDontKnow; + FC_RETURN_BOOL(iRetVal); } +FCIMPLEND // Returns an enum saying whether you can copy an array of srcType into destType. @@ -233,21 +209,16 @@ ArrayNative::AssignArrayEnum ArrayNative::CanAssignArrayType(const BASEARRAYREF } CONTRACTL_END; - // The next 50 lines are a little tricky. Change them with great care. - // - // This first bit is a minor optimization: e.g. when copying byte[] to byte[] // we do not need to call GetArrayElementTypeHandle(). MethodTable *pSrcMT = pSrc->GetMethodTable(); MethodTable *pDestMT = pDest->GetMethodTable(); - if (pSrcMT == pDestMT) - return AssignWillWork; + _ASSERTE(pSrcMT != pDestMT); // Handled by fast path TypeHandle srcTH = pSrcMT->GetApproxArrayElementTypeHandle(); TypeHandle destTH = pDestMT->GetApproxArrayElementTypeHandle(); - if (srcTH == destTH) // This check kicks for different array kind or dimensions - return AssignWillWork; - + _ASSERTE(srcTH != destTH); // Handled by fast path + // Value class boxing if (srcTH.IsValueType() && !destTH.IsValueType()) { @@ -276,8 +247,7 @@ ArrayNative::AssignArrayEnum ArrayNative::CanAssignArrayType(const BASEARRAYREF // Copying primitives from one type to another if (CorTypeInfo::IsPrimitiveType_NoThrow(srcElType) && CorTypeInfo::IsPrimitiveType_NoThrow(destElType)) { - if (srcElType == destElType) - return AssignWillWork; + _ASSERTE(srcElType != destElType); // Handled by fast path if (InvokeUtil::CanPrimitiveWiden(destElType, srcElType)) return AssignPrimitiveWiden; else @@ -285,8 +255,7 @@ ArrayNative::AssignArrayEnum ArrayNative::CanAssignArrayType(const BASEARRAYREF } // dest Object extends src - if (srcTH.CanCastTo(destTH)) - return AssignWillWork; + _ASSERTE(!srcTH.CanCastTo(destTH)); // Handled by fast path // src Object extends dest if (destTH.CanCastTo(srcTH)) @@ -766,38 +735,7 @@ void memmoveGCRefs(void *dest, const void *src, size_t len) } } -void ArrayNative::ArrayCopyNoTypeCheck(BASEARRAYREF pSrc, unsigned int srcIndex, BASEARRAYREF pDest, unsigned int destIndex, unsigned int length) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - PRECONDITION(pSrc != NULL); - PRECONDITION(srcIndex >= 0); - PRECONDITION(pDest != NULL); - PRECONDITION(length > 0); - } - CONTRACTL_END; - - BYTE *src = (BYTE*)pSrc->GetDataPtr(); - BYTE *dst = (BYTE*)pDest->GetDataPtr(); - SIZE_T size = pSrc->GetComponentSize(); - - src += srcIndex * size; - dst += destIndex * size; - - if (pDest->GetMethodTable()->ContainsPointers()) - { - memmoveGCRefs(dst, src, length * size); - } - else - { - memmove(dst, src, length * size); - } -} - -FCIMPL6(void, ArrayNative::ArrayCopy, ArrayBase* m_pSrc, INT32 m_iSrcIndex, ArrayBase* m_pDst, INT32 m_iDstIndex, INT32 m_iLength, CLR_BOOL reliable) +FCIMPL5(void, ArrayNative::CopySlow, ArrayBase* pSrc, INT32 iSrcIndex, ArrayBase* pDst, INT32 iDstIndex, INT32 iLength) { FCALL_CONTRACT; @@ -807,115 +745,50 @@ FCIMPL6(void, ArrayNative::ArrayCopy, ArrayBase* m_pSrc, INT32 m_iSrcIndex, Arra BASEARRAYREF pDst; } gc; - gc.pSrc = (BASEARRAYREF)m_pSrc; - gc.pDst = (BASEARRAYREF)m_pDst; - - // - // creating a HelperMethodFrame is quite expensive, - // so we want to delay this for the most common case which doesn't trigger a GC. - // FCThrow is needed to throw an exception without a HelperMethodFrame - // + gc.pSrc = (BASEARRAYREF)pSrc; + gc.pDst = (BASEARRAYREF)pDst; // cannot pass null for source or destination - if (gc.pSrc == NULL || gc.pDst == NULL) { - FCThrowArgumentNullVoid(gc.pSrc==NULL ? W("sourceArray") : W("destinationArray")); - } + _ASSERTE(gc.pSrc == NULL && gc.pDst == NULL); // source and destination must be arrays _ASSERTE(gc.pSrc->GetMethodTable()->IsArray()); _ASSERTE(gc.pDst->GetMethodTable()->IsArray()); - // Equal method tables should imply equal rank - _ASSERTE(!(gc.pSrc->GetMethodTable() == gc.pDst->GetMethodTable() && gc.pSrc->GetRank() != gc.pDst->GetRank())); + _ASSERTE(gc.pSrc->GetRank() == gc.pDst->GetRank()); - // Which enables us to avoid touching the EEClass in simple cases - if (gc.pSrc->GetMethodTable() != gc.pDst->GetMethodTable() && gc.pSrc->GetRank() != gc.pDst->GetRank()) { - FCThrowResVoid(kRankException, W("Rank_MustMatch")); - } - - g_IBCLogger.LogMethodTableAccess(gc.pSrc->GetMethodTable()); - g_IBCLogger.LogMethodTableAccess(gc.pDst->GetMethodTable()); - - int srcLB = gc.pSrc->GetLowerBoundsPtr()[0]; - int destLB = gc.pDst->GetLowerBoundsPtr()[0]; // array bounds checking - const unsigned int srcLen = gc.pSrc->GetNumComponents(); - const unsigned int destLen = gc.pDst->GetNumComponents(); - if (m_iLength < 0) - FCThrowArgumentOutOfRangeVoid(W("length"), W("ArgumentOutOfRange_NeedNonNegNum")); - - if (m_iSrcIndex < srcLB || (m_iSrcIndex - srcLB < 0)) - FCThrowArgumentOutOfRangeVoid(W("sourceIndex"), W("ArgumentOutOfRange_ArrayLB")); - - if (m_iDstIndex < destLB || (m_iDstIndex - destLB < 0)) - FCThrowArgumentOutOfRangeVoid(W("destinationIndex"), W("ArgumentOutOfRange_ArrayLB")); - - if ((DWORD)(m_iSrcIndex - srcLB + m_iLength) > srcLen) - FCThrowArgumentVoid(W("sourceArray"), W("Arg_LongerThanSrcArray")); - - if ((DWORD)(m_iDstIndex - destLB + m_iLength) > destLen) - FCThrowArgumentVoid(W("destinationArray"), W("Arg_LongerThanDestArray")); - - int r = 0; - - // Small perf optimization - we copy from one portion of an array back to - // itself a lot when resizing collections, etc. The cost of doing the type - // checking is significant for copying small numbers of bytes (~half of the time - // for copying 1 byte within one array from element 0 to element 1). - if (gc.pSrc == gc.pDst) - r = AssignWillWork; - else - r = CanAssignArrayTypeNoGC(gc.pSrc, gc.pDst); - - if (r == AssignWrongType) { - FCThrowResVoid(kArrayTypeMismatchException, W("ArrayTypeMismatch_CantAssignType")); - } - - if (r == AssignWillWork) { - if (m_iLength > 0) - ArrayCopyNoTypeCheck(gc.pSrc, m_iSrcIndex - srcLB, gc.pDst, m_iDstIndex - destLB, m_iLength); - - FC_GC_POLL(); - return; - } + _ASSERTE(iLength >= 0); + _ASSERTE(iSrcIndex >= 0); + _ASSERTE(iDstIndex >= 0); + _ASSERTE((DWORD)(iSrcIndex + iLength) <= gc.pSrc->GetNumComponents()); + _ASSERTE((DWORD)(iDstIndex + iLength) > gc.pDst->GetNumComponents()); HELPER_METHOD_FRAME_BEGIN_PROTECT(gc); - if (r == AssignDontKnow) - { - r = CanAssignArrayType(gc.pSrc, gc.pDst); - } - CONSISTENCY_CHECK(r != AssignDontKnow); - // If we were called from Array.ConstrainedCopy, ensure that the array copy - // is guaranteed to succeed. - if (reliable && r != AssignWillWork) - COMPlusThrow(kArrayTypeMismatchException, W("ArrayTypeMismatch_ConstrainedCopy")); + int r = CanAssignArrayType(gc.pSrc, gc.pDst); if (r == AssignWrongType) COMPlusThrow(kArrayTypeMismatchException, W("ArrayTypeMismatch_CantAssignType")); - if (m_iLength > 0) + if (iLength > 0) { switch (r) { - case AssignWillWork: - ArrayCopyNoTypeCheck(gc.pSrc, m_iSrcIndex - srcLB, gc.pDst, m_iDstIndex - destLB, m_iLength); - break; - case AssignUnboxValueClass: - UnBoxEachElement(gc.pSrc, m_iSrcIndex - srcLB, gc.pDst, m_iDstIndex - destLB, m_iLength); + UnBoxEachElement(gc.pSrc, iSrcIndex, gc.pDst, iDstIndex, iLength); break; case AssignBoxValueClassOrPrimitive: - BoxEachElement(gc.pSrc, m_iSrcIndex - srcLB, gc.pDst, m_iDstIndex - destLB, m_iLength); + BoxEachElement(gc.pSrc, iSrcIndex, gc.pDst, iDstIndex, iLength); break; case AssignMustCast: - CastCheckEachElement(gc.pSrc, m_iSrcIndex - srcLB, gc.pDst, m_iDstIndex - destLB, m_iLength); + CastCheckEachElement(gc.pSrc, iSrcIndex, gc.pDst, iDstIndex, iLength); break; case AssignPrimitiveWiden: - PrimitiveWiden(gc.pSrc, m_iSrcIndex - srcLB, gc.pDst, m_iDstIndex - destLB, m_iLength); + PrimitiveWiden(gc.pSrc, iSrcIndex, gc.pDst, iDstIndex, iLength); break; default: diff --git a/src/classlibnative/bcltype/arraynative.h b/src/classlibnative/bcltype/arraynative.h index 02197bfdedfb..2f587ade4157 100644 --- a/src/classlibnative/bcltype/arraynative.h +++ b/src/classlibnative/bcltype/arraynative.h @@ -27,7 +27,8 @@ class ArrayNative public: static FCDECL1(void, Initialize, ArrayBase* pArray); - static FCDECL6(void, ArrayCopy, ArrayBase* m_pSrc, INT32 m_iSrcIndex, ArrayBase* m_pDst, INT32 m_iDstIndex, INT32 m_iLength, CLR_BOOL reliable); + static FCDECL2(FC_BOOL_RET, IsSimpleCopy, ArrayBase* pSrc, ArrayBase* pDst); + static FCDECL5(void, CopySlow, ArrayBase* pSrc, INT32 iSrcIndex, ArrayBase* pDst, INT32 iDstIndex, INT32 iLength); // This method will create a new array of type type, with zero lower // bounds and rank. @@ -51,18 +52,14 @@ class ArrayNative enum AssignArrayEnum { AssignWrongType, - AssignWillWork, AssignMustCast, AssignBoxValueClassOrPrimitive, AssignUnboxValueClass, AssignPrimitiveWiden, - AssignDontKnow, }; // The following functions are all helpers for ArrayCopy - static AssignArrayEnum CanAssignArrayTypeNoGC(const BASEARRAYREF pSrc, const BASEARRAYREF pDest); static AssignArrayEnum CanAssignArrayType(const BASEARRAYREF pSrc, const BASEARRAYREF pDest); - static void ArrayCopyNoTypeCheck(BASEARRAYREF pSrc, unsigned int srcIndex, BASEARRAYREF pDest, unsigned int destIndex, unsigned int length); static void CastCheckEachElement(BASEARRAYREF pSrc, unsigned int srcIndex, BASEARRAYREF pDest, unsigned int destIndex, unsigned int length); static void BoxEachElement(BASEARRAYREF pSrc, unsigned int srcIndex, BASEARRAYREF pDest, unsigned int destIndex, unsigned int length); static void UnBoxEachElement(BASEARRAYREF pSrc, unsigned int srcIndex, BASEARRAYREF pDest, unsigned int destIndex, unsigned int length); diff --git a/src/vm/ecalllist.h b/src/vm/ecalllist.h index 56de255796ab..746bd011493a 100644 --- a/src/vm/ecalllist.h +++ b/src/vm/ecalllist.h @@ -716,7 +716,8 @@ FCFuncEnd() FCFuncStart(gArrayFuncs) FCFuncElement("Initialize", ArrayNative::Initialize) - FCFuncElement("Copy", ArrayNative::ArrayCopy) + FCFuncElement("IsSimpleCopy", ArrayNative::IsSimpleCopy) + FCFuncElement("CopySlow", ArrayNative::CopySlow) FCFuncElement("InternalCreate", ArrayNative::CreateInstance) FCFuncElement("InternalGetReference", ArrayNative::GetReference) FCFuncElement("InternalSetValue", ArrayNative::SetValue) From 53a48173ef79f2604f5a34f7c1902c2822ecc4bf Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Nov 2019 09:28:20 -0700 Subject: [PATCH 2/8] Cleanup --- .../src/System/Array.CoreCLR.cs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs index 1a796e702658..d543c5d19dcb 100644 --- a/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs +++ b/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs @@ -162,7 +162,7 @@ public static unsafe void Copy(Array sourceArray, Array destinationArray, int le } // Less common - Copy(sourceArray, sourceArray.GetLowerBound(0), destinationArray, destinationArray.GetLowerBound(0), length, reliable: false); + Copy(sourceArray, sourceArray.GetLowerBound(), destinationArray, destinationArray.GetLowerBound(), length, reliable: false); } // Copies length elements from sourceArray, starting at sourceIndex, to @@ -211,12 +211,12 @@ private static unsafe void Copy(Array sourceArray, int sourceIndex, Array destin if (length < 0) throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NeedNonNegNum); - int srcLB = sourceArray.GetLowerBound(0); + int srcLB = sourceArray.GetLowerBound(); if (sourceIndex < srcLB || sourceIndex - srcLB < 0) throw new ArgumentOutOfRangeException(nameof(sourceIndex), SR.ArgumentOutOfRange_ArrayLB); sourceIndex -= srcLB; - int dstLB = destinationArray.GetLowerBound(0); + int dstLB = destinationArray.GetLowerBound(); if (destinationIndex < dstLB || destinationIndex - dstLB < 0) throw new ArgumentOutOfRangeException(nameof(destinationIndex), SR.ArgumentOutOfRange_ArrayLB); destinationIndex -= dstLB; @@ -240,6 +240,7 @@ private static unsafe void Copy(Array sourceArray, int sourceIndex, Array destin else Buffer.Memmove(ref dst, ref src, byteCount); + // GC.KeepAlive(sourceArray) not required. pMT kept alive via sourceArray return; } @@ -500,16 +501,6 @@ public unsafe int GetLowerBound(int dimension) return Unsafe.Add(ref RuntimeHelpers.GetMultiDimensionalArrayBounds(this), rank + dimension); } - internal unsafe int GetLowerBound() - { - // TODO: Optimize? - int rank = RuntimeHelpers.GetMethodTable(this)->MultiDimensionalArrayRank; - if (rank == 0) - return 0; - - return Unsafe.Add(ref RuntimeHelpers.GetMultiDimensionalArrayBounds(this), rank); - } - [MethodImpl(MethodImplOptions.InternalCall)] private static extern bool TrySZBinarySearch(Array sourceArray, int sourceIndex, int count, object? value, out int retVal); From 50fb056395a34fdbb6366231907b94dd2040c4ee Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Nov 2019 09:50:40 -0700 Subject: [PATCH 3/8] Build breaks --- .../src/System/Array.CoreCLR.cs | 12 ++++++------ .../Runtime/InteropServices/Marshal.CoreCLR.cs | 1 + .../src/System/Threading/Thread.CoreCLR.cs | 1 + 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs index d543c5d19dcb..f723ef8ec5c5 100644 --- a/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs +++ b/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs @@ -144,8 +144,8 @@ public static unsafe void Copy(Array sourceArray, Array destinationArray, int le MethodTable* pMT = RuntimeHelpers.GetMethodTable(sourceArray); if (pMT == RuntimeHelpers.GetMethodTable(destinationArray) && - !pMT->IsMultiDimensionalArray && - (uint)length < (uint)sourceArray.Length && + !pMT->IsMultiDimensionalArray && + (uint)length < (uint)sourceArray.Length && (uint)length < (uint)destinationArray.Length) { nuint byteCount = (uint)length * (nuint)pMT->ComponentSize; @@ -162,7 +162,7 @@ public static unsafe void Copy(Array sourceArray, Array destinationArray, int le } // Less common - Copy(sourceArray, sourceArray.GetLowerBound(), destinationArray, destinationArray.GetLowerBound(), length, reliable: false); + Copy(sourceArray, sourceArray.GetLowerBound(0), destinationArray, destinationArray.GetLowerBound(0), length, reliable: false); } // Copies length elements from sourceArray, starting at sourceIndex, to @@ -173,7 +173,7 @@ public static unsafe void Copy(Array sourceArray, int sourceIndex, Array destina if (sourceArray != null && destinationArray != null) { MethodTable* pMT = RuntimeHelpers.GetMethodTable(sourceArray); - if (pMT == RuntimeHelpers.GetMethodTable(destinationArray) && + if (pMT == RuntimeHelpers.GetMethodTable(destinationArray) && !pMT->IsMultiDimensionalArray && length >= 0 && sourceIndex >= 0 && destinationIndex >= 0 && (uint)(sourceIndex + length) <= (uint)sourceArray.Length && @@ -211,12 +211,12 @@ private static unsafe void Copy(Array sourceArray, int sourceIndex, Array destin if (length < 0) throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NeedNonNegNum); - int srcLB = sourceArray.GetLowerBound(); + int srcLB = sourceArray.GetLowerBound(0); if (sourceIndex < srcLB || sourceIndex - srcLB < 0) throw new ArgumentOutOfRangeException(nameof(sourceIndex), SR.ArgumentOutOfRange_ArrayLB); sourceIndex -= srcLB; - int dstLB = destinationArray.GetLowerBound(); + int dstLB = destinationArray.GetLowerBound(0); if (destinationIndex < dstLB || destinationIndex - dstLB < 0) throw new ArgumentOutOfRangeException(nameof(destinationIndex), SR.ArgumentOutOfRange_ArrayLB); destinationIndex -= dstLB; diff --git a/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs index d83b3c67b88f..91ada9172ffb 100644 --- a/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs +++ b/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs @@ -193,6 +193,7 @@ private static void PrelinkCore(MethodInfo m) /// structure contains pointers to allocated blocks and "fDeleteOld" is /// true, this routine will call DestroyStructure() first. /// + [MethodImpl(MethodImplOptions.InternalCall)] public static extern void StructureToPtr(object structure, IntPtr ptr, bool fDeleteOld); private static object PtrToStructureHelper(IntPtr ptr, Type structureType) diff --git a/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs index 89e936838a93..fe03fd6f5c4c 100644 --- a/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs +++ b/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs @@ -276,6 +276,7 @@ private void SetCultureOnUnstartedThreadNoCheck(CultureInfo value, bool uiCultur [MethodImpl(MethodImplOptions.NoInlining)] private static Thread InitializeCurrentThread() => t_currentThread = GetCurrentThreadNative(); + [MethodImpl(MethodImplOptions.InternalCall)] private static extern Thread GetCurrentThreadNative(); private void SetStartHelper(Delegate start, int maxStackSize) From 7a16344c7dd76327de5503e9c4c92e969cc40c7f Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Nov 2019 09:58:06 -0700 Subject: [PATCH 4/8] Fix --- src/System.Private.CoreLib/src/System/Array.CoreCLR.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs index f723ef8ec5c5..935f17b55524 100644 --- a/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs +++ b/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs @@ -145,8 +145,8 @@ public static unsafe void Copy(Array sourceArray, Array destinationArray, int le MethodTable* pMT = RuntimeHelpers.GetMethodTable(sourceArray); if (pMT == RuntimeHelpers.GetMethodTable(destinationArray) && !pMT->IsMultiDimensionalArray && - (uint)length < (uint)sourceArray.Length && - (uint)length < (uint)destinationArray.Length) + (uint)length <= (uint)sourceArray.Length && + (uint)length <= (uint)destinationArray.Length) { nuint byteCount = (uint)length * (nuint)pMT->ComponentSize; ref byte src = ref sourceArray.GetRawSzArrayData(); From 24b64321313ebc2d54003acaf8979795d41e8b13 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Nov 2019 12:28:30 -0700 Subject: [PATCH 5/8] Fix assert --- src/classlibnative/bcltype/arraynative.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/classlibnative/bcltype/arraynative.cpp b/src/classlibnative/bcltype/arraynative.cpp index c7062972d090..bb9f0c9d00f4 100644 --- a/src/classlibnative/bcltype/arraynative.cpp +++ b/src/classlibnative/bcltype/arraynative.cpp @@ -138,7 +138,7 @@ FCIMPL2(FC_BOOL_RET, ArrayNative::IsSimpleCopy, ArrayBase* pSrc, ArrayBase* pDst _ASSERTE(pDst != NULL); // This case is expected to be handled by the fast path - _ASSERTE(pSrc->GetMethodTable() != pSrc->GetMethodTable()); + _ASSERTE(pSrc->GetMethodTable() != pDst->GetMethodTable()); TypeHandle srcTH = pSrc->GetMethodTable()->GetApproxArrayElementTypeHandle(); TypeHandle destTH = pDst->GetMethodTable()->GetApproxArrayElementTypeHandle(); From 1f0532161f19446f4790ffebc15427e4c6499df2 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Nov 2019 14:20:25 -0700 Subject: [PATCH 6/8] Fix assert --- src/classlibnative/bcltype/arraynative.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/classlibnative/bcltype/arraynative.cpp b/src/classlibnative/bcltype/arraynative.cpp index bb9f0c9d00f4..aa6fe22f2165 100644 --- a/src/classlibnative/bcltype/arraynative.cpp +++ b/src/classlibnative/bcltype/arraynative.cpp @@ -749,7 +749,7 @@ FCIMPL5(void, ArrayNative::CopySlow, ArrayBase* pSrc, INT32 iSrcIndex, ArrayBase gc.pDst = (BASEARRAYREF)pDst; // cannot pass null for source or destination - _ASSERTE(gc.pSrc == NULL && gc.pDst == NULL); + _ASSERTE(gc.pSrc != NULL && gc.pDst != NULL); // source and destination must be arrays _ASSERTE(gc.pSrc->GetMethodTable()->IsArray()); From a8f6bce24756b68c9b1a9eeb8b17d8d6497d3bc9 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Nov 2019 17:41:27 -0700 Subject: [PATCH 7/8] Fix assert --- src/classlibnative/bcltype/arraynative.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/classlibnative/bcltype/arraynative.cpp b/src/classlibnative/bcltype/arraynative.cpp index aa6fe22f2165..6c9d655cf0d3 100644 --- a/src/classlibnative/bcltype/arraynative.cpp +++ b/src/classlibnative/bcltype/arraynative.cpp @@ -762,7 +762,7 @@ FCIMPL5(void, ArrayNative::CopySlow, ArrayBase* pSrc, INT32 iSrcIndex, ArrayBase _ASSERTE(iSrcIndex >= 0); _ASSERTE(iDstIndex >= 0); _ASSERTE((DWORD)(iSrcIndex + iLength) <= gc.pSrc->GetNumComponents()); - _ASSERTE((DWORD)(iDstIndex + iLength) > gc.pDst->GetNumComponents()); + _ASSERTE((DWORD)(iDstIndex + iLength) <= gc.pDst->GetNumComponents()); HELPER_METHOD_FRAME_BEGIN_PROTECT(gc); From a2fac31224595920cfcb42a1a29413ecf885fcbe Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Nov 2019 19:22:53 -0700 Subject: [PATCH 8/8] Argument order --- src/System.Private.CoreLib/src/System/Array.CoreCLR.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs index 935f17b55524..0665278a5ec1 100644 --- a/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs +++ b/src/System.Private.CoreLib/src/System/Array.CoreCLR.cs @@ -222,9 +222,9 @@ private static unsafe void Copy(Array sourceArray, int sourceIndex, Array destin destinationIndex -= dstLB; if ((uint)(sourceIndex + length) > (uint)sourceArray.Length) - throw new ArgumentException(nameof(sourceArray), SR.Arg_LongerThanSrcArray); + throw new ArgumentException(SR.Arg_LongerThanSrcArray, nameof(sourceArray)); if ((uint)(destinationIndex + length) > (uint)destinationArray.Length) - throw new ArgumentException(nameof(destinationArray), SR.Arg_LongerThanDestArray); + throw new ArgumentException(SR.Arg_LongerThanDestArray, nameof(destinationArray)); if (sourceArray.GetType() == destinationArray.GetType() || IsSimpleCopy(sourceArray, destinationArray)) {