Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Rewrite Buffer.BlockCopy in C# #27216

Merged
merged 3 commits into from
Oct 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions src/System.Private.CoreLib/shared/System/Buffer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,54 @@ namespace System
{
public static partial class Buffer
{
// Copies from one primitive array to another primitive array without
// respecting types. This calls memmove internally. The count and
// offset parameters here are in bytes. If you want to use traditional
// array element indices and counts, use Array.Copy.
public static unsafe void BlockCopy(Array src, int srcOffset, Array dst, int dstOffset, int count)
{
if (src == null)
throw new ArgumentNullException(nameof(src));
if (dst == null)
throw new ArgumentNullException(nameof(dst));

nuint uSrcLen = (nuint)src.LongLength;
if (src.GetType() != typeof(byte[]))
{
if (!IsPrimitiveTypeArray(src))
throw new ArgumentException(SR.Arg_MustBePrimArray, nameof(src));
uSrcLen *= (nuint)src.GetElementSize();
}

nuint uDstLen = uSrcLen;
if (src != dst)
{
uDstLen = (nuint)dst.LongLength;
if (dst.GetType() != typeof(byte[]))
{
if (!IsPrimitiveTypeArray(dst))
jkotas marked this conversation as resolved.
Show resolved Hide resolved
throw new ArgumentException(SR.Arg_MustBePrimArray, nameof(dst));
uDstLen *= (nuint)dst.GetElementSize();
}
}

if (srcOffset < 0)
throw new ArgumentOutOfRangeException(nameof(srcOffset), SR.ArgumentOutOfRange_MustBeNonNegInt32);
if (dstOffset < 0)
throw new ArgumentOutOfRangeException(nameof(dstOffset), SR.ArgumentOutOfRange_MustBeNonNegInt32);
if (count < 0)
throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_MustBeNonNegInt32);

nuint uCount = (nuint)count;
nuint uSrcOffset = (nuint)srcOffset;
nuint uDstOffset = (nuint)dstOffset;

if ((uSrcLen < uSrcOffset + uCount) || (uDstLen < uDstOffset + uCount))
throw new ArgumentException(SR.Argument_InvalidOffLen);

Memmove(ref Unsafe.AddByteOffset(ref dst.GetRawArrayData(), uDstOffset), ref Unsafe.AddByteOffset(ref src.GetRawArrayData(), uSrcOffset), uCount);
}

public static int ByteLength(Array array)
{
// Is the array present?
Expand Down
3 changes: 0 additions & 3 deletions src/System.Private.CoreLib/src/System/Array.CoreCLR.cs
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,6 @@ public extern int Rank
[MethodImpl(MethodImplOptions.InternalCall)]
public extern int GetLowerBound(int dimension);

[MethodImpl(MethodImplOptions.InternalCall)]
internal extern ref byte GetRawArrayData();

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern bool TrySZBinarySearch(Array sourceArray, int sourceIndex, int count, object? value, out int retVal);

Expand Down
9 changes: 1 addition & 8 deletions src/System.Private.CoreLib/src/System/Buffer.CoreCLR.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Internal.Runtime.CompilerServices;

#pragma warning disable SA1121 // explicitly using type aliases instead of built-in types
#if BIT64
Expand All @@ -17,14 +18,6 @@ namespace System
{
public partial class Buffer
{
// Copies from one primitive array to another primitive array without
// respecting types. This calls memmove internally. The count and
// offset parameters here are in bytes. If you want to use traditional
// array element indices and counts, use Array.Copy.
[MethodImpl(MethodImplOptions.InternalCall)]
public static extern void BlockCopy(Array src, int srcOffset,
Array dst, int dstOffset, int count);

// Returns a bool to indicate if the array is of primitive data types
// or not.
[MethodImpl(MethodImplOptions.InternalCall)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@
using System.Runtime.InteropServices;
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.Runtime.CompilerServices
{
public static partial class RuntimeHelpers
Expand Down Expand Up @@ -164,12 +171,14 @@ internal static ref byte GetRawData(this object obj) =>
internal static ref byte GetRawSzArrayData(this Array array) =>
ref Unsafe.As<RawArrayData>(array).Data;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static unsafe ref byte GetRawArrayData(this Array array) =>
ref Unsafe.AddByteOffset(ref Unsafe.As<RawData>(array).Data, (nuint)GetObjectMethodTablePointer(array)->BaseSize - (nuint)(2 * sizeof(IntPtr)));
stephentoub marked this conversation as resolved.
Show resolved Hide resolved

internal static unsafe ushort GetElementSize(this Array array)
{
Debug.Assert(ObjectHasComponentSize(array));

// The first UINT16 of the method table is component size.
return *(ushort*)GetObjectMethodTablePointer(array);
return GetObjectMethodTablePointer(array)->ComponentSize;
}

// Returns true iff the object has a component size;
Expand All @@ -181,28 +190,38 @@ internal static unsafe bool ObjectHasComponentSize(object obj)
// [ pMethodTable || .. object data .. ]
// ^-- the object reference points here
//
// The first DWORD of the method table class will have its high bit set if the
// The m_dwFlags field of the method table class will have its high bit set if the
// method table has component size info stored somewhere. See member
// MethodTable:IsStringOrArray in src\vm\methodtable.h for full details.
//
// So in effect this method is the equivalent of
// return ((MethodTable*)(*obj))->IsStringOrArray();
return (int)GetObjectMethodTablePointer(obj)->Flags < 0;
}

Debug.Assert(obj != null);
return *(int*)GetObjectMethodTablePointer(obj) < 0;
// Subset of src\vm\methodtable.h
[StructLayout(LayoutKind.Explicit)]
private struct MethodTable
jkotas marked this conversation as resolved.
Show resolved Hide resolved
{
[FieldOffset(0)]
public ushort ComponentSize;
[FieldOffset(0)]
public uint Flags;
[FieldOffset(4)]
public uint BaseSize;
}

// Given an object reference, returns its MethodTable* as an IntPtr.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static IntPtr GetObjectMethodTablePointer(object obj)
private static unsafe MethodTable* GetObjectMethodTablePointer(object obj)
{
Debug.Assert(obj != null);

// We know that the first data field in any managed object is immediately after the
// method table pointer, so just back up one pointer and immediately deref.
// This is not ideal in terms of minimizing instruction count but is the best we can do at the moment.

return Unsafe.Add(ref Unsafe.As<byte, IntPtr>(ref obj.GetRawData()), -1);
return (MethodTable *)Unsafe.Add(ref Unsafe.As<byte, IntPtr>(ref obj.GetRawData()), -1);

// The JIT currently implements this as:
// lea tmp, [rax + 8h] ; assume rax contains the object reference, tmp is type IntPtr&
Expand Down
1 change: 0 additions & 1 deletion src/inc/dacvars.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_pEnumClass, ::g_pEnumClass)
DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_pThreadClass, ::g_pThreadClass)
DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_pPredefinedArrayTypes, ::g_pPredefinedArrayTypes)
DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_TypedReferenceMT, ::g_TypedReferenceMT)
DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_pByteArrayMT, ::g_pByteArrayMT)

#ifdef FEATURE_COMINTEROP
DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_pBaseCOMObject, ::g_pBaseCOMObject)
Expand Down
15 changes: 7 additions & 8 deletions src/inc/jithelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,17 +146,16 @@
JITHELPER(CORINFO_HELP_GETSYNCFROMCLASSHANDLE, JIT_GetSyncFromClassHandle, CORINFO_HELP_SIG_REG_ONLY)

