From 8a286ac58ff514fbaad6dfa96abadca4a541eba5 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sat, 27 Apr 2024 16:19:51 -0700 Subject: [PATCH 1/6] Code Versioning - move native code versioning state Remove the dictionary from the CodeVersionManager and instead store the data directly on the MethodDesc. --- src/coreclr/vm/amd64/asmconstants.h | 8 ------ src/coreclr/vm/codeversion.cpp | 34 +++++++++++--------------- src/coreclr/vm/codeversion.h | 35 +-------------------------- src/coreclr/vm/i386/asmconstants.h | 2 +- src/coreclr/vm/method.cpp | 26 ++++++++++++++++++++ src/coreclr/vm/method.hpp | 33 +++++++++++++------------ src/coreclr/vm/methodtablebuilder.cpp | 2 ++ 7 files changed, 61 insertions(+), 79 deletions(-) diff --git a/src/coreclr/vm/amd64/asmconstants.h b/src/coreclr/vm/amd64/asmconstants.h index f51f3b00836b7..3e2c50cd61131 100644 --- a/src/coreclr/vm/amd64/asmconstants.h +++ b/src/coreclr/vm/amd64/asmconstants.h @@ -98,14 +98,6 @@ ASMCONSTANTS_C_ASSERT(SIZEOF__ComPrestubMethodFrame ASMCONSTANTS_C_ASSERT(SIZEOF__ComMethodFrame == sizeof(ComMethodFrame)); -#define OFFSETOF__ComPlusCallMethodDesc__m_pComPlusCallInfo DBG_FRE(0x30, 0x08) -ASMCONSTANTS_C_ASSERT(OFFSETOF__ComPlusCallMethodDesc__m_pComPlusCallInfo - == offsetof(ComPlusCallMethodDesc, m_pComPlusCallInfo)); - -#define OFFSETOF__ComPlusCallInfo__m_pILStub 0x0 -ASMCONSTANTS_C_ASSERT(OFFSETOF__ComPlusCallInfo__m_pILStub - == offsetof(ComPlusCallInfo, m_pILStub)); - #endif // FEATURE_COMINTEROP #define OFFSETOF__Thread__m_fPreemptiveGCDisabled 0x04 diff --git a/src/coreclr/vm/codeversion.cpp b/src/coreclr/vm/codeversion.cpp index ea95e274419ce..6d50d6fcacf16 100644 --- a/src/coreclr/vm/codeversion.cpp +++ b/src/coreclr/vm/codeversion.cpp @@ -1306,9 +1306,6 @@ void ILCodeVersioningState::LinkILCodeVersionNode(ILCodeVersionNode* pILCodeVers bool CodeVersionManager::s_initialNativeCodeVersionMayNotBeTheDefaultNativeCodeVersion = false; #endif -CodeVersionManager::CodeVersionManager() -{} - PTR_ILCodeVersioningState CodeVersionManager::GetILCodeVersioningState(PTR_Module pModule, mdMethodDef methodDef) const { LIMITED_METHOD_DAC_CONTRACT; @@ -1319,7 +1316,7 @@ PTR_ILCodeVersioningState CodeVersionManager::GetILCodeVersioningState(PTR_Modul PTR_MethodDescVersioningState CodeVersionManager::GetMethodDescVersioningState(PTR_MethodDesc pClosedMethodDesc) const { LIMITED_METHOD_DAC_CONTRACT; - return m_methodDescVersioningStateMap.Lookup(pClosedMethodDesc); + return pClosedMethodDesc->GetMethodDescVersionState(); } #ifndef DACCESS_COMPILE @@ -1354,28 +1351,25 @@ HRESULT CodeVersionManager::GetOrCreateILCodeVersioningState(Module* pModule, md HRESULT CodeVersionManager::GetOrCreateMethodDescVersioningState(MethodDesc* pMethod, MethodDescVersioningState** ppMethodVersioningState) { - LIMITED_METHOD_CONTRACT; - HRESULT hr = S_OK; - MethodDescVersioningState* pMethodVersioningState = m_methodDescVersioningStateMap.Lookup(pMethod); + CONTRACTL + { + NOTHROW; + PRECONDITION(pMethod != NULL); + PRECONDITION(ppMethodVersioningState != NULL); + } + CONTRACTL_END; + + MethodDescVersioningState* pMethodVersioningState = pMethod->GetMethodDescVersionState(); if (pMethodVersioningState == NULL) { pMethodVersioningState = new (nothrow) MethodDescVersioningState(pMethod); if (pMethodVersioningState == NULL) - { return E_OUTOFMEMORY; - } - EX_TRY - { - // This throws when out of memory, but remains internally - // consistent (without adding the new element) - m_methodDescVersioningStateMap.Add(pMethodVersioningState); - } - EX_CATCH_HRESULT(hr); - if (FAILED(hr)) - { + + if (!pMethod->SetMethodDescVersionState(pMethodVersioningState)) delete pMethodVersioningState; - return hr; - } + + pMethodVersioningState = pMethod->GetMethodDescVersionState(); } *ppMethodVersioningState = pMethodVersioningState; return S_OK; diff --git a/src/coreclr/vm/codeversion.h b/src/coreclr/vm/codeversion.h index a6fabc38c99a0..391137a142d1a 100644 --- a/src/coreclr/vm/codeversion.h +++ b/src/coreclr/vm/codeversion.h @@ -475,36 +475,6 @@ class MethodDescVersioningState PTR_NativeCodeVersionNode m_pFirstVersionNode; }; -class MethodDescVersioningStateHashTraits : public NoRemoveSHashTraits> -{ -public: - typedef typename DefaultSHashTraits::element_t element_t; - typedef typename DefaultSHashTraits::count_t count_t; - - typedef const PTR_MethodDesc key_t; - - static key_t GetKey(element_t e) - { - LIMITED_METHOD_CONTRACT; - return e->GetMethodDesc(); - } - static BOOL Equals(key_t k1, key_t k2) - { - LIMITED_METHOD_CONTRACT; - return k1 == k2; - } - static count_t Hash(key_t k) - { - LIMITED_METHOD_CONTRACT; - return (count_t)dac_cast(k); - } - - static element_t Null() { LIMITED_METHOD_CONTRACT; return dac_cast(nullptr); } - static bool IsNull(const element_t &e) { LIMITED_METHOD_CONTRACT; return e == NULL; } -}; - -typedef SHash MethodDescVersioningStateHash; - class ILCodeVersioningState { public: @@ -572,7 +542,7 @@ class CodeVersionManager friend class ILCodeVersion; public: - CodeVersionManager(); + CodeVersionManager() = default; DWORD GetNonDefaultILVersionCount(); ILCodeVersionCollection GetILCodeVersions(PTR_MethodDesc pMethod); @@ -648,9 +618,6 @@ class CodeVersionManager //Module,MethodDef -> ILCodeVersioningState ILCodeVersioningStateHash m_ilCodeVersioningStateMap; - //closed MethodDesc -> MethodDescVersioningState - MethodDescVersioningStateHash m_methodDescVersioningStateMap; - private: static CrstStatic s_lock; diff --git a/src/coreclr/vm/i386/asmconstants.h b/src/coreclr/vm/i386/asmconstants.h index 475a0f857ebd9..c1a577caa0f86 100644 --- a/src/coreclr/vm/i386/asmconstants.h +++ b/src/coreclr/vm/i386/asmconstants.h @@ -233,7 +233,7 @@ ASMCONSTANTS_C_ASSERT(OFFSETOF__FrameHandlerExRecord__m_pEntryFrame == offsetof( #endif -#define ComPlusCallMethodDesc__m_pComPlusCallInfo DBG_FRE(0x1C, 0x8) +#define ComPlusCallMethodDesc__m_pComPlusCallInfo DBG_FRE(0x20, 0xC) ASMCONSTANTS_C_ASSERT(ComPlusCallMethodDesc__m_pComPlusCallInfo == offsetof(ComPlusCallMethodDesc, m_pComPlusCallInfo)) #define ComPlusCallInfo__m_pRetThunk 0x10 diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index bbbed6bd33a48..604a4a0d1263e 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -209,8 +209,33 @@ LoaderAllocator * MethodDesc::GetDomainSpecificLoaderAllocator() } +void MethodDesc::AllocateCodeData() +{ + CONTRACTL + { + THROWS; + PRECONDITION(m_codeData == NULL); + } + CONTRACTL_END; + + m_codeData = (MethodDescCodeData*)(void*)GetLoaderAllocator()->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(sizeof(MethodDescCodeData))); +} + +bool MethodDesc::SetMethodDescVersionState(PTR_MethodDescVersioningState state) +{ + WRAPPER_NO_CONTRACT; + _ASSERTE(m_codeData != NULL); + return InterlockedCompareExchangeT(&m_codeData->MDState, state, NULL) == NULL; +} + #endif //!DACCESS_COMPILE +PTR_MethodDescVersioningState MethodDesc::GetMethodDescVersionState() +{ + WRAPPER_NO_CONTRACT; + _ASSERTE(m_codeData != NULL); + return m_codeData->MDState; +} //******************************************************************************* LPCUTF8 MethodDesc::GetNameThrowing() @@ -1719,6 +1744,7 @@ MethodDescChunk *MethodDescChunk::CreateChunk(LoaderHeap *pHeap, DWORD methodDes { pMD->SetChunkIndex(pChunk); pMD->SetMethodDescIndex(i); + pMD->AllocateCodeData(); pMD->SetClassification(classification); if (fNonVtableSlot) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index ad662fa960749..e0651e7490c8a 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -35,7 +35,6 @@ class Dictionary; class GCCoverageInfo; class DynamicMethodDesc; class ReJitManager; -class CodeVersionManager; class PrepareCodeConfig; typedef DPTR(FCallMethodDesc) PTR_FCallMethodDesc; @@ -159,6 +158,12 @@ enum MethodDescFlags mdfIsIntrinsic = 0x8000 // Jit may expand method as an intrinsic }; +struct MethodDescCodeData +{ + PTR_MethodDescVersioningState MDState; +}; +using PTR_MethodDescCodeData = DPTR(MethodDescCodeData); + // The size of this structure needs to be a multiple of MethodDesc::ALIGNMENT // // @GENERICS: @@ -177,18 +182,6 @@ enum MethodDescFlags // It also knows how to get at its IL code (code:IMAGE_COR_ILMETHOD) class MethodDesc { - friend class EEClass; - friend class MethodTableBuilder; - friend class ArrayClass; - friend class NDirect; - friend class MethodDescChunk; - friend class InstantiatedMethodDesc; - friend class MethodImpl; - friend class CheckAsmOffsets; - friend class ClrDataAccess; - - friend class MethodDescCallSite; - public: #ifdef TARGET_64BIT @@ -1624,6 +1617,7 @@ class MethodDesc WORD m_wSlotNumber; // The slot number of this MethodDesc in the vtable array. WORD m_wFlags; // See MethodDescFlags + PTR_MethodDescCodeData m_codeData; public: #ifdef DACCESS_COMPILE @@ -1632,15 +1626,25 @@ class MethodDesc BYTE GetMethodDescIndex() { + LIMITED_METHOD_CONTRACT; return m_methodIndex; } void SetMethodDescIndex(COUNT_T index) { + LIMITED_METHOD_CONTRACT; _ASSERTE(index <= 255); m_methodIndex = (BYTE)index; } +#ifndef DACCESS_COMPILE + void AllocateCodeData(); + + bool SetMethodDescVersionState(PTR_MethodDescVersioningState state); +#endif //!DACCESS_COMPILE + + PTR_MethodDescVersioningState GetMethodDescVersionState(); + public: inline DWORD GetClassification() const { @@ -2108,7 +2112,6 @@ class MulticoreJitPrepareCodeConfig : public PrepareCodeConfig class MethodDescChunk { friend class MethodDesc; - friend class CheckAsmOffsets; enum { enum_flag_TokenRangeMask = 0x0FFF, // This must equal METHOD_TOKEN_RANGE_MASK calculated higher in this file @@ -2411,9 +2414,7 @@ typedef DPTR(DynamicResolver) PTR_DynamicResolver; class DynamicMethodDesc : public StoredSigMethodDesc { friend class ILStubCache; - friend class ILStubState; friend class DynamicMethodTable; - friend class MethodDesc; protected: PTR_CUTF8 m_pszMethodName; diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index db015d227538e..12b51a1ea1b34 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -7065,6 +7065,7 @@ VOID MethodTableBuilder::AllocAndInitMethodDescChunk(COUNT_T startIndex, COUNT_T pMD->SetChunkIndex(pChunk); pMD->SetMethodDescIndex(methodDescCount); + pMD->AllocateCodeData(); InitNewMethodDesc(pMDMethod, pMD); @@ -7109,6 +7110,7 @@ VOID MethodTableBuilder::AllocAndInitMethodDescChunk(COUNT_T startIndex, COUNT_T // Reset the chunk index pUnboxedMD->SetChunkIndex(pChunk); pUnboxedMD->SetMethodDescIndex(methodDescCount); + pUnboxedMD->AllocateCodeData(); if (bmtGenerics->GetNumGenericArgs() == 0) { pUnboxedMD->SetHasNonVtableSlot(); From b9c05e5735d1bb5d9b1b5ab9493a27510a0987cf Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 28 Apr 2024 07:04:57 -0700 Subject: [PATCH 2/6] Pass allocator for codedata allocation. --- src/coreclr/vm/method.cpp | 10 +++++++--- src/coreclr/vm/method.hpp | 2 +- src/coreclr/vm/methodtablebuilder.cpp | 7 ++++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 604a4a0d1263e..14d9699ee5136 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -209,16 +209,20 @@ LoaderAllocator * MethodDesc::GetDomainSpecificLoaderAllocator() } -void MethodDesc::AllocateCodeData() +void MethodDesc::AllocateCodeData(LoaderHeap* pHeap, AllocMemTracker* pamTracker) { CONTRACTL { THROWS; + PRECONDITION(pHeap != NULL); + PRECONDITION(pamTracker != NULL); PRECONDITION(m_codeData == NULL); + POSTCONDITION(m_codeData != NULL); } CONTRACTL_END; - m_codeData = (MethodDescCodeData*)(void*)GetLoaderAllocator()->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(sizeof(MethodDescCodeData))); + m_codeData = (MethodDescCodeData*)pamTracker->Track( + pHeap->AllocMem(S_SIZE_T(sizeof(MethodDescCodeData)))); } bool MethodDesc::SetMethodDescVersionState(PTR_MethodDescVersioningState state) @@ -1744,7 +1748,7 @@ MethodDescChunk *MethodDescChunk::CreateChunk(LoaderHeap *pHeap, DWORD methodDes { pMD->SetChunkIndex(pChunk); pMD->SetMethodDescIndex(i); - pMD->AllocateCodeData(); + pMD->AllocateCodeData(pHeap, pamTracker); pMD->SetClassification(classification); if (fNonVtableSlot) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index e0651e7490c8a..49ffbb0a1e155 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1638,7 +1638,7 @@ class MethodDesc } #ifndef DACCESS_COMPILE - void AllocateCodeData(); + void AllocateCodeData(LoaderHeap* pHeap, AllocMemTracker* pamTracker); bool SetMethodDescVersionState(PTR_MethodDescVersioningState state); #endif //!DACCESS_COMPILE diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 12b51a1ea1b34..b60be41230f57 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -7039,8 +7039,9 @@ VOID MethodTableBuilder::AllocAndInitMethodDescChunk(COUNT_T startIndex, COUNT_T PRECONDITION(sizeOfMethodDescs <= MethodDescChunk::MaxSizeOfMethodDescs); } CONTRACTL_END; + PTR_LoaderHeap pHeap = GetLoaderAllocator()->GetHighFrequencyHeap(); void * pMem = GetMemTracker()->Track( - GetLoaderAllocator()->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(sizeof(TADDR) + sizeof(MethodDescChunk) + sizeOfMethodDescs))); + pHeap->AllocMem(S_SIZE_T(sizeof(TADDR) + sizeof(MethodDescChunk) + sizeOfMethodDescs))); // Skip pointer to temporary entrypoints MethodDescChunk * pChunk = (MethodDescChunk *)((BYTE*)pMem + sizeof(TADDR)); @@ -7065,7 +7066,7 @@ VOID MethodTableBuilder::AllocAndInitMethodDescChunk(COUNT_T startIndex, COUNT_T pMD->SetChunkIndex(pChunk); pMD->SetMethodDescIndex(methodDescCount); - pMD->AllocateCodeData(); + pMD->AllocateCodeData(pHeap, GetMemTracker()); InitNewMethodDesc(pMDMethod, pMD); @@ -7110,7 +7111,7 @@ VOID MethodTableBuilder::AllocAndInitMethodDescChunk(COUNT_T startIndex, COUNT_T // Reset the chunk index pUnboxedMD->SetChunkIndex(pChunk); pUnboxedMD->SetMethodDescIndex(methodDescCount); - pUnboxedMD->AllocateCodeData(); + pUnboxedMD->AllocateCodeData(pHeap, GetMemTracker()); if (bmtGenerics->GetNumGenericArgs() == 0) { pUnboxedMD->SetHasNonVtableSlot(); From c7e9cb2a71ef039182afa9c8f2060f5bc32d09ca Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 29 Apr 2024 08:49:40 -0700 Subject: [PATCH 3/6] Remove POSTCONDITION --- src/coreclr/vm/method.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 14d9699ee5136..e7403290d2e45 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -217,7 +217,6 @@ void MethodDesc::AllocateCodeData(LoaderHeap* pHeap, AllocMemTracker* pamTracker PRECONDITION(pHeap != NULL); PRECONDITION(pamTracker != NULL); PRECONDITION(m_codeData == NULL); - POSTCONDITION(m_codeData != NULL); } CONTRACTL_END; From 2f424c19d4b00c0997b788b457752833b9c28b50 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 30 Apr 2024 10:59:44 -0700 Subject: [PATCH 4/6] Remove PRECONDITION --- src/coreclr/vm/method.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index e7403290d2e45..830640f18144f 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -216,7 +216,6 @@ void MethodDesc::AllocateCodeData(LoaderHeap* pHeap, AllocMemTracker* pamTracker THROWS; PRECONDITION(pHeap != NULL); PRECONDITION(pamTracker != NULL); - PRECONDITION(m_codeData == NULL); } CONTRACTL_END; From e8b34a69bb189ee3a276cbcacd5e63c84b6920a9 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 1 May 2024 07:59:38 -0700 Subject: [PATCH 5/6] Add GC_NOTRIGGER --- src/coreclr/vm/method.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 830640f18144f..c882f9c2c8e94 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -214,6 +214,7 @@ void MethodDesc::AllocateCodeData(LoaderHeap* pHeap, AllocMemTracker* pamTracker CONTRACTL { THROWS; + GC_NOTRIGGER; PRECONDITION(pHeap != NULL); PRECONDITION(pamTracker != NULL); } From 1a2d0b72473316dd673095c2fa93a9d9121f0121 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 7 May 2024 09:38:50 -0700 Subject: [PATCH 6/6] Update contract --- src/coreclr/vm/codeversion.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/vm/codeversion.cpp b/src/coreclr/vm/codeversion.cpp index 6d50d6fcacf16..96ce1ea28d237 100644 --- a/src/coreclr/vm/codeversion.cpp +++ b/src/coreclr/vm/codeversion.cpp @@ -1354,6 +1354,7 @@ HRESULT CodeVersionManager::GetOrCreateMethodDescVersioningState(MethodDesc* pMe CONTRACTL { NOTHROW; + GC_NOTRIGGER; PRECONDITION(pMethod != NULL); PRECONDITION(ppMethodVersioningState != NULL); }