From 34f4f0dca29f06f471d813cd1a252031c00762be Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 24 Aug 2023 15:11:38 -0700 Subject: [PATCH] Use reader/writer lock instead of mutex for RCW cache --- src/coreclr/inc/CrstTypes.def | 3 - src/coreclr/inc/crsttypes_generated.h | 173 +++++++++--------- .../vm/interoplibinterface_comwrappers.cpp | 62 ++++--- 3 files changed, 124 insertions(+), 114 deletions(-) diff --git a/src/coreclr/inc/CrstTypes.def b/src/coreclr/inc/CrstTypes.def index 3a1e81f84f8681..3bccb73e03a599 100644 --- a/src/coreclr/inc/CrstTypes.def +++ b/src/coreclr/inc/CrstTypes.def @@ -394,9 +394,6 @@ End Crst RCWCleanupList End -Crst ExternalObjectContextCache -End - Crst Reflection AcquiredBefore LoaderHeap UnresolvedClassLock End diff --git a/src/coreclr/inc/crsttypes_generated.h b/src/coreclr/inc/crsttypes_generated.h index 9710f65afcd399..70847a5b367fcd 100644 --- a/src/coreclr/inc/crsttypes_generated.h +++ b/src/coreclr/inc/crsttypes_generated.h @@ -50,92 +50,91 @@ enum CrstType CrstException = 32, CrstExecutableAllocatorLock = 33, CrstExecuteManRangeLock = 34, - CrstExternalObjectContextCache = 35, - CrstFCall = 36, - CrstFrozenObjectHeap = 37, - CrstFuncPtrStubs = 38, - CrstFusionAppCtx = 39, - CrstGCCover = 40, - CrstGlobalStrLiteralMap = 41, - CrstHandleTable = 42, - CrstIbcProfile = 43, - CrstIJWFixupData = 44, - CrstIJWHash = 45, - CrstILStubGen = 46, - CrstInlineTrackingMap = 47, - CrstInstMethodHashTable = 48, - CrstInterop = 49, - CrstInteropData = 50, - CrstIsJMCMethod = 51, - CrstISymUnmanagedReader = 52, - CrstJit = 53, - CrstJitGenericHandleCache = 54, - CrstJitInlineTrackingMap = 55, - CrstJitPatchpoint = 56, - CrstJitPerf = 57, - CrstJumpStubCache = 58, - CrstLeafLock = 59, - CrstListLock = 60, - CrstLoaderAllocator = 61, - CrstLoaderAllocatorReferences = 62, - CrstLoaderHeap = 63, - CrstManagedObjectWrapperMap = 64, - CrstMethodDescBackpatchInfoTracker = 65, - CrstMethodTableExposedObject = 66, - CrstModule = 67, - CrstModuleFixup = 68, - CrstModuleLookupTable = 69, - CrstMulticoreJitHash = 70, - CrstMulticoreJitManager = 71, - CrstNativeImageEagerFixups = 72, - CrstNativeImageLoad = 73, - CrstNls = 74, - CrstNotifyGdb = 75, - CrstObjectList = 76, - CrstPEImage = 77, - CrstPendingTypeLoadEntry = 78, - CrstPerfMap = 79, - CrstPgoData = 80, - CrstPinnedByrefValidation = 81, - CrstPinnedHeapHandleTable = 82, - CrstProfilerGCRefDataFreeList = 83, - CrstProfilingAPIStatus = 84, - CrstRCWCache = 85, - CrstRCWCleanupList = 86, - CrstReadyToRunEntryPointToMethodDescMap = 87, - CrstReflection = 88, - CrstReJITGlobalRequest = 89, - CrstRetThunkCache = 90, - CrstSavedExceptionInfo = 91, - CrstSaveModuleProfileData = 92, - CrstSecurityStackwalkCache = 93, - CrstSigConvert = 94, - CrstSingleUseLock = 95, - CrstSpecialStatics = 96, - CrstStackSampler = 97, - CrstStaticBoxInit = 98, - CrstStressLog = 99, - CrstStubCache = 100, - CrstStubDispatchCache = 101, - CrstStubUnwindInfoHeapSegments = 102, - CrstSyncBlockCache = 103, - CrstSyncHashLock = 104, - CrstSystemBaseDomain = 105, - CrstSystemDomain = 106, - CrstSystemDomainDelayedUnloadList = 107, - CrstThreadIdDispenser = 108, - CrstThreadStore = 109, - CrstTieredCompilation = 110, - CrstTypeEquivalenceMap = 111, - CrstTypeIDMap = 112, - CrstUMEntryThunkCache = 113, - CrstUMEntryThunkFreeListLock = 114, - CrstUniqueStack = 115, - CrstUnresolvedClassLock = 116, - CrstUnwindInfoTableLock = 117, - CrstVSDIndirectionCellLock = 118, - CrstWrapperTemplate = 119, - kNumberOfCrstTypes = 120 + CrstFCall = 35, + CrstFrozenObjectHeap = 36, + CrstFuncPtrStubs = 37, + CrstFusionAppCtx = 38, + CrstGCCover = 39, + CrstGlobalStrLiteralMap = 40, + CrstHandleTable = 41, + CrstIbcProfile = 42, + CrstIJWFixupData = 43, + CrstIJWHash = 44, + CrstILStubGen = 45, + CrstInlineTrackingMap = 46, + CrstInstMethodHashTable = 47, + CrstInterop = 48, + CrstInteropData = 49, + CrstIsJMCMethod = 50, + CrstISymUnmanagedReader = 51, + CrstJit = 52, + CrstJitGenericHandleCache = 53, + CrstJitInlineTrackingMap = 54, + CrstJitPatchpoint = 55, + CrstJitPerf = 56, + CrstJumpStubCache = 57, + CrstLeafLock = 58, + CrstListLock = 59, + CrstLoaderAllocator = 60, + CrstLoaderAllocatorReferences = 61, + CrstLoaderHeap = 62, + CrstManagedObjectWrapperMap = 63, + CrstMethodDescBackpatchInfoTracker = 64, + CrstMethodTableExposedObject = 65, + CrstModule = 66, + CrstModuleFixup = 67, + CrstModuleLookupTable = 68, + CrstMulticoreJitHash = 69, + CrstMulticoreJitManager = 70, + CrstNativeImageEagerFixups = 71, + CrstNativeImageLoad = 72, + CrstNls = 73, + CrstNotifyGdb = 74, + CrstObjectList = 75, + CrstPEImage = 76, + CrstPendingTypeLoadEntry = 77, + CrstPerfMap = 78, + CrstPgoData = 79, + CrstPinnedByrefValidation = 80, + CrstPinnedHeapHandleTable = 81, + CrstProfilerGCRefDataFreeList = 82, + CrstProfilingAPIStatus = 83, + CrstRCWCache = 84, + CrstRCWCleanupList = 85, + CrstReadyToRunEntryPointToMethodDescMap = 86, + CrstReflection = 87, + CrstReJITGlobalRequest = 88, + CrstRetThunkCache = 89, + CrstSavedExceptionInfo = 90, + CrstSaveModuleProfileData = 91, + CrstSecurityStackwalkCache = 92, + CrstSigConvert = 93, + CrstSingleUseLock = 94, + CrstSpecialStatics = 95, + CrstStackSampler = 96, + CrstStaticBoxInit = 97, + CrstStressLog = 98, + CrstStubCache = 99, + CrstStubDispatchCache = 100, + CrstStubUnwindInfoHeapSegments = 101, + CrstSyncBlockCache = 102, + CrstSyncHashLock = 103, + CrstSystemBaseDomain = 104, + CrstSystemDomain = 105, + CrstSystemDomainDelayedUnloadList = 106, + CrstThreadIdDispenser = 107, + CrstThreadStore = 108, + CrstTieredCompilation = 109, + CrstTypeEquivalenceMap = 110, + CrstTypeIDMap = 111, + CrstUMEntryThunkCache = 112, + CrstUMEntryThunkFreeListLock = 113, + CrstUniqueStack = 114, + CrstUnresolvedClassLock = 115, + CrstUnwindInfoTableLock = 116, + CrstVSDIndirectionCellLock = 117, + CrstWrapperTemplate = 118, + kNumberOfCrstTypes = 119 }; #endif // __CRST_TYPES_INCLUDED @@ -181,7 +180,6 @@ int g_rgCrstLevelMap[] = 0, // CrstException 0, // CrstExecutableAllocatorLock 0, // CrstExecuteManRangeLock - 0, // CrstExternalObjectContextCache 4, // CrstFCall -1, // CrstFrozenObjectHeap 7, // CrstFuncPtrStubs @@ -306,7 +304,6 @@ LPCSTR g_rgCrstNameMap[] = "CrstException", "CrstExecutableAllocatorLock", "CrstExecuteManRangeLock", - "CrstExternalObjectContextCache", "CrstFCall", "CrstFrozenObjectHeap", "CrstFuncPtrStubs", diff --git a/src/coreclr/vm/interoplibinterface_comwrappers.cpp b/src/coreclr/vm/interoplibinterface_comwrappers.cpp index 93fa6a448d4687..4b41b68b8e6151 100644 --- a/src/coreclr/vm/interoplibinterface_comwrappers.cpp +++ b/src/coreclr/vm/interoplibinterface_comwrappers.cpp @@ -5,6 +5,7 @@ // Runtime headers #include "common.h" +#include "simplerwlock.hpp" #include "rcwrefcache.h" #ifdef FEATURE_COMINTEROP_APARTMENT_SUPPORT #include "olecontexthelpers.h" @@ -257,32 +258,36 @@ namespace using Element = SHash::element_t; using Iterator = SHash::Iterator; - class LockHolder : public CrstHolder + class ReaderLock final { + SimpleReadLockHolder _lock; public: - LockHolder(_In_ ExtObjCxtCache *cache) - : CrstHolder(&cache->_lock) - { - // This cache must be locked in Cooperative mode - // since releases of wrappers can occur during a GC. - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - } - CONTRACTL_END; - } + ReaderLock(_In_ ExtObjCxtCache* cache) + : _lock{ &cache->_lock } + { } + + ~ReaderLock() = default; + }; + + class WriterLock final + { + SimpleWriteLockHolder _lock; + public: + WriterLock(_In_ ExtObjCxtCache* cache) + : _lock{ &cache->_lock } + { } + + ~WriterLock() = default; }; private: friend struct InteropLibImports::RuntimeCallContext; SHash _hashMap; - Crst _lock; + SimpleRWLock _lock; ExtObjCxtRefCache* _refCache; ExtObjCxtCache() - : _lock(CrstExternalObjectContextCache, CRST_UNSAFE_COOPGC) + : _lock(COOPERATIVE, LOCK_TYPE_DEFAULT) , _refCache(GetAppDomain()->GetRCWRefCache()) { } ~ExtObjCxtCache() = default; @@ -292,7 +297,7 @@ namespace bool IsLockHeld() { WRAPPER_NO_CONTRACT; - return (_lock.OwnedByCurrentThread() != FALSE); + return (_lock.LockTaken() != FALSE); } #endif // _DEBUG @@ -343,7 +348,7 @@ namespace // Determine the count of objects to return. SIZE_T objCountMax = 0; { - LockHolder lock(this); + ReaderLock lock(this); Iterator end = _hashMap.End(); for (Iterator curr = _hashMap.Begin(); curr != end; ++curr) { @@ -365,7 +370,7 @@ namespace SIZE_T objCount = 0; if (0 < objCountMax) { - LockHolder lock(this); + ReaderLock lock(this); Iterator end = _hashMap.End(); for (Iterator curr = _hashMap.Begin(); curr != end; ++curr) { @@ -823,11 +828,22 @@ namespace if (!uniqueInstance) { bool objectFound = false; + bool tryRemove = false; { - // Query the external object cache - ExtObjCxtCache::LockHolder lock(cache); + // Perform a quick look up to determine if we know of the object and if + // we need to perform a more expensive cleanup operation below. + ExtObjCxtCache::ReaderLock lock(cache); extObjCxt = cache->Find(cacheKey); + objectFound = extObjCxt != NULL; + tryRemove = objectFound && extObjCxt->IsSet(ExternalObjectContext::Flags_Detached); + } + if (tryRemove) + { + // Perform the slower cleanup operation that may be appropriate + // if the object still exists and has been detached. + ExtObjCxtCache::WriterLock lock(cache); + extObjCxt = cache->Find(cacheKey); objectFound = extObjCxt != NULL; if (objectFound && extObjCxt->IsSet(ExternalObjectContext::Flags_Detached)) { @@ -958,7 +974,7 @@ namespace else { // Attempt to insert the new context into the cache. - ExtObjCxtCache::LockHolder lock(cache); + ExtObjCxtCache::WriterLock lock(cache); extObjCxt = cache->FindOrAdd(cacheKey, resultHolder.GetContext()); } @@ -980,7 +996,7 @@ namespace { // Failed to set the context; one must already exist. // Remove from the cache above as well. - ExtObjCxtCache::LockHolder lock(cache); + ExtObjCxtCache::WriterLock lock(cache); cache->Remove(resultHolder.GetContext()); COMPlusThrow(kNotSupportedException);