diff --git a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj index c9c3bca531be0..99a4a8ce520d8 100644 --- a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -201,8 +201,6 @@ - - diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CrossLoaderAllocatorHashHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CrossLoaderAllocatorHashHelpers.cs deleted file mode 100644 index 9bb4628a3be64..0000000000000 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CrossLoaderAllocatorHashHelpers.cs +++ /dev/null @@ -1,35 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.InteropServices; - -namespace System.Runtime.CompilerServices -{ - /// - /// Managed structure used by CrossLoaderAllocatorHeap to isolate per LoaderAllocator - /// data. - /// - [StructLayout(LayoutKind.Sequential)] - internal sealed class LAHashDependentHashTracker - { - private GCHandle _dependentHandle; - private IntPtr _loaderAllocator; - - ~LAHashDependentHashTracker() - { - if (_dependentHandle.IsAllocated) - _dependentHandle.Free(); - } - } - - /// - /// Managed structure used by CrossLoaderAllocatorHeap to hold a set of references - /// to LAHashDependentHashTracker's - /// - [StructLayout(LayoutKind.Sequential)] - internal sealed class LAHashKeyToTrackers - { - private object? _trackerOrTrackerSet; - private object? _laLocalKeyValueStore; - } -} diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/GCHeapHash.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/GCHeapHash.cs deleted file mode 100644 index 82f99197a6a7e..0000000000000 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/GCHeapHash.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.InteropServices; - -namespace System.Runtime.CompilerServices -{ - /// - /// Managed structure used by GCHeapHash in CLR to provide a hashtable manipulated - /// by C++ runtime code which manages its memory in the GC heap. - /// - [StructLayout(LayoutKind.Sequential)] - internal sealed class GCHeapHash - { - private Array? _data; - private int _count; - private int _deletedCount; - } -} diff --git a/src/coreclr/inc/CrstTypes.def b/src/coreclr/inc/CrstTypes.def index 279118f3012ae..d4f8118fc8e42 100644 --- a/src/coreclr/inc/CrstTypes.def +++ b/src/coreclr/inc/CrstTypes.def @@ -544,7 +544,7 @@ Crst InlineTrackingMap End Crst JitInlineTrackingMap - AcquiredBefore CodeVersioning ThreadStore LoaderAllocator + AcquiredBefore CodeVersioning ThreadStore MethodDescBackpatchInfoTracker End Crst EventPipe @@ -568,8 +568,8 @@ Crst COMCallWrapper End Crst MethodDescBackpatchInfoTracker - AcquiredBefore FuncPtrStubs ThreadStore SystemDomain - AcquiredAfter ReJITGlobalRequest + AcquiredBefore FuncPtrStubs + AcquiredAfter ReJITGlobalRequest ThreadStore SystemDomain End Crst NativeImageEagerFixups diff --git a/src/coreclr/inc/crsttypes.h b/src/coreclr/inc/crsttypes.h index 815b263646617..93ea7f8847d59 100644 --- a/src/coreclr/inc/crsttypes.h +++ b/src/coreclr/inc/crsttypes.h @@ -200,7 +200,7 @@ int g_rgCrstLevelMap[] = 7, // CrstISymUnmanagedReader 11, // CrstJit 0, // CrstJitGenericHandleCache - 16, // CrstJitInlineTrackingMap + 13, // CrstJitInlineTrackingMap 4, // CrstJitPatchpoint -1, // CrstJitPerf 6, // CrstJumpStubCache @@ -210,7 +210,7 @@ int g_rgCrstLevelMap[] = 16, // CrstLoaderAllocatorReferences 3, // CrstLoaderHeap 3, // CrstManagedObjectWrapperMap - 14, // CrstMethodDescBackpatchInfoTracker + 10, // CrstMethodDescBackpatchInfoTracker 5, // CrstModule 15, // CrstModuleFixup 4, // CrstModuleLookupTable @@ -231,7 +231,7 @@ int g_rgCrstLevelMap[] = 0, // CrstRCWCleanupList 10, // CrstReadyToRunEntryPointToMethodDescMap 8, // CrstReflection - 17, // CrstReJITGlobalRequest + 14, // CrstReJITGlobalRequest 4, // CrstRetThunkCache 3, // CrstSavedExceptionInfo 0, // CrstSaveModuleProfileData diff --git a/src/coreclr/inc/shash.h b/src/coreclr/inc/shash.h index f2ef79e5c1738..c00fc72c17e64 100644 --- a/src/coreclr/inc/shash.h +++ b/src/coreclr/inc/shash.h @@ -71,9 +71,14 @@ // default traits if it can be assigned from 0. // static const bool IsDeleted(const ELEMENT &e) Compare element with Deleted sentinal value. May be inherited from the // default traits if it can be assigned from -1. +// static bool ShouldDelete(const ELEMENT &e) Called in addition to IsDeleted() when s_supports_autoremove is true, see more +// information there. // // static void OnDestructPerEntryCleanupAction(ELEMENT& e) Called on every element when in hashtable destructor. // s_DestructPerEntryCleanupAction must be set to true if implemented. +// static void OnRemovePerEntryCleanupAction(ELEMENT& e) Called when an element is removed from the hashtable, including when +// the hashtable is destructed. s_RemovePerEntryCleanupAction must be set to true +// if implemented. // // s_growth_factor_numerator // s_growth_factor_denominator Factor to grow allocation (numerator/denominator). @@ -92,7 +97,19 @@ // in that there may be more copies of the template instantiated through // the system as different variants are used. // +// s_supports_autoremove When autoremove is supported, ShouldDelete() is called in addition to +// IsDeleted() in any situation that involves walking the table's elements, to +// determine if an element should be deleted. It enables the hash table to +// self-clean elements whose underlying lifetime may be controlled externally. Note +// that since some lookup/iteration operations are const (can operate on a +// "const SHash"), when autoremove is supported, any such const operation may still +// modify the hash table. If this is set to true, s_supports_remove must also be +// true. +// // s_DestructPerEntryCleanupAction Set to true if OnDestructPerEntryCleanupAction has non-empty implementation. +// s_RemovePerEntryCleanupAction Set to true if OnRemovePerEntryCleanupAction has non-empty implementation. +// Only one of s_DestructPerEntryCleanupAction and s_RemovePerEntryCleanupAction +// may be set to true. // // DefaultHashTraits provides defaults for seldomly customized values in traits classes. @@ -115,15 +132,20 @@ class DefaultSHashTraits static const COUNT_T s_minimum_allocation = 7; static const bool s_supports_remove = true; + static const bool s_supports_autoremove = false; static ELEMENT Null() { return (ELEMENT)(TADDR)0; } static ELEMENT Deleted() { return (ELEMENT)(TADDR)-1; } static bool IsNull(const ELEMENT &e) { return e == (ELEMENT)(TADDR)0; } static bool IsDeleted(const ELEMENT &e) { return e == (ELEMENT)(TADDR)-1; } + static bool ShouldDelete(const ELEMENT &e) { return false; } static inline void OnDestructPerEntryCleanupAction(const ELEMENT& e) { /* Do nothing */ } static const bool s_DestructPerEntryCleanupAction = false; + static void OnRemovePerEntryCleanupAction(const ELEMENT &e) {} + static const bool s_RemovePerEntryCleanupAction = false; + static const bool s_NoThrow = true; // No defaults - must specify: @@ -177,6 +199,10 @@ class EMPTY_BASES_DECL SHash : public TRAITS const element_t* LookupPtr(key_t key) const; + // Pointer-based flavor to replace an existing element (allows efficient access to tables of structures) + + void ReplacePtr(const element_t *elementPtr, const element_t &newElement, bool invokeCleanupAction = true); + // Add an element to the hash table. This will never replace an element; multiple // elements may be stored with the same key. @@ -298,7 +324,7 @@ class EMPTY_BASES_DECL SHash : public TRAITS // it is perfectly fine for the element to be a duplicate - if so it // is added an additional time. Returns TRUE if a new empty spot was used; // FALSE if an existing deleted slot. - static BOOL Add(element_t *table, count_t tableSize, const element_t &element); + BOOL Add(element_t *table, count_t tableSize, const element_t &element); // Utility function to add a new element to the hash table, if no element with the same key // is already there. Otherwise, it will replace the existing element. This has the effect of @@ -308,7 +334,7 @@ class EMPTY_BASES_DECL SHash : public TRAITS // Utility function to find the first element with the given key in // the hash table. - static const element_t* Lookup(PTR_element_t table, count_t tableSize, key_t key); + const element_t* Lookup(PTR_element_t table, count_t tableSize, key_t key) const; // Utility function to remove the first element with the given key // in the hash table. @@ -334,13 +360,17 @@ class EMPTY_BASES_DECL SHash : public TRAITS // Some compilers won't compile the separate implementation in shash.inl protected: + SHash *m_hash; PTR_element_t m_table; count_t m_tableSize; count_t m_index; - + // Iteration may modify the hash table if s_supports_autoremove is true. Since it typically does not modify the hash + // table, it should be possible to iterate over a "const SHash". So this takes a "const SHash *" and casts away the + // const to support autoremove. Index(const SHash *hash, BOOL begin) - : m_table(hash->m_table), + : m_hash(const_cast(hash)), + m_table(hash->m_table), m_tableSize(hash->m_tableSize), m_index(begin ? 0 : m_tableSize) { @@ -358,9 +388,24 @@ class EMPTY_BASES_DECL SHash : public TRAITS { LIMITED_METHOD_CONTRACT; - if (m_index < m_tableSize) - if (TRAITS::IsNull(m_table[m_index]) || TRAITS::IsDeleted(m_table[m_index])) - Next(); + if (m_index >= m_tableSize) + { + return; + } + + if (!TRAITS::IsNull(m_table[m_index]) && !TRAITS::IsDeleted(m_table[m_index])) + { + if (TRAITS::s_supports_autoremove && TRAITS::ShouldDelete(m_table[m_index])) + { + m_hash->RemoveElement(m_table, m_tableSize, &m_table[m_index]); + } + else + { + return; + } + } + + Next(); } void Next() @@ -376,7 +421,16 @@ class EMPTY_BASES_DECL SHash : public TRAITS if (m_index >= m_tableSize) break; if (!TRAITS::IsNull(m_table[m_index]) && !TRAITS::IsDeleted(m_table[m_index])) - break; + { + if (TRAITS::s_supports_autoremove && TRAITS::ShouldDelete(m_table[m_index])) + { + m_hash->RemoveElement(m_table, m_tableSize, &m_table[m_index]); + } + else + { + break; + } + } } } @@ -442,11 +496,26 @@ class EMPTY_BASES_DECL SHash : public TRAITS m_increment = (hash % (this->m_tableSize-1)) + 1; // Find first valid element + if (TRAITS::IsNull(this->m_table[this->m_index])) + { this->m_index = this->m_tableSize; - else if (TRAITS::IsDeleted(this->m_table[this->m_index]) - || !TRAITS::Equals(m_key, TRAITS::GetKey(this->m_table[this->m_index]))) - Next(); + return; + } + + if (!TRAITS::IsDeleted(this->m_table[this->m_index])) + { + if (TRAITS::s_supports_autoremove && TRAITS::ShouldDelete(this->m_table[this->m_index])) + { + this->m_hash->RemoveElement(this->m_table, this->m_tableSize, &this->m_table[this->m_index]); + } + else if (TRAITS::Equals(m_key, TRAITS::GetKey(this->m_table[this->m_index]))) + { + return; + } + } + + Next(); } } @@ -466,8 +535,18 @@ class EMPTY_BASES_DECL SHash : public TRAITS break; } - if (!TRAITS::IsDeleted(this->m_table[this->m_index]) - && TRAITS::Equals(m_key, TRAITS::GetKey(this->m_table[this->m_index]))) + if (TRAITS::IsDeleted(this->m_table[this->m_index])) + { + continue; + } + + if (TRAITS::s_supports_autoremove && TRAITS::ShouldDelete(this->m_table[this->m_index])) + { + this->m_hash->RemoveElement(this->m_table, this->m_tableSize, &this->m_table[this->m_index]); + continue; + } + + if (TRAITS::Equals(m_key, TRAITS::GetKey(this->m_table[this->m_index]))) { break; } diff --git a/src/coreclr/inc/shash.inl b/src/coreclr/inc/shash.inl index d5c2bd41d781e..65ca26ce18fc3 100644 --- a/src/coreclr/inc/shash.inl +++ b/src/coreclr/inc/shash.inl @@ -24,6 +24,9 @@ SHash::SHash() static_assert_no_msg(SHash::s_growth_factor_numerator > SHash::s_growth_factor_denominator); static_assert_no_msg(SHash::s_density_factor_numerator < SHash::s_density_factor_denominator); #endif + + static_assert_no_msg(TRAITS::s_supports_remove || !TRAITS::s_supports_autoremove); + static_assert_no_msg(!TRAITS::s_DestructPerEntryCleanupAction || !TRAITS::s_RemovePerEntryCleanupAction); } template @@ -38,6 +41,13 @@ SHash::~SHash() TRAITS::OnDestructPerEntryCleanupAction(*i); } } + else if (TRAITS::s_RemovePerEntryCleanupAction) + { + for (Iterator i = Begin(); i != End(); i++) + { + TRAITS::OnRemovePerEntryCleanupAction(*i); + } + } delete [] m_table; } @@ -90,6 +100,33 @@ const typename SHash::element_t * SHash::LookupPtr(key_t key) co RETURN Lookup(m_table, m_tableSize, key); } +template +void SHash::ReplacePtr(const element_t *elementPtr, const element_t &newElement, bool invokeCleanupAction) +{ + CONTRACT_VOID + { + NOTHROW; + GC_NOTRIGGER; + INSTANCE_CHECK; + PRECONDITION(m_table <= elementPtr); + PRECONDITION(elementPtr < m_table + m_tableSize); + PRECONDITION(!TRAITS::IsNull(*elementPtr)); + PRECONDITION(!TRAITS::IsDeleted(*elementPtr)); + PRECONDITION(!TRAITS::IsNull(newElement)); + PRECONDITION(!TRAITS::IsDeleted(newElement)); + PRECONDITION(TRAITS::Equals(TRAITS::GetKey(newElement), TRAITS::GetKey(*elementPtr))); + } + CONTRACT_END; + + if (TRAITS::s_RemovePerEntryCleanupAction && invokeCleanupAction) + { + TRAITS::OnRemovePerEntryCleanupAction(*elementPtr); + } + + *const_cast(elementPtr) = newElement; + RETURN; +} + template void SHash::Add(const element_t & element) { @@ -277,11 +314,11 @@ void SHash::RemoveAll() } CONTRACT_END; - if (TRAITS::s_DestructPerEntryCleanupAction) + if (TRAITS::s_RemovePerEntryCleanupAction) { for (Iterator i = Begin(); i != End(); i++) { - TRAITS::OnDestructPerEntryCleanupAction(*i); + TRAITS::OnRemovePerEntryCleanupAction(*i); } } @@ -495,10 +532,18 @@ void SHash::ForEach(Functor &functor) for (count_t i = 0; i < m_tableSize; i++) { element_t element = m_table[i]; - if (!TRAITS::IsNull(element) && !TRAITS::IsDeleted(element)) + if (TRAITS::IsNull(element) || TRAITS::IsDeleted(element)) + { + continue; + } + + if (TRAITS::s_supports_autoremove && TRAITS::ShouldDelete(element)) { - functor(element); + RemoveElement(m_table, m_tableSize, &m_table[i]); + continue; } + + functor(element); } } @@ -587,8 +632,9 @@ SHash::ReplaceTable(element_t * newTable, count_t newTableSize) for (Iterator i = Begin(), end = End(); i != end; i++) { const element_t & cur = (*i); - if (!TRAITS::IsNull(cur) && !TRAITS::IsDeleted(cur)) - Add(newTable, newTableSize, cur); + _ASSERTE(!TRAITS::IsNull(cur)); + _ASSERTE(!TRAITS::IsDeleted(cur)); + Add(newTable, newTableSize, cur); } m_table = PTR_element_t(newTable); @@ -619,7 +665,7 @@ SHash::DeleteOldTable(element_t * oldTable) } template -const typename SHash::element_t * SHash::Lookup(PTR_element_t table, count_t tableSize, key_t key) +const typename SHash::element_t * SHash::Lookup(PTR_element_t table, count_t tableSize, key_t key) const { CONTRACT(const element_t *) { @@ -644,10 +690,16 @@ const typename SHash::element_t * SHash::Lookup(PTR_element_t ta if (TRAITS::IsNull(current)) RETURN NULL; - if (!TRAITS::IsDeleted(current) - && TRAITS::Equals(key, TRAITS::GetKey(current))) + if (!TRAITS::IsDeleted(current)) { - RETURN ¤t; + if (TRAITS::s_supports_autoremove && TRAITS::ShouldDelete(current)) + { + const_cast *>(this)->RemoveElement(table, tableSize, ¤t); + } + else if (TRAITS::Equals(key, TRAITS::GetKey(current))) + { + RETURN ¤t; + } } if (increment == 0) @@ -692,6 +744,13 @@ BOOL SHash::Add(element_t * table, count_t tableSize, const element_t & RETURN FALSE; } + if (TRAITS::s_supports_autoremove && TRAITS::ShouldDelete(current)) + { + RemoveElement(table, tableSize, ¤t); + table[index] = element; + RETURN FALSE; + } + if (increment == 0) increment = (hash % (tableSize-1)) + 1; @@ -733,6 +792,11 @@ void SHash::AddOrReplace(element_t *table, count_t tableSize, const elem } else if (TRAITS::Equals(key, TRAITS::GetKey(current))) { + if (TRAITS::s_RemovePerEntryCleanupAction) + { + TRAITS::OnRemovePerEntryCleanupAction(current); + } + table[index] = element; RETURN; } @@ -769,12 +833,13 @@ void SHash::Remove(element_t *table, count_t tableSize, key_t key) if (TRAITS::IsNull(current)) RETURN; - if (!TRAITS::IsDeleted(current) - && TRAITS::Equals(key, TRAITS::GetKey(current))) + if (!TRAITS::IsDeleted(current)) { - table[index] = TRAITS::Deleted(); - m_tableCount--; - RETURN; + if ((TRAITS::s_supports_autoremove && TRAITS::ShouldDelete(current)) || + TRAITS::Equals(key, TRAITS::GetKey(current))) + { + RemoveElement(table, tableSize, ¤t); + } } if (increment == 0) @@ -793,12 +858,17 @@ void SHash::RemoveElement(element_t *table, count_t tableSize, element_t { NOTHROW; GC_NOTRIGGER; - static_assert(TRAITS::s_supports_remove, "This SHash does not support remove operations."); + PRECONDITION(TRAITS::s_supports_remove); PRECONDITION(table <= element && element < table + tableSize); PRECONDITION(!TRAITS::IsNull(*element) && !TRAITS::IsDeleted(*element)); } CONTRACT_END; + if (TRAITS::s_RemovePerEntryCleanupAction) + { + TRAITS::OnRemovePerEntryCleanupAction(*element); + } + *element = TRAITS::Deleted(); m_tableCount--; RETURN; diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index 55076560a46a4..1e1563ee04291 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -1394,8 +1394,6 @@ void SystemDomain::LoadBaseSystemClasses() g_pDelegateClass = CoreLibBinder::GetClass(CLASS__DELEGATE); g_pMulticastDelegateClass = CoreLibBinder::GetClass(CLASS__MULTICAST_DELEGATE); - CrossLoaderAllocatorHashSetup::EnsureTypesLoaded(); - // further loading of nonprimitive types may need casting support. // initialize cast cache here. CastCache::Initialize(); diff --git a/src/coreclr/vm/callcounting.cpp b/src/coreclr/vm/callcounting.cpp index 2ff450334494b..344f68b17b5ca 100644 --- a/src/coreclr/vm/callcounting.cpp +++ b/src/coreclr/vm/callcounting.cpp @@ -553,20 +553,8 @@ bool CallCountingManager::SetCodeEntryPoint( { THROWS; GC_NOTRIGGER; - - // Backpatching entry point slots requires cooperative GC mode, see MethodDescBackpatchInfoTracker::Backpatch_Locked(). - // The code version manager's table lock is an unsafe lock that may be taken in any GC mode. The lock is taken in - // cooperative GC mode on other paths, so the caller must use the same ordering to prevent deadlock (switch to - // cooperative GC mode before taking the lock). PRECONDITION(!activeCodeVersion.IsNull()); - if (activeCodeVersion.GetMethodDesc()->MayHaveEntryPointSlotsToBackpatch()) - { - MODE_COOPERATIVE; - } - else - { - MODE_ANY; - } + MODE_ANY; } CONTRACTL_END; @@ -874,13 +862,7 @@ void CallCountingManager::CompleteCallCounting() TieredCompilationManager *tieredCompilationManager = appDomain->GetTieredCompilationManager(); CodeVersionManager *codeVersionManager = appDomain->GetCodeVersionManager(); - MethodDescBackpatchInfoTracker::ConditionalLockHolderForGCCoop slotBackpatchLockHolder; - - // Backpatching entry point slots requires cooperative GC mode, see - // MethodDescBackpatchInfoTracker::Backpatch_Locked(). The code version manager's table lock is an unsafe lock that - // may be taken in any GC mode. The lock is taken in cooperative GC mode on some other paths, so the same ordering - // must be used here to prevent deadlock. - GCX_COOP(); + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder; CodeVersionManager::LockHolder codeVersioningLockHolder; for (auto itEnd = s_callCountingManagers->End(), it = s_callCountingManagers->Begin(); it != itEnd; ++it) @@ -1002,8 +984,6 @@ void CallCountingManager::StopAndDeleteAllCallCountingStubs() TieredCompilationManager *tieredCompilationManager = GetAppDomain()->GetTieredCompilationManager(); - MethodDescBackpatchInfoTracker::ConditionalLockHolderForGCCoop slotBackpatchLockHolder; - ThreadSuspend::SuspendEE(ThreadSuspend::SUSPEND_OTHER); struct AutoRestartEE { @@ -1014,11 +994,7 @@ void CallCountingManager::StopAndDeleteAllCallCountingStubs() } } autoRestartEE; - // Backpatching entry point slots requires cooperative GC mode, see - // MethodDescBackpatchInfoTracker::Backpatch_Locked(). The code version manager's table lock is an unsafe lock that - // may be taken in any GC mode. The lock is taken in cooperative GC mode on some other paths, so the same ordering - // must be used here to prevent deadlock. - GCX_COOP(); + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder; CodeVersionManager::LockHolder codeVersioningLockHolder; // After the following, no method's entry point would be pointing to a call counting stub diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 8404413beb345..3800fc4df0a1c 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -666,6 +666,7 @@ void EEStartupHelper() Thread::StaticInitialize(); ThreadpoolMgr::StaticInitialize(); + JITInlineTrackingMap::StaticInitialize(); MethodDescBackpatchInfoTracker::StaticInitialize(); CodeVersionManager::StaticInitialize(); TieredCompilationManager::StaticInitialize(); diff --git a/src/coreclr/vm/codeversion.cpp b/src/coreclr/vm/codeversion.cpp index d59e13d45d45d..ffb0b73157a67 100644 --- a/src/coreclr/vm/codeversion.cpp +++ b/src/coreclr/vm/codeversion.cpp @@ -1475,7 +1475,7 @@ HRESULT CodeVersionManager::SetActiveILCodeVersions(ILCodeVersion* pActiveVersio CONTRACTL { NOTHROW; - GC_TRIGGERS; + GC_NOTRIGGER; MODE_PREEMPTIVE; CAN_TAKE_LOCK; PRECONDITION(CheckPointer(pActiveVersions)); @@ -1496,6 +1496,8 @@ HRESULT CodeVersionManager::SetActiveILCodeVersions(ILCodeVersion* pActiveVersio } #endif + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder; + // Assume that a new IL code version will be added for a method def, and that an instantiation may not yet have native code // for the default native code version CodeVersionManager::SetInitialNativeCodeVersionMayNotBeTheDefaultNativeCodeVersion(); @@ -1544,11 +1546,6 @@ HRESULT CodeVersionManager::SetActiveILCodeVersions(ILCodeVersion* pActiveVersio // step 3 - update each pre-existing method instantiation { - // Backpatching entry point slots requires cooperative GC mode, see - // MethodDescBackpatchInfoTracker::Backpatch_Locked(). The code version manager's table lock is an unsafe lock that - // may be taken in any GC mode. The lock is taken in cooperative GC mode on some other paths, so the same ordering - // must be used here to prevent deadlock. - GCX_COOP(); LockHolder codeVersioningLockHolder; for (DWORD i = 0; i < cActiveVersions; i++) @@ -1758,10 +1755,6 @@ PCODE CodeVersionManager::PublishVersionableCodeIfNecessary( NativeCodeVersion newActiveVersion; do { - bool mayHaveEntryPointSlotsToBackpatch = doPublish && pMethodDesc->MayHaveEntryPointSlotsToBackpatch(); - MethodDescBackpatchInfoTracker::ConditionalLockHolderForGCCoop slotBackpatchLockHolder( - mayHaveEntryPointSlotsToBackpatch); - // Try a faster check to see if we can avoid checking the currently active code version // - For the default code version, if a profiler is attached it may be notified of JIT events and may request rejit // synchronously on the same thread. In that case, for back-compat as described below, the returned code must be for @@ -1774,7 +1767,10 @@ PCODE CodeVersionManager::PublishVersionableCodeIfNecessary( { if (doPublish) { - pMethodDesc->TrySetInitialCodeEntryPointForVersionableMethod(pCode, mayHaveEntryPointSlotsToBackpatch); + bool mayHaveEntryPointSlotsToBackpatch2 = pMethodDesc->MayHaveEntryPointSlotsToBackpatch(); + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder2( + mayHaveEntryPointSlotsToBackpatch2); + pMethodDesc->TrySetInitialCodeEntryPointForVersionableMethod(pCode, mayHaveEntryPointSlotsToBackpatch2); } else { @@ -1785,11 +1781,8 @@ PCODE CodeVersionManager::PublishVersionableCodeIfNecessary( break; } - // Backpatching entry point slots requires cooperative GC mode, see - // MethodDescBackpatchInfoTracker::Backpatch_Locked(). The code version manager's table lock is an unsafe lock that - // may be taken in any GC mode. The lock is taken in cooperative GC mode on some other paths, so the same ordering - // must be used here to prevent deadlock. - GCX_MAYBE_COOP(mayHaveEntryPointSlotsToBackpatch); + bool mayHaveEntryPointSlotsToBackpatch = doPublish && pMethodDesc->MayHaveEntryPointSlotsToBackpatch(); + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder(mayHaveEntryPointSlotsToBackpatch); LockHolder codeVersioningLockHolder; hr = GetActiveILCodeVersion(pMethodDesc).GetOrCreateActiveNativeCodeVersion(pMethodDesc, &newActiveVersion); @@ -1880,21 +1873,9 @@ HRESULT CodeVersionManager::PublishNativeCodeVersion(MethodDesc* pMethod, Native { CONTRACTL { - GC_NOTRIGGER; - - // Backpatching entry point slots requires cooperative GC mode, see MethodDescBackpatchInfoTracker::Backpatch_Locked(). - // The code version manager's table lock is an unsafe lock that may be taken in any GC mode. The lock is taken in - // cooperative GC mode on other paths, so the caller must use the same ordering to prevent deadlock (switch to - // cooperative GC mode before taking the lock). - if (pMethod->MayHaveEntryPointSlotsToBackpatch()) - { - MODE_COOPERATIVE; - } - else - { - MODE_ANY; - } NOTHROW; + GC_NOTRIGGER; + MODE_ANY; } CONTRACTL_END; @@ -1941,7 +1922,7 @@ HRESULT CodeVersionManager::EnumerateClosedMethodDescs( CONTRACTL { NOTHROW; - GC_TRIGGERS; + GC_NOTRIGGER; MODE_PREEMPTIVE; CAN_TAKE_LOCK; PRECONDITION(CheckPointer(pMD, NULL_OK)); diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index ee95e5fd8a6d4..33f9d7e30d4d8 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1236,13 +1236,6 @@ DEFINE_CLASS(NULLABLE_COMPARER, CollectionsGeneric, NullableComparer`1) DEFINE_CLASS(INATTRIBUTE, Interop, InAttribute) -DEFINE_CLASS_U(CompilerServices, GCHeapHash, GCHeapHashObject) -DEFINE_FIELD_U(_data, GCHeapHashObject, _data) -DEFINE_FIELD_U(_count, GCHeapHashObject, _count) -DEFINE_FIELD_U(_deletedCount, GCHeapHashObject, _deletedCount) - -DEFINE_CLASS(GCHEAPHASH, CompilerServices, GCHeapHash) - DEFINE_CLASS(CASTHELPERS, CompilerServices, CastHelpers) DEFINE_FIELD(CASTHELPERS, TABLE, s_table) DEFINE_METHOD(CASTHELPERS, ISINSTANCEOFANY, IsInstanceOfAny, SM_PtrVoid_Obj_RetObj) @@ -1256,21 +1249,6 @@ DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, SM_Ptr DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_Array_Int_Obj_RetVoid) DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_Array_Int_PtrVoid_RetRefObj) -DEFINE_CLASS_U(CompilerServices, LAHashDependentHashTracker, LAHashDependentHashTrackerObject) -DEFINE_FIELD_U(_dependentHandle, LAHashDependentHashTrackerObject,_dependentHandle) -DEFINE_FIELD_U(_loaderAllocator, LAHashDependentHashTrackerObject,_loaderAllocator) - -DEFINE_CLASS(LAHASHDEPENDENTHASHTRACKER, CompilerServices, LAHashDependentHashTracker) -#if FOR_ILLINK -DEFINE_METHOD(LAHASHDEPENDENTHASHTRACKER, CTOR, .ctor, IM_RetVoid) -#endif - -DEFINE_CLASS_U(CompilerServices, LAHashKeyToTrackers, LAHashKeyToTrackersObject) -DEFINE_FIELD_U(_trackerOrTrackerSet, LAHashKeyToTrackersObject, _trackerOrTrackerSet) -DEFINE_FIELD_U(_laLocalKeyValueStore, LAHashKeyToTrackersObject, _laLocalKeyValueStore) - -DEFINE_CLASS(LAHASHKEYTOTRACKERS, CompilerServices, LAHashKeyToTrackers) - DEFINE_CLASS_U(System, GCMemoryInfoData, GCMemoryInfoData) DEFINE_FIELD_U(_highMemoryLoadThresholdBytes, GCMemoryInfoData, highMemLoadThresholdBytes) DEFINE_FIELD_U(_totalAvailableMemoryBytes, GCMemoryInfoData, totalAvailableMemoryBytes) diff --git a/src/coreclr/vm/crossloaderallocatorhash.h b/src/coreclr/vm/crossloaderallocatorhash.h index 2e01343dc1263..9f16cc07f21c6 100644 --- a/src/coreclr/vm/crossloaderallocatorhash.h +++ b/src/coreclr/vm/crossloaderallocatorhash.h @@ -4,7 +4,9 @@ #ifndef CROSSLOADERALLOCATORHASH_H #define CROSSLOADERALLOCATORHASH_H -#include "gcheaphashtable.h" +#define DISABLE_COPY(T) \ + T(const T &) = delete; \ + T &operator =(const T &) = delete class LoaderAllocator; @@ -14,53 +16,66 @@ class NoRemoveDefaultCrossLoaderAllocatorHashTraits public: typedef TKey_ TKey; typedef TValue_ TValue; + typedef COUNT_T TCount; - static bool IsNull(const TValue &value) { return value == NULL; } - static TValue NullValue() { return NULL; } + static const bool s_supports_remove = false; -#ifndef DACCESS_COMPILE - static void SetUsedEntries(TValue* pStartOfValuesData, DWORD entriesInArrayTotal, DWORD usedEntries); - static bool AddToValuesInHeapMemory(OBJECTREF *pKeyValueStore, const TKey& key, const TValue& value); -#endif //!DACCESS_COMPILE - static DWORD ComputeUsedEntries(OBJECTREF *pKeyValueStore, DWORD *pEntriesInArrayTotal); - template - static bool VisitKeyValueStore(OBJECTREF *pLoaderAllocatorRef, OBJECTREF *pKeyValueStore, Visitor &visitor); - static TKey ReadKeyFromKeyValueStore(OBJECTREF *pKeyValueStore); + // CrossLoaderAllocatorHash requires that a particular null value exist, which represents an empty value slot + static bool IsNullValue(const TValue &value) { return value == NULL; } + static TValue NullValue() { return NULL; } + + static BOOL KeyEquals(const TKey &k1, const TKey &k2) { return k1 == k2; } + static BOOL ValueEquals(const TValue &v1, const TValue &v2) { return v1 == v2; } + static TCount Hash(const TKey &k) { return (TCount)(size_t)k; } + + static LoaderAllocator *GetLoaderAllocator(const TKey &k) { return k->GetLoaderAllocator(); } }; template class DefaultCrossLoaderAllocatorHashTraits : public NoRemoveDefaultCrossLoaderAllocatorHashTraits { public: - typedef TKey_ TKey; - typedef TValue_ TValue; - -#ifndef DACCESS_COMPILE - static void DeleteValueInHeapMemory(OBJECTREF keyValueStore, const TValue& value); -#endif //!DACCESS_COMPILE + static const bool s_supports_remove = true; }; -struct GCHeapHashDependentHashTrackerHashTraits : public DefaultGCHeapHashTraits +// Base class for a native object that depends on a LoaderAllocator's lifetime +class LADependentNativeObject { - typedef LoaderAllocator* PtrTypeKey; +protected: + LADependentNativeObject() = default; - static INT32 Hash(PtrTypeKey *pValue); - static INT32 Hash(PTRARRAYREF arr, INT32 index); - static bool DoesEntryMatchKey(PTRARRAYREF arr, INT32 index, PtrTypeKey *pKey); - static bool IsDeleted(PTRARRAYREF arr, INT32 index, GCHEAPHASHOBJECTREF gcHeap); -}; +public: + virtual ~LADependentNativeObject() = default; -typedef GCHeapHash GCHeapHashDependentHashTrackerHash; + DISABLE_COPY(LADependentNativeObject); +}; -template -struct KeyToValuesGCHeapHashTraits : public DefaultGCHeapHashTraits +// A handle to an LADependentNativeObject that is registered with the LoaderAllocator. The handle would be cleared when the +// LoaderAllocator is collected. Appropriate locking must be used with this class and in the LoaderAllocator's code that clears +// handles. +class LADependentHandleToNativeObject { - template - static INT32 Hash(TKey *pValue); - static INT32 Hash(PTRARRAYREF arr, INT32 index); +private: + LADependentNativeObject *m_dependentObject; + +public: + LADependentHandleToNativeObject(LADependentNativeObject *dependentObject) : m_dependentObject(dependentObject) {} + ~LADependentHandleToNativeObject() { delete m_dependentObject; } - template - static bool DoesEntryMatchKey(PTRARRAYREF arr, INT32 index, TKey *pKey); + // See notes about synchronization in Clear() + LADependentNativeObject *GetDependentObject() const { return m_dependentObject; } + + void Clear() + { + _ASSERTE(m_dependentObject != nullptr); + + // The callers of GetDependentObject() and Clear() must ensure that they cannot run concurrently, and that a + // GetDependentObject() following a Clear() sees a null dependent object in a thread-safe manner + delete m_dependentObject; + m_dependentObject = nullptr; + } + + DISABLE_COPY(LADependentHandleToNativeObject); }; // Hashtable of key to a list of values where the key may live in a different loader allocator @@ -86,7 +101,16 @@ struct KeyToValuesGCHeapHashTraits : public DefaultGCHeapHashTraits // // VisitValuesOfKey will visit all values that have the same key. // +// IMPORTANT NOTE ABOUT SYNCHRONIZATION +// +// Any lock used to synchronize access to an instance of this data structure must also be +// acquired in AssemblyLoaderAllocator::CleanupDependentHandlesToNativeObjects() to ensure that +// while visiting values, the LoaderAllocator of the value remains alive inside the lock. +// Visited values must not be used outside the lock, as the LoaderAllocator of the value may +// may not be alive outside the lock. +// // IMPLEMENTATION DESIGN +// // This data structure is a series of hashtables and lists. // // In general, this data structure builds a set of values associated with a key per @@ -127,10 +151,246 @@ struct KeyToValuesGCHeapHashTraits : public DefaultGCHeapHashTraits template class CrossLoaderAllocatorHash { + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // Types used by CrossLoaderAllocatorHash + private: typedef typename TRAITS::TKey TKey; typedef typename TRAITS::TValue TValue; - typedef GCHeapHash> KeyToValuesGCHeapHash; + typedef typename TRAITS::TCount TCount; + + class KeyValueStoreOrLAHashKeyToTrackers + { + protected: + KeyValueStoreOrLAHashKeyToTrackers() = default; + + public: + virtual ~KeyValueStoreOrLAHashKeyToTrackers() = default; + + virtual bool IsLAHashKeyToTrackers() const { return false; } + }; + + class LAHashDependentHashTrackerOrTrackerSet + { + private: + const bool _isTrackerSet; + + protected: + LAHashDependentHashTrackerOrTrackerSet(bool isTrackerSet) : _isTrackerSet(isTrackerSet) {} + + public: + bool IsTrackerSet() const { return _isTrackerSet; } + }; + + class KeyValueStore : public KeyValueStoreOrLAHashKeyToTrackers + { + private: + const TCount _capacity; + const TKey _key; + TValue _values[0]; + + private: + KeyValueStore(TCount capacity, const TKey &key) : _capacity(capacity), _key(key) {} + + public: + static KeyValueStore *Create(TCount capacity, const TKey &key); + + TCount GetCapacity() const { return _capacity; } + TKey GetKey() const { return _key; } + TValue *GetValues() { return _values; } + }; + + class LAHashKeyToTrackers : public KeyValueStoreOrLAHashKeyToTrackers + { + public: + LAHashDependentHashTrackerOrTrackerSet *_trackerOrTrackerSet; + + // _laLocalKeyValueStore holds an object that represents a Key value (which must always be valid for the lifetime of the + // CrossLoaderAllocatorHeapHash, and the values which must also be valid for that entire lifetime. When a value might + // have a shorter lifetime it is accessed through the _trackerOrTrackerSet variable, which allows access to hashtables which + // are associated with that remote loaderallocator through a dependent handle, so that lifetime can be managed. + KeyValueStore *_laLocalKeyValueStore; + + LAHashKeyToTrackers(KeyValueStore *laLocalKeyValueStore) + : _trackerOrTrackerSet(NULL), _laLocalKeyValueStore(laLocalKeyValueStore) + {} + + virtual ~LAHashKeyToTrackers() override; + + virtual bool IsLAHashKeyToTrackers() const override { return true; } + }; + + class EMPTY_BASES_DECL KeyToValuesHashTraits : public DefaultSHashTraits + { + private: + typedef DefaultSHashTraits Base; + + public: + typedef TCount count_t; + typedef TKey key_t; + + static const bool s_supports_remove = TRAITS::s_supports_remove; + static const bool s_RemovePerEntryCleanupAction = true; + + static KeyValueStoreOrLAHashKeyToTrackers *Deleted() + { + if (s_supports_remove) + { + return Base::Deleted(); + } + + UNREACHABLE(); + } + + static bool IsDeleted(KeyValueStoreOrLAHashKeyToTrackers *e) { return s_supports_remove && Base::IsDeleted(e); } + static void OnRemovePerEntryCleanupAction(KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry) { delete hashKeyEntry; } + static TKey GetKey(KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry); + static BOOL Equals(const TKey &k1, const TKey &k2) { return TRAITS::KeyEquals(k1, k2); } + static TCount Hash(const TKey &k) { return TRAITS::Hash(k); } + + #ifndef DACCESS_COMPILE + static void SetUsedEntries(KeyValueStore *keyValueStore, TCount entriesInArrayTotal, TCount usedEntries); + static bool AddToValuesInHeapMemory( + KeyValueStore **pKeyValueStore, + NewHolder &keyValueStoreHolder, + const TKey& key, + const TValue& value); + #endif // !DACCESS_COMPILE + static TCount ComputeUsedEntries(KeyValueStore *keyValueStore, TCount *pEntriesInArrayTotal); + template + static bool VisitKeyValueStore(LoaderAllocator *loaderAllocator, KeyValueStore *keyValueStore, Visitor &visitor); + #ifndef DACCESS_COMPILE + static void DeleteValueInHeapMemory(KeyValueStore *keyValueStore, const TValue& value); + #endif // !DACCESS_COMPILE + }; + + typedef SHash KeyToValuesHash; + + class LADependentKeyToValuesHash : public LADependentNativeObject + { + private: + KeyToValuesHash _keyToValuesHash; + + public: + virtual ~LADependentKeyToValuesHash() override = default; + + KeyToValuesHash *GetKeyToValuesHash() { return &_keyToValuesHash; } + }; + + class LAHashDependentHashTracker : public LAHashDependentHashTrackerOrTrackerSet + { + private: + LoaderAllocator *const _loaderAllocator; + LADependentHandleToNativeObject *const _dependentHandle; + UINT64 _refCount; + + public: + LAHashDependentHashTracker(LoaderAllocator *loaderAllocator, LADependentKeyToValuesHash *dependentKeyValueStoreHash) + : LAHashDependentHashTrackerOrTrackerSet(false /* isTrackerSet */), + _loaderAllocator(loaderAllocator), + _dependentHandle(CreateDependentHandle(loaderAllocator, dependentKeyValueStoreHash)), + _refCount(1) + {} + + private: + static LADependentHandleToNativeObject *CreateDependentHandle( + LoaderAllocator *loaderAllocator, + LADependentKeyToValuesHash *dependentKeyValueStoreHash); + ~LAHashDependentHashTracker(); // only accessible via DecRefCount() + + public: + bool IsLoaderAllocatorLive() const { return _dependentHandle->GetDependentObject() != NULL; } + + bool IsTrackerFor(LoaderAllocator *loaderAllocator) const + { + return loaderAllocator == _loaderAllocator && IsLoaderAllocatorLive(); + } + + KeyToValuesHash *GetDependentKeyToValuesHash() const; + + // Be careful with this. This isn't safe to use unless something is keeping the LoaderAllocator live, or there is no + // intention to dereference this pointer. + LoaderAllocator *GetLoaderAllocatorUnsafe() const { return _loaderAllocator; } + + #ifndef DACCESS_COMPILE + void IncRefCount() + { + _ASSERTE(_refCount != 0); + ++_refCount; + } + + void DecRefCount() + { + _ASSERTE(_refCount != 0); + if (--_refCount == 0) + { + delete this; + } + } + + static void StaticDecRefCount(LAHashDependentHashTracker *dependentTracker) + { + if (dependentTracker != NULL) + { + dependentTracker->DecRefCount(); + } + } + + using NewTrackerHolder = SpecializedWrapper; + #endif // !DACCESS_COMPILE + }; + + class EMPTY_BASES_DECL LAHashDependentHashTrackerHashTraits : public DefaultSHashTraits + { + public: + typedef TCount count_t; + typedef LoaderAllocator *key_t; + + static const bool s_supports_autoremove = true; + static const bool s_RemovePerEntryCleanupAction = true; + + static bool ShouldDelete(LAHashDependentHashTracker *dependentTracker) + { + #ifndef DACCESS_COMPILE + // This is a tricky bit of logic used which detects freed loader allocators lazily + // and deletes them from the hash table while looking up or otherwise walking the hashtable + // for any purpose. OnRemovePerEntryCleanupAction() is invoked for removed elements to + // handle cleanup of the actual data. + return !dependentTracker->IsLoaderAllocatorLive(); + #else + return false; + #endif + } + + static void OnRemovePerEntryCleanupAction(LAHashDependentHashTracker *dependentTracker) + { + #ifndef DACCESS_COMPILE + // Dependent trackers may be stored in multiple hash tables and are ref-counted when added/removed from a hash + // table. The ref count decrement may also delete the tracker. + dependentTracker->DecRefCount(); + #endif + } + + static LoaderAllocator *GetKey(LAHashDependentHashTracker *tracker) { return tracker->GetLoaderAllocatorUnsafe(); } + static BOOL Equals(LoaderAllocator *la1, LoaderAllocator *la2) { return la1 == la2; } + static TCount Hash(LoaderAllocator *la) { return (TCount)(size_t)la; } + }; + + typedef SHash LAHashDependentHashTrackerHash; + + class LAHashDependentHashTrackerSetWrapper : public LAHashDependentHashTrackerOrTrackerSet + { + private: + LAHashDependentHashTrackerHash _trackerSet; + + public: + LAHashDependentHashTrackerSetWrapper() : LAHashDependentHashTrackerOrTrackerSet(true /* isTrackerSet */) {} + + LAHashDependentHashTrackerHash *GetTrackerSet() { return &_trackerSet; } + }; + + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // CrossLoaderAllocatorHash members public: @@ -164,32 +424,28 @@ class CrossLoaderAllocatorHash private: #ifndef DACCESS_COMPILE - void EnsureManagedObjectsInitted(); - LAHASHDEPENDENTHASHTRACKERREF GetDependentTrackerForLoaderAllocator(LoaderAllocator* pLoaderAllocator); - GCHEAPHASHOBJECTREF GetKeyToValueCrossLAHashForHashkeyToTrackers(LAHASHKEYTOTRACKERSREF hashKeyToTrackersUnsafe, LoaderAllocator* pValueLoaderAllocator); + LAHashDependentHashTracker *GetDependentTrackerForLoaderAllocator(LoaderAllocator *pLoaderAllocator); + KeyToValuesHash *GetKeyToValueCrossLAHashForHashkeyToTrackers( + LAHashKeyToTrackers *hashKeyToTrackers, + LoaderAllocator *pValueLoaderAllocator); #endif // !DACCESS_COMPILE template - static bool VisitKeyValueStore(OBJECTREF *pLoaderAllocatorRef, OBJECTREF *pKeyValueStore, Visitor &visitor); + static bool VisitKeyValueStore(LoaderAllocator *loaderAllocator, KeyValueStore *keyValueStore, Visitor &visitor); template - static bool VisitTracker(TKey key, LAHASHDEPENDENTHASHTRACKERREF trackerUnsafe, Visitor &visitor); + static bool VisitTracker(TKey key, LAHashDependentHashTracker *tracker, Visitor &visitor); template - static bool VisitTrackerAllEntries(LAHASHDEPENDENTHASHTRACKERREF trackerUnsafe, Visitor &visitor); + static bool VisitTrackerAllEntries(LAHashDependentHashTracker *tracker, Visitor &visitor); template - static bool VisitKeyToTrackerAllEntries(OBJECTREF hashKeyEntryUnsafe, Visitor &visitor); - static void DeleteEntryTracker(TKey key, LAHASHDEPENDENTHASHTRACKERREF trackerUnsafe); + static bool VisitKeyToTrackerAllLALocalEntries(KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry, Visitor &visitor); + static void DeleteEntryTracker(TKey key, LAHashDependentHashTracker *tracker); private: LoaderAllocator *m_pLoaderAllocator = 0; - OBJECTHANDLE m_loaderAllocatorToDependentTrackerHash = 0; - OBJECTHANDLE m_keyToDependentTrackersHash = 0; - OBJECTHANDLE m_globalDependentTrackerRootHandle = 0; + LAHashDependentHashTrackerHash m_loaderAllocatorToDependentTrackerHash; + KeyToValuesHash m_keyToDependentTrackersHash; }; -class CrossLoaderAllocatorHashSetup -{ -public: - inline static void EnsureTypesLoaded(); -}; +#undef DISABLE_COPY #endif // CROSSLOADERALLOCATORHASH_H diff --git a/src/coreclr/vm/crossloaderallocatorhash.inl b/src/coreclr/vm/crossloaderallocatorhash.inl index 2b3d08403f45d..9037da4f36a4a 100644 --- a/src/coreclr/vm/crossloaderallocatorhash.inl +++ b/src/coreclr/vm/crossloaderallocatorhash.inl @@ -5,32 +5,36 @@ #define CROSSLOADERALLOCATORHASH_INL #ifdef CROSSLOADERALLOCATORHASH_H -#include "gcheaphashtable.inl" - -template -/*static*/ DWORD NoRemoveDefaultCrossLoaderAllocatorHashTraits::ComputeUsedEntries(OBJECTREF *pKeyValueStore, DWORD *pEntriesInArrayTotal) +template +/*static*/ typename CrossLoaderAllocatorHash::TCount +CrossLoaderAllocatorHash::KeyToValuesHashTraits::ComputeUsedEntries( + KeyValueStore *keyValueStore, + TCount *pEntriesInArrayTotal) { CONTRACTL { NOTHROW; GC_NOTRIGGER; - MODE_COOPERATIVE; + MODE_ANY; } CONTRACTL_END; - DWORD entriesInArrayTotal = (((I1ARRAYREF)*pKeyValueStore)->GetNumComponents() - sizeof(TKey))/sizeof(TValue); - DWORD usedEntries; - TValue* pStartOfValuesData = (TValue*)(((I1ARRAYREF)*pKeyValueStore)->GetDirectPointerToNonObjectElements() + sizeof(TKey)); + // An empty value slot may be used to store a count + static_assert_no_msg(sizeof(TValue) >= sizeof(TCount)); + + TCount entriesInArrayTotal = keyValueStore->GetCapacity(); + TCount usedEntries; + TValue *pStartOfValuesData = keyValueStore->GetValues(); if (entriesInArrayTotal == 0) { usedEntries = 0; } - else if ((entriesInArrayTotal >= 2) && (pStartOfValuesData[entriesInArrayTotal - 2] == (TValue)0)) + else if ((entriesInArrayTotal >= 2) && TRAITS::IsNullValue(pStartOfValuesData[entriesInArrayTotal - 2])) { - usedEntries = (DWORD)(SIZE_T)pStartOfValuesData[entriesInArrayTotal - 1]; + usedEntries = *(TCount *)&pStartOfValuesData[entriesInArrayTotal - 1]; } - else if (pStartOfValuesData[entriesInArrayTotal - 1] == (TValue)0) + else if (TRAITS::IsNullValue(pStartOfValuesData[entriesInArrayTotal - 1])) { usedEntries = entriesInArrayTotal - 1; } @@ -44,59 +48,132 @@ template } #ifndef DACCESS_COMPILE -template -/*static*/ void NoRemoveDefaultCrossLoaderAllocatorHashTraits::SetUsedEntries(TValue* pStartOfValuesData, DWORD entriesInArrayTotal, DWORD usedEntries) +template +/*static*/ void CrossLoaderAllocatorHash::KeyToValuesHashTraits::SetUsedEntries( + KeyValueStore *keyValueStore, + TCount entriesInArrayTotal, + TCount usedEntries) { + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END; + + // An empty value slot may be used to store a count + static_assert_no_msg(sizeof(TValue) >= sizeof(TCount)); + + _ASSERTE(entriesInArrayTotal == keyValueStore->GetCapacity()); + if (usedEntries < entriesInArrayTotal) { + TValue *pStartOfValuesData = keyValueStore->GetValues(); if (usedEntries == (entriesInArrayTotal - 1)) { - pStartOfValuesData[entriesInArrayTotal - 1] = (TValue)0; + pStartOfValuesData[entriesInArrayTotal - 1] = TRAITS::NullValue(); } else { - pStartOfValuesData[entriesInArrayTotal - 1] = (TValue)((INT_PTR)usedEntries); - pStartOfValuesData[entriesInArrayTotal - 2] = (TValue)0; + *(TCount *)&pStartOfValuesData[entriesInArrayTotal - 1] = usedEntries; + pStartOfValuesData[entriesInArrayTotal - 2] = TRAITS::NullValue(); } } } -template -/*static*/ bool NoRemoveDefaultCrossLoaderAllocatorHashTraits::AddToValuesInHeapMemory(OBJECTREF *pKeyValueStore, const TKey& key, const TValue& value) +template +/*static*/ typename CrossLoaderAllocatorHash::KeyValueStore *CrossLoaderAllocatorHash::KeyValueStore::Create( + TCount capacity, + const TKey &key) { CONTRACTL { THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END; + + NewArrayHolder bufferHolder = new BYTE[sizeof(KeyValueStore) + capacity * sizeof(TValue)]; + KeyValueStore *keyValueStore = new(bufferHolder) KeyValueStore(capacity, key); + bufferHolder.SuppressRelease(); + for (TCount i = 0; i < capacity; i++) + { + keyValueStore->GetValues()[i] = TRAITS::NullValue(); + } + + return keyValueStore; +} + +template +CrossLoaderAllocatorHash::LAHashKeyToTrackers::~LAHashKeyToTrackers() +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; } CONTRACTL_END; - static_assert(sizeof(TKey)==sizeof(TValue), "Assume keys and values are the same size"); + delete _laLocalKeyValueStore; + + LAHashDependentHashTrackerOrTrackerSet *trackerOrTrackerSet = _trackerOrTrackerSet; + if (trackerOrTrackerSet == NULL) + { + return; + } + + if (trackerOrTrackerSet->IsTrackerSet()) + { + delete static_cast(trackerOrTrackerSet); + } + else + { + static_cast(trackerOrTrackerSet)->DecRefCount(); + } +} + +template +/*static*/ bool CrossLoaderAllocatorHash::KeyToValuesHashTraits::AddToValuesInHeapMemory( + KeyValueStore **pKeyValueStore, + NewHolder &keyValueStoreHolder, + const TKey& key, + const TValue& value) +{ + CONTRACTL + { + THROWS; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END; bool updatedKeyValueStore = false; + KeyValueStore *keyValueStore = *pKeyValueStore; - if (*pKeyValueStore == NULL) + if (keyValueStore == NULL) { - *pKeyValueStore = AllocatePrimitiveArray(ELEMENT_TYPE_I1, IsNull(value) ? sizeof(TKey) : sizeof(TKey) + sizeof(TValue)); + keyValueStoreHolder = *pKeyValueStore = keyValueStore = + KeyValueStore::Create(TRAITS::IsNullValue(value) ? 0 : 1, key); updatedKeyValueStore = true; - TKey* pKeyLoc = (TKey*)((I1ARRAYREF)*pKeyValueStore)->GetDirectPointerToNonObjectElements(); - *pKeyLoc = key; - if (!IsNull(value)) + if (!TRAITS::IsNullValue(value)) { - TValue* pValueLoc = (TValue*)(((I1ARRAYREF)*pKeyValueStore)->GetDirectPointerToNonObjectElements() + sizeof(TKey)); - *pValueLoc = value; + keyValueStore->GetValues()[0] = value; } } - else if (!IsNull(value)) + else if (!TRAITS::IsNullValue(value)) { - DWORD entriesInArrayTotal; - DWORD usedEntries = ComputeUsedEntries(pKeyValueStore, &entriesInArrayTotal); + _ASSERTE(TRAITS::KeyEquals(key, keyValueStore->GetKey())); + + TCount entriesInArrayTotal; + TCount usedEntries = ComputeUsedEntries(keyValueStore, &entriesInArrayTotal); if (usedEntries == entriesInArrayTotal) { // There isn't free space. Build a new, bigger array with the existing data - DWORD newSize; + TCount newSize; if (usedEntries < 8) newSize = usedEntries + 1; // Grow very slowly initially. The cost of allocation/copy is cheap, and this holds very tight on memory usage else @@ -106,56 +183,40 @@ template COMPlusThrow(kOverflowException); // Allocate the new array. - I1ARRAYREF newKeyValueStore = (I1ARRAYREF)AllocatePrimitiveArray(ELEMENT_TYPE_I1, newSize*sizeof(TValue) + sizeof(TKey)); + KeyValueStore *newKeyValueStore = KeyValueStore::Create(newSize, key); + memcpyNoGCRefs(newKeyValueStore->GetValues(), keyValueStore->GetValues(), entriesInArrayTotal * sizeof(TValue)); - // Since, AllocatePrimitiveArray may have triggered a GC, recapture all data pointers from GC objects - void* pStartOfNewArray = newKeyValueStore->GetDirectPointerToNonObjectElements(); - void* pStartOfOldArray = ((I1ARRAYREF)*pKeyValueStore)->GetDirectPointerToNonObjectElements(); - - memcpyNoGCRefs(pStartOfNewArray, pStartOfOldArray, ((I1ARRAYREF)*pKeyValueStore)->GetNumComponents()); - - *pKeyValueStore = (OBJECTREF)newKeyValueStore; + keyValueStoreHolder = *pKeyValueStore = keyValueStore = newKeyValueStore; updatedKeyValueStore = true; entriesInArrayTotal = newSize; } // There is free space. Append on the end - TValue* pStartOfValuesData = (TValue*)(((I1ARRAYREF)*pKeyValueStore)->GetDirectPointerToNonObjectElements() + sizeof(TKey)); - SetUsedEntries(pStartOfValuesData, entriesInArrayTotal, usedEntries + 1); - pStartOfValuesData[usedEntries] = value; + SetUsedEntries(keyValueStore, entriesInArrayTotal, usedEntries + 1); + keyValueStore->GetValues()[usedEntries] = value; } return updatedKeyValueStore; } #endif //!DACCESS_COMPILE -template -/*static*/ TKey_ NoRemoveDefaultCrossLoaderAllocatorHashTraits::ReadKeyFromKeyValueStore(OBJECTREF *pKeyValueStore) -{ - WRAPPER_NO_CONTRACT; - - TKey* pKeyLoc = (TKey*)((I1ARRAYREF)*pKeyValueStore)->GetDirectPointerToNonObjectElements(); - return *pKeyLoc; -} - -template +template template -/*static*/ bool NoRemoveDefaultCrossLoaderAllocatorHashTraits::VisitKeyValueStore(OBJECTREF *pLoaderAllocatorRef, OBJECTREF *pKeyValueStore, Visitor &visitor) +/*static*/ bool CrossLoaderAllocatorHash::KeyToValuesHashTraits::VisitKeyValueStore( + LoaderAllocator *loaderAllocator, + KeyValueStore *keyValueStore, + Visitor &visitor) { WRAPPER_NO_CONTRACT; - DWORD entriesInArrayTotal; - DWORD usedEntries = ComputeUsedEntries(pKeyValueStore, &entriesInArrayTotal); + TKey key = keyValueStore->GetKey(); + TCount entriesInArrayTotal; + TCount usedEntries = ComputeUsedEntries(keyValueStore, &entriesInArrayTotal); - for (DWORD index = 0; index < usedEntries; ++index) + for (TCount index = 0; index < usedEntries; ++index) { - // Capture pKeyLoc and pStartOfValuesData inside of loop, as we aren't protecting these pointers into the GC heap, so they - // are not permitted to live across the call to visitor (in case visitor triggers a GC) - TKey* pKeyLoc = (TKey*)((I1ARRAYREF)*pKeyValueStore)->GetDirectPointerToNonObjectElements(); - TValue* pStartOfValuesData = (TValue*)(((I1ARRAYREF)*pKeyValueStore)->GetDirectPointerToNonObjectElements() + sizeof(TKey)); - - if (!visitor(*pLoaderAllocatorRef, *pKeyLoc, pStartOfValuesData[index])) + if (!visitor(loaderAllocator, key, keyValueStore->GetValues()[index])) { return false; } @@ -165,14 +226,16 @@ template } #ifndef DACCESS_COMPILE -template -/*static*/ void DefaultCrossLoaderAllocatorHashTraits::DeleteValueInHeapMemory(OBJECTREF keyValueStore, const TValue& value) +template +/*static*/ void CrossLoaderAllocatorHash::KeyToValuesHashTraits::DeleteValueInHeapMemory( + KeyValueStore *keyValueStore, + const TValue& value) { CONTRACTL { NOTHROW; GC_NOTRIGGER; - MODE_COOPERATIVE; + MODE_ANY; } CONTRACTL_END; @@ -180,159 +243,105 @@ template // values list is sorted, and then doing a binary search for the value instead // of the linear search - DWORD entriesInArrayTotal; - DWORD usedEntries = NoRemoveDefaultCrossLoaderAllocatorHashTraits::ComputeUsedEntries(&keyValueStore, &entriesInArrayTotal); - TValue* pStartOfValuesData = (TValue*)(((I1ARRAYREF)keyValueStore)->GetDirectPointerToNonObjectElements() + sizeof(TKey)); + TCount entriesInArrayTotal; + TCount usedEntries = ComputeUsedEntries(keyValueStore, &entriesInArrayTotal); + TValue *pStartOfValuesData = keyValueStore->GetValues(); - for (DWORD iEntry = 0; iEntry < usedEntries; iEntry++) + for (TCount iEntry = 0; iEntry < usedEntries; iEntry++) { - if (pStartOfValuesData[iEntry] == value) + if (TRAITS::ValueEquals(pStartOfValuesData[iEntry], value)) { memmove(pStartOfValuesData + iEntry, pStartOfValuesData + iEntry + 1, (usedEntries - iEntry - 1) * sizeof(TValue)); - SetUsedEntries(pStartOfValuesData, entriesInArrayTotal, usedEntries - 1); + SetUsedEntries(keyValueStore, entriesInArrayTotal, usedEntries - 1); return; } } } -#endif //!DACCESS_COMPILE - -/*static*/ inline INT32 GCHeapHashDependentHashTrackerHashTraits::Hash(PtrTypeKey *pValue) -{ - LIMITED_METHOD_CONTRACT; - return (INT32)(SIZE_T)*pValue; -} - -/*static*/ inline INT32 GCHeapHashDependentHashTrackerHashTraits::Hash(PTRARRAYREF arr, INT32 index) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - LAHASHDEPENDENTHASHTRACKERREF value = (LAHASHDEPENDENTHASHTRACKERREF)arr->GetAt(index); - LoaderAllocator *pLoaderAllocator = value->GetLoaderAllocatorUnsafe(); - return Hash(&pLoaderAllocator); -} -/*static*/ inline bool GCHeapHashDependentHashTrackerHashTraits::DoesEntryMatchKey(PTRARRAYREF arr, INT32 index, PtrTypeKey *pKey) +template +/*static*/ LADependentHandleToNativeObject *CrossLoaderAllocatorHash::LAHashDependentHashTracker::CreateDependentHandle( + LoaderAllocator *loaderAllocator, + LADependentKeyToValuesHash *dependentKeyValueStoreHash) { CONTRACTL { - NOTHROW; + THROWS; GC_NOTRIGGER; - MODE_COOPERATIVE; + MODE_ANY; } CONTRACTL_END; - LAHASHDEPENDENTHASHTRACKERREF value = (LAHASHDEPENDENTHASHTRACKERREF)arr->GetAt(index); - - return value->IsTrackerFor(*pKey); + NewHolder dependentHandleHolder = + new LADependentHandleToNativeObject(dependentKeyValueStoreHash); + loaderAllocator->RegisterDependentHandleToNativeObjectForCleanup(dependentHandleHolder); + return dependentHandleHolder.Extract(); } -/*static*/ inline bool GCHeapHashDependentHashTrackerHashTraits::IsDeleted(PTRARRAYREF arr, INT32 index, GCHEAPHASHOBJECTREF gcHeap) +template +CrossLoaderAllocatorHash::LAHashDependentHashTracker::~LAHashDependentHashTracker() { CONTRACTL { NOTHROW; GC_NOTRIGGER; - MODE_COOPERATIVE; + MODE_ANY; } CONTRACTL_END; - OBJECTREF valueInHeap = arr->GetAt(index); - - if (valueInHeap == NULL) - return false; - - if (gcHeap == valueInHeap) - return true; - - // This is a tricky bit of logic used which detects freed loader allocators lazily - // and deletes them from the GCHeapHash while looking up or otherwise walking the hashtable - // for any purpose. - LAHASHDEPENDENTHASHTRACKERREF value = (LAHASHDEPENDENTHASHTRACKERREF)valueInHeap; - if (!value->IsLoaderAllocatorLive()) + if (IsLoaderAllocatorLive()) { -#ifndef DACCESS_COMPILE - arr->SetAt(index, gcHeap); - gcHeap->DecrementCount(true); -#endif // DACCESS_COMPILE - - return true; + _loaderAllocator->UnregisterDependentHandleToNativeObjectFromCleanup(_dependentHandle); } - return false; -} - -template -template -/*static*/ INT32 KeyToValuesGCHeapHashTraits::Hash(TKey *pValue) -{ - LIMITED_METHOD_CONTRACT; - return (INT32)(SIZE_T)*pValue; + delete _dependentHandle; } +#endif //!DACCESS_COMPILE -template -/*static*/ inline INT32 KeyToValuesGCHeapHashTraits::Hash(PTRARRAYREF arr, INT32 index) +template +typename CrossLoaderAllocatorHash::KeyToValuesHash * +CrossLoaderAllocatorHash::LAHashDependentHashTracker::GetDependentKeyToValuesHash() const { CONTRACTL { NOTHROW; GC_NOTRIGGER; - MODE_COOPERATIVE; + MODE_ANY; } CONTRACTL_END; - OBJECTREF hashKeyEntry = arr->GetAt(index); - LAHASHKEYTOTRACKERSREF hashKeyToTrackers; - OBJECTREF keyValueStore; - - if (hashKeyEntry->GetMethodTable() == CoreLibBinder::GetExistingClass(CLASS__LAHASHKEYTOTRACKERS)) + LADependentNativeObject *dependentObject = _dependentHandle->GetDependentObject(); + if (dependentObject == NULL) { - hashKeyToTrackers = (LAHASHKEYTOTRACKERSREF)hashKeyEntry; - keyValueStore = hashKeyToTrackers->_laLocalKeyValueStore; - } - else - { - keyValueStore = hashKeyEntry; + return NULL; } - typename TRAITS::TKey key = TRAITS::ReadKeyFromKeyValueStore(&keyValueStore); - return Hash(&key); + return static_cast(dependentObject)->GetKeyToValuesHash(); } -template -template -/*static*/ bool KeyToValuesGCHeapHashTraits::DoesEntryMatchKey(PTRARRAYREF arr, INT32 index, TKey *pKey) +template +/*static*/ typename CrossLoaderAllocatorHash::TKey CrossLoaderAllocatorHash::KeyToValuesHashTraits::GetKey( + KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry) { CONTRACTL { NOTHROW; GC_NOTRIGGER; - MODE_COOPERATIVE; + MODE_ANY; } CONTRACTL_END; - OBJECTREF hashKeyEntry = arr->GetAt(index); - LAHASHKEYTOTRACKERSREF hashKeyToTrackers; - OBJECTREF keyValueStore; - - if (hashKeyEntry->GetMethodTable() == CoreLibBinder::GetExistingClass(CLASS__LAHASHKEYTOTRACKERS)) + KeyValueStore *keyValueStore; + if (hashKeyEntry->IsLAHashKeyToTrackers()) { - hashKeyToTrackers = (LAHASHKEYTOTRACKERSREF)hashKeyEntry; + LAHashKeyToTrackers *hashKeyToTrackers = static_cast(hashKeyEntry); keyValueStore = hashKeyToTrackers->_laLocalKeyValueStore; } else { - keyValueStore = hashKeyEntry; + keyValueStore = static_cast(hashKeyEntry); } - TKey key = TRAITS::ReadKeyFromKeyValueStore(&keyValueStore); - - return key == *pKey; + return keyValueStore->GetKey(); } #ifndef DACCESS_COMPILE @@ -342,128 +351,126 @@ void CrossLoaderAllocatorHash::Add(TKey key, TValue value, LoaderAllocat CONTRACTL { THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; + GC_NOTRIGGER; + MODE_ANY; } CONTRACTL_END; + // This data structure actually doesn't have this invariant, but it is expected that uses of this + // data structure will require that the key's loader allocator is the same as that of this data structure. + _ASSERTE(TRAITS::GetLoaderAllocator(key) == m_pLoaderAllocator); - struct { - KeyToValuesGCHeapHash keyToTrackersHash; - KeyToValuesGCHeapHash keyToValuePerLAHash; - OBJECTREF keyValueStore; - OBJECTREF hashKeyEntry; - LAHASHKEYTOTRACKERSREF hashKeyToTrackers; - } gc; - ZeroMemory(&gc, sizeof(gc)); - GCPROTECT_BEGIN(gc) - { - EnsureManagedObjectsInitted(); - - bool addToKeyValuesHash = false; - // This data structure actually doesn't have this invariant, but it is expected that uses of this - // data structure will require that the key's loader allocator is the same as that of this data structure. - _ASSERTE(key->GetLoaderAllocator() == m_pLoaderAllocator); + // Add may involve multiple changes to the data structure. The data structure is set up such that each of those changes + // keeps the data structure in a valid state, so OOMs in the middle of an Add operation may end up making some changes but + // the data structure remains in a valid state. Holders are used to clean up memory that was allocated and not yet inserted + // into the data structure. Once inserted into the data structure, it takes over the lifetime of the memory. - gc.keyToTrackersHash = KeyToValuesGCHeapHash((GCHEAPHASHOBJECTREF)ObjectFromHandle(m_keyToDependentTrackersHash)); - INT32 index = gc.keyToTrackersHash.GetValueIndex(&key); + NewHolder keyValueStoreHolder; + NewHolder hashKeyToTrackersHolder; + KeyValueStore *keyValueStore = NULL; + LAHashKeyToTrackers *hashKeyToTrackers = NULL; - if (index == -1) - { - addToKeyValuesHash = true; - TRAITS::AddToValuesInHeapMemory(&gc.keyValueStore, key, pLoaderAllocatorOfValue == m_pLoaderAllocator ? value : TRAITS::NullValue()); + KeyToValuesHash &keyToTrackersHash = m_keyToDependentTrackersHash; + KeyValueStoreOrLAHashKeyToTrackers *const *hashKeyEntryPtr = keyToTrackersHash.LookupPtr(key); + KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry; - if (pLoaderAllocatorOfValue != m_pLoaderAllocator) - { - gc.hashKeyToTrackers = (LAHASHKEYTOTRACKERSREF)AllocateObject(CoreLibBinder::GetExistingClass(CLASS__LAHASHKEYTOTRACKERS)); - SetObjectReference(&gc.hashKeyToTrackers->_laLocalKeyValueStore, gc.keyValueStore); - gc.hashKeyEntry = gc.hashKeyToTrackers; - } - else - { - gc.hashKeyEntry = gc.keyValueStore; - } + if (hashKeyEntryPtr == NULL) + { + KeyToValuesHashTraits::AddToValuesInHeapMemory( + &keyValueStore, + keyValueStoreHolder, + key, + pLoaderAllocatorOfValue == m_pLoaderAllocator ? value : TRAITS::NullValue()); - gc.keyToTrackersHash.Add(&key, [&gc](PTRARRAYREF arr, INT32 index) - { - arr->SetAt(index, (OBJECTREF)gc.hashKeyEntry); - }); + if (pLoaderAllocatorOfValue != m_pLoaderAllocator) + { + hashKeyToTrackersHolder = hashKeyToTrackers = new LAHashKeyToTrackers(keyValueStore); + keyValueStoreHolder.SuppressRelease(); + hashKeyEntry = hashKeyToTrackers; } else { - gc.keyToTrackersHash.GetElement(index, gc.hashKeyEntry); - - if (gc.hashKeyEntry->GetMethodTable() == CoreLibBinder::GetExistingClass(CLASS__LAHASHKEYTOTRACKERS)) - { - gc.hashKeyToTrackers = (LAHASHKEYTOTRACKERSREF)gc.hashKeyEntry; - gc.keyValueStore = gc.hashKeyToTrackers->_laLocalKeyValueStore; - } - else - { - gc.keyValueStore = gc.hashKeyEntry; - } - - bool updatedKeyValueStore = false; + hashKeyEntry = keyValueStore; + } - if (pLoaderAllocatorOfValue == m_pLoaderAllocator) - { - updatedKeyValueStore = TRAITS::AddToValuesInHeapMemory(&gc.keyValueStore, key, value); - } + keyToTrackersHash.Add(hashKeyEntry); + keyValueStoreHolder.SuppressRelease(); + hashKeyToTrackersHolder.SuppressRelease(); + } + else + { + hashKeyEntry = *hashKeyEntryPtr; + if (hashKeyEntry->IsLAHashKeyToTrackers()) + { + hashKeyToTrackers = static_cast(hashKeyEntry); + keyValueStore = hashKeyToTrackers->_laLocalKeyValueStore; + } + else + { + keyValueStore = static_cast(hashKeyEntry); + } + if (pLoaderAllocatorOfValue == m_pLoaderAllocator) + { + bool updatedKeyValueStore = + KeyToValuesHashTraits::AddToValuesInHeapMemory(&keyValueStore, keyValueStoreHolder, key, value); if (updatedKeyValueStore) { - if (gc.hashKeyToTrackers != NULL) + if (hashKeyToTrackers != NULL) { - SetObjectReference(&gc.hashKeyToTrackers->_laLocalKeyValueStore, gc.keyValueStore); + delete hashKeyToTrackers->_laLocalKeyValueStore; + hashKeyToTrackers->_laLocalKeyValueStore = keyValueStore; } else { - gc.hashKeyEntry = gc.keyValueStore; - gc.keyToTrackersHash.SetElement(index, gc.hashKeyEntry); + hashKeyEntry = keyValueStore; + keyToTrackersHash.ReplacePtr(hashKeyEntryPtr, hashKeyEntry); } + + keyValueStoreHolder.SuppressRelease(); } } + } - // If the LoaderAllocator matches, we've finished adding by now, otherwise, we need to get the remove hash and work with that - if (pLoaderAllocatorOfValue != m_pLoaderAllocator) + // If the LoaderAllocator matches, we've finished adding by now, otherwise, we need to get the remove hash and work with that + if (pLoaderAllocatorOfValue != m_pLoaderAllocator) + { + if (hashKeyToTrackers == NULL) { - if (gc.hashKeyToTrackers == NULL) - { - // Nothing has yet caused the trackers proxy object to be setup. Create it now, and update the keyToTrackersHash - gc.hashKeyToTrackers = (LAHASHKEYTOTRACKERSREF)AllocateObject(CoreLibBinder::GetExistingClass(CLASS__LAHASHKEYTOTRACKERS)); - SetObjectReference(&gc.hashKeyToTrackers->_laLocalKeyValueStore, gc.keyValueStore); - gc.hashKeyEntry = gc.hashKeyToTrackers; - gc.keyToTrackersHash.SetElement(index, gc.hashKeyEntry); - } + // Nothing has yet caused the trackers proxy object to be setup. Create it now, and update the keyToTrackersHash. + // Don't need to use the holder here since there's no allocation before it's put into the hash table. The previous + // element was the key-value store, and should not be deleted because its lifetime is being assigned to the new + // LAHashKeyToTrackers, so replace the element without cleanup. + hashKeyToTrackers = new LAHashKeyToTrackers(keyValueStore); + hashKeyEntry = hashKeyToTrackers; + keyToTrackersHash.ReplacePtr(hashKeyEntryPtr, hashKeyEntry, false /* invokeCleanupAction */); + } - // Must add it to the cross LA structure - GCHEAPHASHOBJECTREF gcheapKeyToValue = GetKeyToValueCrossLAHashForHashkeyToTrackers(gc.hashKeyToTrackers, pLoaderAllocatorOfValue); + // Must add it to the cross LA structure + KeyToValuesHash *keyToValuePerLAHash = + GetKeyToValueCrossLAHashForHashkeyToTrackers(hashKeyToTrackers, pLoaderAllocatorOfValue); - gc.keyToValuePerLAHash = KeyToValuesGCHeapHash(gcheapKeyToValue); + KeyValueStoreOrLAHashKeyToTrackers *const *hashKeyEntryInPerLAHashPtr = keyToValuePerLAHash->LookupPtr(key); + if (hashKeyEntryInPerLAHashPtr != NULL) + { + _ASSERTE(!(*hashKeyEntryInPerLAHashPtr)->IsLAHashKeyToTrackers()); + keyValueStore = static_cast(*hashKeyEntryInPerLAHashPtr); - INT32 indexInKeyValueHash = gc.keyToValuePerLAHash.GetValueIndex(&key); - if (indexInKeyValueHash != -1) + if (KeyToValuesHashTraits::AddToValuesInHeapMemory(&keyValueStore, keyValueStoreHolder, key, value)) { - gc.keyToValuePerLAHash.GetElement(indexInKeyValueHash, gc.keyValueStore); - - if (TRAITS::AddToValuesInHeapMemory(&gc.keyValueStore, key, value)) - { - gc.keyToValuePerLAHash.SetElement(indexInKeyValueHash, gc.keyValueStore); - } + keyToValuePerLAHash->ReplacePtr(hashKeyEntryInPerLAHashPtr, keyValueStore); + keyValueStoreHolder.SuppressRelease(); } - else - { - gc.keyValueStore = NULL; - TRAITS::AddToValuesInHeapMemory(&gc.keyValueStore, key, value); + } + else + { + keyValueStore = NULL; + KeyToValuesHashTraits::AddToValuesInHeapMemory(&keyValueStore, keyValueStoreHolder, key, value); - gc.keyToValuePerLAHash.Add(&key, [&gc](PTRARRAYREF arr, INT32 index) - { - arr->SetAt(index, gc.keyValueStore); - }); - } + keyToValuePerLAHash->Add(keyValueStore); + keyValueStoreHolder.SuppressRelease(); } } - GCPROTECT_END(); } #endif // !DACCESS_COMPILE @@ -473,72 +480,54 @@ void CrossLoaderAllocatorHash::Remove(TKey key, TValue value, LoaderAllo { CONTRACTL { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; } CONTRACTL_END; // This data structure actually doesn't have this invariant, but it is expected that uses of this // data structure will require that the key's loader allocator is the same as that of this data structure. - _ASSERTE(key->GetLoaderAllocator() == m_pLoaderAllocator); + _ASSERTE(TRAITS::GetLoaderAllocator(key) == m_pLoaderAllocator); - if (m_keyToDependentTrackersHash == NULL) + KeyToValuesHash &keyToTrackersHash = m_keyToDependentTrackersHash; + KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry = keyToTrackersHash.Lookup(key); + if (hashKeyEntry == NULL) { - // If the heap objects haven't been initted, then there is nothing to delete return; } - struct { - KeyToValuesGCHeapHash keyToTrackersHash; - KeyToValuesGCHeapHash keyToValuePerLAHash; - OBJECTREF hashKeyEntry; - LAHASHKEYTOTRACKERSREF hashKeyToTrackers; - OBJECTREF keyValueStore; - } gc; + LAHashKeyToTrackers *hashKeyToTrackers = NULL; + KeyValueStore *keyValueStore; - ZeroMemory(&gc, sizeof(gc)); - GCPROTECT_BEGIN(gc) + if (hashKeyEntry->IsLAHashKeyToTrackers()) { - gc.keyToTrackersHash = KeyToValuesGCHeapHash((GCHEAPHASHOBJECTREF)ObjectFromHandle(m_keyToDependentTrackersHash)); - INT32 index = gc.keyToTrackersHash.GetValueIndex(&key); + hashKeyToTrackers = static_cast(hashKeyEntry); + keyValueStore = hashKeyToTrackers->_laLocalKeyValueStore; + } + else + { + keyValueStore = static_cast(hashKeyEntry); + } - if (index != -1) + // Check to see if value can be removed from this data structure directly. + if (m_pLoaderAllocator == pLoaderAllocatorOfValue) + { + KeyToValuesHashTraits::DeleteValueInHeapMemory(keyValueStore, value); + } + else if (hashKeyToTrackers != NULL) + { + // Must remove it from the cross LA structure + KeyToValuesHash *keyToValuePerLAHash = + GetKeyToValueCrossLAHashForHashkeyToTrackers(hashKeyToTrackers, pLoaderAllocatorOfValue); + hashKeyEntry = keyToValuePerLAHash->Lookup(key); + if (hashKeyEntry != NULL) { - gc.keyToTrackersHash.GetElement(index, gc.hashKeyEntry); - - if (gc.hashKeyEntry->GetMethodTable() == CoreLibBinder::GetExistingClass(CLASS__LAHASHKEYTOTRACKERS)) - { - gc.hashKeyToTrackers = (LAHASHKEYTOTRACKERSREF)gc.hashKeyEntry; - gc.keyValueStore = gc.hashKeyToTrackers->_laLocalKeyValueStore; - } - else - { - gc.keyValueStore = gc.hashKeyEntry; - } - - // Check to see if value can be added to this data structure directly. - if (m_pLoaderAllocator == pLoaderAllocatorOfValue) - { - TRAITS::DeleteValueInHeapMemory(gc.keyValueStore, value); - } - else if (gc.hashKeyToTrackers != NULL) - { - // Must remove it from the cross LA structure - GCHEAPHASHOBJECTREF gcheapKeyToValue = GetKeyToValueCrossLAHashForHashkeyToTrackers(gc.hashKeyToTrackers, pLoaderAllocatorOfValue); - - gc.keyToValuePerLAHash = KeyToValuesGCHeapHash(gcheapKeyToValue); - - INT32 indexInKeyValueHash = gc.keyToValuePerLAHash.GetValueIndex(&key); - if (indexInKeyValueHash != -1) - { - gc.keyToValuePerLAHash.GetElement(indexInKeyValueHash, gc.keyValueStore); - TRAITS::DeleteValueInHeapMemory(gc.keyValueStore, value); - } - } + _ASSERTE(!hashKeyEntry->IsLAHashKeyToTrackers()); + keyValueStore = static_cast(hashKeyEntry); + KeyToValuesHashTraits::DeleteValueInHeapMemory(keyValueStore, value); } } - GCPROTECT_END(); } #endif // !DACCESS_COMPILE @@ -548,178 +537,102 @@ bool CrossLoaderAllocatorHash::VisitValuesOfKey(TKey key, Visitor &visit { WRAPPER_NO_CONTRACT; - class VisitIndividualEntryKeyValueHash - { - public: - TKey m_key; - Visitor *m_pVisitor; - GCHeapHashDependentHashTrackerHash *m_pDependentTrackerHash; - - VisitIndividualEntryKeyValueHash(TKey key, Visitor *pVisitor, GCHeapHashDependentHashTrackerHash *pDependentTrackerHash) : - m_key(key), - m_pVisitor(pVisitor), - m_pDependentTrackerHash(pDependentTrackerHash) - {} - - bool operator()(INT32 index) - { - WRAPPER_NO_CONTRACT; - - LAHASHDEPENDENTHASHTRACKERREF dependentTracker; - m_pDependentTrackerHash->GetElement(index, dependentTracker); - return VisitTracker(m_key, dependentTracker, *m_pVisitor); - } - }; - // This data structure actually doesn't have this invariant, but it is expected that uses of this // data structure will require that the key's loader allocator is the same as that of this data structure. - _ASSERTE(key->GetLoaderAllocator() == m_pLoaderAllocator); + _ASSERTE(TRAITS::GetLoaderAllocator(key) == m_pLoaderAllocator); - // Check to see that something has been added - if (m_keyToDependentTrackersHash == NULL) + KeyToValuesHash &keyToTrackersHash = m_keyToDependentTrackersHash; + KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry = keyToTrackersHash.Lookup(key); + if (hashKeyEntry == NULL) + { return true; + } + + // We have an entry in the hashtable for the key/dependenthandle. + LAHashKeyToTrackers *hashKeyToTrackers = NULL; + KeyValueStore *keyValueStore; - bool result = true; - struct + if (hashKeyEntry->IsLAHashKeyToTrackers()) { - KeyToValuesGCHeapHash keyToTrackersHash; - GCHeapHashDependentHashTrackerHash dependentTrackerHash; - LAHASHDEPENDENTHASHTRACKERREF dependentTrackerMaybe; - LAHASHDEPENDENTHASHTRACKERREF dependentTracker; - OBJECTREF hashKeyEntry; - LAHASHKEYTOTRACKERSREF hashKeyToTrackers; - OBJECTREF keyValueStore; - OBJECTREF nullref; - } gc; - ZeroMemory(&gc, sizeof(gc)); - GCPROTECT_BEGIN(gc) + hashKeyToTrackers = static_cast(hashKeyEntry); + keyValueStore = hashKeyToTrackers->_laLocalKeyValueStore; + } + else { - gc.keyToTrackersHash = KeyToValuesGCHeapHash((GCHEAPHASHOBJECTREF)ObjectFromHandle(m_keyToDependentTrackersHash)); - INT32 index = gc.keyToTrackersHash.GetValueIndex(&key); - if (index != -1) - { - // We have an entry in the hashtable for the key/dependenthandle. - gc.keyToTrackersHash.GetElement(index, gc.hashKeyEntry); + keyValueStore = static_cast(hashKeyEntry); + } - if (gc.hashKeyEntry->GetMethodTable() == CoreLibBinder::GetExistingClass(CLASS__LAHASHKEYTOTRACKERS)) - { - gc.hashKeyToTrackers = (LAHASHKEYTOTRACKERSREF)gc.hashKeyEntry; - gc.keyValueStore = gc.hashKeyToTrackers->_laLocalKeyValueStore; - } - else - { - gc.keyValueStore = gc.hashKeyEntry; - } + // Now hashKeyToTrackers is filled in and keyValueStore - // Now gc.hashKeyToTrackers is filled in and keyValueStore + // visit local entries + if (!VisitKeyValueStore(NULL, keyValueStore, visitor)) + { + return false; + } - // visit local entries - result = VisitKeyValueStore(&gc.nullref, &gc.keyValueStore, visitor); + if (hashKeyToTrackers == NULL) + { + return true; + } - if (gc.hashKeyToTrackers != NULL) - { - // Is there a single dependenttracker here, or a set. + // Is there a single dependenttracker here, or a set. - if (gc.hashKeyToTrackers->_trackerOrTrackerSet->GetMethodTable() == CoreLibBinder::GetExistingClass(CLASS__LAHASHDEPENDENTHASHTRACKER)) - { - gc.dependentTracker = (LAHASHDEPENDENTHASHTRACKERREF)gc.hashKeyToTrackers->_trackerOrTrackerSet; - result = VisitTracker(key, gc.dependentTracker, visitor); - } - else - { - gc.dependentTrackerHash = GCHeapHashDependentHashTrackerHash(gc.hashKeyToTrackers->_trackerOrTrackerSet); - VisitIndividualEntryKeyValueHash visitIndivididualKeys(key, &visitor, &gc.dependentTrackerHash); - result = gc.dependentTrackerHash.VisitAllEntryIndices(visitIndivididualKeys); - } + if (!hashKeyToTrackers->_trackerOrTrackerSet->IsTrackerSet()) + { + LAHashDependentHashTracker *dependentTracker = + static_cast(hashKeyToTrackers->_trackerOrTrackerSet); + return VisitTracker(key, dependentTracker, visitor); + } + else + { + LAHashDependentHashTrackerHash *dependentTrackerHash = + static_cast(hashKeyToTrackers->_trackerOrTrackerSet)->GetTrackerSet(); + for (typename LAHashDependentHashTrackerHash::Iterator it = dependentTrackerHash->Begin(), + itEnd = dependentTrackerHash->End(); + it != itEnd; + ++it) + { + LAHashDependentHashTracker *dependentTracker = *it; + if (!VisitTracker(key, dependentTracker, visitor)) + { + return false; } } - } - GCPROTECT_END(); - return result; + return true; + } } template template bool CrossLoaderAllocatorHash::VisitAllKeyValuePairs(Visitor &visitor) { - CONTRACTL - { - MODE_COOPERATIVE; - GC_NOTRIGGER; - } - CONTRACTL_END; - - class VisitAllEntryKeyToDependentTrackerHash - { - public: - Visitor *m_pVisitor; - KeyToValuesGCHeapHash *m_pKeyToTrackerHash; - - VisitAllEntryKeyToDependentTrackerHash(Visitor *pVisitor, KeyToValuesGCHeapHash *pKeyToTrackerHash) : - m_pVisitor(pVisitor), - m_pKeyToTrackerHash(pKeyToTrackerHash) - {} - - bool operator()(INT32 index) - { - WRAPPER_NO_CONTRACT; - - OBJECTREF hashKeyEntry; - m_pKeyToTrackerHash->GetElement(index, hashKeyEntry); - return VisitKeyToTrackerAllEntries(hashKeyEntry, *m_pVisitor); - } - }; + WRAPPER_NO_CONTRACT; - class VisitAllEntryDependentTrackerHash + KeyToValuesHash &keyToTrackersHash = m_keyToDependentTrackersHash; + for (typename KeyToValuesHash::Iterator it = keyToTrackersHash.Begin(), itEnd = keyToTrackersHash.End(); it != itEnd; ++it) { - public: - Visitor *m_pVisitor; - GCHeapHashDependentHashTrackerHash *m_pDependentTrackerHash; - - VisitAllEntryDependentTrackerHash(Visitor *pVisitor, GCHeapHashDependentHashTrackerHash *pDependentTrackerHash) : - m_pVisitor(pVisitor), - m_pDependentTrackerHash(pDependentTrackerHash) - {} - - bool operator()(INT32 index) + KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry = *it; + if (!VisitKeyToTrackerAllLALocalEntries(hashKeyEntry, visitor)) { - WRAPPER_NO_CONTRACT; - - LAHASHDEPENDENTHASHTRACKERREF dependentTracker; - m_pDependentTrackerHash->GetElement(index, dependentTracker); - return VisitTrackerAllEntries(dependentTracker, *m_pVisitor); + return false; } - }; + } - struct + LAHashDependentHashTrackerHash &dependentTrackerHash = m_loaderAllocatorToDependentTrackerHash; + for (typename LAHashDependentHashTrackerHash::Iterator it = dependentTrackerHash.Begin(), + itEnd = dependentTrackerHash.End(); + it != itEnd; + ++it) { - KeyToValuesGCHeapHash keyToTrackersHash; - GCHeapHashDependentHashTrackerHash dependentTrackerHash; - } gc; - ZeroMemory(&gc, sizeof(gc)); - bool result = true; - GCPROTECT_BEGIN(gc) - { - if (m_keyToDependentTrackersHash != NULL) - { - // Visit all local entries - gc.keyToTrackersHash = KeyToValuesGCHeapHash((GCHEAPHASHOBJECTREF)ObjectFromHandle(m_keyToDependentTrackersHash)); - VisitAllEntryKeyToDependentTrackerHash visitAllEntryKeys(&visitor, &gc.keyToTrackersHash); - result = gc.keyToTrackersHash.VisitAllEntryIndices(visitAllEntryKeys); - } - - if (m_loaderAllocatorToDependentTrackerHash != NULL) + LAHashDependentHashTracker *dependentTracker = *it; + if (!VisitTrackerAllEntries(dependentTracker, visitor)) { - // Visit the non-local data - gc.dependentTrackerHash = GCHeapHashDependentHashTrackerHash((GCHEAPHASHOBJECTREF)ObjectFromHandle(m_loaderAllocatorToDependentTrackerHash)); - VisitAllEntryDependentTrackerHash visitDependentTrackers(&visitor, &gc.dependentTrackerHash); - result = gc.dependentTrackerHash.VisitAllEntryIndices(visitDependentTrackers); + return false; } } - GCPROTECT_END(); - return result; + return true; } #ifndef DACCESS_COMPILE @@ -728,96 +641,67 @@ void CrossLoaderAllocatorHash::RemoveAll(TKey key) { CONTRACTL { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; } CONTRACTL_END; - class DeleteIndividualEntryKeyValueHash - { - public: - TKey m_key; - GCHeapHashDependentHashTrackerHash *m_pDependentTrackerHash; - - DeleteIndividualEntryKeyValueHash(TKey key, GCHeapHashDependentHashTrackerHash *pDependentTrackerHash) : - m_key(key), - m_pDependentTrackerHash(pDependentTrackerHash) - {} - - bool operator()(INT32 index) - { - WRAPPER_NO_CONTRACT; - - LAHASHDEPENDENTHASHTRACKERREF dependentTracker; - m_pDependentTrackerHash->GetElement(index, dependentTracker); - DeleteEntryTracker(m_key, dependentTracker); - return true; - } - }; - // This data structure actually doesn't have this invariant, but it is expected that uses of this // data structure will require that the key's loader allocator is the same as that of this data structure. - _ASSERTE(key->GetLoaderAllocator() == m_pLoaderAllocator); + _ASSERTE(TRAITS::GetLoaderAllocator(key) == m_pLoaderAllocator); - if (m_keyToDependentTrackersHash == NULL) + KeyToValuesHash &keyToTrackersHash = m_keyToDependentTrackersHash; + KeyValueStoreOrLAHashKeyToTrackers *const *hashKeyEntryPtr = keyToTrackersHash.LookupPtr(key); + if (hashKeyEntryPtr == NULL) { - return; // Nothing was ever added, so removing all is easy + return; } - struct + // We have an entry in the hashtable for the key/dependenthandle. + KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry = *hashKeyEntryPtr; + LAHashKeyToTrackers *hashKeyToTrackers = NULL; + KeyValueStore *keyValueStore; + + if (hashKeyEntry->IsLAHashKeyToTrackers()) { - KeyToValuesGCHeapHash keyToTrackersHash; - GCHeapHashDependentHashTrackerHash dependentTrackerHash; - LAHASHDEPENDENTHASHTRACKERREF dependentTracker; - OBJECTREF hashKeyEntry; - LAHASHKEYTOTRACKERSREF hashKeyToTrackers; - OBJECTREF keyValueStore; - } gc; - ZeroMemory(&gc, sizeof(gc)); - GCPROTECT_BEGIN(gc) + hashKeyToTrackers = static_cast(hashKeyEntry); + keyValueStore = hashKeyToTrackers->_laLocalKeyValueStore; + } + else { - gc.keyToTrackersHash = KeyToValuesGCHeapHash((GCHEAPHASHOBJECTREF)ObjectFromHandle(m_keyToDependentTrackersHash)); - INT32 index = gc.keyToTrackersHash.GetValueIndex(&key); - if (index != -1) - { - // We have an entry in the hashtable for the key/dependenthandle. - gc.keyToTrackersHash.GetElement(index, gc.hashKeyEntry); + keyValueStore = static_cast(hashKeyEntry); + } - if (gc.hashKeyEntry->GetMethodTable() == CoreLibBinder::GetExistingClass(CLASS__LAHASHKEYTOTRACKERS)) - { - gc.hashKeyToTrackers = (LAHASHKEYTOTRACKERSREF)gc.hashKeyEntry; - gc.keyValueStore = gc.hashKeyToTrackers->_laLocalKeyValueStore; - } - else - { - gc.keyValueStore = gc.hashKeyEntry; - } + // Now hashKeyToTrackers is filled in - // Now gc.hashKeyToTrackers is filled in + if (hashKeyToTrackers != NULL) + { + // Is there a single dependenttracker here, or a set. - if (gc.hashKeyToTrackers != NULL) + if (!hashKeyToTrackers->_trackerOrTrackerSet->IsTrackerSet()) + { + LAHashDependentHashTracker *dependentTracker = + static_cast(hashKeyToTrackers->_trackerOrTrackerSet); + DeleteEntryTracker(key, dependentTracker); + } + else + { + LAHashDependentHashTrackerHash *dependentTrackerHash = + static_cast(hashKeyToTrackers->_trackerOrTrackerSet)->GetTrackerSet(); + for (typename LAHashDependentHashTrackerHash::Iterator it = dependentTrackerHash->Begin(), + itEnd = dependentTrackerHash->End(); + it != itEnd; + ++it) { - // Is there a single dependenttracker here, or a set. - - if (gc.hashKeyToTrackers->_trackerOrTrackerSet->GetMethodTable() == CoreLibBinder::GetExistingClass(CLASS__LAHASHDEPENDENTHASHTRACKER)) - { - gc.dependentTracker = (LAHASHDEPENDENTHASHTRACKERREF)gc.hashKeyToTrackers->_trackerOrTrackerSet; - DeleteEntryTracker(key, gc.dependentTracker); - } - else - { - gc.dependentTrackerHash = GCHeapHashDependentHashTrackerHash(gc.hashKeyToTrackers->_trackerOrTrackerSet); - DeleteIndividualEntryKeyValueHash deleteIndividualKeyValues(key, &gc.dependentTrackerHash); - gc.dependentTrackerHash.VisitAllEntryIndices(deleteIndividualKeyValues); - } + LAHashDependentHashTracker *dependentTracker = *it; + DeleteEntryTracker(key, dependentTracker); } - - // Remove entry from key to tracker hash - gc.keyToTrackersHash.DeleteEntry(&key); } } - GCPROTECT_END(); + + // Remove entry from key to tracker hash + keyToTrackersHash.RemovePtr(const_cast(hashKeyEntryPtr)); } #endif // !DACCESS_COMPILE @@ -830,399 +714,203 @@ void CrossLoaderAllocatorHash::Init(LoaderAllocator *pAssociatedLoaderAl template template -/*static*/ bool CrossLoaderAllocatorHash::VisitKeyValueStore(OBJECTREF *pLoaderAllocatorRef, OBJECTREF *pKeyValueStore, Visitor &visitor) +/*static*/ bool CrossLoaderAllocatorHash::VisitKeyValueStore(LoaderAllocator *loaderAllocator, KeyValueStore *keyValueStore, Visitor &visitor) { WRAPPER_NO_CONTRACT; - return TRAITS::VisitKeyValueStore(pLoaderAllocatorRef, pKeyValueStore, visitor); + return KeyToValuesHashTraits::VisitKeyValueStore(loaderAllocator, keyValueStore, visitor); } template template -/*static*/ bool CrossLoaderAllocatorHash::VisitTracker(TKey key, LAHASHDEPENDENTHASHTRACKERREF trackerUnsafe, Visitor &visitor) +/*static*/ bool CrossLoaderAllocatorHash::VisitTracker(TKey key, LAHashDependentHashTracker *tracker, Visitor &visitor) { - CONTRACTL - { - MODE_COOPERATIVE; - GC_NOTRIGGER; - } - CONTRACTL_END; - - struct - { - LAHASHDEPENDENTHASHTRACKERREF tracker; - OBJECTREF loaderAllocatorRef; - GCHEAPHASHOBJECTREF keyToValuesHashObject; - KeyToValuesGCHeapHash keyToValuesHash; - OBJECTREF keyValueStore; - }gc; - - ZeroMemory(&gc, sizeof(gc)); - gc.tracker = trackerUnsafe; - - bool result = true; + WRAPPER_NO_CONTRACT; - GCPROTECT_BEGIN(gc); + KeyToValuesHash *keyToValuesHash = tracker->GetDependentKeyToValuesHash(); + if (keyToValuesHash != NULL) { - gc.tracker->GetDependentAndLoaderAllocator(&gc.loaderAllocatorRef, &gc.keyToValuesHashObject); - if (gc.keyToValuesHashObject != NULL) + KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry = keyToValuesHash->Lookup(key); + if (hashKeyEntry != NULL) { - gc.keyToValuesHash = KeyToValuesGCHeapHash(gc.keyToValuesHashObject); - INT32 indexInKeyValueHash = gc.keyToValuesHash.GetValueIndex(&key); - if (indexInKeyValueHash != -1) - { - gc.keyToValuesHash.GetElement(indexInKeyValueHash, gc.keyValueStore); - - result = VisitKeyValueStore(&gc.loaderAllocatorRef, &gc.keyValueStore, visitor); - } + _ASSERTE(!hashKeyEntry->IsLAHashKeyToTrackers()); + return VisitKeyValueStore(tracker->GetLoaderAllocatorUnsafe(), static_cast(hashKeyEntry), visitor); } } - GCPROTECT_END(); - return result; + return true; } template template -/*static*/ bool CrossLoaderAllocatorHash::VisitTrackerAllEntries(LAHASHDEPENDENTHASHTRACKERREF trackerUnsafe, Visitor &visitor) +/*static*/ bool CrossLoaderAllocatorHash::VisitTrackerAllEntries(LAHashDependentHashTracker *tracker, Visitor &visitor) { - CONTRACTL - { - MODE_COOPERATIVE; - GC_NOTRIGGER; - } - CONTRACTL_END; - - struct - { - LAHASHDEPENDENTHASHTRACKERREF tracker; - OBJECTREF loaderAllocatorRef; - GCHEAPHASHOBJECTREF keyToValuesHashObject; - KeyToValuesGCHeapHash keyToValuesHash; - OBJECTREF keyValueStore; - }gc; - - class VisitAllEntryKeyValueHash - { - public: - Visitor *m_pVisitor; - KeyToValuesGCHeapHash *m_pKeysToValueHash; - OBJECTREF *m_pKeyValueStore; - OBJECTREF *m_pLoaderAllocatorRef; - - VisitAllEntryKeyValueHash(Visitor *pVisitor, KeyToValuesGCHeapHash *pKeysToValueHash, OBJECTREF *pKeyValueStore, OBJECTREF *pLoaderAllocatorRef) : - m_pVisitor(pVisitor), - m_pKeysToValueHash(pKeysToValueHash), - m_pKeyValueStore(pKeyValueStore), - m_pLoaderAllocatorRef(pLoaderAllocatorRef) - {} - - bool operator()(INT32 index) - { - WRAPPER_NO_CONTRACT; - - m_pKeysToValueHash->GetElement(index, *m_pKeyValueStore); - return VisitKeyValueStore(m_pLoaderAllocatorRef, m_pKeyValueStore, m_pVisitor); - } - }; - - ZeroMemory(&gc, sizeof(gc)); - gc.tracker = trackerUnsafe; - - bool result = true; + WRAPPER_NO_CONTRACT; - GCPROTECT_BEGIN(gc); + KeyToValuesHash *keyToValuesHash = tracker->GetDependentKeyToValuesHash(); + if (keyToValuesHash != NULL) { - gc.tracker->GetDependentAndLoaderAllocator(&gc.loaderAllocatorRef, &gc.keyToValuesHashObject); - if (gc.keyToValuesHashObject != NULL) + LoaderAllocator *loaderAllocator = tracker->GetLoaderAllocatorUnsafe(); + for (typename KeyToValuesHash::Iterator it = keyToValuesHash->Begin(), itEnd = keyToValuesHash->End(); + it != itEnd; + ++it) { - gc.keyToValuesHash = KeyToValuesGCHeapHash(gc.keyToValuesHashObject); - result = gc.keyToValuesHash.VisitAllEntryIndices(VisitAllEntryKeyValueHash(&visitor, &gc.keyToValuesHash, &gc.keyValueStore, &gc.loaderAllocatorRef)); + KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry = *it; + _ASSERTE(!hashKeyEntry->IsLAHashKeyToTrackers()); + if (!VisitKeyValueStore(loaderAllocator, static_cast(hashKeyEntry), visitor)) + { + return false; + } } } - GCPROTECT_END(); - return result; + return true; } template template -/*static*/ bool CrossLoaderAllocatorHash::VisitKeyToTrackerAllEntries(OBJECTREF hashKeyEntryUnsafe, Visitor &visitor) +/*static*/ bool CrossLoaderAllocatorHash::VisitKeyToTrackerAllLALocalEntries(KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry, Visitor &visitor) { - CONTRACTL - { - MODE_COOPERATIVE; - GC_NOTRIGGER; - } - CONTRACTL_END; + WRAPPER_NO_CONTRACT; - struct + KeyValueStore *keyValueStore; + if (hashKeyEntry->IsLAHashKeyToTrackers()) { - OBJECTREF hashKeyEntry; - LAHASHKEYTOTRACKERSREF hashKeyToTrackers; - OBJECTREF keyValueStore; - OBJECTREF loaderAllocatorRef; - } gc; - - ZeroMemory(&gc, sizeof(gc)); - gc.hashKeyEntry = hashKeyEntryUnsafe; - - bool result = true; - - GCPROTECT_BEGIN(gc); + LAHashKeyToTrackers *hashKeyToTrackers = static_cast(hashKeyEntry); + keyValueStore = hashKeyToTrackers->_laLocalKeyValueStore; + } + else { - if (gc.hashKeyEntry->GetMethodTable() == CoreLibBinder::GetExistingClass(CLASS__LAHASHKEYTOTRACKERS)) - { - gc.hashKeyToTrackers = (LAHASHKEYTOTRACKERSREF)gc.hashKeyEntry; - gc.keyValueStore = gc.hashKeyToTrackers->_laLocalKeyValueStore; - } - else - { - gc.keyValueStore = gc.hashKeyEntry; - } - - result = VisitKeyValueStore(&gc.loaderAllocatorRef, &gc.keyValueStore, visitor); + keyValueStore = static_cast(hashKeyEntry); } - GCPROTECT_END(); - return result; + return VisitKeyValueStore(NULL, keyValueStore, visitor); } template -/*static*/ void CrossLoaderAllocatorHash::DeleteEntryTracker(TKey key, LAHASHDEPENDENTHASHTRACKERREF trackerUnsafe) +/*static*/ void CrossLoaderAllocatorHash::DeleteEntryTracker(TKey key, LAHashDependentHashTracker *tracker) { CONTRACTL { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; } CONTRACTL_END; - struct - { - LAHASHDEPENDENTHASHTRACKERREF tracker; - OBJECTREF loaderAllocatorRef; - GCHEAPHASHOBJECTREF keyToValuesHashObject; - KeyToValuesGCHeapHash keyToValuesHash; - }gc; - - ZeroMemory(&gc, sizeof(gc)); - gc.tracker = trackerUnsafe; - - GCPROTECT_BEGIN(gc); + KeyToValuesHash *keyToValuesHash = tracker->GetDependentKeyToValuesHash(); + if (keyToValuesHash != NULL) { - gc.tracker->GetDependentAndLoaderAllocator(&gc.loaderAllocatorRef, &gc.keyToValuesHashObject); - if (gc.keyToValuesHashObject != NULL) - { - gc.keyToValuesHash = KeyToValuesGCHeapHash(gc.keyToValuesHashObject); - gc.keyToValuesHash.DeleteEntry(&key); - } + keyToValuesHash->Remove(key); } - GCPROTECT_END(); } #ifndef DACCESS_COMPILE -/*static */inline void CrossLoaderAllocatorHashSetup::EnsureTypesLoaded() -{ - STANDARD_VM_CONTRACT; - - // Force these types to be loaded, so that the hashtable logic can use CoreLibBinder::GetExistingClass - // throughout and avoid lock ordering issues - CoreLibBinder::GetClass(CLASS__LAHASHKEYTOTRACKERS); - CoreLibBinder::GetClass(CLASS__LAHASHDEPENDENTHASHTRACKER); - CoreLibBinder::GetClass(CLASS__GCHEAPHASH); - TypeHandle elemType = TypeHandle(CoreLibBinder::GetElementType(ELEMENT_TYPE_I1)); - TypeHandle typHnd = ClassLoader::LoadArrayTypeThrowing(elemType, ELEMENT_TYPE_SZARRAY, 0); - elemType = TypeHandle(CoreLibBinder::GetElementType(ELEMENT_TYPE_OBJECT)); - typHnd = ClassLoader::LoadArrayTypeThrowing(elemType, ELEMENT_TYPE_SZARRAY, 0); -} - template -void CrossLoaderAllocatorHash::EnsureManagedObjectsInitted() +typename CrossLoaderAllocatorHash::LAHashDependentHashTracker * +CrossLoaderAllocatorHash::GetDependentTrackerForLoaderAllocator(LoaderAllocator *pLoaderAllocator) { CONTRACTL { THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; + GC_NOTRIGGER; + MODE_ANY; } CONTRACTL_END; - if (m_loaderAllocatorToDependentTrackerHash == NULL) + LAHashDependentHashTrackerHash &dependentTrackerHash = m_loaderAllocatorToDependentTrackerHash; + LAHashDependentHashTracker *dependentTracker = dependentTrackerHash.Lookup(pLoaderAllocator); + if (dependentTracker != NULL) { - OBJECTREF laToDependentHandleHashObject = AllocateObject(CoreLibBinder::GetExistingClass(CLASS__GCHEAPHASH)); - m_loaderAllocatorToDependentTrackerHash = m_pLoaderAllocator->GetDomain()->CreateHandle(laToDependentHandleHashObject); - m_pLoaderAllocator->RegisterHandleForCleanup(m_loaderAllocatorToDependentTrackerHash); + // We have an entry in the hashtable for the key/dependenthandle. + return dependentTracker; } - if (m_keyToDependentTrackersHash == NULL) - { - OBJECTREF m_keyToDependentTrackersHashObject = AllocateObject(CoreLibBinder::GetExistingClass(CLASS__GCHEAPHASH)); - m_keyToDependentTrackersHash = m_pLoaderAllocator->GetDomain()->CreateHandle(m_keyToDependentTrackersHashObject); - m_pLoaderAllocator->RegisterHandleForCleanup(m_keyToDependentTrackersHash); - } + NewHolder laDependentKeyToValuesHashHolder = new LADependentKeyToValuesHash(); + typename LAHashDependentHashTracker::NewTrackerHolder dependentTrackerHolder = + new LAHashDependentHashTracker(pLoaderAllocator, laDependentKeyToValuesHashHolder); + laDependentKeyToValuesHashHolder.SuppressRelease(); + + dependentTrackerHash.Add(dependentTrackerHolder); + return dependentTrackerHolder.Extract(); } #endif // !DACCESS_COMPILE #ifndef DACCESS_COMPILE template -LAHASHDEPENDENTHASHTRACKERREF CrossLoaderAllocatorHash::GetDependentTrackerForLoaderAllocator(LoaderAllocator* pLoaderAllocator) +typename CrossLoaderAllocatorHash::KeyToValuesHash * +CrossLoaderAllocatorHash::GetKeyToValueCrossLAHashForHashkeyToTrackers( + LAHashKeyToTrackers *hashKeyToTrackers, + LoaderAllocator *pValueLoaderAllocator) { CONTRACTL { THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; + GC_NOTRIGGER; + MODE_ANY; } CONTRACTL_END; - struct - { - GCHeapHashDependentHashTrackerHash dependentTrackerHash; - LAHASHDEPENDENTHASHTRACKERREF dependentTracker; - GCHEAPHASHOBJECTREF GCHeapHashForKeyToValueStore; - } gc; - ZeroMemory(&gc, sizeof(gc)); - GCPROTECT_BEGIN(gc) - { - gc.dependentTrackerHash = GCHeapHashDependentHashTrackerHash((GCHEAPHASHOBJECTREF)ObjectFromHandle(m_loaderAllocatorToDependentTrackerHash)); - INT32 index = gc.dependentTrackerHash.GetValueIndex(&pLoaderAllocator); - if (index != -1) - { - // We have an entry in the hashtable for the key/dependenthandle. - gc.dependentTrackerHash.GetElement(index, gc.dependentTracker); - } - else - { - gc.dependentTracker = (LAHASHDEPENDENTHASHTRACKERREF)AllocateObject(CoreLibBinder::GetExistingClass(CLASS__LAHASHDEPENDENTHASHTRACKER)); - gc.GCHeapHashForKeyToValueStore = (GCHEAPHASHOBJECTREF)AllocateObject(CoreLibBinder::GetExistingClass(CLASS__GCHEAPHASH)); + LAHashDependentHashTracker *dependentTracker; - OBJECTREF exposedObject = pLoaderAllocator->GetExposedObject(); - if (exposedObject == NULL) - { - if (m_globalDependentTrackerRootHandle == NULL) - { - // Global LoaderAllocator does not have an exposed object, so create a fake one - exposedObject = AllocateObject(CoreLibBinder::GetExistingClass(CLASS__OBJECT)); - m_globalDependentTrackerRootHandle = GetAppDomain()->CreateHandle(exposedObject); - m_pLoaderAllocator->RegisterHandleForCleanup(m_globalDependentTrackerRootHandle); - } - else - { - exposedObject = ObjectFromHandle(m_globalDependentTrackerRootHandle); - } - } - - OBJECTHANDLE dependentHandle = GetAppDomain()->CreateDependentHandle(exposedObject, gc.GCHeapHashForKeyToValueStore); - gc.dependentTracker->Init(dependentHandle, pLoaderAllocator); - gc.dependentTrackerHash.Add(&pLoaderAllocator, [&gc](PTRARRAYREF arr, INT32 index) - { - arr->SetAt(index, (OBJECTREF)gc.dependentTracker); - }); - } - } - GCPROTECT_END(); - - return gc.dependentTracker; -} -#endif // !DACCESS_COMPILE - -#ifndef DACCESS_COMPILE -template -GCHEAPHASHOBJECTREF CrossLoaderAllocatorHash::GetKeyToValueCrossLAHashForHashkeyToTrackers(LAHASHKEYTOTRACKERSREF hashKeyToTrackersUnsafe, LoaderAllocator* pValueLoaderAllocator) -{ - CONTRACTL + // Is there a single dependenttracker here, or a set, or no dependenttracker at all + if (hashKeyToTrackers->_trackerOrTrackerSet == NULL) { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; + dependentTracker = GetDependentTrackerForLoaderAllocator(pValueLoaderAllocator); + hashKeyToTrackers->_trackerOrTrackerSet = dependentTracker; + dependentTracker->IncRefCount(); } - CONTRACTL_END; - - struct - { - GCHeapHashDependentHashTrackerHash dependentTrackerHash; - LAHASHDEPENDENTHASHTRACKERREF dependentTrackerMaybe; - LAHASHDEPENDENTHASHTRACKERREF dependentTracker; - LAHASHKEYTOTRACKERSREF hashKeyToTrackers; - GCHEAPHASHOBJECTREF returnValue; - } gc; - ZeroMemory(&gc, sizeof(gc)); - // Now gc.hashKeyToTrackers is filled in. - gc.hashKeyToTrackers = hashKeyToTrackersUnsafe; - GCPROTECT_BEGIN(gc) + else if (!hashKeyToTrackers->_trackerOrTrackerSet->IsTrackerSet()) { - EnsureManagedObjectsInitted(); - - // Is there a single dependenttracker here, or a set, or no dependenttracker at all - if (gc.hashKeyToTrackers->_trackerOrTrackerSet == NULL) + LAHashDependentHashTracker *dependentTrackerMaybe = + static_cast(hashKeyToTrackers->_trackerOrTrackerSet); + if (dependentTrackerMaybe->IsTrackerFor(pValueLoaderAllocator)) { - gc.dependentTracker = GetDependentTrackerForLoaderAllocator(pValueLoaderAllocator); - SetObjectReference(&gc.hashKeyToTrackers->_trackerOrTrackerSet, gc.dependentTracker); + // We've found the right dependent tracker. + dependentTracker = dependentTrackerMaybe; } - else if (gc.hashKeyToTrackers->_trackerOrTrackerSet->GetMethodTable() == CoreLibBinder::GetExistingClass(CLASS__LAHASHDEPENDENTHASHTRACKER)) + else { - gc.dependentTrackerMaybe = (LAHASHDEPENDENTHASHTRACKERREF)gc.hashKeyToTrackers->_trackerOrTrackerSet; - if (gc.dependentTrackerMaybe->IsTrackerFor(pValueLoaderAllocator)) + dependentTracker = GetDependentTrackerForLoaderAllocator(pValueLoaderAllocator); + if (!dependentTrackerMaybe->IsLoaderAllocatorLive()) { - // We've found the right dependent tracker. - gc.dependentTracker = gc.dependentTrackerMaybe; + hashKeyToTrackers->_trackerOrTrackerSet = dependentTracker; + dependentTrackerMaybe->DecRefCount(); + dependentTracker->IncRefCount(); } else { - gc.dependentTracker = GetDependentTrackerForLoaderAllocator(pValueLoaderAllocator); - if (!gc.dependentTrackerMaybe->IsLoaderAllocatorLive()) - { - SetObjectReference(&gc.hashKeyToTrackers->_trackerOrTrackerSet, gc.dependentTracker); - } - else - { - // Allocate the dependent tracker hash - // Fill with the existing dependentTrackerMaybe, and gc.DependentTracker - gc.dependentTrackerHash = GCHeapHashDependentHashTrackerHash(AllocateObject(CoreLibBinder::GetExistingClass(CLASS__GCHEAPHASH))); - LoaderAllocator *pLoaderAllocatorKey = gc.dependentTracker->GetLoaderAllocatorUnsafe(); - gc.dependentTrackerHash.Add(&pLoaderAllocatorKey, [&gc](PTRARRAYREF arr, INT32 index) - { - arr->SetAt(index, (OBJECTREF)gc.dependentTracker); - }); - pLoaderAllocatorKey = gc.dependentTrackerMaybe->GetLoaderAllocatorUnsafe(); - gc.dependentTrackerHash.Add(&pLoaderAllocatorKey, [&gc](PTRARRAYREF arr, INT32 index) - { - arr->SetAt(index, (OBJECTREF)gc.dependentTrackerMaybe); - }); - SetObjectReference(&gc.hashKeyToTrackers->_trackerOrTrackerSet, gc.dependentTrackerHash.GetGCHeapRef()); - } + // Allocate the dependent tracker hash + // Fill with the existing dependentTrackerMaybe, and DependentTracker + NewHolder dependentTrackerHashWrapperHolder = + new LAHashDependentHashTrackerSetWrapper(); + LAHashDependentHashTrackerHash *dependentTrackerHash = dependentTrackerHashWrapperHolder->GetTrackerSet(); + dependentTrackerHash->Add(dependentTracker); + dependentTracker->IncRefCount(); + dependentTrackerHash->Add(dependentTrackerMaybe); + hashKeyToTrackers->_trackerOrTrackerSet = dependentTrackerHashWrapperHolder.Extract(); } } - else + } + else + { + LAHashDependentHashTrackerHash *dependentTrackerHash = + static_cast(hashKeyToTrackers->_trackerOrTrackerSet)->GetTrackerSet(); + + dependentTracker = dependentTrackerHash->Lookup(pValueLoaderAllocator); + if (dependentTracker == NULL) { - gc.dependentTrackerHash = GCHeapHashDependentHashTrackerHash(gc.hashKeyToTrackers->_trackerOrTrackerSet); + // Dependent tracker not yet attached to this key - INT32 indexOfTracker = gc.dependentTrackerHash.GetValueIndex(&pValueLoaderAllocator); - if (indexOfTracker == -1) - { - // Dependent tracker not yet attached to this key - - // Get dependent tracker - gc.dependentTracker = GetDependentTrackerForLoaderAllocator(pValueLoaderAllocator); - gc.dependentTrackerHash.Add(&pValueLoaderAllocator, [&gc](PTRARRAYREF arr, INT32 index) - { - arr->SetAt(index, (OBJECTREF)gc.dependentTracker); - }); - } - else - { - gc.dependentTrackerHash.GetElement(indexOfTracker, gc.dependentTracker); - } + // Get dependent tracker + dependentTracker = GetDependentTrackerForLoaderAllocator(pValueLoaderAllocator); + dependentTrackerHash->Add(dependentTracker); + dependentTracker->IncRefCount(); } - - // At this stage gc.dependentTracker is setup to have a good value - gc.dependentTracker->GetDependentAndLoaderAllocator(NULL, &gc.returnValue); } - GCPROTECT_END(); - return gc.returnValue; + // At this stage dependentTracker is setup to have a good value + return dependentTracker->GetDependentKeyToValuesHash(); } #endif // !DACCESS_COMPILE diff --git a/src/coreclr/vm/fptrstubs.cpp b/src/coreclr/vm/fptrstubs.cpp index cd3dd33d4d863..05c4fedf22d74 100644 --- a/src/coreclr/vm/fptrstubs.cpp +++ b/src/coreclr/vm/fptrstubs.cpp @@ -156,7 +156,7 @@ PCODE FuncPtrStubs::GetFuncPtrStub(MethodDesc * pMD, PrecodeType type) _ASSERTE(pMD->IsVersionableWithVtableSlotBackpatch()); PCODE temporaryEntryPoint = pMD->GetTemporaryEntryPoint(); - MethodDescBackpatchInfoTracker::ConditionalLockHolderForGCPreemp slotBackpatchLockHolder; + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder; // Set the funcptr stub's entry point to the current entry point inside the lock and after the funcptr stub is exposed, // to synchronize with backpatching in MethodDesc::BackpatchEntryPointSlots() diff --git a/src/coreclr/vm/gcheaphashtable.h b/src/coreclr/vm/gcheaphashtable.h deleted file mode 100644 index d47ebaf232299..0000000000000 --- a/src/coreclr/vm/gcheaphashtable.h +++ /dev/null @@ -1,141 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -#ifndef GCHEAPHASHTABLE_H -#define GCHEAPHASHTABLE_H - -class GCHeapHashObject; - -template -struct DefaultGCHeapHashTraits -{ - typedef PTRARRAYREF THashArrayType; - static const INT32 s_growth_factor_numerator = 3; - static const INT32 s_growth_factor_denominator = 2; - - static const INT32 s_density_factor_numerator = 3; - static const INT32 s_density_factor_denominator = 4; - - static const INT32 s_densitywithdeletes_factor_numerator = 7; - static const INT32 s_densitywithdeletes_factor_denominator = 8; - - static const INT32 s_minimum_allocation = 7; - - static bool IsNull(PTRARRAYREF arr, INT32 index); - static bool IsDeleted(PTRARRAYREF arr, INT32 index, GCHEAPHASHOBJECTREF gcHeap); -#ifndef DACCESS_COMPILE - static THashArrayType AllocateArray(INT32 size); -#endif - - // Not a part of the traits api, but used to allow derived traits to save on code - static OBJECTREF GetValueAtIndex(GCHEAPHASHOBJECTREF *pgcHeap, INT32 index); - -#ifndef DACCESS_COMPILE - static void CopyValue(THashArrayType srcArray, INT32 indexSrc, THashArrayType destinationArray, INT32 indexDest); - static void DeleteEntry(GCHEAPHASHOBJECTREF *pgcHeap, INT32 index); -#endif // !DACCESS_COMPILE - - template - static void GetElement(GCHEAPHASHOBJECTREF *pgcHeap, INT32 index, TElement& foundElement); - -#ifndef DACCESS_COMPILE - template - static void SetElement(GCHEAPHASHOBJECTREF *pgcHeap, INT32 index, TElement& foundElement); -#endif // !DACCESS_COMPILE -}; - -template -struct GCHeapHashTraitsPointerToPointerList : public DefaultGCHeapHashTraits -{ - static INT32 Hash(PtrTypeKey *pValue); - static INT32 Hash(PTRARRAYREF arr, INT32 index); - static bool DoesEntryMatchKey(PTRARRAYREF arr, INT32 index, PtrTypeKey *pKey); -}; - - -// GCHeapHash is based on the logic of SHash, and utilizes the same basic structure (which allows the key/value -// to be one and the same, or other interesting memory tweaks.) To avoid GC pointer issues, responsibility for allocating -// the underlying arrays and manipulating the entries is entirely extracted to the traits class, and responsibility -// for creation of elements is deferred into the caller of the add function. (See example uses in CrossLoaderAllocatorHash) -// As the GCHeapHash is actually a managed object, but the code for manipulating the hash is written here in native code, -// allocating an instance of this class does not actually allocate a hashtable. Instead, the hashtable is allocated by -// allocating an instance of the GCHeapHash type, and then passing the allocated object into this type's constructor to -// assign the value. This class is designed to be used protected within a GC_PROTECT region. See examples in CrossLoaderAllocatorHash. -template -class GCHeapHash -{ - GCHEAPHASHOBJECTREF m_gcHeapHash; - - typedef typename TRAITS::THashArrayType THashArrayType; - typedef INT32 count_t; - - private: - // Insert into hashtable without growing. GCHEAPHASHOBJECTREF must be GC protected as must be TKey if needed - template - void Insert(TKey *pKey, const TValueSetter &valueSetter); - void CheckGrowth(); - void Grow(); - THashArrayType Grow_OnlyAllocateNewTable(); - - bool IsPrime(count_t number); - count_t NextPrime(count_t number); - - void ReplaceTable(THashArrayType newTable); - - template - count_t CallHash(TKey* pValue) - { - WRAPPER_NO_CONTRACT; - - count_t hash = TRAITS::Hash(pValue); - hash = hash < 0 ? -hash : hash; - if (hash < 0) - return 1; - else - return hash; - } - - count_t CallHash(THashArrayType arr, count_t index) - { - WRAPPER_NO_CONTRACT; - - count_t hash = TRAITS::Hash(arr, index); - hash = hash < 0 ? -hash : hash; - if (hash < 0) - return 1; - else - return hash; - } - - public: - - template - bool VisitAllEntryIndices(TVisitor &visitor); - - template - void Add(TKey *pKey, const TValueSetter &valueSetter); - - // Get the index in the hashtable of the value which matches key, or -1 if there are no matches - template - INT32 GetValueIndex(TKey *pKey); - - template - void GetElement(INT32 index, TElement& foundElement); - - // Use this to update an value within the hashtable directly. - // It is ONLY safe to do if the index already points at an element - // which already exists and has the same key as the newElementValue - template - void SetElement(INT32 index, TElement& newElementValue); - - template - void DeleteEntry(TKey *pKey); - - GCHEAPHASHOBJECTREF GetGCHeapRef() { LIMITED_METHOD_CONTRACT; return m_gcHeapHash; } - - GCHeapHash(GCHEAPHASHOBJECTREF gcHeap) : m_gcHeapHash(gcHeap) {} - GCHeapHash(OBJECTREF gcHeap) : m_gcHeapHash((GCHEAPHASHOBJECTREF)gcHeap) {} - GCHeapHash() : m_gcHeapHash((GCHEAPHASHOBJECTREF)TADDR(NULL)) {} -}; - -#endif // GCHEAPHASHTABLE_H diff --git a/src/coreclr/vm/gcheaphashtable.inl b/src/coreclr/vm/gcheaphashtable.inl deleted file mode 100644 index 6149dcc52dbc5..0000000000000 --- a/src/coreclr/vm/gcheaphashtable.inl +++ /dev/null @@ -1,529 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -#ifdef GCHEAPHASHTABLE_H -#ifndef GCHEAPHASHTABLE_INL -#define GCHEAPHASHTABLE_INL - -template -/*static */bool DefaultGCHeapHashTraits::IsNull(PTRARRAYREF arr, INT32 index) -{ - LIMITED_METHOD_CONTRACT; - - return arr->GetAt(index) == 0; -} - -template -/*static */bool DefaultGCHeapHashTraits::IsDeleted(PTRARRAYREF arr, INT32 index, GCHEAPHASHOBJECTREF gcHeap) -{ - LIMITED_METHOD_CONTRACT; - - if (removeSupported) - { - return gcHeap == arr->GetAt(index); - } - else - { - return false; - } -} - -#ifndef DACCESS_COMPILE -template -/*static*/ typename DefaultGCHeapHashTraits::THashArrayType DefaultGCHeapHashTraits::AllocateArray(INT32 size) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - return (THashArrayType)AllocateObjectArray(size, g_pObjectClass); -} -#endif // !DACCESS_COMPILE - - // Not a part of the traits api, but used to allow derived traits to save on code -template -/*static*/ OBJECTREF DefaultGCHeapHashTraits::GetValueAtIndex(GCHEAPHASHOBJECTREF *pgcHeap, INT32 index) -{ - LIMITED_METHOD_CONTRACT; - - PTRARRAYREF arr((PTRARRAYREF)(*pgcHeap)->GetData()); - - OBJECTREF value = arr->GetAt(index); - - return value; -} - -#ifndef DACCESS_COMPILE -template -/*static*/ void DefaultGCHeapHashTraits::CopyValue(THashArrayType srcArray, INT32 indexSrc, THashArrayType destinationArray, INT32 indexDest) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - if (srcArray == NULL) - COMPlusThrow(kNullReferenceException); - - if ((INT32)srcArray->GetNumComponents() < indexSrc) - COMPlusThrow(kIndexOutOfRangeException); - - OBJECTREF value = srcArray->GetAt(indexSrc); - - if ((INT32)destinationArray->GetNumComponents() < indexDest) - COMPlusThrow(kIndexOutOfRangeException); - - destinationArray->SetAt(indexDest, value); -} -#endif // !DACCESS_COMPILE - -#ifndef DACCESS_COMPILE -template -/*static*/ void DefaultGCHeapHashTraits::DeleteEntry(GCHEAPHASHOBJECTREF *pgcHeap, INT32 index) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - static_assert(removeSupported, "This hash doesn't support remove"); - - PTRARRAYREF arr((PTRARRAYREF)(*pgcHeap)->GetData()); - - if (arr == NULL) - COMPlusThrow(kNullReferenceException); - - if ((INT32)arr->GetNumComponents() < index) - COMPlusThrow(kIndexOutOfRangeException); - - // The deleted sentinel is a self-pointer - arr->SetAt(index, *pgcHeap); -} -#endif // !DACCESS_COMPILE - -template -template -/*static*/ void DefaultGCHeapHashTraits::GetElement(GCHEAPHASHOBJECTREF *pgcHeap, INT32 index, TElement& foundElement) -{ - LIMITED_METHOD_CONTRACT; - - foundElement = (TElement)GetValueAtIndex(pgcHeap, index); -} - -#ifndef DACCESS_COMPILE -template -template -/*static*/ void DefaultGCHeapHashTraits::SetElement(GCHEAPHASHOBJECTREF *pgcHeap, INT32 index, TElement& foundElement) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - PTRARRAYREF arr((PTRARRAYREF)(*pgcHeap)->GetData()); - - if (arr == NULL) - COMPlusThrow(kNullReferenceException); - - if ((INT32)arr->GetNumComponents() < index) - COMPlusThrow(kIndexOutOfRangeException); - - arr->SetAt(index, foundElement); -} -#endif // !DACCESS_COMPILE - -template -/*static */INT32 GCHeapHashTraitsPointerToPointerList::Hash(PtrTypeKey *pValue) -{ - LIMITED_METHOD_CONTRACT; - return (INT32)*pValue; -} - -template -/*static */INT32 GCHeapHashTraitsPointerToPointerList::Hash(PTRARRAYREF arr, INT32 index) -{ - LIMITED_METHOD_CONTRACT; - - UPTRARRAYREF value = (UPTRARRAYREF)arr->GetAt(index); - - return (INT32)*value->GetDirectConstPointerToNonObjectElements(); -} - -template -/*static */bool GCHeapHashTraitsPointerToPointerList::DoesEntryMatchKey(PTRARRAYREF arr, INT32 index, PtrTypeKey *pKey) -{ - LIMITED_METHOD_CONTRACT; - - UPTRARRAYREF value = (UPTRARRAYREF)arr->GetAt(index); - UPTR uptrValue = *value->GetDirectConstPointerToNonObjectElements(); - - return ((UPTR)*pKey) == uptrValue; -} - -template -template -void GCHeapHash::Insert(TKey *pKey, const TValueSetter &valueSetter) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - count_t hash = CallHash(pKey); - count_t tableSize = m_gcHeapHash->GetCapacity(); - count_t index = hash % tableSize; - count_t increment = 0; // delay computation - - while (TRUE) - { - THashArrayType arr((THashArrayType)(m_gcHeapHash)->GetData()); - - bool isNull = TRAITS::IsNull(arr, index); - bool isDeleted = false; - if (!isNull && TRAITS::IsDeleted(arr, index, m_gcHeapHash)) - isDeleted = true; - - if (isNull || isDeleted) - { - if (arr == NULL) - COMPlusThrow(kNullReferenceException); - - if ((INT32)arr->GetNumComponents() < index) - COMPlusThrow(kIndexOutOfRangeException); - - valueSetter(arr, index); - m_gcHeapHash->IncrementCount(isDeleted); - return; - } - - if (increment == 0) - increment = (hash % (tableSize-1)) + 1; - - index += increment; - if (index >= tableSize) - index -= tableSize; - } -} - -template -void GCHeapHash::CheckGrowth() -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - count_t tableMax = (count_t) (m_gcHeapHash->GetCapacity() * TRAITS::s_density_factor_numerator / TRAITS::s_density_factor_denominator); - if (m_gcHeapHash->GetCount() == tableMax) - Grow(); - else - { - tableMax = (count_t) (m_gcHeapHash->GetCapacity() * TRAITS::s_densitywithdeletes_factor_numerator / TRAITS::s_densitywithdeletes_factor_denominator); - if ((m_gcHeapHash->GetCount() + m_gcHeapHash->GetDeletedCount()) >= tableMax) - { - THashArrayType newTable = TRAITS::AllocateArray(m_gcHeapHash->GetCapacity()); - ReplaceTable(newTable); - } - } -} - -template -void GCHeapHash::Grow() -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - THashArrayType newTable = Grow_OnlyAllocateNewTable(); - ReplaceTable(newTable); -} - -template -typename GCHeapHash::THashArrayType GCHeapHash::Grow_OnlyAllocateNewTable() -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - INSTANCE_CHECK; - } - CONTRACTL_END; - - count_t newSize = (count_t) (m_gcHeapHash->GetCount() - * TRAITS::s_growth_factor_numerator / TRAITS::s_growth_factor_denominator - * TRAITS::s_density_factor_denominator / TRAITS::s_density_factor_numerator); - if (newSize < TRAITS::s_minimum_allocation) - newSize = TRAITS::s_minimum_allocation; - - // handle potential overflow - if (newSize < m_gcHeapHash->GetCount()) - ThrowOutOfMemory(); - - return TRAITS::AllocateArray(NextPrime(newSize)); -} - -template -bool GCHeapHash::IsPrime(count_t number) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - } - CONTRACTL_END; - - // This is a very low-tech check for primality, which doesn't scale very well. - // There are more efficient tests if this proves to be burdensome for larger - // tables. - - if ((number & 1) == 0) - return false; - - count_t factor = 3; - while (factor * factor <= number) - { - if ((number % factor) == 0) - return false; - factor += 2; - } - - return true; -} - -template -typename GCHeapHash::count_t GCHeapHash::NextPrime(count_t number) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - } - CONTRACTL_END; - - static_assert(sizeof(INT32) == sizeof(g_shash_primes[0]), "the cast below of g_shash_primes[] to INT32 isn't safe due to loss of precision."); - - for (int i = 0; i < (int) (sizeof(g_shash_primes) / sizeof(g_shash_primes[0])); i++) { - if ((INT32)g_shash_primes[i] >= number) - return (INT32)g_shash_primes[i]; - } - - if ((number&1) == 0) - number++; - - while (number != 1) { - if (IsPrime(number)) - return number; - number +=2; - } - - // overflow - ThrowOutOfMemory(); -} - -template -void GCHeapHash::ReplaceTable(THashArrayType newTable) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - GCPROTECT_BEGIN(newTable); - { - count_t newTableSize = (count_t)newTable->GetNumComponents(); - count_t oldTableSize = m_gcHeapHash->GetCapacity(); - - // Move all entries over to the new table - count_t capacity = m_gcHeapHash->GetCapacity(); - - for (count_t index = 0; index < capacity; ++index) - { - THashArrayType arr((THashArrayType)(m_gcHeapHash)->GetData()); - if (!TRAITS::IsNull(arr, index) && !TRAITS::IsDeleted(arr, index, m_gcHeapHash)) - { - count_t hash = CallHash(arr, index); - count_t tableSize = (count_t)newTable->GetNumComponents(); - count_t newIndex = hash % tableSize; - count_t increment = 0; // delay computation - - // Value to copy is in index - while (TRUE) - { - if (TRAITS::IsNull(newTable, newIndex)) - { - arr = (THashArrayType)(m_gcHeapHash)->GetData(); - TRAITS::CopyValue(arr, index, newTable, newIndex); - break; - } - - if (increment == 0) - increment = (hash % (tableSize-1)) + 1; - - newIndex += increment; - if (newIndex >= tableSize) - newIndex -= tableSize; - } - } - } - - m_gcHeapHash->SetTable((BASEARRAYREF)newTable); - - // We've just copied the table to a new table. There are no deleted items as - // we skipped them all, so reset the deleted count to zero - m_gcHeapHash->SetDeletedCountToZero(); - } - GCPROTECT_END(); -} - -template -template -bool GCHeapHash::VisitAllEntryIndices(TVisitor &visitor) -{ - WRAPPER_NO_CONTRACT; - - count_t capacity = m_gcHeapHash->GetCapacity(); - - for (count_t index = 0; index < capacity; ++index) - { - THashArrayType arr((THashArrayType)(m_gcHeapHash)->GetData()); - if (!TRAITS::IsNull(arr, index) && !TRAITS::IsDeleted(arr, index, m_gcHeapHash)) - { - if (!visitor(index)) - return false; - } - } - - return true; -} - -template -template -void GCHeapHash::Add(TKey *pKey, const TValueSetter &valueSetter) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - CheckGrowth(); - Insert(pKey, valueSetter); -} - - // Get the index in the hashtable of the value which matches key, or -1 if there are no matches -template -template -INT32 GCHeapHash::GetValueIndex(TKey *pKey) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - count_t hash = CallHash(pKey); - count_t tableSize = m_gcHeapHash->GetCapacity(); - - // If the table is empty, then there aren't any entries. Just return. - if (m_gcHeapHash->GetCount() == 0) - return -1; - - count_t index = hash % tableSize; - count_t increment = 0; // delay computation - - THashArrayType arr((THashArrayType)(m_gcHeapHash)->GetData()); - - while (m_gcHeapHash->GetCount() != 0) /* the TRAITS::IsDeleted function is allowed to reduce the count */ - { - if (TRAITS::IsNull(arr, index)) - { - return -1; - } - - if (!TRAITS::IsDeleted(arr, index, m_gcHeapHash) && TRAITS::DoesEntryMatchKey(arr, index, pKey)) - { - return index; - } - - if (increment == 0) - increment = (hash % (tableSize-1)) + 1; - - index += increment; - if (index >= tableSize) - index -= tableSize; - } - - return -1; -} - -template -template -void GCHeapHash::GetElement(INT32 index, TElement& foundElement) -{ - WRAPPER_NO_CONTRACT; - - TRAITS::GetElement(&m_gcHeapHash, index, foundElement); -} - - // Use this to update an value within the hashtable directly. - // It is ONLY safe to do if the index already points at an element - // which already exists and has the same key as the newElementValue -template -template -void GCHeapHash::SetElement(INT32 index, TElement& newElementValue) -{ - TRAITS::SetElement(&m_gcHeapHash, index, newElementValue); -} - -template -template -void GCHeapHash::DeleteEntry(TKey *pKey) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - INT32 index = GetValueIndex(pKey); - if (index != -1) - { - TRAITS::DeleteEntry(&m_gcHeapHash, index); - m_gcHeapHash->DecrementCount(true); - } -} - -#endif // GCHEAPHASHTABLE_INL -#endif // GCHEAPHASHTABLE_H diff --git a/src/coreclr/vm/inlinetracking.cpp b/src/coreclr/vm/inlinetracking.cpp index 86d73ea0a7657..4bd60ab598788 100644 --- a/src/coreclr/vm/inlinetracking.cpp +++ b/src/coreclr/vm/inlinetracking.cpp @@ -432,9 +432,9 @@ Module* PersistentInlineTrackingMapR2R2::GetModuleByIndex(DWORD index) #if !defined(DACCESS_COMPILE) -JITInlineTrackingMap::JITInlineTrackingMap(LoaderAllocator *pAssociatedLoaderAllocator) : - m_mapCrst(CrstJitInlineTrackingMap), - m_map() +CrstStatic JITInlineTrackingMap::s_mapCrst; + +JITInlineTrackingMap::JITInlineTrackingMap(LoaderAllocator *pAssociatedLoaderAllocator) : m_map() { LIMITED_METHOD_CONTRACT; @@ -445,12 +445,12 @@ BOOL JITInlineTrackingMap::InliningExistsDontTakeLock(MethodDesc *inliner, Metho { LIMITED_METHOD_CONTRACT; - _ASSERTE(m_mapCrst.OwnedByCurrentThread()); + _ASSERTE(s_mapCrst.OwnedByCurrentThread()); _ASSERTE(inliner != NULL); _ASSERTE(inlinee != NULL); BOOL found = FALSE; - auto lambda = [&](OBJECTREF obj, MethodDesc *lambdaInlinee, MethodDesc *lambdaInliner) + auto lambda = [&](LoaderAllocator *loaderAllocatorOfInliner, MethodDesc *lambdaInlinee, MethodDesc *lambdaInliner) { _ASSERTE(inlinee == lambdaInlinee); @@ -474,7 +474,7 @@ void JITInlineTrackingMap::AddInlining(MethodDesc *inliner, MethodDesc *inlinee) inlinee = inlinee->LoadTypicalMethodDefinition(); - CrstHolder holder(&m_mapCrst); + CrstHolder holder(&s_mapCrst); AddInliningDontTakeLock(inliner, inlinee); } @@ -482,12 +482,10 @@ void JITInlineTrackingMap::AddInliningDontTakeLock(MethodDesc *inliner, MethodDe { LIMITED_METHOD_CONTRACT; - _ASSERTE(m_mapCrst.OwnedByCurrentThread()); + _ASSERTE(s_mapCrst.OwnedByCurrentThread()); _ASSERTE(inliner != NULL); _ASSERTE(inlinee != NULL); - GCX_COOP(); - if (!InliningExistsDontTakeLock(inliner, inlinee)) { LoaderAllocator *loaderAllocatorOfInliner = inliner->GetLoaderAllocator(); diff --git a/src/coreclr/vm/inlinetracking.h b/src/coreclr/vm/inlinetracking.h index 9c01669db840a..d0d2621dbc767 100644 --- a/src/coreclr/vm/inlinetracking.h +++ b/src/coreclr/vm/inlinetracking.h @@ -370,10 +370,9 @@ class JITInlineTrackingMap } CONTRACTL_END; - GCX_COOP(); - CrstHolder holder(&m_mapCrst); + CrstHolder holder(&s_mapCrst); - auto lambda = [&](OBJECTREF obj, MethodDesc *lambdaInlinee, MethodDesc *lambdaInliner) + auto lambda = [&](LoaderAllocator *loaderAllocatorOfInliner, MethodDesc *lambdaInlinee, MethodDesc *lambdaInliner) { _ASSERTE(lambdaInlinee == inlinee); @@ -383,10 +382,18 @@ class JITInlineTrackingMap m_map.VisitValuesOfKey(inlinee, lambda); } + static void StaticInitialize() + { + WRAPPER_NO_CONTRACT; + s_mapCrst.Init(CrstJitInlineTrackingMap); + } + + static CrstBase *GetMapCrst() { return &s_mapCrst; } + private: BOOL InliningExistsDontTakeLock(MethodDesc *inliner, MethodDesc *inlinee); - Crst m_mapCrst; + static CrstStatic s_mapCrst; InliningInfoTrackerHash m_map; }; diff --git a/src/coreclr/vm/loaderallocator.cpp b/src/coreclr/vm/loaderallocator.cpp index 039bb3825837a..cbfda12770007 100644 --- a/src/coreclr/vm/loaderallocator.cpp +++ b/src/coreclr/vm/loaderallocator.cpp @@ -577,6 +577,10 @@ void LoaderAllocator::GCLoaderAllocators(LoaderAllocator* pOriginalLoaderAllocat pDomainLoaderAllocatorDestroyIterator->ReleaseManagedAssemblyLoadContext(); + // The native objects in dependent handles may refer to the virtual call stub manager's heaps, so clear the dependent + // handles first + pDomainLoaderAllocatorDestroyIterator->CleanupDependentHandlesToNativeObjects(); + // The following code was previously happening on delete ~DomainAssembly->Terminate // We are moving this part here in order to make sure that we can unload a LoaderAllocator // that didn't have a DomainAssembly @@ -1681,6 +1685,11 @@ void AssemblyLoaderAllocator::SetCollectible() void AssemblyLoaderAllocator::Init(AppDomain* pAppDomain) { m_Id.Init(); + + // This is CRST_UNSAFE_ANYMODE to enable registering/unregistering dependent handles to native objects without changing the + // GC mode, in case the caller requires that + m_dependentHandleToNativeObjectSetCrst.Init(CrstLeafLock, CRST_UNSAFE_ANYMODE); + LoaderAllocator::Init((BaseDomain *)pAppDomain); if (IsCollectible()) { @@ -1876,6 +1885,70 @@ void AssemblyLoaderAllocator::CleanupHandles() } } +void AssemblyLoaderAllocator::RegisterDependentHandleToNativeObjectForCleanup(LADependentHandleToNativeObject *dependentHandle) +{ + CONTRACTL + { + THROWS; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END; + + _ASSERTE(dependentHandle != nullptr); + + CrstHolder setLockHolder(&m_dependentHandleToNativeObjectSetCrst); + + _ASSERTE(m_dependentHandleToNativeObjectSet.Lookup(dependentHandle) == NULL); + m_dependentHandleToNativeObjectSet.Add(dependentHandle); +} + +void AssemblyLoaderAllocator::UnregisterDependentHandleToNativeObjectFromCleanup(LADependentHandleToNativeObject *dependentHandle) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END; + + _ASSERTE(dependentHandle != nullptr); + + CrstHolder setLockHolder(&m_dependentHandleToNativeObjectSetCrst); + + _ASSERTE(m_dependentHandleToNativeObjectSet.Lookup(dependentHandle) != NULL); + m_dependentHandleToNativeObjectSet.Remove(dependentHandle); +} + +void AssemblyLoaderAllocator::CleanupDependentHandlesToNativeObjects() +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_PREEMPTIVE; + } + CONTRACTL_END; + + // Locks under which dependent handles may be used must all be taken here to ensure that a thread using a dependent handle + // would either observe it cleared, or that the dependent object remains valid under those locks. In particular, any locks + // used to synchronize uses of CrossLoaderAllocatorHash instances must also be taken here. + CrstHolder jitInlineTrackingMapLockHolder(JITInlineTrackingMap::GetMapCrst()); + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder; + + CrstHolder setLockHolder(&m_dependentHandleToNativeObjectSetCrst); + + for (DependentHandleToNativeObjectSet::Iterator it = m_dependentHandleToNativeObjectSet.Begin(), + itEnd = m_dependentHandleToNativeObjectSet.End(); + it != itEnd; + ++it) + { + LADependentHandleToNativeObject *dependentHandle = *it; + dependentHandle->Clear(); + } +} + void LoaderAllocator::RegisterFailedTypeInitForCleanup(ListLockEntry *pListLockEntry) { CONTRACTL diff --git a/src/coreclr/vm/loaderallocator.hpp b/src/coreclr/vm/loaderallocator.hpp index 846ec6346d418..90311d3a01ca1 100644 --- a/src/coreclr/vm/loaderallocator.hpp +++ b/src/coreclr/vm/loaderallocator.hpp @@ -673,6 +673,12 @@ class LoaderAllocator PTR_OnStackReplacementManager GetOnStackReplacementManager(); #endif // FEATURE_ON_STACK_REPLACEMENT +#ifndef DACCESS_COMPILE +public: + virtual void RegisterDependentHandleToNativeObjectForCleanup(LADependentHandleToNativeObject *dependentHandle) {}; + virtual void UnregisterDependentHandleToNativeObjectFromCleanup(LADependentHandleToNativeObject *dependentHandle) {}; + virtual void CleanupDependentHandlesToNativeObjects() {}; +#endif }; // class LoaderAllocator typedef VPTR(LoaderAllocator) PTR_LoaderAllocator; @@ -766,6 +772,20 @@ class AssemblyLoaderAllocator : public LoaderAllocator #if !defined(DACCESS_COMPILE) CustomAssemblyBinder* m_binderToRelease; #endif + +private: + class DependentHandleToNativeObjectHashTraits : public PtrSetSHashTraits {}; + typedef SHash DependentHandleToNativeObjectSet; + + CrstExplicitInit m_dependentHandleToNativeObjectSetCrst; + DependentHandleToNativeObjectSet m_dependentHandleToNativeObjectSet; + +#ifndef DACCESS_COMPILE +public: + virtual void RegisterDependentHandleToNativeObjectForCleanup(LADependentHandleToNativeObject *dependentHandle); + virtual void UnregisterDependentHandleToNativeObjectFromCleanup(LADependentHandleToNativeObject *dependentHandle); + virtual void CleanupDependentHandlesToNativeObjects(); +#endif }; typedef VPTR(AssemblyLoaderAllocator) PTR_AssemblyLoaderAllocator; diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 6449386918767..08e4a3a278da6 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -3110,7 +3110,7 @@ void MethodDesc::RecordAndBackpatchEntryPointSlot( GCX_PREEMP(); LoaderAllocator *mdLoaderAllocator = GetLoaderAllocator(); - MethodDescBackpatchInfoTracker::ConditionalLockHolderForGCCoop slotBackpatchLockHolder; + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder; RecordAndBackpatchEntryPointSlot_Locked( mdLoaderAllocator, diff --git a/src/coreclr/vm/methoddescbackpatchinfo.cpp b/src/coreclr/vm/methoddescbackpatchinfo.cpp index 9a63100726776..2a8d438048ab7 100644 --- a/src/coreclr/vm/methoddescbackpatchinfo.cpp +++ b/src/coreclr/vm/methoddescbackpatchinfo.cpp @@ -77,9 +77,7 @@ void MethodDescBackpatchInfoTracker::Backpatch_Locked(MethodDesc *pMethodDesc, P _ASSERTE(IsLockOwnedByCurrentThread()); _ASSERTE(pMethodDesc != nullptr); - GCX_COOP(); - - auto lambda = [&entryPoint](OBJECTREF obj, MethodDesc *pMethodDesc, UINT_PTR slotData) + auto lambda = [&entryPoint](LoaderAllocator *pLoaderAllocatorOfSlot, MethodDesc *pMethodDesc, UINT_PTR slotData) { TADDR slot; @@ -101,8 +99,6 @@ void MethodDescBackpatchInfoTracker::AddSlotAndPatch_Locked(MethodDesc *pMethodD _ASSERTE(pMethodDesc != nullptr); _ASSERTE(pMethodDesc->MayHaveEntryPointSlotsToBackpatch()); - GCX_COOP(); - UINT_PTR slotData; slotData = EntryPointSlots::ConvertSlotAndTypePairToUINT_PTR(slot, slotType); diff --git a/src/coreclr/vm/methoddescbackpatchinfo.h b/src/coreclr/vm/methoddescbackpatchinfo.h index b94882bbf7271..d9c706cac6530 100644 --- a/src/coreclr/vm/methoddescbackpatchinfo.h +++ b/src/coreclr/vm/methoddescbackpatchinfo.h @@ -95,42 +95,23 @@ class MethodDescBackpatchInfoTracker static bool IsLockOwnedByCurrentThread(); #endif -public: - // To be used when the thread will remain in preemptive GC mode while holding the lock - class ConditionalLockHolderForGCPreemp : private CrstHolderWithState - { - public: - ConditionalLockHolderForGCPreemp(bool acquireLock = true) - : CrstHolderWithState( -#ifndef DACCESS_COMPILE - acquireLock ? &s_lock : nullptr -#else - nullptr -#endif - ) - { - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_PREEMPTIVE; - } - CONTRACTL_END; - } - - DISABLE_COPY(ConditionalLockHolderForGCPreemp); - }; - #ifndef DACCESS_COMPILE public: - // To be used when the thread may enter cooperative GC mode while holding the lock. The thread enters a - // forbid-suspend-for-debugger region along with acquiring the lock, such that it would not suspend for the debugger while - // holding the lock, as that may otherwise cause a FuncEval to deadlock when trying to acquire the lock. - class ConditionalLockHolderForGCCoop : private CrstAndForbidSuspendForDebuggerHolder + class ConditionalLockHolder : private CrstHolderWithState { + #ifdef ENABLE_CONTRACTS_IMPL + private: + GCNoTrigger m_gcNoTriggerHolder; + #endif + public: - ConditionalLockHolderForGCCoop(bool acquireLock = true) - : CrstAndForbidSuspendForDebuggerHolder(acquireLock ? &s_lock : nullptr) + // Acquire the lock and put the thread in a GC_NOTRIGGER scope to ensure that this lock cannot be held while the thread + // suspends for the debugger, otherwise FuncEvals that run may deadlock on acquiring this lock + ConditionalLockHolder(bool acquireLock = true) + : CrstHolderWithState(acquireLock ? &s_lock : nullptr) + #ifdef ENABLE_CONTRACTS_IMPL + , m_gcNoTriggerHolder(acquireLock, __FUNCTION__, __FILE__, __LINE__) + #endif { CONTRACTL { @@ -141,7 +122,7 @@ class MethodDescBackpatchInfoTracker CONTRACTL_END; } - DISABLE_COPY(ConditionalLockHolderForGCCoop); + DISABLE_COPY(ConditionalLockHolder); }; #endif diff --git a/src/coreclr/vm/object.cpp b/src/coreclr/vm/object.cpp index 8071392c9b4cc..34d5863d73f71 100644 --- a/src/coreclr/vm/object.cpp +++ b/src/coreclr/vm/object.cpp @@ -1952,20 +1952,3 @@ void ExceptionObject::GetStackTrace(StackTraceArray & stackTrace, PTRARRAYREF * #endif // !defined(DACCESS_COMPILE) } - -bool LAHashDependentHashTrackerObject::IsLoaderAllocatorLive() -{ - return (ObjectFromHandle(_dependentHandle) != NULL); -} - -void LAHashDependentHashTrackerObject::GetDependentAndLoaderAllocator(OBJECTREF *pLoaderAllocatorRef, GCHEAPHASHOBJECTREF *pGCHeapHash) -{ - OBJECTREF primary = ObjectFromHandle(_dependentHandle); - if (pLoaderAllocatorRef != NULL) - *pLoaderAllocatorRef = primary; - - IGCHandleManager *mgr = GCHandleUtilities::GetGCHandleManager(); - // Secondary is tracked only if primary is non-null - if (pGCHeapHash != NULL) - *pGCHeapHash = (GCHEAPHASHOBJECTREF)(OBJECTREF)((primary != NULL) ? mgr->GetDependentHandleSecondary(_dependentHandle) : NULL); -} diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h index bf3f9c0bc17bc..e64ece892585b 100644 --- a/src/coreclr/vm/object.h +++ b/src/coreclr/vm/object.h @@ -2700,133 +2700,4 @@ typedef REF EXCEPTIONREF; typedef PTR_ExceptionObject EXCEPTIONREF; #endif // USE_CHECKED_OBJECTREFS -class GCHeapHashObject : public Object -{ -#ifdef DACCESS_COMPILE - friend class ClrDataAccess; -#endif - friend class GCHeap; - friend class JIT_TrialAlloc; - friend class CheckAsmOffsets; - friend class COMString; - friend class CoreLibBinder; - - private: - BASEARRAYREF _data; - INT32 _count; - INT32 _deletedCount; - - public: - INT32 GetCount() { LIMITED_METHOD_CONTRACT; return _count; } - void IncrementCount(bool replacingDeletedItem) - { - LIMITED_METHOD_CONTRACT; - ++_count; - if (replacingDeletedItem) - --_deletedCount; - } - - void DecrementCount(bool deletingItem) - { - LIMITED_METHOD_CONTRACT; - --_count; - if (deletingItem) - ++_deletedCount; - } - INT32 GetDeletedCount() { LIMITED_METHOD_CONTRACT; return _deletedCount; } - void SetDeletedCountToZero() { LIMITED_METHOD_CONTRACT; _deletedCount = 0; } - INT32 GetCapacity() { LIMITED_METHOD_CONTRACT; if (_data == NULL) return 0; else return (_data->GetNumComponents()); } - BASEARRAYREF GetData() { LIMITED_METHOD_CONTRACT; return _data; } - - void SetTable(BASEARRAYREF data) - { - STATIC_CONTRACT_NOTHROW; - STATIC_CONTRACT_GC_NOTRIGGER; - STATIC_CONTRACT_MODE_COOPERATIVE; - - SetObjectReference((OBJECTREF*)&_data, (OBJECTREF)data); - } - - protected: - GCHeapHashObject() {LIMITED_METHOD_CONTRACT; } - ~GCHeapHashObject() {LIMITED_METHOD_CONTRACT; } -}; - -typedef DPTR(GCHeapHashObject) PTR_GCHeapHashObject; - -#ifdef USE_CHECKED_OBJECTREFS -typedef REF GCHEAPHASHOBJECTREF; -#else // USE_CHECKED_OBJECTREFS -typedef PTR_GCHeapHashObject GCHEAPHASHOBJECTREF; -#endif // USE_CHECKED_OBJECTREFS - -class LAHashDependentHashTrackerObject : public Object -{ -#ifdef DACCESS_COMPILE - friend class ClrDataAccess; -#endif - friend class CheckAsmOffsets; - friend class CoreLibBinder; - - private: - OBJECTHANDLE _dependentHandle; - LoaderAllocator* _loaderAllocator; - - public: - bool IsLoaderAllocatorLive(); - bool IsTrackerFor(LoaderAllocator *pLoaderAllocator) - { - if (pLoaderAllocator != _loaderAllocator) - return false; - - return IsLoaderAllocatorLive(); - } - - void GetDependentAndLoaderAllocator(OBJECTREF *pLoaderAllocatorRef, GCHEAPHASHOBJECTREF *pGCHeapHash); - - // Be careful with this. This isn't safe to use unless something is keeping the LoaderAllocator live, or there is no intention to dereference this pointer - LoaderAllocator* GetLoaderAllocatorUnsafe() - { - return _loaderAllocator; - } - - void Init(OBJECTHANDLE dependentHandle, LoaderAllocator* loaderAllocator) - { - LIMITED_METHOD_CONTRACT; - _dependentHandle = dependentHandle; - _loaderAllocator = loaderAllocator; - } -}; - -class LAHashKeyToTrackersObject : public Object -{ -#ifdef DACCESS_COMPILE - friend class ClrDataAccess; -#endif - friend class CheckAsmOffsets; - friend class CoreLibBinder; - - public: - // _trackerOrTrackerSet is either a reference to a LAHashDependentHashTracker, or to a GCHeapHash of LAHashDependentHashTracker objects. - OBJECTREF _trackerOrTrackerSet; - // _laLocalKeyValueStore holds an object that represents a Key value (which must always be valid for the lifetime of the - // CrossLoaderAllocatorHeapHash, and the values which must also be valid for that entire lifetime. When a value might - // have a shorter lifetime it is accessed through the _trackerOrTrackerSet variable, which allows access to hashtables which - // are associated with that remote loaderallocator through a dependent handle, so that lifetime can be managed. - OBJECTREF _laLocalKeyValueStore; -}; - -typedef DPTR(LAHashDependentHashTrackerObject) PTR_LAHashDependentHashTrackerObject; -typedef DPTR(LAHashKeyToTrackersObject) PTR_LAHashKeyToTrackersObject; - - -#ifdef USE_CHECKED_OBJECTREFS -typedef REF LAHASHDEPENDENTHASHTRACKERREF; -typedef REF LAHASHKEYTOTRACKERSREF; -#else // USE_CHECKED_OBJECTREFS -typedef PTR_LAHashDependentHashTrackerObject LAHASHDEPENDENTHASHTRACKERREF; -typedef PTR_LAHashKeyToTrackersObject LAHASHKEYTOTRACKERSREF; -#endif // USE_CHECKED_OBJECTREFS - - #endif // _OBJECT_H_ diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 9d75ccc075c2d..df1178befd6e2 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -90,8 +90,7 @@ PCODE MethodDesc::DoBackpatch(MethodTable * pMT, MethodTable *pDispatchingMT, BO // Only take the lock if the method is versionable with vtable slot backpatch, for recording slots and synchronizing with // backpatching slots - MethodDescBackpatchInfoTracker::ConditionalLockHolderForGCCoop slotBackpatchLockHolder( - isVersionableWithVtableSlotBackpatch); + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder(isVersionableWithVtableSlotBackpatch); // Get the method entry point inside the lock above to synchronize with backpatching in // MethodDesc::BackpatchEntryPointSlots() diff --git a/src/coreclr/vm/rejit.cpp b/src/coreclr/vm/rejit.cpp index 80f05489ad3b2..8807b0934e285 100644 --- a/src/coreclr/vm/rejit.cpp +++ b/src/coreclr/vm/rejit.cpp @@ -643,8 +643,6 @@ HRESULT ReJitManager::UpdateActiveILVersions( SHash::Iterator endIter = mgrToCodeActivationBatch.End(); { - MethodDescBackpatchInfoTracker::ConditionalLockHolderForGCCoop slotBackpatchLockHolder; - for (SHash::Iterator iter = beginIter; iter != endIter; iter++) { CodeActivationBatch * pCodeActivationBatch = *iter; diff --git a/src/coreclr/vm/tieredcompilation.cpp b/src/coreclr/vm/tieredcompilation.cpp index 9adb2ca09785e..9611c33356a93 100644 --- a/src/coreclr/vm/tieredcompilation.cpp +++ b/src/coreclr/vm/tieredcompilation.cpp @@ -579,13 +579,7 @@ bool TieredCompilationManager::TryDeactivateTieringDelay() COUNT_T methodCount = methodsPendingCounting->GetCount(); CodeVersionManager *codeVersionManager = GetAppDomain()->GetCodeVersionManager(); - MethodDescBackpatchInfoTracker::ConditionalLockHolderForGCCoop slotBackpatchLockHolder; - - // Backpatching entry point slots requires cooperative GC mode, see - // MethodDescBackpatchInfoTracker::Backpatch_Locked(). The code version manager's table lock is an unsafe lock that - // may be taken in any GC mode. The lock is taken in cooperative GC mode on some other paths, so the same ordering - // must be used here to prevent deadlock. - GCX_COOP(); + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder; CodeVersionManager::LockHolder codeVersioningLockHolder; for (COUNT_T i = 0; i < methodCount; ++i) @@ -946,14 +940,7 @@ void TieredCompilationManager::ActivateCodeVersion(NativeCodeVersion nativeCodeV HRESULT hr = S_OK; { bool mayHaveEntryPointSlotsToBackpatch = pMethod->MayHaveEntryPointSlotsToBackpatch(); - MethodDescBackpatchInfoTracker::ConditionalLockHolderForGCCoop slotBackpatchLockHolder( - mayHaveEntryPointSlotsToBackpatch); - - // Backpatching entry point slots requires cooperative GC mode, see - // MethodDescBackpatchInfoTracker::Backpatch_Locked(). The code version manager's table lock is an unsafe lock that - // may be taken in any GC mode. The lock is taken in cooperative GC mode on some other paths, so the same ordering - // must be used here to prevent deadlock. - GCX_MAYBE_COOP(mayHaveEntryPointSlotsToBackpatch); + MethodDescBackpatchInfoTracker::ConditionalLockHolder slotBackpatchLockHolder(mayHaveEntryPointSlotsToBackpatch); CodeVersionManager::LockHolder codeVersioningLockHolder; // As long as we are exclusively using any non-JumpStamp publishing for tiered compilation