// Security callout support
JITHELPER(CORINFO_HELP_SECURITY_PROLOG, JIT_Security_Prolog,CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_SECURITY_PROLOG_FRAMED, JIT_Security_Prolog_Framed,CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_SECURITY_PROLOG, NULL,CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_SECURITY_PROLOG_FRAMED, NULL,CORINFO_HELP_SIG_REG_ONLY)

JITHELPER(CORINFO_HELP_METHOD_ACCESS_CHECK, JIT_MethodAccessCheck,CORINFO_HELP_SIG_8_STACK)
JITHELPER(CORINFO_HELP_FIELD_ACCESS_CHECK, JIT_FieldAccessCheck,CORINFO_HELP_SIG_4_STACK)
JITHELPER(CORINFO_HELP_CLASS_ACCESS_CHECK, JIT_ClassAccessCheck,CORINFO_HELP_SIG_4_STACK)
JITHELPER(CORINFO_HELP_METHOD_ACCESS_CHECK, NULL,CORINFO_HELP_SIG_8_STACK)
JITHELPER(CORINFO_HELP_FIELD_ACCESS_CHECK, NULL,CORINFO_HELP_SIG_4_STACK)
JITHELPER(CORINFO_HELP_CLASS_ACCESS_CHECK, NULL,CORINFO_HELP_SIG_4_STACK)

