From 43bfdd7260eb4430efb8f5cb43542763cfa5a2df Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 6 Nov 2021 18:54:02 -0700 Subject: [PATCH 1/3] remove m_hostAssemblyMap and ceremony around it --- src/coreclr/binder/assembly.cpp | 1 + src/coreclr/binder/inc/assembly.hpp | 14 +++ src/coreclr/inc/CrstTypes.def | 9 +- src/coreclr/inc/crsttypes.h | 163 +++++++++++++--------------- src/coreclr/vm/appdomain.cpp | 111 +------------------ src/coreclr/vm/appdomain.hpp | 61 ----------- src/coreclr/vm/domainfile.cpp | 36 +++++- src/coreclr/vm/domainfile.h | 2 + 8 files changed, 133 insertions(+), 264 deletions(-) diff --git a/src/coreclr/binder/assembly.cpp b/src/coreclr/binder/assembly.cpp index d13c20b90ef6d..9348c7b67e4e8 100644 --- a/src/coreclr/binder/assembly.cpp +++ b/src/coreclr/binder/assembly.cpp @@ -24,6 +24,7 @@ namespace BINDER_SPACE m_pAssemblyName = NULL; m_isInTPA = false; m_pBinder = NULL; + m_domainAssembly = NULL; } Assembly::~Assembly() diff --git a/src/coreclr/binder/inc/assembly.hpp b/src/coreclr/binder/inc/assembly.hpp index 19cf15ed402d2..8c24ebb6d30e0 100644 --- a/src/coreclr/binder/inc/assembly.hpp +++ b/src/coreclr/binder/inc/assembly.hpp @@ -27,6 +27,8 @@ #include "bundle.h" +class DomainAssembly; + namespace BINDER_SPACE { // BINDER_SPACE::Assembly represents a result of binding to an actual assembly (PEImage) @@ -53,12 +55,24 @@ namespace BINDER_SPACE return m_pBinder; } + DomainAssembly* GetDomainAssembly() + { + return m_domainAssembly; + } + + void SetDomainAssembly(DomainAssembly* value) + { + _ASSERTE(value == NULL || m_domainAssembly == NULL); + m_domainAssembly = value; + } + private: LONG m_cRef; PEImage *m_pPEImage; AssemblyName *m_pAssemblyName; AssemblyBinder *m_pBinder; bool m_isInTPA; + DomainAssembly *m_domainAssembly; inline void SetBinder(AssemblyBinder *pBinder) { diff --git a/src/coreclr/inc/CrstTypes.def b/src/coreclr/inc/CrstTypes.def index d6fb713a98a1e..0335e4965f110 100644 --- a/src/coreclr/inc/CrstTypes.def +++ b/src/coreclr/inc/CrstTypes.def @@ -236,13 +236,6 @@ Crst HandleTable SameLevelAs HandleTable End -Crst HostAssemblyMap -End - -Crst HostAssemblyMapAdd - AcquiredBefore HostAssemblyMap -End - Crst IbcProfile End @@ -586,4 +579,4 @@ End Crst PgoData AcquiredBefore LoaderHeap -End \ No newline at end of file +End diff --git a/src/coreclr/inc/crsttypes.h b/src/coreclr/inc/crsttypes.h index 23070d7820d61..d462cbd3c88e1 100644 --- a/src/coreclr/inc/crsttypes.h +++ b/src/coreclr/inc/crsttypes.h @@ -1,7 +1,6 @@ // // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. // #ifndef __CRST_TYPES_INCLUDED @@ -11,7 +10,7 @@ // This file describes the range of Crst types available and their mapping to a numeric level (used by the // runtime in debug mode to validate we're deadlock free). To modify these settings edit the -// file:CrstTypes.def file and run the clr\bin\CrstTypeTool utility to generate a new version of this file. +// file:CrstTypes.def file and run the clr\artifacts\CrstTypeTool utility to generate a new version of this file. // Each Crst type is declared as a value in the following CrstType enum. enum CrstType @@ -59,85 +58,83 @@ enum CrstType CrstGCCover = 40, CrstGlobalStrLiteralMap = 41, CrstHandleTable = 42, - CrstHostAssemblyMap = 43, - CrstHostAssemblyMapAdd = 44, - CrstIbcProfile = 45, - CrstIJWFixupData = 46, - CrstIJWHash = 47, - CrstILStubGen = 48, - CrstInlineTrackingMap = 49, - CrstInstMethodHashTable = 50, - CrstInterop = 51, - CrstInteropData = 52, - CrstIsJMCMethod = 53, - CrstISymUnmanagedReader = 54, - CrstJit = 55, - CrstJitGenericHandleCache = 56, - CrstJitInlineTrackingMap = 57, - CrstJitPatchpoint = 58, - CrstJitPerf = 59, - CrstJumpStubCache = 60, - CrstLeafLock = 61, - CrstListLock = 62, - CrstLoaderAllocator = 63, - CrstLoaderAllocatorReferences = 64, - CrstLoaderHeap = 65, - CrstManagedObjectWrapperMap = 66, - CrstMethodDescBackpatchInfoTracker = 67, - CrstModule = 68, - CrstModuleFixup = 69, - CrstModuleLookupTable = 70, - CrstMulticoreJitHash = 71, - CrstMulticoreJitManager = 72, - CrstNativeImageEagerFixups = 73, - CrstNativeImageLoad = 74, - CrstNls = 75, - CrstNotifyGdb = 76, - CrstObjectList = 77, - CrstPEImage = 78, - CrstPendingTypeLoadEntry = 79, - CrstPgoData = 80, - CrstPinnedByrefValidation = 81, - CrstProfilerGCRefDataFreeList = 82, - CrstProfilingAPIStatus = 83, - CrstRCWCache = 84, - CrstRCWCleanupList = 85, - CrstReadyToRunEntryPointToMethodDescMap = 86, - CrstReflection = 87, - CrstReJITGlobalRequest = 88, - CrstRetThunkCache = 89, - CrstSavedExceptionInfo = 90, - CrstSaveModuleProfileData = 91, - CrstSecurityStackwalkCache = 92, - CrstSigConvert = 93, - CrstSingleUseLock = 94, - CrstSpecialStatics = 95, - CrstStackSampler = 96, - CrstStressLog = 97, - CrstStubCache = 98, - CrstStubDispatchCache = 99, - CrstStubUnwindInfoHeapSegments = 100, - CrstSyncBlockCache = 101, - CrstSyncHashLock = 102, - CrstSystemBaseDomain = 103, - CrstSystemDomain = 104, - CrstSystemDomainDelayedUnloadList = 105, - CrstThreadIdDispenser = 106, - CrstThreadpoolTimerQueue = 107, - CrstThreadpoolWaitThreads = 108, - CrstThreadpoolWorker = 109, - CrstThreadStore = 110, - CrstTieredCompilation = 111, - CrstTypeEquivalenceMap = 112, - CrstTypeIDMap = 113, - CrstUMEntryThunkCache = 114, - CrstUMEntryThunkFreeListLock = 115, - CrstUniqueStack = 116, - CrstUnresolvedClassLock = 117, - CrstUnwindInfoTableLock = 118, - CrstVSDIndirectionCellLock = 119, - CrstWrapperTemplate = 120, - kNumberOfCrstTypes = 121 + CrstIbcProfile = 43, + CrstIJWFixupData = 44, + CrstIJWHash = 45, + CrstILStubGen = 46, + CrstInlineTrackingMap = 47, + CrstInstMethodHashTable = 48, + CrstInterop = 49, + CrstInteropData = 50, + CrstIsJMCMethod = 51, + CrstISymUnmanagedReader = 52, + CrstJit = 53, + CrstJitGenericHandleCache = 54, + CrstJitInlineTrackingMap = 55, + CrstJitPatchpoint = 56, + CrstJitPerf = 57, + CrstJumpStubCache = 58, + CrstLeafLock = 59, + CrstListLock = 60, + CrstLoaderAllocator = 61, + CrstLoaderAllocatorReferences = 62, + CrstLoaderHeap = 63, + CrstManagedObjectWrapperMap = 64, + CrstMethodDescBackpatchInfoTracker = 65, + CrstModule = 66, + CrstModuleFixup = 67, + CrstModuleLookupTable = 68, + CrstMulticoreJitHash = 69, + CrstMulticoreJitManager = 70, + CrstNativeImageEagerFixups = 71, + CrstNativeImageLoad = 72, + CrstNls = 73, + CrstNotifyGdb = 74, + CrstObjectList = 75, + CrstPEImage = 76, + CrstPendingTypeLoadEntry = 77, + CrstPgoData = 78, + CrstPinnedByrefValidation = 79, + CrstProfilerGCRefDataFreeList = 80, + CrstProfilingAPIStatus = 81, + CrstRCWCache = 82, + CrstRCWCleanupList = 83, + CrstReadyToRunEntryPointToMethodDescMap = 84, + CrstReflection = 85, + CrstReJITGlobalRequest = 86, + CrstRetThunkCache = 87, + CrstSavedExceptionInfo = 88, + CrstSaveModuleProfileData = 89, + CrstSecurityStackwalkCache = 90, + CrstSigConvert = 91, + CrstSingleUseLock = 92, + CrstSpecialStatics = 93, + CrstStackSampler = 94, + CrstStressLog = 95, + CrstStubCache = 96, + CrstStubDispatchCache = 97, + CrstStubUnwindInfoHeapSegments = 98, + CrstSyncBlockCache = 99, + CrstSyncHashLock = 100, + CrstSystemBaseDomain = 101, + CrstSystemDomain = 102, + CrstSystemDomainDelayedUnloadList = 103, + CrstThreadIdDispenser = 104, + CrstThreadpoolTimerQueue = 105, + CrstThreadpoolWaitThreads = 106, + CrstThreadpoolWorker = 107, + CrstThreadStore = 108, + CrstTieredCompilation = 109, + CrstTypeEquivalenceMap = 110, + CrstTypeIDMap = 111, + CrstUMEntryThunkCache = 112, + CrstUMEntryThunkFreeListLock = 113, + CrstUniqueStack = 114, + CrstUnresolvedClassLock = 115, + CrstUnwindInfoTableLock = 116, + CrstVSDIndirectionCellLock = 117, + CrstWrapperTemplate = 118, + kNumberOfCrstTypes = 119 }; #endif // __CRST_TYPES_INCLUDED @@ -191,8 +188,6 @@ int g_rgCrstLevelMap[] = 10, // CrstGCCover 13, // CrstGlobalStrLiteralMap 1, // CrstHandleTable - 0, // CrstHostAssemblyMap - 3, // CrstHostAssemblyMapAdd 0, // CrstIbcProfile 8, // CrstIJWFixupData 0, // CrstIJWHash @@ -317,8 +312,6 @@ LPCSTR g_rgCrstNameMap[] = "CrstGCCover", "CrstGlobalStrLiteralMap", "CrstHandleTable", - "CrstHostAssemblyMap", - "CrstHostAssemblyMapAdd", "CrstIbcProfile", "CrstIJWFixupData", "CrstIJWHash", diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index 61c4c08e24925..7439fd6c35078 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -2039,17 +2039,6 @@ void AppDomain::Init() SetStage( STAGE_CREATING); - - // The lock is taken also during stack walking (GC or profiler) - // - To prevent deadlock with GC thread, we cannot trigger GC while holding the lock - // - To prevent deadlock with profiler thread, we cannot allow thread suspension - m_crstHostAssemblyMap.Init( - CrstHostAssemblyMap, - (CrstFlags)(CRST_GC_NOTRIGGER_WHEN_TAKEN - | CRST_DEBUGGER_THREAD - INDEBUG(| CRST_DEBUG_ONLY_CHECK_FORBID_SUSPEND_THREAD))); - m_crstHostAssemblyMapAdd.Init(CrstHostAssemblyMapAdd); - //Allocate the threadpool entry before the appdomain id list. Otherwise, //the thread pool list will be out of sync if insertion of id in //the appdomain fails. @@ -3196,10 +3185,9 @@ DomainAssembly * AppDomain::FindAssembly(PEAssembly * pPEAssembly, FindAssemblyO { CONTRACTL { - THROWS; - GC_TRIGGERS; + NOTHROW; + GC_NOTRIGGER; MODE_ANY; - INJECT_FAULT(COMPlusThrowOM();); } CONTRACTL_END; @@ -3207,7 +3195,7 @@ DomainAssembly * AppDomain::FindAssembly(PEAssembly * pPEAssembly, FindAssemblyO if (pPEAssembly->HasHostAssembly()) { - DomainAssembly * pDA = FindAssembly(pPEAssembly->GetHostAssembly()); + DomainAssembly * pDA = pPEAssembly->GetHostAssembly()->GetDomainAssembly(); if (pDA != nullptr && (pDA->IsLoaded() || (includeFailedToLoad && pDA->IsError()))) { return pDA; @@ -5337,99 +5325,6 @@ TypeEquivalenceHashTable * AppDomain::GetTypeEquivalenceCache() #endif //FEATURE_TYPEEQUIVALENCE -#if !defined(DACCESS_COMPILE) - -//--------------------------------------------------------------------------------------------------------------------- -void AppDomain::PublishHostedAssembly( - DomainAssembly * pDomainAssembly) -{ - CONTRACTL - { - THROWS; - GC_NOTRIGGER; - MODE_ANY; - } - CONTRACTL_END - - if (pDomainAssembly->GetPEAssembly()->HasHostAssembly()) - { - // We have to serialize all Add operations - CrstHolder lockAdd(&m_crstHostAssemblyMapAdd); - _ASSERTE(m_hostAssemblyMap.Lookup(pDomainAssembly->GetPEAssembly()->GetHostAssembly()) == nullptr); - - // Wrapper for m_hostAssemblyMap.Add that avoids call out into host - HostAssemblyMap::AddPhases addCall; - - // 1. Preallocate one element - addCall.PreallocateForAdd(&m_hostAssemblyMap); - { - // 2. Take the reader lock which can be taken during stack walking - // We cannot call out into host from ForbidSuspend region (i.e. no allocations/deallocations) - ForbidSuspendThreadHolder suspend; - { - CrstHolder lock(&m_crstHostAssemblyMap); - // 3. Add the element to the hash table (no call out into host) - addCall.Add(pDomainAssembly); - } - } - // 4. Cleanup the old memory (if any) - addCall.DeleteOldTable(); - } - else - { - } -} - -//--------------------------------------------------------------------------------------------------------------------- -void AppDomain::UnPublishHostedAssembly( - DomainAssembly * pAssembly) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - CAN_TAKE_LOCK; - } - CONTRACTL_END - - if (pAssembly->GetPEAssembly()->HasHostAssembly()) - { - ForbidSuspendThreadHolder suspend; - { - CrstHolder lock(&m_crstHostAssemblyMap); - _ASSERTE(m_hostAssemblyMap.Lookup(pAssembly->GetPEAssembly()->GetHostAssembly()) != nullptr); - m_hostAssemblyMap.Remove(pAssembly->GetPEAssembly()->GetHostAssembly()); - } - } -} - -#endif //!DACCESS_COMPILE - -//--------------------------------------------------------------------------------------------------------------------- -PTR_DomainAssembly AppDomain::FindAssembly(PTR_BINDER_SPACE_Assembly pHostAssembly) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - SUPPORTS_DAC; - } - CONTRACTL_END - - if (pHostAssembly == nullptr) - return NULL; - - { - ForbidSuspendThreadHolder suspend; - { - CrstHolder lock(&m_crstHostAssemblyMap); - return m_hostAssemblyMap.Lookup(pHostAssembly); - } - } -} - #ifndef DACCESS_COMPILE // Return native image for a given composite image file name, NULL when not found. PTR_NativeImage AppDomain::GetNativeImage(LPCUTF8 simpleFileName) diff --git a/src/coreclr/vm/appdomain.hpp b/src/coreclr/vm/appdomain.hpp index 27509c601e4cc..a369823a53937 100644 --- a/src/coreclr/vm/appdomain.hpp +++ b/src/coreclr/vm/appdomain.hpp @@ -2359,67 +2359,6 @@ class AppDomain : public BaseDomain TieredCompilationManager m_tieredCompilationManager; #endif - -private: - //----------------------------------------------------------- - // Static BINDER_SPACE::Assembly -> DomainAssembly mapping functions. - // This map does not maintain a reference count to either key or value. - // PEAssembly maintains a reference count on the BINDER_SPACE::Assembly through its code:PEAssembly::m_pHostAssembly field. - // It is removed from this hash table by code:DomainAssembly::~DomainAssembly. - struct HostAssemblyHashTraits : public DefaultSHashTraits - { - public: - typedef PTR_BINDER_SPACE_Assembly key_t; - - static key_t GetKey(element_t const & elem) - { - STATIC_CONTRACT_WRAPPER; - return elem->GetPEAssembly()->GetHostAssembly(); - } - - static BOOL Equals(key_t key1, key_t key2) - { - LIMITED_METHOD_CONTRACT; - return dac_cast(key1) == dac_cast(key2); - } - - static count_t Hash(key_t key) - { - STATIC_CONTRACT_LIMITED_METHOD; - //return reinterpret_cast(dac_cast(key)); - return (count_t)(dac_cast(key)); - } - - static element_t Null() { return NULL; } - static element_t Deleted() { return (element_t)(TADDR)-1; } - static bool IsNull(const element_t & e) { return e == NULL; } - static bool IsDeleted(const element_t & e) { return dac_cast(e) == (TADDR)-1; } - }; - - typedef SHash HostAssemblyMap; - HostAssemblyMap m_hostAssemblyMap; - CrstExplicitInit m_crstHostAssemblyMap; - // Lock to serialize all Add operations (in addition to the "read-lock" above) - CrstExplicitInit m_crstHostAssemblyMapAdd; - -public: - // Returns DomainAssembly. - PTR_DomainAssembly FindAssembly(PTR_BINDER_SPACE_Assembly pHostAssembly); - -#ifndef DACCESS_COMPILE -private: - friend void DomainAssembly::Allocate(); - friend DomainAssembly::~DomainAssembly(); - - // Called from DomainAssembly::Begin. - void PublishHostedAssembly( - DomainAssembly* pAssembly); - - // Called from DomainAssembly::~DomainAssembly - void UnPublishHostedAssembly( - DomainAssembly* pAssembly); -#endif // DACCESS_COMPILE - }; // class AppDomain // Just a ref holder diff --git a/src/coreclr/vm/domainfile.cpp b/src/coreclr/vm/domainfile.cpp index ca9ea6565d3f7..8938aea83765d 100644 --- a/src/coreclr/vm/domainfile.cpp +++ b/src/coreclr/vm/domainfile.cpp @@ -723,7 +723,7 @@ DomainAssembly::~DomainAssembly() if (m_fHostAssemblyPublished) { // Remove association first. - GetAppDomain()->UnPublishHostedAssembly(this); + UnRegisterFromHostedAssembly(); } ModuleIterator i = IterateModules(kModIterIncludeLoading); @@ -853,10 +853,42 @@ void DomainAssembly::Begin() m_pDomain->AddAssembly(this); } // Make it possible to find this DomainAssembly object from associated BINDER_SPACE::Assembly. - GetAppDomain()->PublishHostedAssembly(this); + RegisterWithHostedAssembly(); m_fHostAssemblyPublished = true; } +void DomainAssembly::RegisterWithHostedAssembly() +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END + + if (GetPEAssembly()->HasHostAssembly()) + { + GetPEAssembly()->GetHostAssembly()->SetDomainAssembly(this); + } +} + +void DomainAssembly::UnRegisterFromHostedAssembly() +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END + + if (GetPEAssembly()->HasHostAssembly()) + { + GetPEAssembly()->GetHostAssembly()->SetDomainAssembly(nullptr); + } +} + void DomainAssembly::Allocate() { CONTRACTL diff --git a/src/coreclr/vm/domainfile.h b/src/coreclr/vm/domainfile.h index 49a8344b21e84..ce36bdd738948 100644 --- a/src/coreclr/vm/domainfile.h +++ b/src/coreclr/vm/domainfile.h @@ -587,6 +587,8 @@ class DomainAssembly : public DomainFile void Allocate(); void DeliverSyncEvents(); void DeliverAsyncEvents(); + void RegisterWithHostedAssembly(); + void UnRegisterFromHostedAssembly(); #endif public: From 3e4e39e3fb7e47ef9df0facf5edaf24bc89cfb40 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 6 Nov 2021 19:12:35 -0700 Subject: [PATCH 2/3] Removes SHash::AddPhases - support for callouts in hosted scenarios. --- src/coreclr/inc/shash.h | 97 +---------------- src/coreclr/inc/shash.inl | 216 -------------------------------------- 2 files changed, 3 insertions(+), 310 deletions(-) diff --git a/src/coreclr/inc/shash.h b/src/coreclr/inc/shash.h index 163e2e8c4dfdb..f2ef79e5c1738 100644 --- a/src/coreclr/inc/shash.h +++ b/src/coreclr/inc/shash.h @@ -264,11 +264,6 @@ class EMPTY_BASES_DECL SHash : public TRAITS // NoThrow version of CheckGrowth function. Returns FALSE on failure. BOOL CheckGrowthNoThrow(); - // See if it is OK to grow the hash table by one element. If not, allocate new - // hash table and return it together with its size *pcNewSize (used by code:AddPhases). - // Returns NULL if there already is space for one element. - element_t * CheckGrowth_OnlyAllocateNewTable(count_t * pcNewSize); - // Allocates new resized hash table for growth. Does not update the hash table on the object. // The new size is computed based on the current population, growth factor, and maximum density factor. element_t * Grow_OnlyAllocateNewTable(count_t * pcNewSize); @@ -278,7 +273,7 @@ class EMPTY_BASES_DECL SHash : public TRAITS // Utility function to allocate new table (does not copy the values into it yet). Returns the size of new table in // *pcNewTableSize (finds next prime). - // Phase 1 of code:Reallocate - it is split to support code:AddPhases. + // Phase 1 of code:Reallocate element_t * AllocateNewTable(count_t requestedSize, count_t * pcNewTableSize); // NoThrow version of AllocateNewTable utility function. Returns NULL on failure. @@ -287,11 +282,11 @@ class EMPTY_BASES_DECL SHash : public TRAITS // Utility function to replace old table with newly allocated table (as allocated by // code:AllocateNewTable). Copies all 'old' values into the new table first. // Returns the old table. Caller is expected to delete it (via code:DeleteOldTable). - // Phase 2 of code:Reallocate - it is split to support code:AddPhases. + // Phase 2 of code:Reallocate element_t * ReplaceTable(element_t * newTable, count_t newTableSize); // Utility function to delete old table (as returned by code:ReplaceTable). - // Phase 3 of code:Reallocate - it is split to support code:AddPhases. + // Phase 3 of code:Reallocate void DeleteOldTable(element_t * oldTable); // Utility function that does not call code:CheckGrowth. @@ -502,92 +497,6 @@ class EMPTY_BASES_DECL SHash : public TRAITS } }; - // Wrapper and holder for adding an element to the hash table. Useful for Add operations that have to happen - // under a rare lock that does not allow call out into host. - // There are 3 phases: - // 1. code:PreallocateForAdd ... Can allocate memory (calls into host). - // 2. code:Add ... Adds one element (does NOT call into host). - // or code:AddNothing_PublishPreallocatedTable ... Publishes the pre-allocated memory from step #1 (if any). - // 3. code:DeleteOldTable (or destructor) ... Can delete the old memory (calls into host). - // Example: - // CrstHolder lockAdd(&crstLockForAdd); // Serialize all Add operations. - // HostAssemblyMap::AddPhases addCall; - // addCall.PreallocateForAdd(&shash); // 1. Allocates memory for one Add call (if needed). addCall serves as holder for the allocated memory. - // { - // // We cannot call out into host from ForbidSuspend region (i.e. no allocations/deallocations). - // ForbidSuspendThreadHolder suspend; // Required by the 'special' read-lock - // { - // CrstHolder lock(&crstLock); - // if (some_condition) - // { // 2a. Add item. This may replace SHash inner table with the one pre-allocated in step 1. - // addCall.Add(shashItem); - // } - // else - // { // 2b. Skip adding item. This may replace SHash inner table with the one pre-allocated in step 1. - // addCall.AddNothing_PublishPreallocatedTable(); - // } - // } - // } - // addCall.DeleteOldTable(); // 3. Cleanup old table memory from shash (if it was replaced by pre-allocated table in step 2). - // // Note: addCall destructor would take care of deleting the memory as well. - class AddPhases - { - public: - AddPhases(); - ~AddPhases(); - - // Prepares object for one call to code:Add. Pre-allocates new table memory if needed, does not publish - // the table yet (it is kept ready only in this holder for call to code:Add). - // Calls out into host. - void PreallocateForAdd(SHash * pHash); - - // Add an element to the hash table. This will never replace an element; multiple elements may be stored - // with the same key. - // Will use/publish pre-allocated memory from code:PreallocateForAdd. - // Does not call out into host. - // Only one Add* method can be called once per object! (Create a new object for each call) - void Add(const element_t & element); - - // Element will not be added to the hash table. - // Will use/publish pre-allocated memory from code:PreallocateForAdd. - // Does not call out into host. - // Only one Add* method can be called once per object! (Create a new object for each call) - void AddNothing_PublishPreallocatedTable(); - - // Deletes old table if it was replaced by call to code:Add or code:AddNothing_PublishPreallocatedTable. - // Calls out into host. - void DeleteOldTable(); - - private: - SHash * m_pHash; - element_t * m_newTable; - count_t m_newTableSize; - element_t * m_oldTable; - - #ifdef _DEBUG - PTR_element_t dbg_m_table; - count_t dbg_m_tableSize; - count_t dbg_m_tableCount; - count_t dbg_m_tableOccupied; - count_t dbg_m_tableMax; - BOOL dbg_m_fAddCalled; - #endif //_DEBUG - }; // class SHash::AddPhases - - // Adds an entry to the hash table according to the guidelines above for - // avoiding a callout to the host while the read lock is held. - // Returns true if elem was added to the map, otherwise false. - // When elem was not added (false is returned), and if ppStoredElem is non-null, - // then it is set to point to the value in the map. - template - bool CheckAddInPhases( - element_t const & elem, - LockT & lock, - AddLockT & addLock, - IUnknown * addRefObject = nullptr); private: diff --git a/src/coreclr/inc/shash.inl b/src/coreclr/inc/shash.inl index ef4e79ed4cf39..d5c2bd41d781e 100644 --- a/src/coreclr/inc/shash.inl +++ b/src/coreclr/inc/shash.inl @@ -374,26 +374,6 @@ BOOL SHash::CheckGrowthNoThrow() RETURN result; } -template -typename SHash::element_t * -SHash::CheckGrowth_OnlyAllocateNewTable(count_t * pcNewSize) -{ - CONTRACT(element_t *) - { - THROWS; - GC_NOTRIGGER; - INSTANCE_CHECK; - } - CONTRACT_END; - - if (m_tableOccupied == m_tableMax) - { - RETURN Grow_OnlyAllocateNewTable(pcNewSize); - } - - RETURN NULL; -} - template void SHash::Grow() { @@ -898,202 +878,6 @@ COUNT_T SHash::NextPrime(COUNT_T number) ThrowOutOfMemory(); } -template -SHash::AddPhases::AddPhases() -{ - LIMITED_METHOD_CONTRACT; - - m_pHash = NULL; - m_newTable = NULL; - m_newTableSize = 0; - m_oldTable = NULL; - - INDEBUG(dbg_m_fAddCalled = FALSE;) -} - -template -SHash::AddPhases::~AddPhases() -{ - LIMITED_METHOD_CONTRACT; - - if (m_newTable != NULL) - { // The new table was not applied to the hash yet - _ASSERTE((m_pHash != NULL) && (m_newTableSize != 0) && (m_oldTable == NULL)); - - delete [] m_newTable; - } - DeleteOldTable(); -} - -template -void SHash::AddPhases::PreallocateForAdd(SHash * pHash) -{ - CONTRACT_VOID - { - THROWS; - GC_NOTRIGGER; - } - CONTRACT_END; - - _ASSERTE((m_pHash == NULL) && (m_newTable == NULL) && (m_newTableSize == 0) && (m_oldTable == NULL)); - - m_pHash = pHash; - // May return NULL if the allocation was not needed - m_newTable = m_pHash->CheckGrowth_OnlyAllocateNewTable(&m_newTableSize); - -#ifdef _DEBUG - dbg_m_table = pHash->m_table; - dbg_m_tableSize = pHash->m_tableSize; - dbg_m_tableCount = pHash->m_tableCount; - dbg_m_tableOccupied = pHash->m_tableOccupied; - dbg_m_tableMax = pHash->m_tableMax; -#endif //_DEBUG - - RETURN; -} - -template -void SHash::AddPhases::Add(const element_t & element) -{ - CONTRACT_VOID - { - NOTHROW_UNLESS_TRAITS_THROWS; - GC_NOTRIGGER; - } - CONTRACT_END; - - _ASSERTE((m_pHash != NULL) && (m_oldTable == NULL)); - // Add can be called only once on this object - _ASSERTE(!dbg_m_fAddCalled); - - // Check that the hash table didn't change since call to code:PreallocateForAdd - _ASSERTE(dbg_m_table == m_pHash->m_table); - _ASSERTE(dbg_m_tableSize == m_pHash->m_tableSize); - _ASSERTE(dbg_m_tableCount >= m_pHash->m_tableCount); // Remove operation might have removed elements - _ASSERTE(dbg_m_tableOccupied == m_pHash->m_tableOccupied); - _ASSERTE(dbg_m_tableMax == m_pHash->m_tableMax); - - if (m_newTable != NULL) - { // We have pre-allocated table from code:PreallocateForAdd, use it. - _ASSERTE(m_newTableSize != 0); - - // May return NULL if there was not table allocated yet - m_oldTable = m_pHash->ReplaceTable(m_newTable, m_newTableSize); - - m_newTable = NULL; - m_newTableSize = 0; - } - // We know that we have enough space, direcly add the element - m_pHash->Add_GrowthChecked(element); - - INDEBUG(dbg_m_fAddCalled = TRUE;) - - RETURN; -} - -template -void SHash::AddPhases::AddNothing_PublishPreallocatedTable() -{ - CONTRACT_VOID - { - NOTHROW_UNLESS_TRAITS_THROWS; - GC_NOTRIGGER; - } - CONTRACT_END; - - _ASSERTE((m_pHash != NULL) && (m_oldTable == NULL)); - // Add can be called only once on this object - _ASSERTE(!dbg_m_fAddCalled); - - // Check that the hash table didn't change since call to code:PreallocateForAdd - _ASSERTE(dbg_m_table == m_pHash->m_table); - _ASSERTE(dbg_m_tableSize == m_pHash->m_tableSize); - _ASSERTE(dbg_m_tableCount >= m_pHash->m_tableCount); // Remove operation might have removed elements - _ASSERTE(dbg_m_tableOccupied == m_pHash->m_tableOccupied); - _ASSERTE(dbg_m_tableMax == m_pHash->m_tableMax); - - if (m_newTable != NULL) - { // We have pre-allocated table from code:PreallocateForAdd, use it. - _ASSERTE(m_newTableSize != 0); - - // May return NULL if there was not table allocated yet - m_oldTable = m_pHash->ReplaceTable(m_newTable, m_newTableSize); - - m_newTable = NULL; - m_newTableSize = 0; - } - - INDEBUG(dbg_m_fAddCalled = TRUE;) - - RETURN; -} - -template -void SHash::AddPhases::DeleteOldTable() -{ - LIMITED_METHOD_CONTRACT; - - if (m_oldTable != NULL) - { - _ASSERTE((m_pHash != NULL) && (m_newTable == NULL) && (m_newTableSize == 0)); - - delete [] m_oldTable; - m_oldTable = NULL; - } -} - -template -template -bool SHash::CheckAddInPhases( - element_t const & elem, - LockT & lock, - AddLockT & addLock, - IUnknown * addRefObject) -{ - CONTRACTL - { - THROWS; - GC_NOTRIGGER; - INSTANCE_CHECK; - } - CONTRACTL_END; - - AddLockHolderT hAddLock(&addLock); - AddPhases addCall; - - // 1. Preallocate one element - addCall.PreallocateForAdd(this); - { - // 2. Take the reader lock. Host callouts now forbidden. - LockHolderT hLock(&lock); - - element_t const * pEntry = LookupPtr(TRAITS::GetKey(elem)); - if (pEntry != nullptr) - { - // 3a. Use the newly allocated table (if any) to avoid later redundant allocation. - addCall.AddNothing_PublishPreallocatedTable(); - return false; - } - else - { - // 3b. Add the element to the hash table. - addCall.Add(elem); - - if (addRefObject != nullptr) - { - clr::SafeAddRef(addRefObject); - } - - return true; - } - } - - // 4. addCall's destructor will take care of any required cleanup. -} - template BOOL MapSHash::Lookup(KEY key, VALUE* pValue) const { From a23468fee13aefbd6ebae4fbe8f631cf5e1d0e14 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 6 Nov 2021 19:26:18 -0700 Subject: [PATCH 3/3] typos --- src/coreclr/vm/domainfile.cpp | 8 ++++---- src/coreclr/vm/domainfile.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/domainfile.cpp b/src/coreclr/vm/domainfile.cpp index 8938aea83765d..19de3c58c4bc9 100644 --- a/src/coreclr/vm/domainfile.cpp +++ b/src/coreclr/vm/domainfile.cpp @@ -723,7 +723,7 @@ DomainAssembly::~DomainAssembly() if (m_fHostAssemblyPublished) { // Remove association first. - UnRegisterFromHostedAssembly(); + UnRegisterFromHostAssembly(); } ModuleIterator i = IterateModules(kModIterIncludeLoading); @@ -853,11 +853,11 @@ void DomainAssembly::Begin() m_pDomain->AddAssembly(this); } // Make it possible to find this DomainAssembly object from associated BINDER_SPACE::Assembly. - RegisterWithHostedAssembly(); + RegisterWithHostAssembly(); m_fHostAssemblyPublished = true; } -void DomainAssembly::RegisterWithHostedAssembly() +void DomainAssembly::RegisterWithHostAssembly() { CONTRACTL { @@ -873,7 +873,7 @@ void DomainAssembly::RegisterWithHostedAssembly() } } -void DomainAssembly::UnRegisterFromHostedAssembly() +void DomainAssembly::UnRegisterFromHostAssembly() { CONTRACTL { diff --git a/src/coreclr/vm/domainfile.h b/src/coreclr/vm/domainfile.h index ce36bdd738948..7650cedbf13db 100644 --- a/src/coreclr/vm/domainfile.h +++ b/src/coreclr/vm/domainfile.h @@ -587,8 +587,8 @@ class DomainAssembly : public DomainFile void Allocate(); void DeliverSyncEvents(); void DeliverAsyncEvents(); - void RegisterWithHostedAssembly(); - void UnRegisterFromHostedAssembly(); + void RegisterWithHostAssembly(); + void UnRegisterFromHostAssembly(); #endif public: