From 8920a59751a646d0146daa9fabc06286c90670a1 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 13 Oct 2021 16:32:02 -0700 Subject: [PATCH 01/24] ValueArray that has its field repeated 64 times --- src/coreclr/vm/methodtablebuilder.cpp | 94 ++++++++++++++----- src/coreclr/vm/methodtablebuilder.h | 2 + .../System.Private.CoreLib.Shared.projitems | 1 + .../src/System/ValueArray.cs | 49 ++++++++++ .../System.Runtime/ref/System.Runtime.cs | 5 + 5 files changed, 127 insertions(+), 24 deletions(-) create mode 100644 src/libraries/System.Private.CoreLib/src/System/ValueArray.cs diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 90f451a1dfde14..1d716914349151 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1704,6 +1704,21 @@ MethodTableBuilder::BuildMethodTableThrowing( &pByValueClassCache, bmtMFDescs, bmtFP, &totalDeclaredFieldSize); + // TODO: VS this is a complete hack which works only in Debug + // ValueArray<> should be bound at load time and + // here we should compare it with constructed-from type. + // + SString cname = SString(SString::Utf8, this->GetHalfBakedClass()->m_szDebugClassName); + if (cname.BeginsWith(SString(SString::Utf8Literal, "System.ValueArray`1["))) + { + if (!bmtGenericsInfo->fContainsGenericVariables) + { + bmtFP->NumValueArrayElements = 42; + } + + printf("Loading: %s \n", this->GetHalfBakedClass()->m_szDebugClassName); + } + // Place regular static fields PlaceRegularStaticFields(); @@ -1723,6 +1738,11 @@ MethodTableBuilder::BuildMethodTableThrowing( _ASSERTE(HasLayout()); + if (bmtFP->NumValueArrayElements) + { + GetLayoutInfo()->m_cbManagedSize *= bmtFP->NumValueArrayElements; + } + bmtFP->NumInstanceFieldBytes = GetLayoutInfo()->m_cbManagedSize; // For simple Blittable types we still need to check if they have any overlapping @@ -8238,6 +8258,18 @@ VOID MethodTableBuilder::PlaceInstanceFields(MethodTable ** pByValueClassCach BuildMethodTableThrowException(IDS_CLASSLOAD_FIELDTOOLARGE); } + if (bmtFP->NumValueArrayElements > 0) + { + dwNumInstanceFieldBytes *= bmtFP->NumValueArrayElements; + + // TODO: VS we will repeat GC series for struct elements for now + // it may be possible to encode them as a repeat pattern (like in arrays) + if (pFieldDescList[0].IsByValue()) + { + dwNumGCPointerSeries *= bmtFP->NumValueArrayElements; + } + } + bmtFP->NumInstanceFieldBytes = dwNumInstanceFieldBytes; bmtFP->NumGCPointerSeries = dwNumGCPointerSeries; @@ -11302,12 +11334,19 @@ VOID MethodTableBuilder::HandleGCForValueClasses(MethodTable ** pByValueClassCac } + DWORD repeat = 1; + if (bmtFP->NumValueArrayElements > 0) + { + _ASSERTE(bmtEnumFields->dwNumInstanceFields == 1); + repeat = bmtFP->NumValueArrayElements; + } + // Build the pointer series map for this pointers in this instance pSeries = ((CGCDesc*)pMT)->GetLowestSeries(); if (bmtFP->NumInstanceGCPointerFields) { // See gcdesc.h for an explanation of why we adjust by subtracting BaseSize - pSeries->SetSeriesSize( (size_t) (bmtFP->NumInstanceGCPointerFields * TARGET_POINTER_SIZE) - (size_t) pMT->GetBaseSize()); + pSeries->SetSeriesSize((size_t)(bmtFP->NumInstanceGCPointerFields * repeat * TARGET_POINTER_SIZE) - (size_t)pMT->GetBaseSize()); pSeries->SetSeriesOffset(bmtFP->GCPointerFieldStart + OBJECT_SIZE); pSeries++; } @@ -11317,44 +11356,51 @@ VOID MethodTableBuilder::HandleGCForValueClasses(MethodTable ** pByValueClassCac { if (pFieldDescList[i].IsByValue()) { - MethodTable *pByValueMT = pByValueClassCache[i]; + MethodTable* pByValueMT = pByValueClassCache[i]; if (pByValueMT->ContainsPointers()) { // Offset of the by value class in the class we are building, does NOT include Object DWORD dwCurrentOffset = pFieldDescList[i].GetOffset_NoLogging(); + DWORD dwElementSize = pByValueMT->GetBaseSize() - OBJECT_BASESIZE; - // The by value class may have more than one pointer series - CGCDescSeries * pByValueSeries = CGCDesc::GetCGCDescFromMT(pByValueMT)->GetLowestSeries(); - SIZE_T dwNumByValueSeries = CGCDesc::GetCGCDescFromMT(pByValueMT)->GetNumSeries(); - - for (SIZE_T j = 0; j < dwNumByValueSeries; j++) + for (DWORD r = 0; r < repeat; r++) { - size_t cbSeriesSize; - size_t cbSeriesOffset; + // The by value class may have more than one pointer series + CGCDescSeries* pByValueSeries = CGCDesc::GetCGCDescFromMT(pByValueMT)->GetLowestSeries(); + SIZE_T dwNumByValueSeries = CGCDesc::GetCGCDescFromMT(pByValueMT)->GetNumSeries(); + + for (SIZE_T j = 0; j < dwNumByValueSeries; j++) + { + size_t cbSeriesSize; + size_t cbSeriesOffset; + + _ASSERTE(pSeries <= CGCDesc::GetCGCDescFromMT(pMT)->GetHighestSeries()); - _ASSERTE(pSeries <= CGCDesc::GetCGCDescFromMT(pMT)->GetHighestSeries()); + cbSeriesSize = pByValueSeries->GetSeriesSize(); - cbSeriesSize = pByValueSeries->GetSeriesSize(); + // Add back the base size of the by value class, since it's being transplanted to this class + cbSeriesSize += pByValueMT->GetBaseSize(); - // Add back the base size of the by value class, since it's being transplanted to this class - cbSeriesSize += pByValueMT->GetBaseSize(); + // Subtract the base size of the class we're building + cbSeriesSize -= pMT->GetBaseSize(); - // Subtract the base size of the class we're building - cbSeriesSize -= pMT->GetBaseSize(); + // Set current series we're building + pSeries->SetSeriesSize(cbSeriesSize); - // Set current series we're building - pSeries->SetSeriesSize(cbSeriesSize); + // Get offset into the value class of the first pointer field (includes a +Object) + cbSeriesOffset = pByValueSeries->GetSeriesOffset(); - // Get offset into the value class of the first pointer field (includes a +Object) - cbSeriesOffset = pByValueSeries->GetSeriesOffset(); + // Add element N offset + cbSeriesOffset += r * dwElementSize; - // Add it to the offset of the by value class in our class - cbSeriesOffset += dwCurrentOffset; + // Add it to the offset of the by value class in our class + cbSeriesOffset += dwCurrentOffset; - pSeries->SetSeriesOffset(cbSeriesOffset); // Offset of field - pSeries++; - pByValueSeries++; + pSeries->SetSeriesOffset(cbSeriesOffset); // Offset of field + pSeries++; + pByValueSeries++; + } } } } diff --git a/src/coreclr/vm/methodtablebuilder.h b/src/coreclr/vm/methodtablebuilder.h index 42cae0b22c1e95..dacb56ed6bfdcc 100644 --- a/src/coreclr/vm/methodtablebuilder.h +++ b/src/coreclr/vm/methodtablebuilder.h @@ -2043,6 +2043,8 @@ class MethodTableBuilder DWORD NumGCPointerSeries; DWORD NumInstanceFieldBytes; + DWORD NumValueArrayElements; + bool fIsByRefLikeType; bool fHasFixedAddressValueTypes; bool fHasSelfReferencingStaticValueTypeField_WithRVA; diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 31613fdb4a950e..071d445925249c 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1128,6 +1128,7 @@ + diff --git a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs new file mode 100644 index 00000000000000..615864e742a8e3 --- /dev/null +++ b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs @@ -0,0 +1,49 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.ComponentModel; +using System.Runtime.CompilerServices; +using Internal.Runtime.CompilerServices; + +namespace System +{ + // [Obsolete("Never use this type directly as that would be an array of unknown length.")] + // [EditorBrowsable(EditorBrowsableState.Never)] + public struct ValueArray + { + // For the array of Length N, we will have N+1 elements immdiately follow. + public T Element0; + + /// + /// NOTE: I am not sure we need the indexer in the final impl. + /// We may not want to JIT it for every length. + /// It may be better to just let C# compiler code-gen the accesses. + /// + public T this[int index] + { + get => this.Address(index); + set => this.Address(index) = value; + } + } + + public static class ValueArrayHelpers + { + /// + /// Returns an address to an element of the ValueArray. + /// Caller must statically know the Length and ensure the "index" is within bounds. + /// + public static ref T Address(this ref ValueArray array, int index) + { + return ref Unsafe.Add(ref array.Element0, (nint)(uint)index /* force zero-extension */); + } + + /// + /// Returns a slice of the ValueArray. + /// Caller must statically know the Length and ensure the "length" is within bounds. + /// + public static Span Slice(this ref ValueArray array, int length) + { + return new Span(ref array.Element0, length); + } + } +} diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 7393345702fe65..40cdd4363a5c6d 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7896,6 +7896,11 @@ public enum UriPartial Path = 2, Query = 3, } + public partial struct ValueArray + { + public T Element0; + public T this[int index] { get { throw null; } set { } } + } public partial struct ValueTuple : System.Collections.IStructuralComparable, System.Collections.IStructuralEquatable, System.IComparable, System.IComparable, System.IEquatable, System.Runtime.CompilerServices.ITuple { object? System.Runtime.CompilerServices.ITuple.this[int index] { get { throw null; } } From 8ff4f56a5da1afba82deac1917b47b6d478ac7f0 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 13 Oct 2021 19:43:24 -0700 Subject: [PATCH 02/24] Update ValueArray API, add range checks. --- .../src/System/ValueArray.cs | 32 ++++++++++--------- .../System.Runtime/ref/System.Runtime.cs | 4 ++- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs index 615864e742a8e3..4153abe49a2c8e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs @@ -15,35 +15,37 @@ public struct ValueArray public T Element0; /// - /// NOTE: I am not sure we need the indexer in the final impl. - /// We may not want to JIT it for every length. - /// It may be better to just let C# compiler code-gen the accesses. + /// Indexer. + /// Caller must statically know the upperBound and pass it in. /// - public T this[int index] + public T this[int index, int upperBound = 42] { - get => this.Address(index); - set => this.Address(index) = value; + get => Address(index, upperBound); + set => Address(index, upperBound) = value; } - } - public static class ValueArrayHelpers - { /// /// Returns an address to an element of the ValueArray. - /// Caller must statically know the Length and ensure the "index" is within bounds. + /// Caller must statically know the upperBound and pass it in. /// - public static ref T Address(this ref ValueArray array, int index) + public ref T Address(int index, int upperBound = 42) { - return ref Unsafe.Add(ref array.Element0, (nint)(uint)index /* force zero-extension */); + if ((uint)index >= (uint)upperBound) + ThrowHelper.ThrowIndexOutOfRangeException(); + + return ref Unsafe.Add(ref new ByReference(ref Element0).Value, (nint)(uint)index /* force zero-extension */); } /// /// Returns a slice of the ValueArray. - /// Caller must statically know the Length and ensure the "length" is within bounds. + /// Caller must statically know the upperBound and pass it in. /// - public static Span Slice(this ref ValueArray array, int length) + public Span Slice(int length, int upperBound = 42) { - return new Span(ref array.Element0, length); + if ((uint)length >= (uint)upperBound) + ThrowHelper.ThrowIndexOutOfRangeException(); + + return new Span(ref Element0, length); } } } diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 40cdd4363a5c6d..82eb7f442e1baa 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7899,7 +7899,9 @@ public enum UriPartial public partial struct ValueArray { public T Element0; - public T this[int index] { get { throw null; } set { } } + public T this[int index, int upperBound = 42] { get { throw null; } set { } } + public ref T Address(int index, int upperBound = 42) { throw null; } + public Span Slice(int length, int upperBound = 42) { throw null; } } public partial struct ValueTuple : System.Collections.IStructuralComparable, System.Collections.IStructuralEquatable, System.IComparable, System.IComparable, System.IEquatable, System.Runtime.CompilerServices.ITuple { From 9513c8edceb45cd9808fdcec0a22d9e0cfe51456 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 14 Oct 2021 15:10:17 -0700 Subject: [PATCH 03/24] less hacky ValueArray detection --- src/coreclr/inc/dacvars.h | 1 + src/coreclr/vm/appdomain.cpp | 2 ++ src/coreclr/vm/corelib.h | 2 ++ src/coreclr/vm/methodtablebuilder.cpp | 17 +++++------------ src/coreclr/vm/vars.cpp | 1 + src/coreclr/vm/vars.hpp | 1 + 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/coreclr/inc/dacvars.h b/src/coreclr/inc/dacvars.h index fdae380a90cb4f..e3cc24237a39fc 100644 --- a/src/coreclr/inc/dacvars.h +++ b/src/coreclr/inc/dacvars.h @@ -168,6 +168,7 @@ DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_pMulticastDelegateClass, ::g_p DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_pFreeObjectMethodTable, ::g_pFreeObjectMethodTable) DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_pOverlappedDataClass, ::g_pOverlappedDataClass) DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_pValueTypeClass, ::g_pValueTypeClass) +DEFINE_DACVAR(ULONG, UNKNOWN_POINTER_TYPE, dac__g_pValueArrayClass, ::g_pValueArrayClass) 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) diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index 61c4c08e249257..d65490134813f1 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -1441,6 +1441,8 @@ void SystemDomain::LoadBaseSystemClasses() g_pICastableInterface = CoreLibBinder::GetClass(CLASS__ICASTABLE); #endif // FEATURE_ICASTABLE + g_pValueArrayClass = CoreLibBinder::GetClass(CLASS__VALUE_ARRAY); + // Make sure that FCall mapping for Monitor.Enter is initialized. We need it in case Monitor.Enter is used only as JIT helper. // For more details, see comment in code:JITutil_MonEnterWorker around "__me = GetEEFuncEntryPointMacro(JIT_MonEnter)". ECall::GetFCallImpl(CoreLibBinder::GetMethod(METHOD__MONITOR__ENTER)); diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index b46562ddb13b75..36c5698054eb74 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -977,6 +977,8 @@ DEFINE_CLASS(VALUE_TYPE, System, ValueType) DEFINE_METHOD(VALUE_TYPE, GET_HASH_CODE, GetHashCode, IM_RetInt) DEFINE_METHOD(VALUE_TYPE, EQUALS, Equals, IM_Obj_RetBool) +DEFINE_CLASS(VALUE_ARRAY, System, ValueArray`1) + DEFINE_CLASS(GC, System, GC) DEFINE_METHOD(GC, KEEP_ALIVE, KeepAlive, SM_Obj_RetVoid) DEFINE_METHOD(GC, COLLECT, Collect, SM_RetVoid) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 1d716914349151..6fa22ceab373c0 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1704,19 +1704,12 @@ MethodTableBuilder::BuildMethodTableThrowing( &pByValueClassCache, bmtMFDescs, bmtFP, &totalDeclaredFieldSize); - // TODO: VS this is a complete hack which works only in Debug - // ValueArray<> should be bound at load time and - // here we should compare it with constructed-from type. - // - SString cname = SString(SString::Utf8, this->GetHalfBakedClass()->m_szDebugClassName); - if (cname.BeginsWith(SString(SString::Utf8Literal, "System.ValueArray`1["))) + if (!bmtGenericsInfo->fContainsGenericVariables && + g_pValueArrayClass != NULL && + g_pValueArrayClass->GetCl() == GetCl() && + g_pValueArrayClass->GetModule() == bmtInternal->pType->GetModule()) { - if (!bmtGenericsInfo->fContainsGenericVariables) - { - bmtFP->NumValueArrayElements = 42; - } - - printf("Loading: %s \n", this->GetHalfBakedClass()->m_szDebugClassName); + bmtFP->NumValueArrayElements = 42; } // Place regular static fields diff --git a/src/coreclr/vm/vars.cpp b/src/coreclr/vm/vars.cpp index 5004599bc779c6..97a4a801d60f9d 100644 --- a/src/coreclr/vm/vars.cpp +++ b/src/coreclr/vm/vars.cpp @@ -71,6 +71,7 @@ GPTR_IMPL(MethodTable, g_pExecutionEngineExceptionClass); GPTR_IMPL(MethodTable, g_pDelegateClass); GPTR_IMPL(MethodTable, g_pMulticastDelegateClass); GPTR_IMPL(MethodTable, g_pValueTypeClass); +GPTR_IMPL(MethodTable, g_pValueArrayClass); GPTR_IMPL(MethodTable, g_pEnumClass); GPTR_IMPL(MethodTable, g_pThreadClass); GPTR_IMPL(MethodTable, g_pFreeObjectMethodTable); diff --git a/src/coreclr/vm/vars.hpp b/src/coreclr/vm/vars.hpp index e4523ef145ab75..dbbc670836f44e 100644 --- a/src/coreclr/vm/vars.hpp +++ b/src/coreclr/vm/vars.hpp @@ -378,6 +378,7 @@ GPTR_DECL(MethodTable, g_pDelegateClass); GPTR_DECL(MethodTable, g_pMulticastDelegateClass); GPTR_DECL(MethodTable, g_pFreeObjectMethodTable); GPTR_DECL(MethodTable, g_pValueTypeClass); +GPTR_DECL(MethodTable, g_pValueArrayClass); GPTR_DECL(MethodTable, g_pEnumClass); GPTR_DECL(MethodTable, g_pThreadClass); GPTR_DECL(MethodTable, g_pOverlappedDataClass); From d9cf4d513ad4722a6a91c62da1df0e1e671739a1 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 14 Oct 2021 22:51:01 -0700 Subject: [PATCH 04/24] Use MD array to encode Length --- src/coreclr/vm/class.h | 7 ++----- src/coreclr/vm/clsload.cpp | 4 ---- src/coreclr/vm/corelib.h | 2 +- src/coreclr/vm/generics.cpp | 8 +++++++ src/coreclr/vm/methodtablebuilder.cpp | 6 +++++- .../src/System/ValueArray.cs | 21 +++++++++---------- .../ConstructorInfoInvokeArrayTests.cs | 1 - .../System.Runtime/ref/System.Runtime.cs | 9 ++++---- 8 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index 1817658119d17e..dd73d6da09af42 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -1975,7 +1975,7 @@ class ArrayClass : public EEClass private: DAC_ALIGNAS(EEClass) // Align the first member to the alignment of the base class - unsigned char m_rank; + DWORD m_rank; CorElementType m_ElementType;// Cache of element type in m_ElementTypeHnd public: @@ -1986,10 +1986,7 @@ class ArrayClass : public EEClass } void SetRank (unsigned Rank) { LIMITED_METHOD_CONTRACT; - // The only code path calling this function is code:ClassLoader::CreateTypeHandleForTypeKey, which has - // checked the rank already. Assert that the rank is less than MAX_RANK and that it fits in one byte. - _ASSERTE((Rank <= MAX_RANK) && (Rank <= (unsigned char)(-1))); - m_rank = (unsigned char)Rank; + m_rank = Rank; } CorElementType GetArrayElementType() { diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index 9392f6c11d93ee..68b26ec3f3884b 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -3046,10 +3046,6 @@ TypeHandle ClassLoader::CreateTypeHandleForTypeKey(TypeKey* pKey, AllocMemTracke } } - if (rank > MAX_RANK) - { - ThrowTypeLoadException(pKey, IDS_CLASSLOAD_RANK_TOOLARGE); - } templateMT = pLoaderModule->CreateArrayMethodTable(paramType, kind, rank, pamTracker); typeHnd = TypeHandle(templateMT); diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 36c5698054eb74..b18c1316ca0db9 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -977,7 +977,7 @@ DEFINE_CLASS(VALUE_TYPE, System, ValueType) DEFINE_METHOD(VALUE_TYPE, GET_HASH_CODE, GetHashCode, IM_RetInt) DEFINE_METHOD(VALUE_TYPE, EQUALS, Equals, IM_Obj_RetBool) -DEFINE_CLASS(VALUE_ARRAY, System, ValueArray`1) +DEFINE_CLASS(VALUE_ARRAY, System, ValueArray`2) DEFINE_CLASS(GC, System, GC) DEFINE_METHOD(GC, KEEP_ALIVE, KeepAlive, SM_Obj_RetVoid) diff --git a/src/coreclr/vm/generics.cpp b/src/coreclr/vm/generics.cpp index 7e449d8c29b08a..8c8aa0267b5919 100644 --- a/src/coreclr/vm/generics.cpp +++ b/src/coreclr/vm/generics.cpp @@ -37,6 +37,14 @@ TypeHandle ClassLoader::CanonicalizeGenericArg(TypeHandle thGenericArg) #if defined(FEATURE_SHARE_GENERIC_CODE) CorElementType et = thGenericArg.GetSignatureCorElementType(); + //TODO: we can narrow the scenario + // by requiring a special/unspeakable/unusable element type + if (et == ELEMENT_TYPE_ARRAY && + thGenericArg.GetArrayElementTypeHandle() == TypeHandle(g_pObjectClass)) + { + RETURN(thGenericArg); + } + // Note that generic variables do not share if (CorTypeInfo::IsObjRef_NoThrow(et)) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 6fa22ceab373c0..3667d5ff85e5b3 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1709,7 +1709,11 @@ MethodTableBuilder::BuildMethodTableThrowing( g_pValueArrayClass->GetCl() == GetCl() && g_pValueArrayClass->GetModule() == bmtInternal->pType->GetModule()) { - bmtFP->NumValueArrayElements = 42; + TypeHandle R = bmtGenericsInfo->GetInstantiation()[1]; + if (R.IsArray()) + { + bmtFP->NumValueArrayElements = bmtGenericsInfo->GetInstantiation()[1].GetRank(); + } } // Place regular static fields diff --git a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs index 4153abe49a2c8e..c4188a5c9fb026 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs @@ -7,30 +7,29 @@ namespace System { - // [Obsolete("Never use this type directly as that would be an array of unknown length.")] - // [EditorBrowsable(EditorBrowsableState.Never)] - public struct ValueArray + public struct ValueArray // where R : System.Array { + public static readonly int Length = typeof(R).GetArrayRank(); + // For the array of Length N, we will have N+1 elements immdiately follow. public T Element0; /// /// Indexer. - /// Caller must statically know the upperBound and pass it in. /// - public T this[int index, int upperBound = 42] + public T this[int index] { - get => Address(index, upperBound); - set => Address(index, upperBound) = value; + get => Address(index); + set => Address(index) = value; } /// /// Returns an address to an element of the ValueArray. /// Caller must statically know the upperBound and pass it in. /// - public ref T Address(int index, int upperBound = 42) + public ref T Address(int index) { - if ((uint)index >= (uint)upperBound) + if ((uint)index >= (uint)Length) ThrowHelper.ThrowIndexOutOfRangeException(); return ref Unsafe.Add(ref new ByReference(ref Element0).Value, (nint)(uint)index /* force zero-extension */); @@ -40,9 +39,9 @@ public ref T Address(int index, int upperBound = 42) /// Returns a slice of the ValueArray. /// Caller must statically know the upperBound and pass it in. /// - public Span Slice(int length, int upperBound = 42) + public Span Slice(int length) { - if ((uint)length >= (uint)upperBound) + if ((uint)length >= (uint)Length) ThrowHelper.ThrowIndexOutOfRangeException(); return new Span(ref Element0, length); diff --git a/src/libraries/System.Reflection.TypeExtensions/tests/ConstructorInfo/ConstructorInfoInvokeArrayTests.cs b/src/libraries/System.Reflection.TypeExtensions/tests/ConstructorInfo/ConstructorInfoInvokeArrayTests.cs index a050476cee765c..47a417a18f6267 100644 --- a/src/libraries/System.Reflection.TypeExtensions/tests/ConstructorInfo/ConstructorInfoInvokeArrayTests.cs +++ b/src/libraries/System.Reflection.TypeExtensions/tests/ConstructorInfo/ConstructorInfoInvokeArrayTests.cs @@ -226,7 +226,6 @@ public void Invoke_LargeDimensionalArrayConstructor() Type type = Type.GetType("System.Type[,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,]"); ConstructorInfo[] cia = TypeExtensions.GetConstructors(type); Assert.Equal(2, cia.Length); - Assert.Throws(() => Type.GetType("System.Type[,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,]")); } [Fact] diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 82eb7f442e1baa..ee4e03ed18e811 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7896,12 +7896,13 @@ public enum UriPartial Path = 2, Query = 3, } - public partial struct ValueArray + public partial struct ValueArray // where R : System.Array { + public static readonly int Length; public T Element0; - public T this[int index, int upperBound = 42] { get { throw null; } set { } } - public ref T Address(int index, int upperBound = 42) { throw null; } - public Span Slice(int length, int upperBound = 42) { throw null; } + public T this[int index] { get { throw null; } set { } } + public ref T Address(int index) { throw null; } + public Span Slice(int length) { throw null; } } public partial struct ValueTuple : System.Collections.IStructuralComparable, System.Collections.IStructuralEquatable, System.IComparable, System.IComparable, System.IEquatable, System.Runtime.CompilerServices.ITuple { From 08d0cd6b7909f58805099cdae854c654d61929b6 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 15 Oct 2021 12:40:35 -0700 Subject: [PATCH 05/24] Tweak ValueArray so that range checks could be inlined/omitted by JIT --- .../src/System/ValueArray.cs | 20 ++++++++++++++++++- .../System.Runtime/ref/System.Runtime.cs | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs index c4188a5c9fb026..595307281ebd07 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs @@ -9,7 +9,7 @@ namespace System { public struct ValueArray // where R : System.Array { - public static readonly int Length = typeof(R).GetArrayRank(); + public static int Length => RankOf.Value; // For the array of Length N, we will have N+1 elements immdiately follow. public T Element0; @@ -47,4 +47,22 @@ public Span Slice(int length) return new Span(ref Element0, length); } } + + // internal helper to compute and cache the Rank of an object array. + internal static class RankOf + { + public static readonly int Value = GetRank(); + + private static int GetRank() + { + var type = typeof(R); + if (!type.IsArray) + throw new ArgumentException("R must be an array"); + + if (type.GetElementType() != typeof(object)) + throw new ArgumentException("R must be an object array"); + + return type.GetArrayRank(); + } + } } diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index ee4e03ed18e811..8ac6e7196b970b 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7898,7 +7898,7 @@ public enum UriPartial } public partial struct ValueArray // where R : System.Array { - public static readonly int Length; + public static int Length { get { throw null; } } public T Element0; public T this[int index] { get { throw null; } set { } } public ref T Address(int index) { throw null; } From 1884e231d135eee6bae5f04245149e3a041c3b1a Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 15 Oct 2021 13:07:37 -0700 Subject: [PATCH 06/24] comments --- .../System.Private.CoreLib/src/System/ValueArray.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs index 595307281ebd07..e1e03d7dcc4aea 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs @@ -15,7 +15,7 @@ public struct ValueArray // where R : System.Array public T Element0; /// - /// Indexer. + /// Gets or sets the element at the specified index. /// public T this[int index] { @@ -24,8 +24,7 @@ public T this[int index] } /// - /// Returns an address to an element of the ValueArray. - /// Caller must statically know the upperBound and pass it in. + /// Gets an element address at the specified index. /// public ref T Address(int index) { @@ -37,7 +36,6 @@ public ref T Address(int index) /// /// Returns a slice of the ValueArray. - /// Caller must statically know the upperBound and pass it in. /// public Span Slice(int length) { From 9b17a5aa2b49148f0c18f29e07612a1592e32b27 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 15 Oct 2021 13:46:11 -0700 Subject: [PATCH 07/24] Make the Element0 field private --- src/libraries/System.Private.CoreLib/src/System/ValueArray.cs | 4 ++-- src/libraries/System.Runtime/ref/System.Runtime.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs index e1e03d7dcc4aea..563be0f4dd39fe 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs @@ -11,8 +11,8 @@ public struct ValueArray // where R : System.Array { public static int Length => RankOf.Value; - // For the array of Length N, we will have N+1 elements immdiately follow. - public T Element0; + // For the array of Length N, runtime will add N-1 elements immediately after this one. + private T Element0; /// /// Gets or sets the element at the specified index. diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 8ac6e7196b970b..72b5b3d1af8194 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7899,7 +7899,7 @@ public enum UriPartial public partial struct ValueArray // where R : System.Array { public static int Length { get { throw null; } } - public T Element0; + private T Element0; public T this[int index] { get { throw null; } set { } } public ref T Address(int index) { throw null; } public Span Slice(int length) { throw null; } From 1e2ef51fbec8255887a968fe3958ba5781ba3b14 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 15 Oct 2021 16:47:25 -0700 Subject: [PATCH 08/24] Make API more similar to Span. --- .../src/System/ValueArray.cs | 43 ++++++++++--------- .../System.Runtime/ref/System.Runtime.cs | 7 ++- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs index 563be0f4dd39fe..aba983f580a80b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs @@ -9,40 +9,43 @@ namespace System { public struct ValueArray // where R : System.Array { - public static int Length => RankOf.Value; + public int Length => RankOf.Value; // For the array of Length N, runtime will add N-1 elements immediately after this one. private T Element0; /// - /// Gets or sets the element at the specified index. + /// Returns a reference to specified element of the value array. /// - public T this[int index] + /// + /// + /// + /// Thrown when index less than 0 or index greater than or equal to Length + /// + public ref T this[int index] { - get => Address(index); - set => Address(index) = value; - } - - /// - /// Gets an element address at the specified index. - /// - public ref T Address(int index) - { - if ((uint)index >= (uint)Length) - ThrowHelper.ThrowIndexOutOfRangeException(); + get + { + if ((uint)index >= (uint)Length) + ThrowHelper.ThrowIndexOutOfRangeException(); - return ref Unsafe.Add(ref new ByReference(ref Element0).Value, (nint)(uint)index /* force zero-extension */); + return ref Unsafe.Add(ref new ByReference(ref Element0).Value, (nint)(uint)index /* force zero-extension */); + } } /// - /// Returns a slice of the ValueArray. + /// Forms a slice out of the given value array, beginning at 'start'. /// - public Span Slice(int length) + /// The index at which to begin this slice. + /// + /// Thrown when the specified index is not in range (<0 or >Length). + /// + public Span Slice(int start) { - if ((uint)length >= (uint)Length) - ThrowHelper.ThrowIndexOutOfRangeException(); + if ((uint)start > (uint)Length) + ThrowHelper.ThrowArgumentOutOfRangeException(); - return new Span(ref Element0, length); + return new Span(ref Unsafe.Add(ref Element0, (nint)(uint)start /* force zero-extension */), Length - start); } } diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 72b5b3d1af8194..6ea90f800c0503 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7898,11 +7898,10 @@ public enum UriPartial } public partial struct ValueArray // where R : System.Array { - public static int Length { get { throw null; } } + public int Length { get { throw null; } } private T Element0; - public T this[int index] { get { throw null; } set { } } - public ref T Address(int index) { throw null; } - public Span Slice(int length) { throw null; } + public ref T this[int index] { get { throw null; } } + public Span Slice(int start) { throw null; } } public partial struct ValueTuple : System.Collections.IStructuralComparable, System.Collections.IStructuralEquatable, System.IComparable, System.IComparable, System.IEquatable, System.Runtime.CompilerServices.ITuple { From b10d2e96c8b7552cf3353708abcf6c1d78895412 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 15 Oct 2021 21:38:46 -0700 Subject: [PATCH 09/24] add a proper test --- .../system/valuearray/ValueArray.cs | 210 ++++++++++++++++++ .../system/valuearray/ValueArray.csproj | 13 ++ .../system/valuearray/ValueArray_ro.csproj | 13 ++ 3 files changed, 236 insertions(+) create mode 100644 src/tests/CoreMangLib/system/valuearray/ValueArray.cs create mode 100644 src/tests/CoreMangLib/system/valuearray/ValueArray.csproj create mode 100644 src/tests/CoreMangLib/system/valuearray/ValueArray_ro.csproj diff --git a/src/tests/CoreMangLib/system/valuearray/ValueArray.cs b/src/tests/CoreMangLib/system/valuearray/ValueArray.cs new file mode 100644 index 00000000000000..e295e5835979c8 --- /dev/null +++ b/src/tests/CoreMangLib/system/valuearray/ValueArray.cs @@ -0,0 +1,210 @@ + + +using System; +using System.Runtime.CompilerServices; + +public class Program +{ + private static int returnVal = 100; + + public static int Main(string[] args) + { + TestSizeOf(); + TestLength(); + TestValueSemantic(); + TestIndexing(); + TestSlice(); + TestGCRootsRef(); + TestGCRootsStruct(); + TestListT(); + + Console.WriteLine("DONE"); + return returnVal; + } + + private unsafe static void TestSizeOf() + { + Console.WriteLine(nameof(TestSizeOf)); + + Test(sizeof(int) * 4, sizeof(ValueArray)); + Test(sizeof(nint) * 5, sizeof(ValueArray)); + Test(sizeof(ValueArray) * 6, sizeof(ValueArray, object[,,,,,]>)); + } + + private static void TestIndexing() + { + Console.WriteLine(nameof(TestIndexing)); + + var arr1 = new ValueArray(); + for (int i = 0; i < arr1.Length; i++) + { + arr1[i] = i % 2 == 0; + } + + for (int i = 0; i < arr1.Length; i++) + { + Test(i % 2 == 0, arr1[i]); + } + } + + private static void TestSlice() + { + Console.WriteLine(nameof(TestSlice)); + + var arr1 = new ValueArray(); + for (int i = 0; i < arr1.Length; i++) + { + arr1[i] = i % 2 == 0; + } + + var span = arr1.Slice(0); + Test(9, span.Length); + for (int i = 0; i < span.Length; i++) + { + Test(i % 2 == 0, span[i]); + } + } + + private static void TestLength() + { + Console.WriteLine(nameof(TestLength)); + + Test(1, new ValueArray().Length); + Test(2, new ValueArray().Length); + Test(3, new ValueArray().Length); + Test(4, new ValueArray().Length); + Test(42, new ValueArray().Length); + } + + private static void TestValueSemantic() + { + Console.WriteLine(nameof(TestValueSemantic)); + + var arr1 = new ValueArray(); + for (int i = 0; i < arr1.Length; i++) + { + arr1[i] = i; + } + + var arr2 = arr1; + + for (int i = 0; i < arr1.Length; i++) + { + arr1[i]++; + } + + for (int i = 0; i < arr1.Length; i++) + { + Test(i, arr2[i]); + Test(i + 1, arr1[i]); + } + } + + private static void TestGCRootsRef() + { + Console.WriteLine(nameof(TestGCRootsRef)); + + var arrStr1 = new ValueArray(); + for (int i = 0; i < arrStr1.Length; i++) + { + arrStr1[i] = (i * 10000).ToString(); + } + + var junk = arrStr1; + for (int i = 0; i < arrStr1.Length; i++) + { + Test((i * 10000).ToString(), junk[i]); + } + + GC.Collect(); + GC.Collect(); + + var arrStr2 = new ValueArray(); + for (int i = 0; i < arrStr2.Length; i++) + { + arrStr2[i] = (i * 12345).ToString(); + } + + for (int i = 0; i < 42; i++) + { + Test((i * 10000).ToString(), arrStr1[i]); + } + } + + internal struct BSI + { + public byte B; + public string S; + public int I; + } + + private static void TestGCRootsStruct() + { + Console.WriteLine(nameof(TestGCRootsStruct)); + + var arrS1 = new ValueArray(); + for (int i = 0; i < 42; i++) + { + arrS1[i] = new BSI { S = (i * 10000).ToString() }; + } + + var arrS11 = arrS1; + for (int i = 0; i < 42; i++) + { + Test((i * 10000).ToString(), arrS11[i].S); + } + + GC.Collect(); + GC.Collect(); + + var arrS2 = new ValueArray(); + for (int i = 0; i < 42; i++) + { + arrS2[i] = new BSI { S = (i * 12345).ToString() }; + } + + for (int i = 0; i < 42; i++) + { + Test((i * 10000).ToString(), arrS1[i].S); + } + } + + private static void TestListT() + { + Console.WriteLine(nameof(TestListT)); + + var list = new List>(); + + for (int j = 0; j < 100; j++) + { + var arrS1 = new ValueArray(); + for (int i = 0; i < arrS1.Length; i++) + { + arrS1[i] = new BSI { S = (i * j).ToString() }; + } + + list.Add(arrS1); + } + + for (int j = 0; j < 100; j++) + { + var arrS1 = list[j]; + int i = 0; + foreach (ref var refS in arrS1.Slice(0)) + { + GC.Collect(); + + Test((i++ * j).ToString(), refS.S); + } + } + } + + public static void Test(T expected, T actual) where T : IEquatable + { + if (!expected.Equals(actual)) + { + Console.WriteLine($"Fail: {expected}, {actual}"); + returnVal++; + } + } +} diff --git a/src/tests/CoreMangLib/system/valuearray/ValueArray.csproj b/src/tests/CoreMangLib/system/valuearray/ValueArray.csproj new file mode 100644 index 00000000000000..c262c5fb3666c2 --- /dev/null +++ b/src/tests/CoreMangLib/system/valuearray/ValueArray.csproj @@ -0,0 +1,13 @@ + + + Exe + + + true + None + + + + + + diff --git a/src/tests/CoreMangLib/system/valuearray/ValueArray_ro.csproj b/src/tests/CoreMangLib/system/valuearray/ValueArray_ro.csproj new file mode 100644 index 00000000000000..8fa33681d43699 --- /dev/null +++ b/src/tests/CoreMangLib/system/valuearray/ValueArray_ro.csproj @@ -0,0 +1,13 @@ + + + Exe + + + true + None + True + + + + + From 48461cf8d736cdb09d71361a39288486dbb3fa6d Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 16 Oct 2021 08:30:32 -0700 Subject: [PATCH 10/24] Add a field test --- .../system/valuearray/ValueArray.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/tests/CoreMangLib/system/valuearray/ValueArray.cs b/src/tests/CoreMangLib/system/valuearray/ValueArray.cs index e295e5835979c8..3481c5230aa000 100644 --- a/src/tests/CoreMangLib/system/valuearray/ValueArray.cs +++ b/src/tests/CoreMangLib/system/valuearray/ValueArray.cs @@ -2,6 +2,7 @@ using System; using System.Runtime.CompilerServices; +using System.Collections.Generic; public class Program { @@ -17,11 +18,47 @@ public static int Main(string[] args) TestGCRootsRef(); TestGCRootsStruct(); TestListT(); + TestAsField(); Console.WriteLine("DONE"); return returnVal; } + class QuadTree + { + private ValueArray _nodes; + + public ValueArray Nodes { get => _nodes; } + + public QuadTree(int depth) + { + if (depth > 0) + { + for (int i = 0; i < _nodes.Length; i++) + { + _nodes[i] = new QuadTree(depth - 1); + } + } + } + + public int CountNodes() + { + int val = 1; + for (int i = 0; i < Nodes.Length; i++) + { + val += Nodes[i]?.CountNodes() ?? 0; + } + + return val; + } + } + + private static void TestAsField() + { + QuadTree tree = new QuadTree(10); + Test(10, tree.CountNodes()); + } + private unsafe static void TestSizeOf() { Console.WriteLine(nameof(TestSizeOf)); From e477426bc9ad6b76b819d2f137814f3d8d325540 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 16 Oct 2021 21:33:29 -0700 Subject: [PATCH 11/24] bug fixes --- src/coreclr/inc/corinfo.h | 2 +- src/coreclr/vm/class.h | 15 ++++++++++++++- src/coreclr/vm/generics.cpp | 3 +-- src/coreclr/vm/jitinterface.cpp | 5 +++++ src/coreclr/vm/methodtablebuilder.cpp | 12 +++++++++--- src/coreclr/vm/siginfo.cpp | 9 +++++++++ 6 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 1abb2077ab7626..42204d5cfdd57b 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -805,7 +805,7 @@ enum CorInfoFlag CORINFO_FLG_ARRAY = 0x00080000, // class is an array class (initialized differently) CORINFO_FLG_OVERLAPPING_FIELDS = 0x00100000, // struct or class has fields that overlap (aka union) CORINFO_FLG_INTERFACE = 0x00200000, // it is an interface - CORINFO_FLG_DONT_PROMOTE = 0x00400000, // don't try to promote fields (used for types outside of AOT compilation version bubble) + CORINFO_FLG_DONT_PROMOTE = 0x00400000, // don't try to promote fields (used for ValueArray and types outside of AOT compilation version bubble) CORINFO_FLG_CUSTOMLAYOUT = 0x00800000, // does this struct have custom layout? CORINFO_FLG_CONTAINS_GC_PTR = 0x01000000, // does the class contain a gc ptr ? CORINFO_FLG_DELEGATE = 0x02000000, // is this a subclass of delegate or multicast delegate ? diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index dd73d6da09af42..5a320682c7cc70 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -1342,6 +1342,18 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! LIMITED_METHOD_CONTRACT; m_VMFlags |= (DWORD)VMFLAG_HAS_FIELDS_WHICH_MUST_BE_INITED; } + + BOOL HasNoPromotionFlagSet() + { + LIMITED_METHOD_CONTRACT; + return (m_VMFlags & VMFLAG_DONT_PROMOTE); + } + void SetNoPromotionFlag() + { + LIMITED_METHOD_CONTRACT; + m_VMFlags |= (DWORD)VMFLAG_DONT_PROMOTE; + } + void SetCannotBeBlittedByObjectCloner() { /* no op */ @@ -1669,7 +1681,8 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! VMFLAG_BESTFITMAPPING = 0x00004000, // BestFitMappingAttribute.Value VMFLAG_THROWONUNMAPPABLECHAR = 0x00008000, // BestFitMappingAttribute.ThrowOnUnmappableChar - // unused = 0x00010000, + // suppress struct promotion (used by ValueArrays) + VMFLAG_DONT_PROMOTE = 0x00010000, VMFLAG_NO_GUID = 0x00020000, VMFLAG_HASNONPUBLICFIELDS = 0x00040000, // unused = 0x00080000, diff --git a/src/coreclr/vm/generics.cpp b/src/coreclr/vm/generics.cpp index 8c8aa0267b5919..7134a148b7a58f 100644 --- a/src/coreclr/vm/generics.cpp +++ b/src/coreclr/vm/generics.cpp @@ -37,8 +37,7 @@ TypeHandle ClassLoader::CanonicalizeGenericArg(TypeHandle thGenericArg) #if defined(FEATURE_SHARE_GENERIC_CODE) CorElementType et = thGenericArg.GetSignatureCorElementType(); - //TODO: we can narrow the scenario - // by requiring a special/unspeakable/unusable element type + // canonical form of MD object array is an MD object array - do not lose the rank if (et == ELEMENT_TYPE_ARRAY && thGenericArg.GetArrayElementTypeHandle() == TypeHandle(g_pObjectClass)) { diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 3ffb87896fa96a..2e455406388b14 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -3701,8 +3701,13 @@ uint32_t CEEInfo::getClassAttribsInternal (CORINFO_CLASS_HANDLE clsHnd) if (pClass->IsUnsafeValueClass()) ret |= CORINFO_FLG_UNSAFE_VALUECLASS; } + if (pClass->HasExplicitFieldOffsetLayout() && pClass->HasOverLayedField()) ret |= CORINFO_FLG_OVERLAPPING_FIELDS; + + if (pClass->HasNoPromotionFlagSet()) + ret |= CORINFO_FLG_DONT_PROMOTE; + if (VMClsHnd.IsCanonicalSubtype()) ret |= CORINFO_FLG_SHAREDINST; diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 3667d5ff85e5b3..aece1a89736cda 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1709,10 +1709,16 @@ MethodTableBuilder::BuildMethodTableThrowing( g_pValueArrayClass->GetCl() == GetCl() && g_pValueArrayClass->GetModule() == bmtInternal->pType->GetModule()) { - TypeHandle R = bmtGenericsInfo->GetInstantiation()[1]; - if (R.IsArray()) + TypeHandle lengthMarker = bmtGenericsInfo->GetInstantiation()[1]; + if (lengthMarker.IsArray()) { - bmtFP->NumValueArrayElements = bmtGenericsInfo->GetInstantiation()[1].GetRank(); + DWORD rank = lengthMarker.GetRank(); + if (rank > 1) + { + bmtFP->NumValueArrayElements = rank; + // there is no scenario when tearing a value array into pieces is desirable. + GetHalfBakedClass()->SetNoPromotionFlag(); + } } } diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index 4c438360513cb6..1759f1b4c7ec25 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -1365,6 +1365,15 @@ TypeHandle SigPointer::GetTypeHandleThrowing( if (tmpEType == ELEMENT_TYPE_CLASS) typeHnd = TypeHandle(g_pCanonMethodTableClass); } + if (elemType == ELEMENT_TYPE_ARRAY) + { + IfFailThrowBF(tempsig.GetElemType(&elemType), BFA_BAD_SIGNATURE, pModule); + // canonical form of MD object array is an MD object array - do not lose the rank + if (elemType != ELEMENT_TYPE_OBJECT) + { + typeHnd = TypeHandle(g_pCanonMethodTableClass); + } + } else if ((elemType == (CorElementType)ELEMENT_TYPE_CANON_ZAPSIG) || (CorTypeInfo::GetGCType_NoThrow(elemType) == TYPE_GC_REF)) { From 3443c2282fa7e620aa00fb914ba116637eb75f41 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sun, 17 Oct 2021 08:26:30 -0700 Subject: [PATCH 12/24] Value semantics should consider all elements. --- .../src/System/ValueArray.cs | 42 +++++++++++++++++-- .../System.Runtime/ref/System.Runtime.cs | 5 +++ .../system/valuearray/ValueArray.cs | 39 +++++++++++++---- 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs index aba983f580a80b..fba939e9763765 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs @@ -1,19 +1,26 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.ComponentModel; -using System.Runtime.CompilerServices; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using Internal.Runtime.CompilerServices; namespace System { public struct ValueArray // where R : System.Array + : IEquatable> { public int Length => RankOf.Value; // For the array of Length N, runtime will add N-1 elements immediately after this one. private T Element0; + /// + /// Returns a reference to the first element of the value array. + /// + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + public ref T GetPinnableReference() => ref new ByReference(ref Element0).Value; + /// /// Returns a reference to specified element of the value array. /// @@ -29,7 +36,7 @@ public ref T this[int index] if ((uint)index >= (uint)Length) ThrowHelper.ThrowIndexOutOfRangeException(); - return ref Unsafe.Add(ref new ByReference(ref Element0).Value, (nint)(uint)index /* force zero-extension */); + return ref Unsafe.Add(ref GetPinnableReference(), (nint)(uint)index /* force zero-extension */); } } @@ -47,6 +54,35 @@ public Span Slice(int start) return new Span(ref Unsafe.Add(ref Element0, (nint)(uint)start /* force zero-extension */), Length - start); } + + public override bool Equals([NotNullWhen(true)] object? obj) + { + return obj is ValueArray other && Equals(other); + } + + public bool Equals(ValueArray other) + { + for (int i = 0; i < Length; i++) + { + if (!EqualityComparer.Default.Equals(this[i], other[i])) + { + return false; + } + } + + return true; + } + + public override int GetHashCode() + { + HashCode hashCode = default; + for (int i = 0; i < Length; i++) + { + hashCode.Add(this[i]); + } + + return hashCode.ToHashCode(); + } } // internal helper to compute and cache the Rank of an object array. diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 6ea90f800c0503..65417afde4f21b 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7897,11 +7897,16 @@ public enum UriPartial Query = 3, } public partial struct ValueArray // where R : System.Array + : System.IEquatable> { public int Length { get { throw null; } } + public ref T GetPinnableReference() { throw null; } private T Element0; public ref T this[int index] { get { throw null; } } public Span Slice(int start) { throw null; } + public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; } + public bool Equals(ValueArray other) { throw null; } + public override int GetHashCode() { throw null; } } public partial struct ValueTuple : System.Collections.IStructuralComparable, System.Collections.IStructuralEquatable, System.IComparable, System.IComparable, System.IEquatable, System.Runtime.CompilerServices.ITuple { diff --git a/src/tests/CoreMangLib/system/valuearray/ValueArray.cs b/src/tests/CoreMangLib/system/valuearray/ValueArray.cs index 3481c5230aa000..31332d2ef0bd75 100644 --- a/src/tests/CoreMangLib/system/valuearray/ValueArray.cs +++ b/src/tests/CoreMangLib/system/valuearray/ValueArray.cs @@ -1,8 +1,8 @@ using System; -using System.Runtime.CompilerServices; using System.Collections.Generic; +using System.Runtime.CompilerServices; public class Program { @@ -12,7 +12,7 @@ public static int Main(string[] args) { TestSizeOf(); TestLength(); - TestValueSemantic(); + TestByValueSemantic(); TestIndexing(); TestSlice(); TestGCRootsRef(); @@ -20,15 +20,25 @@ public static int Main(string[] args) TestListT(); TestAsField(); - Console.WriteLine("DONE"); + if (returnVal == 100) + Console.WriteLine("PASS"); + return returnVal; } class QuadTree { + // embedded indexable data with no indirections!! private ValueArray _nodes; - public ValueArray Nodes { get => _nodes; } + // NB: intentionally returning byval here - just to test byval returning. + public ValueArray Nodes + { + get + { + return _nodes; + } + } public QuadTree(int depth) { @@ -55,8 +65,9 @@ public int CountNodes() private static void TestAsField() { + Console.WriteLine(nameof(TestAsField)); QuadTree tree = new QuadTree(10); - Test(10, tree.CountNodes()); + Test(1398101, tree.CountNodes()); } private unsafe static void TestSizeOf() @@ -113,9 +124,9 @@ private static void TestLength() Test(42, new ValueArray().Length); } - private static void TestValueSemantic() + private static void TestByValueSemantic() { - Console.WriteLine(nameof(TestValueSemantic)); + Console.WriteLine(nameof(TestByValueSemantic)); var arr1 = new ValueArray(); for (int i = 0; i < arr1.Length; i++) @@ -124,7 +135,6 @@ private static void TestValueSemantic() } var arr2 = arr1; - for (int i = 0; i < arr1.Length; i++) { arr1[i]++; @@ -135,6 +145,17 @@ private static void TestValueSemantic() Test(i, arr2[i]); Test(i + 1, arr1[i]); } + + // NB: also testing boxing here. + IEquatable> ieq = arr1; + + var arr3 = arr1; + Test(true, ieq.Equals(arr3)); + Test(arr3, arr1); + arr3[10] = -1; + + Test(false, ieq.Equals(arr3)); + Test(false, arr1.Equals(arr3)); } private static void TestGCRootsRef() @@ -168,7 +189,7 @@ private static void TestGCRootsRef() } } - internal struct BSI + public struct BSI { public byte B; public string S; From b698d94e9dff6ecf569f511f1073b36f8cb27325 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sun, 17 Oct 2021 09:11:14 -0700 Subject: [PATCH 13/24] Hide `GetPinnableReference` per convention, since it is for use in `fixed` statements. --- src/libraries/System.Runtime/ref/System.Runtime.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 65417afde4f21b..06ba5c1a6c64a1 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7900,6 +7900,7 @@ public partial struct ValueArray // where R : System.Array : System.IEquatable> { public int Length { get { throw null; } } + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] public ref T GetPinnableReference() { throw null; } private T Element0; public ref T this[int index] { get { throw null; } } From 5e174f799565277210f25aa6924a5527723e0c30 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sun, 17 Oct 2021 09:20:13 -0700 Subject: [PATCH 14/24] Add a test for use with pointers/fixed --- .../system/valuearray/ValueArray.cs | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/tests/CoreMangLib/system/valuearray/ValueArray.cs b/src/tests/CoreMangLib/system/valuearray/ValueArray.cs index 31332d2ef0bd75..8fee12e0280636 100644 --- a/src/tests/CoreMangLib/system/valuearray/ValueArray.cs +++ b/src/tests/CoreMangLib/system/valuearray/ValueArray.cs @@ -19,6 +19,7 @@ public static int Main(string[] args) TestGCRootsStruct(); TestListT(); TestAsField(); + TestInFixed(); if (returnVal == 100) Console.WriteLine("PASS"); @@ -26,9 +27,32 @@ public static int Main(string[] args) return returnVal; } + private unsafe static void TestInFixed() + { + Console.WriteLine(nameof(TestInFixed)); + + var arr1 = new ValueArray(); + for (int i = 0; i < arr1.Length; i++) + { + arr1[i] = i; + } + + fixed (long* p = arr1) + { + for (long* pElement = p; pElement < p + 9; pElement++) + { + *pElement = *pElement + 1; + } + } + + for (int i = 0; i < arr1.Length; i++) + { + Test(i + 1, arr1[i]); + } + } + class QuadTree { - // embedded indexable data with no indirections!! private ValueArray _nodes; // NB: intentionally returning byval here - just to test byval returning. From ff3ef33b21be29d193be05dff436207d2e05bf3d Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 19 Oct 2021 21:54:55 -0700 Subject: [PATCH 15/24] A few test fixes to match the changes. --- .../RuntimeDeterminedCanonicalizationAlgorithm.cs | 6 ++++++ src/libraries/System.Runtime/tests/System/ArrayTests.cs | 4 +++- src/libraries/System.Runtime/tests/System/Type/TypeTests.cs | 1 + 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/TypeSystem/RuntimeDetermined/RuntimeDeterminedCanonicalizationAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/RuntimeDetermined/RuntimeDeterminedCanonicalizationAlgorithm.cs index a0e7c07778374a..89e21a10843df9 100644 --- a/src/coreclr/tools/Common/TypeSystem/RuntimeDetermined/RuntimeDeterminedCanonicalizationAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/RuntimeDetermined/RuntimeDeterminedCanonicalizationAlgorithm.cs @@ -127,6 +127,12 @@ public static TypeDesc ConvertToCanon(TypeDesc typeToConvert, ref CanonicalFormK } else if (typeToConvert.IsArray) { + TypeDesc elementType = ((ArrayType)typeToConvert).ElementType; + if (elementType.IsObject) + { + return typeToConvert; + } + return context.CanonType; } else diff --git a/src/libraries/System.Runtime/tests/System/ArrayTests.cs b/src/libraries/System.Runtime/tests/System/ArrayTests.cs index b7bdc5fab35834..90bf9ff045cc20 100644 --- a/src/libraries/System.Runtime/tests/System/ArrayTests.cs +++ b/src/libraries/System.Runtime/tests/System/ArrayTests.cs @@ -2010,7 +2010,9 @@ public void CreateInstance_RankMoreThanMaxRank_ThrowsTypeLoadException(int lengt { var lengths = new int[length]; var lowerBounds = new int[length]; - Assert.Throws(() => Array.CreateInstance(typeof(int), lengths, lowerBounds)); + + // does not throw + Array.CreateInstance(typeof(int), lengths, lowerBounds); } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNonZeroLowerBoundArraySupported))] diff --git a/src/libraries/System.Runtime/tests/System/Type/TypeTests.cs b/src/libraries/System.Runtime/tests/System/Type/TypeTests.cs index a749e939d226e3..ecd962a916b940 100644 --- a/src/libraries/System.Runtime/tests/System/Type/TypeTests.cs +++ b/src/libraries/System.Runtime/tests/System/Type/TypeTests.cs @@ -319,6 +319,7 @@ public void MakeArrayType_NegativeOrZeroArrayRank_ThrowsIndexOutOfRangeException Assert.Throws(() => t.MakeArrayType(rank)); } + [ActiveIssue("HACKATHON: right now this works on a special version of CoreCLR only")] [Theory] [InlineData(33)] [InlineData(256)] From cc67b002d8dedc83cab52992065f3bf9e78b9ea5 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 20 Oct 2021 08:27:30 -0700 Subject: [PATCH 16/24] Disable ValueArray tests on mono until implemented. --- src/tests/issues.targets | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 7fec1cd073c2df..b1303c6a6a5ad7 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -846,6 +846,10 @@ + + ValueArray NYI on mono + + https://github.com/dotnet/runtime/issues/56887 From d053de05b7e9c0b64bde3cccd777ff03b1140392 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 21 Oct 2021 12:08:20 -0700 Subject: [PATCH 17/24] teach crossgen about ValueArray --- .../tools/Common/JitInterface/CorInfoImpl.cs | 11 +++++ .../Common/MetadataFieldLayoutAlgorithm.cs | 42 +++++++++++++++++++ .../Utilities/GCPointerMap.Algorithm.cs | 36 ++++++++++++---- 3 files changed, 81 insertions(+), 8 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 84ee78fb0f2432..ed0705d58a0209 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1928,6 +1928,17 @@ private uint getClassAttribsInternal(TypeDesc type) result |= CorInfoFlag.CORINFO_FLG_ABSTRACT; } + if (type is InstantiatedType it) + { + if (it.Name == "ValueArray`2" && it.Namespace == "System") + { + if (it.Instantiation[1] is ArrayType arr && arr.Rank > 1) + { + result |= CorInfoFlag.CORINFO_FLG_DONT_PROMOTE; + } + } + } + #if READYTORUN if (!_compilation.CompilationModuleGroup.VersionsWithType(type)) { diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index f445bb1a1fc70b..d5c4e0bae9ea88 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -434,6 +434,27 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType alignUpInstanceByteSize: true, out instanceByteSizeAndAlignment); + if (type is InstantiatedType it) + { + if (it.Name == "ValueArray`2" && it.Namespace == "System") + { + if (it.Instantiation[1] is ArrayType arr) + { + int repeat = arr.Rank; + + if (!instanceSizeAndAlignment.Size.IsIndeterminate) + { + instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); + } + + if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) + { + instanceByteSizeAndAlignment.Size = new LayoutInt(instanceByteSizeAndAlignment.Size.AsInt * repeat); + } + } + } + } + ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; computedLayout.FieldSize = instanceSizeAndAlignment.Size; @@ -722,6 +743,27 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, alignUpInstanceByteSize: true, out instanceByteSizeAndAlignment); + if (type is InstantiatedType it) + { + if (it.Name == "ValueArray`2" && it.Namespace == "System") + { + if (it.Instantiation[1] is ArrayType arr) + { + int repeat = arr.Rank; + + if (!instanceSizeAndAlignment.Size.IsIndeterminate) + { + instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); + } + + if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) + { + instanceByteSizeAndAlignment.Size = new LayoutInt(instanceByteSizeAndAlignment.Size.AsInt * repeat); + } + } + } + } + ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; computedLayout.FieldSize = instanceSizeAndAlignment.Size; diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs index af5938677a4b4d..9fefec8ea729c9 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs @@ -14,19 +14,32 @@ public static GCPointerMap FromInstanceLayout(DefType type) { Debug.Assert(type.ContainsGCPointers); - GCPointerMapBuilder builder = new GCPointerMapBuilder(type.InstanceByteCount.AsInt, type.Context.Target.PointerSize); - FromInstanceLayoutHelper(ref builder, type); + int pointerSize = type.Context.Target.PointerSize; + GCPointerMapBuilder builder = new GCPointerMapBuilder(type.InstanceByteCount.AsInt, pointerSize); + FromInstanceLayoutHelper(ref builder, type, pointerSize); return builder.ToGCMap(); } - private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, DefType type) + private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, DefType type, int pointerSize) { if (!type.IsValueType && type.HasBaseType) { DefType baseType = type.BaseType; GCPointerMapBuilder baseLayoutBuilder = builder.GetInnerBuilder(0, baseType.InstanceByteCount.AsInt); - FromInstanceLayoutHelper(ref baseLayoutBuilder, baseType); + FromInstanceLayoutHelper(ref baseLayoutBuilder, baseType, pointerSize); + } + + int repeat = 1; + if (type is InstantiatedType it) + { + if (it.Name == "ValueArray`2" && it.Namespace == "System") + { + if (it.Instantiation[1] is ArrayType arr) + { + repeat = arr.Rank; + } + } } foreach (FieldDesc field in type.GetFields()) @@ -37,16 +50,23 @@ private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, De TypeDesc fieldType = field.FieldType; if (fieldType.IsGCPointer) { - builder.MarkGCPointer(field.Offset.AsInt); + for (int i = 0; i < repeat; i++) + { + builder.MarkGCPointer(field.Offset.AsInt + pointerSize * i); + } } else if (fieldType.IsValueType) { var fieldDefType = (DefType)fieldType; if (fieldDefType.ContainsGCPointers) { - GCPointerMapBuilder innerBuilder = - builder.GetInnerBuilder(field.Offset.AsInt, fieldDefType.InstanceByteCount.AsInt); - FromInstanceLayoutHelper(ref innerBuilder, fieldDefType); + for (int i = 0; i < repeat; i++) + { + int fieldSize = fieldDefType.InstanceByteCount.AsInt; + GCPointerMapBuilder innerBuilder = + builder.GetInnerBuilder(field.Offset.AsInt + fieldSize * i, fieldSize); + FromInstanceLayoutHelper(ref innerBuilder, fieldDefType, pointerSize); + } } } } From f3d856965e92853aaadf9a1d107ac24781710f6f Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 21 Oct 2021 19:51:05 -0700 Subject: [PATCH 18/24] introducing IValueArray --- .../src/System/ValueArray.cs | 34 ++++++++++++++++++- .../System.Runtime/ref/System.Runtime.cs | 10 +++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs index fba939e9763765..aa428e9cd28fad 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs @@ -7,9 +7,41 @@ namespace System { + public interface IValueArray + { + /// The number of elements this ValueArray contains. + public int Length { get; } + + /// + /// Returns a reference to the first element of the value array. + /// + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + public ref T GetPinnableReference(); + + /// + /// Returns a reference to specified element of the value array. + /// + /// + /// + /// + /// Thrown when index less than 0 or index greater than or equal to Length + /// + public ref T this[int index] { get; } + + /// + /// Forms a slice out of the given value array, beginning at 'start'. + /// + /// The index at which to begin this slice. + /// + /// Thrown when the specified index is not in range (<0 or >Length). + /// + public Span Slice(int start); + } + public struct ValueArray // where R : System.Array - : IEquatable> + : IValueArray, IEquatable> { + /// The number of elements this ValueArray contains. public int Length => RankOf.Value; // For the array of Length N, runtime will add N-1 elements immediately after this one. diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 06ba5c1a6c64a1..718dc08bc14dd6 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7896,8 +7896,16 @@ public enum UriPartial Path = 2, Query = 3, } + public partial interface IValueArray + { + public int Length { get; } + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + public ref T GetPinnableReference(); + public ref T this[int index] { get; } + public Span Slice(int start); + } public partial struct ValueArray // where R : System.Array - : System.IEquatable> + : IValueArray, System.IEquatable> { public int Length { get { throw null; } } [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] From 057bd96f408aad56b67b4a7c2d047e933919fc11 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 27 Oct 2021 13:48:27 -0700 Subject: [PATCH 19/24] Rename R --> Size --- .../src/System/ValueArray.cs | 18 +++++++++--------- .../System.Runtime/ref/System.Runtime.cs | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs index aa428e9cd28fad..e0a4d198034f26 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ValueArray.cs @@ -38,11 +38,11 @@ public interface IValueArray public Span Slice(int start); } - public struct ValueArray // where R : System.Array - : IValueArray, IEquatable> + public struct ValueArray // where Size : System.Array + : IValueArray, IEquatable> { /// The number of elements this ValueArray contains. - public int Length => RankOf.Value; + public int Length => RankOf.Value; // For the array of Length N, runtime will add N-1 elements immediately after this one. private T Element0; @@ -89,10 +89,10 @@ public Span Slice(int start) public override bool Equals([NotNullWhen(true)] object? obj) { - return obj is ValueArray other && Equals(other); + return obj is ValueArray other && Equals(other); } - public bool Equals(ValueArray other) + public bool Equals(ValueArray other) { for (int i = 0; i < Length; i++) { @@ -118,18 +118,18 @@ public override int GetHashCode() } // internal helper to compute and cache the Rank of an object array. - internal static class RankOf + internal static class RankOf { public static readonly int Value = GetRank(); private static int GetRank() { - var type = typeof(R); + var type = typeof(T); if (!type.IsArray) - throw new ArgumentException("R must be an array"); + throw new ArgumentException("T must be an array"); if (type.GetElementType() != typeof(object)) - throw new ArgumentException("R must be an object array"); + throw new ArgumentException("T must be an object array"); return type.GetArrayRank(); } diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 718dc08bc14dd6..66b39e3f58a53a 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7904,8 +7904,8 @@ public partial interface IValueArray public ref T this[int index] { get; } public Span Slice(int start); } - public partial struct ValueArray // where R : System.Array - : IValueArray, System.IEquatable> + public partial struct ValueArray // where Size : System.Array + : IValueArray, System.IEquatable> { public int Length { get { throw null; } } [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] @@ -7914,7 +7914,7 @@ public partial struct ValueArray // where R : System.Array public ref T this[int index] { get { throw null; } } public Span Slice(int start) { throw null; } public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; } - public bool Equals(ValueArray other) { throw null; } + public bool Equals(ValueArray other) { throw null; } public override int GetHashCode() { throw null; } } public partial struct ValueTuple : System.Collections.IStructuralComparable, System.Collections.IStructuralEquatable, System.IComparable, System.IComparable, System.IEquatable, System.Runtime.CompilerServices.ITuple From 618ffe674b6bdf2263c4845f304c8935988d7c98 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 22 Oct 2021 00:36:11 -0700 Subject: [PATCH 20/24] Use ValueArray in couple trivial cases.(just as an example, there are more cases) --- .../src/Interop/BSD/System.Native/Interop.Sysctl.cs | 11 +---------- .../Interop/FreeBSD/Interop.Process.GetProcInfo.cs | 9 ++++++--- .../Common/src/Interop/FreeBSD/Interop.Process.cs | 13 +++++++++++-- .../Runtime/Serialization/Json/XmlJsonReader.cs | 9 ++++++--- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/libraries/Common/src/Interop/BSD/System.Native/Interop.Sysctl.cs b/src/libraries/Common/src/Interop/BSD/System.Native/Interop.Sysctl.cs index 3c98d2d828d51d..ce2508c218095a 100644 --- a/src/libraries/Common/src/Interop/BSD/System.Native/Interop.Sysctl.cs +++ b/src/libraries/Common/src/Interop/BSD/System.Native/Interop.Sysctl.cs @@ -21,16 +21,7 @@ internal static partial class Sys // This is 'raw' sysctl call, only wrapped to allocate memory if needed // caller always needs to free returned buffer using Marshal.FreeHGlobal() - - internal static unsafe void Sysctl(Span name, ref byte* value, ref int len) - { - fixed (int* ptr = &MemoryMarshal.GetReference(name)) - { - Sysctl(ptr, name.Length, ref value, ref len); - } - } - - private static unsafe void Sysctl(int* name, int name_len, ref byte* value, ref int len) + internal static unsafe void Sysctl(int* name, int name_len, ref byte* value, ref int len) { IntPtr bytesLength = (IntPtr)len; int ret = -1; diff --git a/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.GetProcInfo.cs b/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.GetProcInfo.cs index 09f7c97d302237..d0cdcaa8086f89 100644 --- a/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.GetProcInfo.cs +++ b/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.GetProcInfo.cs @@ -186,8 +186,11 @@ public unsafe struct kinfo_proc /// The number of kinfo_proc entries returned. public static unsafe kinfo_proc* GetProcInfo(int pid, bool threads, out int count) { - Span sysctlName = stackalloc int[4]; - +#if NICE_SYNTAX + int[4] sysctlName = default; +#else + ValueArray sysctlName = default; +#endif if (pid == 0) { // get all processes @@ -205,7 +208,7 @@ public unsafe struct kinfo_proc byte* pBuffer = null; int bytesLength = 0; - Interop.Sys.Sysctl(sysctlName, ref pBuffer, ref bytesLength); + Interop.Sys.Sysctl((int*)&sysctlName, sysctlName.Length, ref pBuffer, ref bytesLength); kinfo_proc* kinfo = (kinfo_proc*)pBuffer; diff --git a/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.cs b/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.cs index 3d18ead32d116d..aab2e21a5498f6 100644 --- a/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.cs +++ b/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.cs @@ -76,13 +76,22 @@ internal static unsafe int[] ListAllPids() /// The PID of the process public static unsafe string? GetProcPath(int pid) { - Span sysctlName = stackalloc int[] { CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, pid }; +#if NICE_SYNTAX + int[4] sysctlName = { CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, pid }; +#else + ValueArray sysctlName = default; + sysctlName[0] = CTL_KERN; + sysctlName[1] = KERN_PROC; + sysctlName[2] = KERN_PROC_PATHNAME; + sysctlName[3] = pid; +#endif + byte* pBuffer = null; int bytesLength = 0; try { - Interop.Sys.Sysctl(sysctlName, ref pBuffer, ref bytesLength); + Interop.Sys.Sysctl((int*)&sysctlName, sysctlName.Length, ref pBuffer, ref bytesLength); return System.Text.Encoding.UTF8.GetString(pBuffer, (int)bytesLength - 1); } finally diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/XmlJsonReader.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/XmlJsonReader.cs index a4faf4835350ce..9a40c8af80a159 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/XmlJsonReader.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/XmlJsonReader.cs @@ -276,7 +276,11 @@ internal sealed class XmlJsonReader : XmlBaseReader, IXmlJsonReaderInitializer CharType.None | CharType.FirstName | CharType.Name, // FF (?) }; private bool _buffered; - private byte[]? _charactersToSkipOnNextRead; +#if NICE_SYNTAX + private byte[2] _charactersToSkipOnNextRead; +#else + private ValueArray _charactersToSkipOnNextRead; +#endif private JsonComplexTextMode _complexTextMode = JsonComplexTextMode.None; private bool _expectingFirstElementInNonPrimitiveChild; private int _maxBytesPerRead; @@ -415,7 +419,6 @@ public override bool Read() BufferReader.SetWindow(ElementNode.BufferOffset, _maxBytesPerRead); } - Debug.Assert(_charactersToSkipOnNextRead != null); byte ch; // Skip whitespace before checking EOF @@ -1548,7 +1551,7 @@ private void ResetState() { _complexTextMode = JsonComplexTextMode.None; _expectingFirstElementInNonPrimitiveChild = false; - _charactersToSkipOnNextRead = new byte[2]; + _charactersToSkipOnNextRead = default; _scopeDepth = 0; if ((_scopes != null) && (_scopes.Length > JsonGlobals.maxScopeSize)) { From 000e29580895bf758d7d96143149fb3592b6441c Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 21 Oct 2021 14:29:06 -0700 Subject: [PATCH 21/24] Use ValueArray in TlsOverPerCoreLockedStacksArrayPool to avoid indirection. --- .../TlsOverPerCoreLockedStacksArrayPool.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs index 108cee7b4a6e39..e850a96c49ce9f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs @@ -362,7 +362,11 @@ public void Trim(int currentMilliseconds, int id, Utilities.MemoryPressure press private sealed class LockedStack { /// The arrays in the stack. - private readonly T[]?[] _arrays = new T[MaxBuffersPerArraySizePerCore][]; +#if NICE_SYNTAX + private T[]?[MaxBuffersPerArraySizePerCore] _arrays; +#else + private ValueArray _arrays; +#endif /// Number of arrays stored in . private int _count; /// Timestamp set by Trim when it sees this as 0. @@ -373,9 +377,8 @@ public bool TryPush(T[] array) { bool enqueued = false; Monitor.Enter(this); - T[]?[] arrays = _arrays; int count = _count; - if ((uint)count < (uint)arrays.Length) + if ((uint)count < (uint)_arrays.Length) { if (count == 0) { @@ -384,7 +387,7 @@ public bool TryPush(T[] array) _millisecondsTimestamp = 0; } - arrays[count] = array; + _arrays[count] = array; _count = count + 1; enqueued = true; } @@ -397,12 +400,11 @@ public bool TryPush(T[] array) { T[]? arr = null; Monitor.Enter(this); - T[]?[] arrays = _arrays; int count = _count - 1; - if ((uint)count < (uint)arrays.Length) + if ((uint)count < (uint)_arrays.Length) { - arr = arrays[count]; - arrays[count] = null; + arr = _arrays[count]; + _arrays[count] = null; _count = count; } Monitor.Exit(this); From 4040fad6fd96a6c8899b0884f66fc5859fe9952c Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 21 Oct 2021 17:40:38 -0700 Subject: [PATCH 22/24] Use ValueArray as inline storage in ValueListBuilder. --- .../Collections/Generic/ValueListBuilder.cs | 31 ++++++++++++----- .../src/System/Globalization/StringInfo.cs | 2 +- .../src/System/String.Manipulation.cs | 12 +++---- .../Text/RegularExpressions/RegexParser.cs | 17 +++++----- .../RegularExpressions/RegexPrefixAnalyzer.cs | 33 +++++++------------ .../RegularExpressions/RegexReplacement.cs | 17 +++------- .../Text/RegularExpressions/RegexWriter.cs | 18 ++-------- 7 files changed, 54 insertions(+), 76 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/ValueListBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/ValueListBuilder.cs index a82372b92b34db..f11b5597e52bf8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/ValueListBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/ValueListBuilder.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Buffers; using System.Diagnostics; using System.Runtime.CompilerServices; @@ -10,16 +11,17 @@ namespace System.Collections.Generic internal ref partial struct ValueListBuilder { private Span _span; + +#if !NETFRAMEWORK && !NETSTANDARD2_0 +#if NICE_SYNTAX + private T[64] _inlineArray; // 64 arbitrarily chosen +#else + private ValueArray _inlineArray; +#endif +#endif private T[]? _arrayFromPool; private int _pos; - public ValueListBuilder(Span initialSpan) - { - _span = initialSpan; - _arrayFromPool = null; - _pos = 0; - } - public int Length { get => _pos; @@ -69,7 +71,20 @@ public void Dispose() private void Grow() { - T[] array = ArrayPool.Shared.Rent(_span.Length * 2); +#if !NETFRAMEWORK && !NETSTANDARD2_0 + if (_span.Length == 0) + { + _span = _inlineArray.Slice(0); + return; + } +#endif + + Rent(); + } + + private void Rent() + { + T[] array = ArrayPool.Shared.Rent(Math.Max(_span.Length * 2, 64)); bool success = _span.TryCopyTo(array); Debug.Assert(success); diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/StringInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/StringInfo.cs index 1e4c69899de7c8..d7d5329757cb51 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/StringInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/StringInfo.cs @@ -211,7 +211,7 @@ public static int[] ParseCombiningCharacters(string str) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.str); } - ValueListBuilder builder = new ValueListBuilder(stackalloc int[64]); // 64 arbitrarily chosen + ValueListBuilder builder = default; ReadOnlySpan remaining = str; while (!remaining.IsEmpty) diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs index d208eb3bedf63a..3e4ea6ab5ca8e5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs @@ -15,8 +15,6 @@ namespace System { public partial class String { - private const int StackallocIntBufferSizeLimit = 128; - private static void FillStringChecked(string dest, int destPos, string src) { Debug.Assert(dest != null); @@ -1067,7 +1065,7 @@ public string Replace(string oldValue, string? newValue) newValue ??= Empty; // Track the locations of oldValue to be replaced. - var replacementIndices = new ValueListBuilder(stackalloc int[StackallocIntBufferSizeLimit]); + ValueListBuilder replacementIndices = default; if (oldValue.Length == 1) { @@ -1380,7 +1378,7 @@ private string[] SplitInternal(ReadOnlySpan separators, int count, StringS options &= ~StringSplitOptions.TrimEntries; } - var sepListBuilder = new ValueListBuilder(stackalloc int[StackallocIntBufferSizeLimit]); + ValueListBuilder sepListBuilder = default; MakeSeparatorList(separators, ref sepListBuilder); ReadOnlySpan sepList = sepListBuilder.AsSpan(); @@ -1470,8 +1468,8 @@ private string[] SplitInternal(string? separator, string?[]? separators, int cou } } - var sepListBuilder = new ValueListBuilder(stackalloc int[StackallocIntBufferSizeLimit]); - var lengthListBuilder = new ValueListBuilder(stackalloc int[StackallocIntBufferSizeLimit]); + ValueListBuilder sepListBuilder = default; + ValueListBuilder lengthListBuilder = default; MakeSeparatorList(separators!, ref sepListBuilder, ref lengthListBuilder); ReadOnlySpan sepList = sepListBuilder.AsSpan(); @@ -1495,7 +1493,7 @@ private string[] SplitInternal(string? separator, string?[]? separators, int cou private string[] SplitInternal(string separator, int count, StringSplitOptions options) { - var sepListBuilder = new ValueListBuilder(stackalloc int[StackallocIntBufferSizeLimit]); + ValueListBuilder sepListBuilder = default; MakeSeparatorList(separator, ref sepListBuilder); ReadOnlySpan sepList = sepListBuilder.AsSpan(); diff --git a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexParser.cs b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexParser.cs index 1f4a05afa47c12..19b3078146e0cd 100644 --- a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexParser.cs +++ b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexParser.cs @@ -16,7 +16,6 @@ internal ref struct RegexParser // ScanBlank() calls are just kind of duct-taped in. private const int EscapeMaxBufferSize = 256; - private const int OptionStackDefaultSize = 32; private const int MaxValueDiv10 = int.MaxValue / 10; private const int MaxValueMod10 = int.MaxValue % 10; @@ -44,12 +43,12 @@ internal ref struct RegexParser private RegexOptions _options; // NOTE: _optionsStack is ValueListBuilder to ensure that // ArrayPool.Shared, not ArrayPool.Shared, - // will be created if the stackalloc'd capacity is ever exceeded. + // will be created if the builder capacity is ever exceeded. private ValueListBuilder _optionsStack; private bool _ignoreNextParen; // flag to skip capturing a parentheses group - private RegexParser(string pattern, RegexOptions options, CultureInfo culture, Hashtable caps, int capsize, Hashtable? capnames, Span optionSpan) + private RegexParser(string pattern, RegexOptions options, CultureInfo culture, Hashtable caps, int capsize, Hashtable? capnames) { Debug.Assert(pattern != null, "Pattern must be set"); Debug.Assert(culture != null, "Culture must be set"); @@ -61,7 +60,7 @@ private RegexParser(string pattern, RegexOptions options, CultureInfo culture, H _capsize = capsize; _capnames = capnames; - _optionsStack = new ValueListBuilder(optionSpan); + _optionsStack = default; _stack = null; _group = null; _alternation = null; @@ -76,14 +75,14 @@ private RegexParser(string pattern, RegexOptions options, CultureInfo culture, H _ignoreNextParen = false; } - private RegexParser(string pattern, RegexOptions options, CultureInfo culture, Span optionSpan) - : this(pattern, options, culture, new Hashtable(), default, null, optionSpan) + private RegexParser(string pattern, RegexOptions options, CultureInfo culture) + : this(pattern, options, culture, new Hashtable(), default, null) { } public static RegexTree Parse(string pattern, RegexOptions options, CultureInfo culture) { - var parser = new RegexParser(pattern, options, culture, stackalloc int[OptionStackDefaultSize]); + var parser = new RegexParser(pattern, options, culture); parser.CountCaptures(); parser.Reset(options); @@ -102,7 +101,7 @@ public static RegexTree Parse(string pattern, RegexOptions options, CultureInfo public static RegexReplacement ParseReplacement(string pattern, RegexOptions options, Hashtable caps, int capsize, Hashtable capnames) { CultureInfo culture = (options & RegexOptions.CultureInvariant) != 0 ? CultureInfo.InvariantCulture : CultureInfo.CurrentCulture; - var parser = new RegexParser(pattern, options, culture, caps, capsize, capnames, stackalloc int[OptionStackDefaultSize]); + var parser = new RegexParser(pattern, options, culture, caps, capsize, capnames); RegexNode root = parser.ScanReplacement(); var regexReplacement = new RegexReplacement(pattern, root, caps); @@ -194,7 +193,7 @@ public static string Unescape(string input) private static string UnescapeImpl(string input, int i) { - var parser = new RegexParser(input, RegexOptions.None, CultureInfo.InvariantCulture, stackalloc int[OptionStackDefaultSize]); + var parser = new RegexParser(input, RegexOptions.None, CultureInfo.InvariantCulture); // In the worst case the escaped string has the same length. // For small inputs we use stack allocation. diff --git a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexPrefixAnalyzer.cs b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexPrefixAnalyzer.cs index 96a709b2338d43..ec3b171d9765ed 100644 --- a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexPrefixAnalyzer.cs +++ b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexPrefixAnalyzer.cs @@ -11,7 +11,6 @@ namespace System.Text.RegularExpressions /// Detects various forms of prefixes in the regular expression that can help FindFirstChars optimize its search. internal ref struct RegexPrefixAnalyzer { - private const int StackBufferSize = 32; private const int BeforeChild = 64; private const int AfterChild = 128; @@ -25,21 +24,12 @@ internal ref struct RegexPrefixAnalyzer public const int Boundary = 0x0040; public const int ECMABoundary = 0x0080; - private readonly List _fcStack; + private ValueListBuilder _fcStack; private ValueListBuilder _intStack; // must not be readonly private bool _skipAllChildren; // don't process any more children at the current level private bool _skipchild; // don't process the current child. private bool _failed; - private RegexPrefixAnalyzer(Span intStack) - { - _fcStack = new List(StackBufferSize); - _intStack = new ValueListBuilder(intStack); - _failed = false; - _skipchild = false; - _skipAllChildren = false; - } - /// Computes the leading substring in . /// It's quite trivial and gives up easily, in which case an empty string is returned. public static (string Prefix, bool CaseInsensitive) ComputeLeadingSubstring(RegexTree tree) @@ -122,7 +112,7 @@ public static (string Prefix, bool CaseInsensitive) ComputeLeadingSubstring(Rege /// true if a character class could be computed; otherwise, false. public static (string CharClass, bool CaseInsensitive)[]? ComputeFirstCharClass(RegexTree tree) { - var s = new RegexPrefixAnalyzer(stackalloc int[StackBufferSize]); + RegexPrefixAnalyzer s = default; RegexFC? fc = s.RegexFCFromRegexTree(tree); s.Dispose(); @@ -392,23 +382,22 @@ public static string AnchorDescription(int anchors) /// /// We also use a stack of RegexFC objects. /// - private void PushFC(RegexFC fc) => _fcStack.Add(fc); + private void PushFC(RegexFC fc) => _fcStack.Append(fc); - private bool FCIsEmpty() => _fcStack.Count == 0; + private bool FCIsEmpty() => _fcStack.Length == 0; - private RegexFC PopFC() - { - RegexFC item = TopFC(); - _fcStack.RemoveAt(_fcStack.Count - 1); - return item; - } + private RegexFC PopFC() => _fcStack.Pop(); - private RegexFC TopFC() => _fcStack[_fcStack.Count - 1]; + private RegexFC TopFC() => _fcStack[_fcStack.Length - 1]; /// /// Return rented buffers. /// - public void Dispose() => _intStack.Dispose(); + public void Dispose() + { + _intStack.Dispose(); + _fcStack.Dispose(); + } /// /// The main FC computation. It does a shortcutted depth-first walk diff --git a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexReplacement.cs b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexReplacement.cs index 814f05e5aeb119..b92341eae7da4d 100644 --- a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexReplacement.cs +++ b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexReplacement.cs @@ -38,9 +38,9 @@ public RegexReplacement(string rep, RegexNode concat, Hashtable _caps) Span vsbStack = stackalloc char[256]; var vsb = new ValueStringBuilder(vsbStack); - FourStackStrings stackStrings = default; - var strings = new ValueListBuilder(MemoryMarshal.CreateSpan(ref stackStrings.Item1!, 4)); - var rules = new ValueListBuilder(stackalloc int[64]); + + ValueListBuilder strings = default; + ValueListBuilder rules = default; int childCount = concat.ChildCount(); for (int i = 0; i < childCount; i++) @@ -89,19 +89,10 @@ public RegexReplacement(string rep, RegexNode concat, Hashtable _caps) _strings = strings.AsSpan().ToArray(); _rules = rules.AsSpan().ToArray(); + strings.Dispose(); rules.Dispose(); } - /// Simple struct of four strings. - [StructLayout(LayoutKind.Sequential)] - private struct FourStackStrings // used to do the equivalent of: Span strings = stackalloc string[4]; - { - public string Item1; - public string Item2; - public string Item3; - public string Item4; - } - /// /// Either returns a weakly cached RegexReplacement helper or creates one and caches it. /// diff --git a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexWriter.cs b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexWriter.cs index 2154947cfaa8d5..e944ad822b8c77 100644 --- a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexWriter.cs +++ b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexWriter.cs @@ -14,33 +14,19 @@ internal ref struct RegexWriter private const int BeforeChild = 64; private const int AfterChild = 128; - // Distribution of common patterns indicates an average amount of 56 op codes. Since we're stackalloc'ing, - // we can afford to make it a bit higher and a power of two for simplicity. - private const int EmittedSize = 64; - private const int IntStackSize = 32; - - private readonly Dictionary _stringTable; + private Dictionary _stringTable; private ValueListBuilder _emitted; private ValueListBuilder _intStack; private Hashtable? _caps; private int _trackCount; - private RegexWriter(Span emittedSpan, Span intStackSpan) - { - _emitted = new ValueListBuilder(emittedSpan); - _intStack = new ValueListBuilder(intStackSpan); - _stringTable = new Dictionary(); - _caps = null; - _trackCount = 0; - } - /// /// This is the only function that should be called from outside. /// It takes a RegexTree and creates a corresponding RegexCode. /// public static RegexCode Write(RegexTree tree) { - var writer = new RegexWriter(stackalloc int[EmittedSize], stackalloc int[IntStackSize]); + var writer = new RegexWriter() { _stringTable = new Dictionary() }; RegexCode code = writer.RegexCodeFromRegexTree(tree); writer.Dispose(); From 88c2c84f97d5ae776a095c2a961f9194716c6370 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 21 Oct 2021 23:37:39 -0700 Subject: [PATCH 23/24] Use ValueArray as inline storage in MethodCallExpression nodes. --- .../src/System.Linq.Expressions.csproj | 3 + .../Linq/Expressions/BlockExpression.cs | 2 + .../Linq/Expressions/DynamicExpression.cs | 2 + .../Linq/Expressions/InvocationExpression.cs | 2 + .../Linq/Expressions/MethodCallExpression.cs | 396 ++++-------------- .../tests/Call/CallFactoryTests.cs | 9 +- 6 files changed, 93 insertions(+), 321 deletions(-) diff --git a/src/libraries/System.Linq.Expressions/src/System.Linq.Expressions.csproj b/src/libraries/System.Linq.Expressions/src/System.Linq.Expressions.csproj index 0a9d7eba08b66c..2c442401e67edc 100644 --- a/src/libraries/System.Linq.Expressions/src/System.Linq.Expressions.csproj +++ b/src/libraries/System.Linq.Expressions/src/System.Linq.Expressions.csproj @@ -217,4 +217,7 @@ + + + diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/BlockExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/BlockExpression.cs index 4d6b22ec6fe734..965111fc9c3650 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/BlockExpression.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/BlockExpression.cs @@ -312,6 +312,8 @@ internal override BlockExpression Rewrite(ReadOnlyCollection : MethodCallExpression, IArgumentProvider + where TArr : struct, IValueArray { - private object _arg0; // storage for the 1st argument or a read-only collection. See IArgumentProvider + // NOTE: _args[0] for historical reasons may store a read-only collection. See IArgumentProvider + // This is why we need to use "object" elements. That is ok. - public MethodCallExpression1(MethodInfo method, Expression arg0) - : base(method) - { - _arg0 = arg0; - } - - public override Expression GetArgument(int index) => - index switch - { - 0 => ExpressionUtils.ReturnObject(_arg0), - _ => throw new ArgumentOutOfRangeException(nameof(index)), - }; - - public override int ArgumentCount => 1; - - internal override ReadOnlyCollection GetOrMakeArguments() - { - return ExpressionUtils.ReturnReadOnly(this, ref _arg0); - } - - internal override bool SameArguments(ICollection? arguments) - { - if (arguments != null && arguments.Count == 1) - { - using (IEnumerator en = arguments.GetEnumerator()) - { - en.MoveNext(); - return en.Current == ExpressionUtils.ReturnObject(_arg0); - } - } - - return false; - } - - internal override MethodCallExpression Rewrite(Expression? instance, IReadOnlyList? args) - { - Debug.Assert(instance == null); - Debug.Assert(args == null || args.Count == 1); - - if (args != null) - { - return Expression.Call(Method, args[0]); - } - - return Expression.Call(Method, ExpressionUtils.ReturnObject(_arg0)); - } - } + // Use ValueArray as inline storage for arguments. + private TArr _args; - internal sealed class MethodCallExpression2 : MethodCallExpression, IArgumentProvider - { - private object _arg0; // storage for the 1st argument or a read-only collection. See IArgumentProvider - private readonly Expression _arg1; // storage for the 2nd arg - - public MethodCallExpression2(MethodInfo method, Expression arg0, Expression arg1) + public MethodCallExpressionInlineArgs(MethodInfo method, TArr args) : base(method) { - _arg0 = arg0; - _arg1 = arg1; + _args = args; } public override Expression GetArgument(int index) { - return index switch + if ((uint)index < (uint)_args.Length) { - 0 => ExpressionUtils.ReturnObject(_arg0), - 1 => _arg1, - _ => throw new ArgumentOutOfRangeException(nameof(index)), - }; - } - - public override int ArgumentCount => 2; - - internal override bool SameArguments(ICollection? arguments) - { - if (arguments != null && arguments.Count == 2) - { - if (_arg0 is ReadOnlyCollection alreadyCollection) - { - return ExpressionUtils.SameElements(arguments, alreadyCollection); - } - - using (IEnumerator en = arguments.GetEnumerator()) - { - en.MoveNext(); - if (en.Current == _arg0) - { - en.MoveNext(); - return en.Current == _arg1; - } - } + return index == 0 ? + ExpressionUtils.ReturnObject(_args[0]) : + Unsafe.As(_args[index]); } - return false; - } - - internal override ReadOnlyCollection GetOrMakeArguments() - { - return ExpressionUtils.ReturnReadOnly(this, ref _arg0); - } - - internal override MethodCallExpression Rewrite(Expression? instance, IReadOnlyList? args) - { - Debug.Assert(instance == null); - Debug.Assert(args == null || args.Count == 2); - - if (args != null) - { - return Expression.Call(Method, args[0], args[1]); - } - return Expression.Call(Method, ExpressionUtils.ReturnObject(_arg0), _arg1); - } - } - - internal sealed class MethodCallExpression3 : MethodCallExpression, IArgumentProvider - { - private object _arg0; // storage for the 1st argument or a read-only collection. See IArgumentProvider - private readonly Expression _arg1, _arg2; // storage for the 2nd - 3rd args. - - public MethodCallExpression3(MethodInfo method, Expression arg0, Expression arg1, Expression arg2) - : base(method) - { - _arg0 = arg0; - _arg1 = arg1; - _arg2 = arg2; - } - - public override Expression GetArgument(int index) - { - return index switch - { - 0 => ExpressionUtils.ReturnObject(_arg0), - 1 => _arg1, - 2 => _arg2, - _ => throw new ArgumentOutOfRangeException(nameof(index)), - }; + throw new ArgumentOutOfRangeException(nameof(index)); } - public override int ArgumentCount => 3; + public override int ArgumentCount => _args.Length; internal override bool SameArguments(ICollection? arguments) { - if (arguments != null && arguments.Count == 3) + if (arguments != null && arguments.Count == _args.Length) { - if (_arg0 is ReadOnlyCollection alreadyCollection) + if (_args[0] is ReadOnlyCollection alreadyCollection) { return ExpressionUtils.SameElements(arguments, alreadyCollection); } using (IEnumerator en = arguments.GetEnumerator()) { - en.MoveNext(); - if (en.Current == _arg0) + for (int i = 0; i < _args.Length; i++) { en.MoveNext(); - if (en.Current == _arg1) + if (_args[i] != en.Current) { - en.MoveNext(); - return en.Current == _arg2; + return false; } } - } - } - - return false; - } - - internal override ReadOnlyCollection GetOrMakeArguments() - { - return ExpressionUtils.ReturnReadOnly(this, ref _arg0); - } - internal override MethodCallExpression Rewrite(Expression? instance, IReadOnlyList? args) - { - Debug.Assert(instance == null); - Debug.Assert(args == null || args.Count == 3); - - if (args != null) - { - return Expression.Call(Method, args[0], args[1], args[2]); - } - return Expression.Call(Method, ExpressionUtils.ReturnObject(_arg0), _arg1, _arg2); - } - } - - internal sealed class MethodCallExpression4 : MethodCallExpression, IArgumentProvider - { - private object _arg0; // storage for the 1st argument or a read-only collection. See IArgumentProvider - private readonly Expression _arg1, _arg2, _arg3; // storage for the 2nd - 4th args. - - public MethodCallExpression4(MethodInfo method, Expression arg0, Expression arg1, Expression arg2, Expression arg3) - : base(method) - { - _arg0 = arg0; - _arg1 = arg1; - _arg2 = arg2; - _arg3 = arg3; - } - - public override Expression GetArgument(int index) - { - return index switch - { - 0 => ExpressionUtils.ReturnObject(_arg0), - 1 => _arg1, - 2 => _arg2, - 3 => _arg3, - _ => throw new ArgumentOutOfRangeException(nameof(index)), - }; - } - - public override int ArgumentCount => 4; - - internal override bool SameArguments(ICollection? arguments) - { - if (arguments != null && arguments.Count == 4) - { - if (_arg0 is ReadOnlyCollection alreadyCollection) - { - return ExpressionUtils.SameElements(arguments, alreadyCollection); - } - - using (IEnumerator en = arguments.GetEnumerator()) - { - en.MoveNext(); - if (en.Current == _arg0) - { - en.MoveNext(); - if (en.Current == _arg1) - { - en.MoveNext(); - if (en.Current == _arg2) - { - en.MoveNext(); - return en.Current == _arg3; - } - } - } + return true; } } @@ -506,103 +316,23 @@ internal override bool SameArguments(ICollection? arguments) internal override ReadOnlyCollection GetOrMakeArguments() { - return ExpressionUtils.ReturnReadOnly(this, ref _arg0); + return ExpressionUtils.ReturnReadOnly(this, ref _args[0]); } internal override MethodCallExpression Rewrite(Expression? instance, IReadOnlyList? args) { Debug.Assert(instance == null); - Debug.Assert(args == null || args.Count == 4); + Debug.Assert(args == null || args.Count == _args.Length); if (args != null) { - return Expression.Call(Method, args[0], args[1], args[2], args[3]); - } - return Expression.Call(Method, ExpressionUtils.ReturnObject(_arg0), _arg1, _arg2, _arg3); - } - } - - internal sealed class MethodCallExpression5 : MethodCallExpression, IArgumentProvider - { - private object _arg0; // storage for the 1st argument or a read-only collection. See IArgumentProvider - private readonly Expression _arg1, _arg2, _arg3, _arg4; // storage for the 2nd - 5th args. - - public MethodCallExpression5(MethodInfo method, Expression arg0, Expression arg1, Expression arg2, Expression arg3, Expression arg4) - : base(method) - { - _arg0 = arg0; - _arg1 = arg1; - _arg2 = arg2; - _arg3 = arg3; - _arg4 = arg4; - } - - public override Expression GetArgument(int index) - { - return index switch - { - 0 => ExpressionUtils.ReturnObject(_arg0), - 1 => _arg1, - 2 => _arg2, - 3 => _arg3, - 4 => _arg4, - _ => throw new ArgumentOutOfRangeException(nameof(index)), - }; - } - - public override int ArgumentCount => 5; - - internal override bool SameArguments(ICollection? arguments) - { - if (arguments != null && arguments.Count == 5) - { - if (_arg0 is ReadOnlyCollection alreadyCollection) - { - return ExpressionUtils.SameElements(arguments, alreadyCollection); - } - - using (IEnumerator en = arguments.GetEnumerator()) - { - en.MoveNext(); - if (en.Current == _arg0) - { - en.MoveNext(); - if (en.Current == _arg1) - { - en.MoveNext(); - if (en.Current == _arg2) - { - en.MoveNext(); - if (en.Current == _arg3) - { - en.MoveNext(); - return en.Current == _arg4; - } - } - } - } - } + return Expression.Call(Method, args); } - return false; - } + TArr newArgs = _args; + newArgs[0] = ExpressionUtils.ReturnObject(_args[0]); - internal override ReadOnlyCollection GetOrMakeArguments() - { - return ExpressionUtils.ReturnReadOnly(this, ref _arg0); - } - - internal override MethodCallExpression Rewrite(Expression? instance, IReadOnlyList? args) - { - Debug.Assert(instance == null); - Debug.Assert(args == null || args.Count == 5); - - if (args != null) - { - return Expression.Call(Method, args[0], args[1], args[2], args[3], args[4]); - } - - return Expression.Call(Method, ExpressionUtils.ReturnObject(_arg0), _arg1, _arg2, _arg3, _arg4); + return new MethodCallExpressionInlineArgs(Method, newArgs); } } @@ -755,6 +485,8 @@ internal override MethodCallExpression Rewrite(Expression instance, IReadOnlyLis } } + // TODO: the whole family of classes can be simplified by using inline storage. + internal sealed class InstanceMethodCallExpression3 : InstanceMethodCallExpression, IArgumentProvider { private object _arg0; // storage for the 1st argument or a read-only collection. See IArgumentProvider @@ -865,9 +597,16 @@ public static MethodCallExpression Call(MethodInfo method, Expression arg0) ValidateArgumentCount(method, ExpressionType.Call, 1, pis); - arg0 = ValidateOneArgument(method, ExpressionType.Call, arg0, pis[0], nameof(method), nameof(arg0)); + ValueArray args = default; + args[0] = ValidateOneArgument(method, ExpressionType.Call, arg0, pis[0], nameof(method), nameof(arg0)); + + return CreateWithInlineArgs(method, args); + } - return new MethodCallExpression1(method, arg0); + private static MethodCallExpression CreateWithInlineArgs(MethodInfo method, TArr args) + where TArr : struct, IValueArray + { + return new MethodCallExpressionInlineArgs(method, args); } /// Creates a that represents a call to a static method that takes two arguments. @@ -887,10 +626,11 @@ public static MethodCallExpression Call(MethodInfo method, Expression arg0, Expr ValidateArgumentCount(method, ExpressionType.Call, 2, pis); - arg0 = ValidateOneArgument(method, ExpressionType.Call, arg0, pis[0], nameof(method), nameof(arg0)); - arg1 = ValidateOneArgument(method, ExpressionType.Call, arg1, pis[1], nameof(method), nameof(arg1)); + ValueArray args = default; + args[0] = ValidateOneArgument(method, ExpressionType.Call, arg0, pis[0], nameof(method), nameof(arg0)); + args[1] = ValidateOneArgument(method, ExpressionType.Call, arg1, pis[1], nameof(method), nameof(arg1)); - return new MethodCallExpression2(method, arg0, arg1); + return CreateWithInlineArgs(method, args); } /// Creates a that represents a call to a static method that takes three arguments. @@ -912,11 +652,12 @@ public static MethodCallExpression Call(MethodInfo method, Expression arg0, Expr ValidateArgumentCount(method, ExpressionType.Call, 3, pis); - arg0 = ValidateOneArgument(method, ExpressionType.Call, arg0, pis[0], nameof(method), nameof(arg0)); - arg1 = ValidateOneArgument(method, ExpressionType.Call, arg1, pis[1], nameof(method), nameof(arg1)); - arg2 = ValidateOneArgument(method, ExpressionType.Call, arg2, pis[2], nameof(method), nameof(arg2)); + ValueArray args = default; + args[0] = ValidateOneArgument(method, ExpressionType.Call, arg0, pis[0], nameof(method), nameof(arg0)); + args[1] = ValidateOneArgument(method, ExpressionType.Call, arg1, pis[1], nameof(method), nameof(arg1)); + args[2] = ValidateOneArgument(method, ExpressionType.Call, arg2, pis[2], nameof(method), nameof(arg2)); - return new MethodCallExpression3(method, arg0, arg1, arg2); + return CreateWithInlineArgs(method, args); } /// Creates a that represents a call to a static method that takes four arguments. @@ -940,12 +681,13 @@ public static MethodCallExpression Call(MethodInfo method, Expression arg0, Expr ValidateArgumentCount(method, ExpressionType.Call, 4, pis); - arg0 = ValidateOneArgument(method, ExpressionType.Call, arg0, pis[0], nameof(method), nameof(arg0)); - arg1 = ValidateOneArgument(method, ExpressionType.Call, arg1, pis[1], nameof(method), nameof(arg1)); - arg2 = ValidateOneArgument(method, ExpressionType.Call, arg2, pis[2], nameof(method), nameof(arg2)); - arg3 = ValidateOneArgument(method, ExpressionType.Call, arg3, pis[3], nameof(method), nameof(arg3)); + ValueArray args = default; + args[0] = ValidateOneArgument(method, ExpressionType.Call, arg0, pis[0], nameof(method), nameof(arg0)); + args[1] = ValidateOneArgument(method, ExpressionType.Call, arg1, pis[1], nameof(method), nameof(arg1)); + args[2] = ValidateOneArgument(method, ExpressionType.Call, arg2, pis[2], nameof(method), nameof(arg2)); + args[3] = ValidateOneArgument(method, ExpressionType.Call, arg3, pis[3], nameof(method), nameof(arg3)); - return new MethodCallExpression4(method, arg0, arg1, arg2, arg3); + return CreateWithInlineArgs(method, args); } /// Creates a that represents a call to a static method that takes five arguments. @@ -972,13 +714,14 @@ public static MethodCallExpression Call(MethodInfo method, Expression arg0, Expr ValidateArgumentCount(method, ExpressionType.Call, 5, pis); - arg0 = ValidateOneArgument(method, ExpressionType.Call, arg0, pis[0], nameof(method), nameof(arg0)); - arg1 = ValidateOneArgument(method, ExpressionType.Call, arg1, pis[1], nameof(method), nameof(arg1)); - arg2 = ValidateOneArgument(method, ExpressionType.Call, arg2, pis[2], nameof(method), nameof(arg2)); - arg3 = ValidateOneArgument(method, ExpressionType.Call, arg3, pis[3], nameof(method), nameof(arg3)); - arg4 = ValidateOneArgument(method, ExpressionType.Call, arg4, pis[4], nameof(method), nameof(arg4)); + ValueArray args = default; + args[0] = ValidateOneArgument(method, ExpressionType.Call, arg0, pis[0], nameof(method), nameof(arg0)); + args[1] = ValidateOneArgument(method, ExpressionType.Call, arg1, pis[1], nameof(method), nameof(arg1)); + args[2] = ValidateOneArgument(method, ExpressionType.Call, arg2, pis[2], nameof(method), nameof(arg2)); + args[3] = ValidateOneArgument(method, ExpressionType.Call, arg3, pis[3], nameof(method), nameof(arg3)); + args[4] = ValidateOneArgument(method, ExpressionType.Call, arg4, pis[4], nameof(method), nameof(arg4)); - return new MethodCallExpression5(method, arg0, arg1, arg2, arg3, arg4); + return CreateWithInlineArgs(method, args); } /// @@ -1062,7 +805,10 @@ internal static MethodCallExpression Call(Expression? instance, MethodInfo metho return new InstanceMethodCallExpression1(method, instance, arg0); } - return new MethodCallExpression1(method, arg0); + ValueArray args = default; + args[0] = arg0; + + return CreateWithInlineArgs(method, args); } /// @@ -1091,7 +837,11 @@ public static MethodCallExpression Call(Expression? instance, MethodInfo method, return new InstanceMethodCallExpression2(method, instance, arg0, arg1); } - return new MethodCallExpression2(method, arg0, arg1); + ValueArray args = default; + args[0] = arg0; + args[1] = arg1; + + return CreateWithInlineArgs(method, args); } /// @@ -1122,7 +872,13 @@ public static MethodCallExpression Call(Expression? instance, MethodInfo method, { return new InstanceMethodCallExpression3(method, instance, arg0, arg1, arg2); } - return new MethodCallExpression3(method, arg0, arg1, arg2); + + ValueArray args = default; + args[0] = arg0; + args[1] = arg1; + args[2] = arg2; + + return CreateWithInlineArgs(method, args); } /// Creates a that represents a call to an instance method by calling the appropriate factory method. diff --git a/src/libraries/System.Linq.Expressions/tests/Call/CallFactoryTests.cs b/src/libraries/System.Linq.Expressions/tests/Call/CallFactoryTests.cs index a81817a8450bd1..bc68675363e3e4 100644 --- a/src/libraries/System.Linq.Expressions/tests/Call/CallFactoryTests.cs +++ b/src/libraries/System.Linq.Expressions/tests/Call/CallFactoryTests.cs @@ -306,7 +306,14 @@ private static void AssertCallIsOptimized(MethodCallExpression expr, Expression private static void AssertStaticMethodCall(int n, object obj) { - AssertTypeName("MethodCallExpression" + n, obj); + if (n == 0) + { + AssertTypeName("MethodCallExpression0", obj); + } + else + { + AssertTypeName("MethodCallExpressionInlineArgs`1", obj); + } } private static void AssertInstanceMethodCall(int n, object obj) From b85d94a39d24895bab78d2d8ee4d1e207ced9367 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 22 Oct 2021 14:05:27 -0700 Subject: [PATCH 24/24] Use ValueArray in ParamsArray. --- .../src/System/IO/StreamWriter.cs | 18 ++-- .../src/System/ParamsArray.cs | 99 +++++++++---------- .../src/System/String.Manipulation.cs | 18 ++-- .../src/System/Text/StringBuilder.cs | 18 ++-- .../Text/ValueStringBuilder.AppendFormat.cs | 18 ++-- 5 files changed, 84 insertions(+), 87 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/StreamWriter.cs b/src/libraries/System.Private.CoreLib/src/System/IO/StreamWriter.cs index 7ba362604d01c7..ccb44424728a53 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/StreamWriter.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/StreamWriter.cs @@ -517,7 +517,7 @@ public override void WriteLine(ReadOnlySpan buffer) } } - private void WriteFormatHelper(string format, ParamsArray args, bool appendNewLine) + private void WriteFormatHelper(string format, ParamsArray args, bool appendNewLine) where TArr : IValueArray { StringBuilder sb = StringBuilderCache.Acquire((format?.Length ?? 0) + args.Length * 8) @@ -542,7 +542,7 @@ public override void Write(string format, object? arg0) { if (GetType() == typeof(StreamWriter)) { - WriteFormatHelper(format, new ParamsArray(arg0), appendNewLine: false); + WriteFormatHelper(format, ParamsArray.Create(arg0), appendNewLine: false); } else { @@ -554,7 +554,7 @@ public override void Write(string format, object? arg0, object? arg1) { if (GetType() == typeof(StreamWriter)) { - WriteFormatHelper(format, new ParamsArray(arg0, arg1), appendNewLine: false); + WriteFormatHelper(format, ParamsArray.Create(arg0, arg1), appendNewLine: false); } else { @@ -566,7 +566,7 @@ public override void Write(string format, object? arg0, object? arg1, object? ar { if (GetType() == typeof(StreamWriter)) { - WriteFormatHelper(format, new ParamsArray(arg0, arg1, arg2), appendNewLine: false); + WriteFormatHelper(format, ParamsArray.Create(arg0, arg1, arg2), appendNewLine: false); } else { @@ -582,7 +582,7 @@ public override void Write(string format, params object?[] arg) { throw new ArgumentNullException((format == null) ? nameof(format) : nameof(arg)); // same as base logic } - WriteFormatHelper(format, new ParamsArray(arg), appendNewLine: false); + WriteFormatHelper(format, ParamsArray.Create(arg), appendNewLine: false); } else { @@ -594,7 +594,7 @@ public override void WriteLine(string format, object? arg0) { if (GetType() == typeof(StreamWriter)) { - WriteFormatHelper(format, new ParamsArray(arg0), appendNewLine: true); + WriteFormatHelper(format, ParamsArray.Create(arg0), appendNewLine: true); } else { @@ -606,7 +606,7 @@ public override void WriteLine(string format, object? arg0, object? arg1) { if (GetType() == typeof(StreamWriter)) { - WriteFormatHelper(format, new ParamsArray(arg0, arg1), appendNewLine: true); + WriteFormatHelper(format, ParamsArray.Create(arg0, arg1), appendNewLine: true); } else { @@ -618,7 +618,7 @@ public override void WriteLine(string format, object? arg0, object? arg1, object { if (GetType() == typeof(StreamWriter)) { - WriteFormatHelper(format, new ParamsArray(arg0, arg1, arg2), appendNewLine: true); + WriteFormatHelper(format, ParamsArray.Create(arg0, arg1, arg2), appendNewLine: true); } else { @@ -634,7 +634,7 @@ public override void WriteLine(string format, params object?[] arg) { throw new ArgumentNullException(nameof(arg)); } - WriteFormatHelper(format, new ParamsArray(arg), appendNewLine: true); + WriteFormatHelper(format, ParamsArray.Create(arg), appendNewLine: true); } else { diff --git a/src/libraries/System.Private.CoreLib/src/System/ParamsArray.cs b/src/libraries/System.Private.CoreLib/src/System/ParamsArray.cs index a995fe70be9af8..e88f243985591d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ParamsArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ParamsArray.cs @@ -3,73 +3,70 @@ namespace System { - internal readonly struct ParamsArray + internal readonly struct ParamsArray where TArr : IValueArray { - // Sentinel fixed-length arrays eliminate the need for a "count" field keeping this - // struct down to just 4 fields. These are only used for their "Length" property, - // that is, their elements are never set or referenced. - private static readonly object?[] s_oneArgArray = new object?[1]; - private static readonly object?[] s_twoArgArray = new object?[2]; - private static readonly object?[] s_threeArgArray = new object?[3]; + // NOTE: arguments in an array form are stored in element #0 of object?[MaxInlineArgs + 1]. + // alternatively we could use object?[2] with element #0 storing a sentinel object and #2 the actual array + // that will take less space, but sentinel check is an actual compare vs. length compare that can be done at JIT time. + // This is a size/cycles tradeoff, which I did not have time to measure, but either way would work. + private const int MaxInlineArgs = 3; - private readonly object? _arg0; - private readonly object? _arg1; - private readonly object? _arg2; + private readonly TArr _args; - // After construction, the first three elements of this array will never be accessed - // because the indexer will retrieve those values from arg0, arg1, and arg2. - private readonly object?[] _args; - - public ParamsArray(object? arg0) + internal ParamsArray(TArr args) { - _arg0 = arg0; - _arg1 = null; - _arg2 = null; - - // Always assign this.args to make use of its "Length" property - _args = s_oneArgArray; + _args = args; } - public ParamsArray(object? arg0, object? arg1) - { - _arg0 = arg0; - _arg1 = arg1; - _arg2 = null; + // NB: _args.Length is a JIT-time constant. + public int Length => _args.Length > MaxInlineArgs ? + ((object?[])_args[0]!).Length : + _args.Length; - // Always assign this.args to make use of its "Length" property - _args = s_twoArgArray; - } + public object? this[int index] => _args.Length > MaxInlineArgs ? + ((object?[])_args[0]!)[index] : + _args[index]; + } - public ParamsArray(object? arg0, object? arg1, object? arg2) - { - _arg0 = arg0; - _arg1 = arg1; - _arg2 = arg2; + internal static class ParamsArray + { + private static ParamsArray Create(TArr args) where TArr : IValueArray + => new ParamsArray(args); - // Always assign this.args to make use of its "Length" property - _args = s_threeArgArray; + public static ParamsArray> Create(object? arg0) + { + ValueArray args = default; + args[0] = arg0; + return Create(args); } - public ParamsArray(object?[] args) + public static ParamsArray> Create(object? arg0, object? arg1) { - int len = args.Length; - _arg0 = len > 0 ? args[0] : null; - _arg1 = len > 1 ? args[1] : null; - _arg2 = len > 2 ? args[2] : null; - _args = args; + ValueArray args = default; + args[0] = arg0; + args[1] = arg1; + return Create(args); } - public int Length => _args.Length; - - public object? this[int index] => index == 0 ? _arg0 : GetAtSlow(index); +#if NICE_SYNTAX + public static ParamsArray Create(object? arg0, object? arg1, object? arg2) + => Create(new object?[3] { arg0, arg1, arg2 }); +#else + public static ParamsArray> Create(object? arg0, object? arg1, object? arg2) + { + ValueArray args = default; + args[0] = arg0; + args[1] = arg1; + args[2] = arg2; + return Create(args); + } +#endif - private object? GetAtSlow(int index) + public static ParamsArray> Create(object?[] args) { - if (index == 1) - return _arg1; - if (index == 2) - return _arg2; - return _args[index]; + ValueArray argsArr = default; + argsArr[0] = args; + return Create(argsArr); } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs index 3e4ea6ab5ca8e5..c38b2314a666d0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs @@ -439,17 +439,17 @@ public static string Concat(params string?[] values) public static string Format(string format, object? arg0) { - return FormatHelper(null, format, new ParamsArray(arg0)); + return FormatHelper(null, format, ParamsArray.Create(arg0)); } public static string Format(string format, object? arg0, object? arg1) { - return FormatHelper(null, format, new ParamsArray(arg0, arg1)); + return FormatHelper(null, format, ParamsArray.Create(arg0, arg1)); } public static string Format(string format, object? arg0, object? arg1, object? arg2) { - return FormatHelper(null, format, new ParamsArray(arg0, arg1, arg2)); + return FormatHelper(null, format, ParamsArray.Create(arg0, arg1, arg2)); } public static string Format(string format, params object?[] args) @@ -461,22 +461,22 @@ public static string Format(string format, params object?[] args) throw new ArgumentNullException((format == null) ? nameof(format) : nameof(args)); } - return FormatHelper(null, format, new ParamsArray(args)); + return FormatHelper(null, format, ParamsArray.Create(args)); } public static string Format(IFormatProvider? provider, string format, object? arg0) { - return FormatHelper(provider, format, new ParamsArray(arg0)); + return FormatHelper(provider, format, ParamsArray.Create(arg0)); } public static string Format(IFormatProvider? provider, string format, object? arg0, object? arg1) { - return FormatHelper(provider, format, new ParamsArray(arg0, arg1)); + return FormatHelper(provider, format, ParamsArray.Create(arg0, arg1)); } public static string Format(IFormatProvider? provider, string format, object? arg0, object? arg1, object? arg2) { - return FormatHelper(provider, format, new ParamsArray(arg0, arg1, arg2)); + return FormatHelper(provider, format, ParamsArray.Create(arg0, arg1, arg2)); } public static string Format(IFormatProvider? provider, string format, params object?[] args) @@ -488,10 +488,10 @@ public static string Format(IFormatProvider? provider, string format, params obj throw new ArgumentNullException((format == null) ? nameof(format) : nameof(args)); } - return FormatHelper(provider, format, new ParamsArray(args)); + return FormatHelper(provider, format, ParamsArray.Create(args)); } - private static string FormatHelper(IFormatProvider? provider, string format, ParamsArray args) + private static string FormatHelper(IFormatProvider? provider, string format, ParamsArray args) where TArr : IValueArray { if (format == null) throw new ArgumentNullException(nameof(format)); diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs index 5957628a6ac846..9601ef5768e058 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs @@ -1499,11 +1499,11 @@ public StringBuilder Insert(int index, ReadOnlySpan value) return this; } - public StringBuilder AppendFormat(string format, object? arg0) => AppendFormatHelper(null, format, new ParamsArray(arg0)); + public StringBuilder AppendFormat(string format, object? arg0) => AppendFormatHelper(null, format, ParamsArray.Create(arg0)); - public StringBuilder AppendFormat(string format, object? arg0, object? arg1) => AppendFormatHelper(null, format, new ParamsArray(arg0, arg1)); + public StringBuilder AppendFormat(string format, object? arg0, object? arg1) => AppendFormatHelper(null, format, ParamsArray.Create(arg0, arg1)); - public StringBuilder AppendFormat(string format, object? arg0, object? arg1, object? arg2) => AppendFormatHelper(null, format, new ParamsArray(arg0, arg1, arg2)); + public StringBuilder AppendFormat(string format, object? arg0, object? arg1, object? arg2) => AppendFormatHelper(null, format, ParamsArray.Create(arg0, arg1, arg2)); public StringBuilder AppendFormat(string format, params object?[] args) { @@ -1515,14 +1515,14 @@ public StringBuilder AppendFormat(string format, params object?[] args) throw new ArgumentNullException(paramName); } - return AppendFormatHelper(null, format, new ParamsArray(args)); + return AppendFormatHelper(null, format, ParamsArray.Create(args)); } - public StringBuilder AppendFormat(IFormatProvider? provider, string format, object? arg0) => AppendFormatHelper(provider, format, new ParamsArray(arg0)); + public StringBuilder AppendFormat(IFormatProvider? provider, string format, object? arg0) => AppendFormatHelper(provider, format, ParamsArray.Create(arg0)); - public StringBuilder AppendFormat(IFormatProvider? provider, string format, object? arg0, object? arg1) => AppendFormatHelper(provider, format, new ParamsArray(arg0, arg1)); + public StringBuilder AppendFormat(IFormatProvider? provider, string format, object? arg0, object? arg1) => AppendFormatHelper(provider, format, ParamsArray.Create(arg0, arg1)); - public StringBuilder AppendFormat(IFormatProvider? provider, string format, object? arg0, object? arg1, object? arg2) => AppendFormatHelper(provider, format, new ParamsArray(arg0, arg1, arg2)); + public StringBuilder AppendFormat(IFormatProvider? provider, string format, object? arg0, object? arg1, object? arg2) => AppendFormatHelper(provider, format, ParamsArray.Create(arg0, arg1, arg2)); public StringBuilder AppendFormat(IFormatProvider? provider, string format, params object?[] args) { @@ -1534,7 +1534,7 @@ public StringBuilder AppendFormat(IFormatProvider? provider, string format, para throw new ArgumentNullException(paramName); } - return AppendFormatHelper(provider, format, new ParamsArray(args)); + return AppendFormatHelper(provider, format, ParamsArray.Create(args)); } private static void FormatError() @@ -1546,7 +1546,7 @@ private static void FormatError() private const int IndexLimit = 1000000; // Note: 0 <= ArgIndex < IndexLimit private const int WidthLimit = 1000000; // Note: -WidthLimit < ArgAlign < WidthLimit - internal StringBuilder AppendFormatHelper(IFormatProvider? provider, string format, ParamsArray args) + internal StringBuilder AppendFormatHelper(IFormatProvider? provider, string format, ParamsArray args) where TArr : IValueArray { if (format == null) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/ValueStringBuilder.AppendFormat.cs b/src/libraries/System.Private.CoreLib/src/System/Text/ValueStringBuilder.AppendFormat.cs index d11eae6ce47b9e..ed6b92e4321b5d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/ValueStringBuilder.AppendFormat.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/ValueStringBuilder.AppendFormat.cs @@ -5,11 +5,11 @@ namespace System.Text { internal ref partial struct ValueStringBuilder { - public void AppendFormat(string format, object? arg0) => AppendFormatHelper(null, format, new ParamsArray(arg0)); + public void AppendFormat(string format, object? arg0) => AppendFormatHelper(null, format, ParamsArray.Create(arg0)); - public void AppendFormat(string format, object? arg0, object? arg1) => AppendFormatHelper(null, format, new ParamsArray(arg0, arg1)); + public void AppendFormat(string format, object? arg0, object? arg1) => AppendFormatHelper(null, format, ParamsArray.Create(arg0, arg1)); - public void AppendFormat(string format, object? arg0, object? arg1, object? arg2) => AppendFormatHelper(null, format, new ParamsArray(arg0, arg1, arg2)); + public void AppendFormat(string format, object? arg0, object? arg1, object? arg2) => AppendFormatHelper(null, format, ParamsArray.Create(arg0, arg1, arg2)); public void AppendFormat(string format, params object?[] args) { @@ -21,14 +21,14 @@ public void AppendFormat(string format, params object?[] args) throw new ArgumentNullException(paramName); } - AppendFormatHelper(null, format, new ParamsArray(args)); + AppendFormatHelper(null, format, ParamsArray.Create(args)); } - public void AppendFormat(IFormatProvider? provider, string format, object? arg0) => AppendFormatHelper(provider, format, new ParamsArray(arg0)); + public void AppendFormat(IFormatProvider? provider, string format, object? arg0) => AppendFormatHelper(provider, format, ParamsArray.Create(arg0)); - public void AppendFormat(IFormatProvider? provider, string format, object? arg0, object? arg1) => AppendFormatHelper(provider, format, new ParamsArray(arg0, arg1)); + public void AppendFormat(IFormatProvider? provider, string format, object? arg0, object? arg1) => AppendFormatHelper(provider, format, ParamsArray.Create(arg0, arg1)); - public void AppendFormat(IFormatProvider? provider, string format, object? arg0, object? arg1, object? arg2) => AppendFormatHelper(provider, format, new ParamsArray(arg0, arg1, arg2)); + public void AppendFormat(IFormatProvider? provider, string format, object? arg0, object? arg1, object? arg2) => AppendFormatHelper(provider, format, ParamsArray.Create(arg0, arg1, arg2)); public void AppendFormat(IFormatProvider? provider, string format, params object?[] args) { @@ -40,7 +40,7 @@ public void AppendFormat(IFormatProvider? provider, string format, params object throw new ArgumentNullException(paramName); } - AppendFormatHelper(provider, format, new ParamsArray(args)); + AppendFormatHelper(provider, format, ParamsArray.Create(args)); } internal void AppendSpanFormattable(T value, string? format, IFormatProvider? provider) where T : ISpanFormattable @@ -57,7 +57,7 @@ internal void AppendSpanFormattable(T value, string? format, IFormatProvider? // Copied from StringBuilder, can't be done via generic extension // as ValueStringBuilder is a ref struct and cannot be used in a generic. - internal void AppendFormatHelper(IFormatProvider? provider, string format, ParamsArray args) + internal void AppendFormatHelper(IFormatProvider? provider, string format, ParamsArray args) where TArr : IValueArray { // Undocumented exclusive limits on the range for Argument Hole Index and Argument Hole Alignment. const int IndexLimit = 1000000; // Note: 0 <= ArgIndex < IndexLimit