JITHELPER(CORINFO_HELP_DELEGATE_SECURITY_CHECK, JIT_DelegateSecurityCheck,CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_DELEGATE_SECURITY_CHECK, NULL,CORINFO_HELP_SIG_REG_ONLY)

// Verification runtime callout support
JITHELPER(CORINFO_HELP_VERIFICATION_RUNTIME_CHECK, JIT_VerificationRuntimeCheck,CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_VERIFICATION_RUNTIME_CHECK, NULL,CORINFO_HELP_SIG_REG_ONLY)

// GC support
DYNAMICJITHELPER(CORINFO_HELP_STOP_FOR_GC, JIT_RareDisableHelper, CORINFO_HELP_SIG_REG_ONLY)
Expand Down
2 changes: 1 addition & 1 deletion src/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12340,7 +12340,7 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree)
}

// We can't answer the equality comparison definitively at jit
// time, but can still simplfy the comparison.
// time, but can still simplify the comparison.
//
// Find out how we can compare the two handles.
// NOTE: We're potentially passing NO_CLASS_HANDLE, but the runtime knows what to do with it here.
Expand Down
4 changes: 0 additions & 4 deletions src/vm/appdomain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2083,10 +2083,6 @@ void SystemDomain::LoadBaseSystemClasses()
g_pUtf8StringClass = MscorlibBinder::GetClass(CLASS__UTF8_STRING);
#endif // FEATURE_UTF8STRING

// Used by Buffer::BlockCopy
g_pByteArrayMT = ClassLoader::LoadArrayTypeThrowing(
TypeHandle(MscorlibBinder::GetElementType(ELEMENT_TYPE_U1))).AsArray()->GetMethodTable();

#ifndef CROSSGEN_COMPILE
CrossLoaderAllocatorHashSetup::EnsureTypesLoaded();
#endif
Expand Down
84 changes: 0 additions & 84 deletions src/vm/comutilnative.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -685,90 +685,6 @@ void QCALLTYPE ExceptionNative::GetMessageFromNativeResources(ExceptionMessageKi
END_QCALL;
}

// BlockCopy
// This method from one primitive array to another based
// upon an offset into each an a byte count.
FCIMPL5(VOID, Buffer::BlockCopy, ArrayBase *src, int srcOffset, ArrayBase *dst, int dstOffset, int count)
{
FCALL_CONTRACT;

// Verify that both the src and dst are Arrays of primitive
// types.
// <TODO>@TODO: We need to check for booleans</TODO>
if (src==NULL || dst==NULL)
FCThrowArgumentNullVoid((src==NULL) ? W("src") : W("dst"));

SIZE_T srcLen, dstLen;

//
// Use specialized fast path for byte arrays because of it is what Buffer::BlockCopy is
// typically used for.
//

MethodTable * pByteArrayMT = g_pByteArrayMT;
_ASSERTE(pByteArrayMT != NULL);

// Optimization: If src is a byte array, we can
// simply set srcLen to GetNumComponents, without having
// to call GetComponentSize or verifying GetArrayElementType
if (src->GetMethodTable() == pByteArrayMT)
{
srcLen = src->GetNumComponents();
}
else
{
srcLen = src->GetNumComponents() * src->GetComponentSize();

// We only want to allow arrays of primitives, no Objects.
const CorElementType srcET = src->GetArrayElementType();
if (!CorTypeInfo::IsPrimitiveType_NoThrow(srcET))
FCThrowArgumentVoid(W("src"), W("Arg_MustBePrimArray"));
}

// Optimization: If copying to/from the same array, then
// we know that dstLen and srcLen must be the same.
if (src == dst)
{
dstLen = srcLen;
}
else if (dst->GetMethodTable() == pByteArrayMT)
{
dstLen = dst->GetNumComponents();
}
else
{
dstLen = dst->GetNumComponents() * dst->GetComponentSize();
if (dst->GetMethodTable() != src->GetMethodTable())
{
const CorElementType dstET = dst->GetArrayElementType();
if (!CorTypeInfo::IsPrimitiveType_NoThrow(dstET))
FCThrowArgumentVoid(W("dst"), W("Arg_MustBePrimArray"));
}
}

if (srcOffset < 0 || dstOffset < 0 || count < 0) {
const WCHAR* str = W("srcOffset");
if (dstOffset < 0) str = W("dstOffset");
if (count < 0) str = W("count");
FCThrowArgumentOutOfRangeVoid(str, W("ArgumentOutOfRange_NeedNonNegNum"));
}

if (srcLen < (SIZE_T)srcOffset + (SIZE_T)count || dstLen < (SIZE_T)dstOffset + (SIZE_T)count) {
FCThrowArgumentVoid(NULL, W("Argument_InvalidOffLen"));
}

PTR_BYTE srcPtr = src->GetDataPtr() + srcOffset;
PTR_BYTE dstPtr = dst->GetDataPtr() + dstOffset;

if ((srcPtr != dstPtr) && (count > 0)) {
memmove(dstPtr, srcPtr, count);
}

FC_GC_POLL();
}
FCIMPLEND


void QCALLTYPE MemoryNative::Clear(void *dst, size_t length)
{
QCALL_CONTRACT;
Expand Down
1 change: 0 additions & 1 deletion src/vm/comutilnative.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ class Buffer {
// BlockCopy
// This method from one primitive array to another based
// upon an offset into each an a byte count.
static FCDECL5(VOID, BlockCopy, ArrayBase *src, int srcOffset, ArrayBase *dst, int dstOffset, int count);
static FCDECL1(FC_BOOL_RET, IsPrimitiveTypeArray, ArrayBase *arrayUNSAFE);

static void QCALLTYPE MemMove(void *dst, void *src, size_t length);
Expand Down
1 change: 0 additions & 1 deletion src/vm/ecalllist.h
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,6 @@ FCFuncStart(gArrayFuncs)
FCFuncEnd()

FCFuncStart(gBufferFuncs)
FCFuncElement("BlockCopy", Buffer::BlockCopy)
FCFuncElement("IsPrimitiveTypeArray", Buffer::IsPrimitiveTypeArray)
#ifdef _TARGET_ARM_
FCFuncElement("Memcpy", FCallMemcpy)
Expand Down
4 changes: 2 additions & 2 deletions src/vm/ilmarshalers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3444,7 +3444,7 @@ void ILArrayWithOffsetMarshaler::EmitConvertSpaceAndContentsCLRToNativeTemp(ILCo
EmitLoadNativeValue(pslILEmit); // dest

pslILEmit->EmitLDLOC(m_dwPinnedLocalNum);
pslILEmit->EmitCALL(METHOD__ARRAY__GET_RAW_ARRAY_DATA, 1, 1);
pslILEmit->EmitCALL(METHOD__RUNTIME_HELPERS__GET_RAW_ARRAY_DATA, 1, 1);
pslILEmit->EmitCONV_I();

EmitLoadManagedValue(pslILEmit);
Expand Down Expand Up @@ -3488,7 +3488,7 @@ void ILArrayWithOffsetMarshaler::EmitConvertContentsNativeToCLR(ILCodeStream* ps
pslILEmit->EmitSTLOC(m_dwPinnedLocalNum);

pslILEmit->EmitLDLOC(m_dwPinnedLocalNum);
pslILEmit->EmitCALL(METHOD__ARRAY__GET_RAW_ARRAY_DATA, 1, 1);
pslILEmit->EmitCALL(METHOD__RUNTIME_HELPERS__GET_RAW_ARRAY_DATA, 1, 1);
pslILEmit->EmitCONV_I();

pslILEmit->EmitLDLOC(m_dwOffsetLocalNum);
Expand Down
Loading