From 6c57747ecf42c3c241c0349cbfd64048bdafa7fd Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Thu, 28 May 2026 15:03:57 -0400 Subject: [PATCH 1/2] gc: collapse single-bucket handle table walks Follow-up to #128646. After that PR the multi-bucket / multi-AppDomain handle table scaffolding has no producer: only g_HandleTableMap.pBuckets[0] is ever populated, pNext is permanently null, and dwMaxIndex is permanently the array size constant. Collapse the dead walk patterns in objecthandle.cpp and the one in handletable.cpp to single-bucket form via a small helper. No DAC/cDAC ABI change -- the on-disk HandleTableMap layout is preserved for gcDacVars->handle_table_map and the cDAC GC_1 reader. The walking patterns in legacy DAC (daccess.cpp) and cDAC (GC_1.cs) are left alone in this change and can be simplified in a separate follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/gc/handletable.cpp | 57 +-- src/coreclr/gc/objecthandle.cpp | 689 +++++++++++++------------------- 2 files changed, 304 insertions(+), 442 deletions(-) diff --git a/src/coreclr/gc/handletable.cpp b/src/coreclr/gc/handletable.cpp index 356b0e0bdc314b..da68bf9db70213 100644 --- a/src/coreclr/gc/handletable.cpp +++ b/src/coreclr/gc/handletable.cpp @@ -989,50 +989,35 @@ uint32_t HndCountHandles(HHANDLETABLE hTable) uint32_t HndCountAllHandles(BOOL fUseLocks) { uint32_t uCount = 0; - int offset = 0; // get number of HandleTables per HandleTableBucket int n_slots = getNumberOfSlots(); - // fetch the pointer to the head of the list - struct HandleTableMap * walk = &g_HandleTableMap; + // After collapsing the legacy multi-bucket / multi-AppDomain handle-table-map + // scaffolding, there is at most a single bucket slot at index 0. + HandleTableBucket* bucket = g_HandleTableMap.pBuckets != NULL ? g_HandleTableMap.pBuckets[0] : NULL; + if (bucket == NULL) + return 0; - // walk the list - while (walk) - { - int nextOffset = walk->dwMaxIndex; - int max = nextOffset - offset; - PTR_PTR_HandleTableBucket pBucket = walk->pBuckets; - PTR_PTR_HandleTableBucket pLastBucket = pBucket + max; + // loop through the HandleTables inside the bucket and accumulate handle counts + HHANDLETABLE* pTable = bucket->pTable; + HHANDLETABLE* pLastTable = pTable + n_slots; - // loop through each slot in this node - for (; pBucket != pLastBucket; ++pBucket) + // if the 'fUseLocks' flag is set, acquire the lock for this handle table before + // calling HndCountHandles() - this will prevent dwCount from being modified and + // it will also prevent any of the main caches from being rebalanced + if (fUseLocks) + { + for (; pTable != pLastTable; ++pTable) { - // if there is a HandleTableBucket in this slot - if (*pBucket) - { - // loop through the HandleTables inside this HandleTableBucket, - // and accumulate the handle count of each HandleTable - HHANDLETABLE * pTable = (*pBucket)->pTable; - HHANDLETABLE * pLastTable = pTable + n_slots; - - // if the 'fUseLocks' flag is set, acquire the lock for this handle table before - // calling HndCountHandles() - this will prevent dwCount from being modified and - // it will also prevent any of the main caches from being rebalanced - if (fUseLocks) - for (; pTable != pLastTable; ++pTable) - { - CrstHolder ch(&(Table(*pTable)->Lock)); - uCount += HndCountHandles(*pTable); - } - else - for (; pTable != pLastTable; ++pTable) - uCount += HndCountHandles(*pTable); - } + CrstHolder ch(&(Table(*pTable)->Lock)); + uCount += HndCountHandles(*pTable); } - - offset = nextOffset; - walk = walk->pNext; + } + else + { + for (; pTable != pLastTable; ++pTable) + uCount += HndCountHandles(*pTable); } //return the total number of handles in all HandleTables diff --git a/src/coreclr/gc/objecthandle.cpp b/src/coreclr/gc/objecthandle.cpp index af7805107e5909..8270b547c21213 100644 --- a/src/coreclr/gc/objecthandle.cpp +++ b/src/coreclr/gc/objecthandle.cpp @@ -30,6 +30,16 @@ DhContext *g_pDependentHandleContexts; #ifndef DACCESS_COMPILE +// Returns the single global handle bucket if Ref_Initialize has run and +// Ref_Shutdown has not. After PR #128646, multi-AppDomain support has been +// gone since 2019 and g_HandleTableMap.pBuckets[0] is the only ever-populated +// bucket. Internal walks use this helper instead of iterating the map. +// g_HandleTableMap itself is retained for DAC layout (gcDacVars->handle_table_map). +static FORCEINLINE HandleTableBucket* GetGlobalHandleTableBucket() +{ + return g_HandleTableMap.pBuckets != NULL ? g_HandleTableMap.pBuckets[0] : NULL; +} + //---------------------------------------------------------------------------- /* @@ -729,11 +739,7 @@ void Ref_Shutdown() // be destroyed externally. // destroy the handle table bucket array - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - delete [] walk->pBuckets; - walk = walk->pNext; - } + delete [] g_HandleTableMap.pBuckets; // null out the handle table array g_HandleTableMap.pNext = NULL; @@ -837,28 +843,14 @@ void Ref_RemoveHandleTableBucket(HandleTableBucket *pBucket) { LIMITED_METHOD_CONTRACT; - size_t index = pBucket->HandleTableIndex; - HandleTableMap* walk = &g_HandleTableMap; - size_t offset = 0; - - while (walk) + // After collapsing the legacy multi-bucket / multi-AppDomain handle-table-map + // scaffolding, there is at most a single bucket slot at index 0. + if (g_HandleTableMap.pBuckets != NULL && + pBucket->HandleTableIndex == 0 && + g_HandleTableMap.pBuckets[0] == pBucket) { - if ((index < walk->dwMaxIndex) && (index >= offset)) - { - // During AppDomain unloading, we first remove a handle table and then destroy - // the table. As soon as the table is removed, the slot can be reused. - if (walk->pBuckets[index - offset] == pBucket) - { - walk->pBuckets[index - offset] = NULL; - return; - } - } - offset = walk->dwMaxIndex; - walk = walk->pNext; + g_HandleTableMap.pBuckets[0] = NULL; } - - // Didn't find it. This will happen typically from Ref_DestroyHandleTableBucket if - // we explicitly call Ref_RemoveHandleTableBucket first. } @@ -995,27 +987,23 @@ void TraceVariableHandles(HANDLESCANPROC pfnTrace, ScanContext *sc, uintptr_t lp uint32_t type = HNDTYPE_VARIABLE; struct VARSCANINFO info = { (uintptr_t)uEnableMask, pfnTrace, lp2 }; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i++) - if (walk->pBuckets[i] != NULL) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) + { + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - { - HndScanHandlesForGC(hTable, VariableTraceDispatcher, - (uintptr_t)sc, (uintptr_t)&info, &type, 1, condemned, maxgen, HNDGCF_EXTRAINFO | flags); - } - } + HndScanHandlesForGC(hTable, VariableTraceDispatcher, + (uintptr_t)sc, (uintptr_t)&info, &type, 1, condemned, maxgen, HNDGCF_EXTRAINFO | flags); } - walk = walk->pNext; + } } } @@ -1031,21 +1019,17 @@ void TraceVariableHandlesBySingleThread(HANDLESCANPROC pfnTrace, uintptr_t lp1, uint32_t type = HNDTYPE_VARIABLE; struct VARSCANINFO info = { (uintptr_t)uEnableMask, pfnTrace, lp2 }; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) - if (walk->pBuckets[i] != NULL) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + // this is the one of Ref_* function performed by single thread in MULTI_HEAPS case, so we need to loop through all HT of the bucket + for (int uCPUindex=0; uCPUindex < getNumberOfSlots(); uCPUindex++) { - // this is the one of Ref_* function performed by single thread in MULTI_HEAPS case, so we need to loop through all HT of the bucket - for (int uCPUindex=0; uCPUindex < getNumberOfSlots(); uCPUindex++) - { - HHANDLETABLE hTable = walk->pBuckets[i]->pTable[uCPUindex]; - if (hTable) - HndScanHandlesForGC(hTable, VariableTraceDispatcher, - lp1, (uintptr_t)&info, &type, 1, condemned, maxgen, HNDGCF_EXTRAINFO | flags); - } + HHANDLETABLE hTable = bucket->pTable[uCPUindex]; + if (hTable) + HndScanHandlesForGC(hTable, VariableTraceDispatcher, + lp1, (uintptr_t)&info, &type, 1, condemned, maxgen, HNDGCF_EXTRAINFO | flags); } - walk = walk->pNext; } } #endif // FEATURE_VARIABLE_HANDLES @@ -1068,32 +1052,28 @@ void Ref_TracePinningRoots(uint32_t condemned, uint32_t maxgen, ScanContext* sc, }; uint32_t flags = sc->concurrent ? HNDGCF_ASYNC : HNDGCF_NORMAL; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) - if (walk->pBuckets[i] != NULL) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) + { + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - { - // Pinned handles and async pinned handles are scanned in separate passes, since async pinned - // handles may require a callback into the EE in order to fully trace an async pinned - // object's object graph. - HndScanHandlesForGC(hTable, PinObject, uintptr_t(sc), uintptr_t(fn), &types[0], 1, condemned, maxgen, flags); + // Pinned handles and async pinned handles are scanned in separate passes, since async pinned + // handles may require a callback into the EE in order to fully trace an async pinned + // object's object graph. + HndScanHandlesForGC(hTable, PinObject, uintptr_t(sc), uintptr_t(fn), &types[0], 1, condemned, maxgen, flags); #ifdef FEATURE_ASYNC_PINNED_HANDLES - HndScanHandlesForGC(hTable, AsyncPinObject, uintptr_t(sc), uintptr_t(fn), &types[1], 1, condemned, maxgen, flags); + HndScanHandlesForGC(hTable, AsyncPinObject, uintptr_t(sc), uintptr_t(fn), &types[1], 1, condemned, maxgen, flags); #endif - } - } } - walk = walk->pNext; + } } #ifdef FEATURE_VARIABLE_HANDLES @@ -1120,26 +1100,22 @@ void Ref_TraceNormalRoots(uint32_t condemned, uint32_t maxgen, ScanContext* sc, uint32_t uTypeCount = (((condemned >= maxgen) && !g_theGCHeap->IsConcurrentGCInProgress()) ? 1 : ARRAY_SIZE(types)); uint32_t flags = (sc->concurrent) ? HNDGCF_ASYNC : HNDGCF_NORMAL; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) - if (walk->pBuckets[i] != NULL) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) + { + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - { - HndScanHandlesForGC(hTable, PromoteObject, uintptr_t(sc), uintptr_t(fn), types, uTypeCount, condemned, maxgen, flags); - } - } + HndScanHandlesForGC(hTable, PromoteObject, uintptr_t(sc), uintptr_t(fn), types, uTypeCount, condemned, maxgen, flags); } - walk = walk->pNext; + } } #ifdef FEATURE_VARIABLE_HANDLES @@ -1154,24 +1130,20 @@ void Ref_TraceNormalRoots(uint32_t condemned, uint32_t maxgen, ScanContext* sc, // promote ref-counted handles uint32_t type = HNDTYPE_REFCOUNTED; - walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) - if (walk->pBuckets[i] != NULL) - { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - HndScanHandlesForGC(hTable, PromoteRefCounted, uintptr_t(sc), uintptr_t(fn), &type, 1, condemned, maxgen, flags ); - } - } - walk = walk->pNext; + bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) + { + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) + HndScanHandlesForGC(hTable, PromoteRefCounted, uintptr_t(sc), uintptr_t(fn), &type, 1, condemned, maxgen, flags ); + } } } #endif // FEATURE_REFCOUNTED_HANDLES @@ -1184,22 +1156,15 @@ void Ref_TraceRefCountHandles(HANDLESCANPROC callback, uintptr_t lParam1, uintpt int max_slots = getNumberOfSlots(); uint32_t handleType = HNDTYPE_REFCOUNTED; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i++) + for (int j = 0; j < max_slots; j++) { - if (walk->pBuckets[i] != NULL) - { - for (int j = 0; j < max_slots; j++) - { - HHANDLETABLE hTable = walk->pBuckets[i]->pTable[j]; - if (hTable) - HndEnumHandles(hTable, &handleType, 1, callback, lParam1, lParam2, false); - } - } + HHANDLETABLE hTable = bucket->pTable[j]; + if (hTable) + HndEnumHandles(hTable, &handleType, 1, callback, lParam1, lParam2, false); } - walk = walk->pNext; } #else UNREFERENCED_PARAMETER(callback); @@ -1227,26 +1192,20 @@ void Ref_CheckReachable(uint32_t condemned, uint32_t maxgen, ScanContext *sc) // check objects pointed to by short weak handles uint32_t flags = sc->concurrent ? HNDGCF_ASYNC : HNDGCF_NORMAL; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) { - if (walk->pBuckets[i] != NULL) - { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - HndScanHandlesForGC(hTable, CheckPromoted, (uintptr_t)sc, 0, types, ARRAY_SIZE(types), condemned, maxgen, flags); - } - } + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) + HndScanHandlesForGC(hTable, CheckPromoted, (uintptr_t)sc, 0, types, ARRAY_SIZE(types), condemned, maxgen, flags); } - walk = walk->pNext; } #ifdef FEATURE_VARIABLE_HANDLES @@ -1316,36 +1275,29 @@ bool Ref_ScanDependentHandlesForPromotion(DhContext *pDhContext) pDhContext->m_fUnpromotedPrimaries = false; pDhContext->m_fPromoted = false; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) + int uCPUindex = getSlotNumber(pDhContext->m_pScanContext); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(pDhContext->m_pScanContext); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) { - if (walk->pBuckets[i] != NULL) + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) { - int uCPUindex = getSlotNumber(pDhContext->m_pScanContext); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(pDhContext->m_pScanContext); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - { - HndScanHandlesForGC(hTable, - PromoteDependentHandle, - uintptr_t(pDhContext->m_pScanContext), - uintptr_t(pDhContext->m_pfnPromoteFunction), - &type, 1, - pDhContext->m_iCondemned, - pDhContext->m_iMaxGen, - flags ); - } - } + HndScanHandlesForGC(hTable, + PromoteDependentHandle, + uintptr_t(pDhContext->m_pScanContext), + uintptr_t(pDhContext->m_pfnPromoteFunction), + &type, 1, + pDhContext->m_iCondemned, + pDhContext->m_iMaxGen, + flags ); } } - walk = walk->pNext; } if (pDhContext->m_fPromoted) @@ -1365,29 +1317,22 @@ void Ref_ScanDependentHandlesForClearing(uint32_t condemned, uint32_t maxgen, Sc uint32_t flags = (sc->concurrent) ? HNDGCF_ASYNC : HNDGCF_NORMAL; flags |= HNDGCF_EXTRAINFO; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) { - if (walk->pBuckets[i] != NULL) + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - { - HndScanHandlesForGC(hTable, ClearDependentHandle, uintptr_t(sc), 0, &type, 1, condemned, maxgen, flags ); - } - } + HndScanHandlesForGC(hTable, ClearDependentHandle, uintptr_t(sc), 0, &type, 1, condemned, maxgen, flags ); } } - walk = walk->pNext; } } @@ -1399,29 +1344,22 @@ void Ref_ScanWeakInteriorPointersForRelocation(uint32_t condemned, uint32_t maxg uint32_t flags = (sc->concurrent) ? HNDGCF_ASYNC : HNDGCF_NORMAL; flags |= HNDGCF_EXTRAINFO; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) { - if (walk->pBuckets[i] != NULL) + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - { - HndScanHandlesForGC(hTable, UpdateWeakInteriorHandle, uintptr_t(sc), uintptr_t(fn), &type, 1, condemned, maxgen, flags ); - } - } + HndScanHandlesForGC(hTable, UpdateWeakInteriorHandle, uintptr_t(sc), uintptr_t(fn), &type, 1, condemned, maxgen, flags ); } } - walk = walk->pNext; } } @@ -1433,29 +1371,22 @@ void Ref_ScanDependentHandlesForRelocation(uint32_t condemned, uint32_t maxgen, uint32_t flags = (sc->concurrent) ? HNDGCF_ASYNC : HNDGCF_NORMAL; flags |= HNDGCF_EXTRAINFO; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) { - if (walk->pBuckets[i] != NULL) + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - { - HndScanHandlesForGC(hTable, UpdateDependentHandle, uintptr_t(sc), uintptr_t(fn), &type, 1, condemned, maxgen, flags ); - } - } + HndScanHandlesForGC(hTable, UpdateDependentHandle, uintptr_t(sc), uintptr_t(fn), &type, 1, condemned, maxgen, flags ); } } - walk = walk->pNext; } } @@ -1472,52 +1403,41 @@ void TraceDependentHandlesBySingleThread(HANDLESCANPROC pfnTrace, uintptr_t lp1, uint32_t type = HNDTYPE_DEPENDENT; struct DIAG_DEPSCANINFO info = { pfnTrace, lp2 }; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) - if (walk->pBuckets[i] != NULL) - { - // this is the one of Ref_* function performed by single thread in MULTI_HEAPS case, so we need to loop through all HT of the bucket - for (int uCPUindex=0; uCPUindex < getNumberOfSlots(); uCPUindex++) - { - HHANDLETABLE hTable = walk->pBuckets[i]->pTable[uCPUindex]; - if (hTable) - HndScanHandlesForGC(hTable, TraceDependentHandle, - lp1, (uintptr_t)&info, &type, 1, condemned, maxgen, HNDGCF_EXTRAINFO | flags); - } - } - walk = walk->pNext; + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + // this is the one of Ref_* function performed by single thread in MULTI_HEAPS case, so we need to loop through all HT of the bucket + for (int uCPUindex=0; uCPUindex < getNumberOfSlots(); uCPUindex++) + { + HHANDLETABLE hTable = bucket->pTable[uCPUindex]; + if (hTable) + HndScanHandlesForGC(hTable, TraceDependentHandle, + lp1, (uintptr_t)&info, &type, 1, condemned, maxgen, HNDGCF_EXTRAINFO | flags); + } } } #ifdef FEATURE_SIZED_REF_HANDLES void ScanSizedRefByCPU(uint32_t maxgen, HANDLESCANPROC scanProc, ScanContext* sc, Ref_promote_func* fn, uint32_t flags) { - HandleTableMap *walk = &g_HandleTableMap; uint32_t type = HNDTYPE_SIZEDREF; - while (walk) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) { - if (walk->pBuckets[i] != NULL) - { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - { - HndScanHandlesForGC(hTable, scanProc, uintptr_t(sc), uintptr_t(fn), &type, 1, maxgen, maxgen, flags); - } - } + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) + { + HndScanHandlesForGC(hTable, scanProc, uintptr_t(sc), uintptr_t(fn), &type, 1, maxgen, maxgen, flags); } } - walk = walk->pNext; } } @@ -1567,22 +1487,15 @@ void Ref_NullBridgeObjectsWeakRefs(size_t length, void* unreachableObjectHandles int max_slots = getNumberOfSlots(); uint32_t handleType[] = { HNDTYPE_WEAK_SHORT, HNDTYPE_WEAK_LONG }; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i++) + for (int j = 0; j < max_slots; j++) { - if (walk->pBuckets[i] != NULL) - { - for (int j = 0; j < max_slots; j++) - { - HHANDLETABLE hTable = walk->pBuckets[i]->pTable[j]; - if (hTable) - HndEnumHandles(hTable, handleType, 2, NullBridgeObjectWeakRef, length, (uintptr_t)unreachableObjectHandles, false); - } - } + HHANDLETABLE hTable = bucket->pTable[j]; + if (hTable) + HndEnumHandles(hTable, handleType, 2, NullBridgeObjectWeakRef, length, (uintptr_t)unreachableObjectHandles, false); } - walk = walk->pNext; } } @@ -1607,20 +1520,16 @@ uint8_t** Ref_ScanBridgeObjects(uint32_t condemned, uint32_t maxgen, ScanContext BridgeResetData(); - HandleTableMap* walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i++) - if (walk->pBuckets[i] != NULL) - { - for (int uCPUindex = 0; uCPUindex < getNumberOfSlots(); uCPUindex++) - { - HHANDLETABLE hTable = walk->pBuckets[i]->pTable[uCPUindex]; - if (hTable) - // or have a local var for bridgeObjectsToPromote/size (instead of NULL) that's passed in as lp2 - HndScanHandlesForGC(hTable, GetBridgeObjectsForProcessing, uintptr_t(sc), 0, &type, 1, condemned, maxgen, HNDGCF_EXTRAINFO | flags); - } - } - walk = walk->pNext; + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + for (int uCPUindex = 0; uCPUindex < getNumberOfSlots(); uCPUindex++) + { + HHANDLETABLE hTable = bucket->pTable[uCPUindex]; + if (hTable) + // or have a local var for bridgeObjectsToPromote/size (instead of NULL) that's passed in as lp2 + HndScanHandlesForGC(hTable, GetBridgeObjectsForProcessing, uintptr_t(sc), 0, &type, 1, condemned, maxgen, HNDGCF_EXTRAINFO | flags); + } } // The callee here will free the allocated memory. @@ -1651,27 +1560,20 @@ void Ref_CheckAlive(uint32_t condemned, uint32_t maxgen, ScanContext *sc) }; uint32_t flags = sc->concurrent ? HNDGCF_ASYNC : HNDGCF_NORMAL; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) { - if (walk->pBuckets[i] != NULL) - { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - HndScanHandlesForGC(hTable, CheckPromoted, (uintptr_t)sc, 0, types, ARRAY_SIZE(types), condemned, maxgen, flags); - } - } + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) + HndScanHandlesForGC(hTable, CheckPromoted, (uintptr_t)sc, 0, types, ARRAY_SIZE(types), condemned, maxgen, flags); } - walk = walk->pNext; } #ifdef FEATURE_VARIABLE_HANDLES @@ -1728,24 +1630,20 @@ void Ref_UpdatePointers(uint32_t condemned, uint32_t maxgen, ScanContext* sc, Re // perform a multi-type scan that updates pointers uint32_t flags = (sc->concurrent) ? HNDGCF_ASYNC : HNDGCF_NORMAL; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) - if (walk->pBuckets[i] != NULL) - { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - HndScanHandlesForGC(hTable, UpdatePointer, uintptr_t(sc), uintptr_t(fn), types, ARRAY_SIZE(types), condemned, maxgen, flags); - } - } - walk = walk->pNext; + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) + { + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) + HndScanHandlesForGC(hTable, UpdatePointer, uintptr_t(sc), uintptr_t(fn), types, ARRAY_SIZE(types), condemned, maxgen, flags); + } } #ifdef FEATURE_VARIABLE_HANDLES @@ -1797,18 +1695,16 @@ void Ref_ScanHandlesForProfilerAndETW(uint32_t maxgen, uintptr_t lp1, handle_sca uint32_t flags = HNDGCF_NORMAL; // perform a multi-type scan that updates pointers - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) - if (walk->pBuckets[i] != NULL) - // this is the one of Ref_* function performed by single thread in MULTI_HEAPS case, so we need to loop through all HT of the bucket - for (int uCPUindex=0; uCPUindex < getNumberOfSlots(); uCPUindex++) - { - HHANDLETABLE hTable = walk->pBuckets[i]->pTable[uCPUindex]; - if (hTable) - HndScanHandlesForGC(hTable, &ScanPointerForProfilerAndETW, lp1, (uintptr_t)fn, types, ARRAY_SIZE(types), maxgen, maxgen, flags); - } - walk = walk->pNext; + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + // this is the one of Ref_* function performed by single thread in MULTI_HEAPS case, so we need to loop through all HT of the bucket + for (int uCPUindex=0; uCPUindex < getNumberOfSlots(); uCPUindex++) + { + HHANDLETABLE hTable = bucket->pTable[uCPUindex]; + if (hTable) + HndScanHandlesForGC(hTable, &ScanPointerForProfilerAndETW, lp1, (uintptr_t)fn, types, ARRAY_SIZE(types), maxgen, maxgen, flags); + } } #ifdef FEATURE_VARIABLE_HANDLES @@ -1859,24 +1755,20 @@ void Ref_UpdatePinnedPointers(uint32_t condemned, uint32_t maxgen, ScanContext* }; uint32_t flags = (sc->concurrent) ? HNDGCF_ASYNC : HNDGCF_NORMAL; - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) - if (walk->pBuckets[i] != NULL) - { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - HndScanHandlesForGC(hTable, UpdatePointerPinned, uintptr_t(sc), uintptr_t(fn), types, ARRAY_SIZE(types), condemned, maxgen, flags); - } - } - walk = walk->pNext; + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) + { + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) + HndScanHandlesForGC(hTable, UpdatePointerPinned, uintptr_t(sc), uintptr_t(fn), types, ARRAY_SIZE(types), condemned, maxgen, flags); + } } #ifdef FEATURE_VARIABLE_HANDLES @@ -1923,24 +1815,20 @@ void Ref_AgeHandles(uint32_t condemned, uint32_t maxgen, ScanContext* sc) }; // perform a multi-type scan that ages the handles - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) - if (walk->pBuckets[i] != NULL) - { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - HndScanHandlesForGC(hTable, NULL, 0, 0, types, ARRAY_SIZE(types), condemned, maxgen, HNDGCF_AGE); - } - } - walk = walk->pNext; + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) + { + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) + HndScanHandlesForGC(hTable, NULL, 0, 0, types, ARRAY_SIZE(types), condemned, maxgen, HNDGCF_AGE); + } } } @@ -1983,24 +1871,20 @@ void Ref_RejuvenateHandles(uint32_t condemned, uint32_t maxgen, ScanContext* sc) }; // reset the ages of these handles - HandleTableMap *walk = &g_HandleTableMap; - while (walk) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) - if (walk->pBuckets[i] != NULL) - { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - HndResetAgeMap(hTable, types, ARRAY_SIZE(types), condemned, maxgen, HNDGCF_NORMAL); - } - } - walk = walk->pNext; + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) + { + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) + { + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) + HndResetAgeMap(hTable, types, ARRAY_SIZE(types), condemned, maxgen, HNDGCF_NORMAL); + } } } @@ -2042,27 +1926,20 @@ void Ref_VerifyHandleTable(uint32_t condemned, uint32_t maxgen, ScanContext* sc) }; // verify these handles - HandleTableMap *walk = &g_HandleTableMap; - while (walk) + HandleTableBucket *bucket = GetGlobalHandleTableBucket(); + if (bucket != NULL) { - for (uint32_t i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; i ++) + int uCPUindex = getSlotNumber(sc); + int uCPUlimit = getNumberOfSlots(); + assert(uCPUlimit > 0); + int uCPUstep = getThreadCount(sc); + HHANDLETABLE* pTable = bucket->pTable; + for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) { - if (walk->pBuckets[i] != NULL) - { - int uCPUindex = getSlotNumber(sc); - int uCPUlimit = getNumberOfSlots(); - assert(uCPUlimit > 0); - int uCPUstep = getThreadCount(sc); - HHANDLETABLE* pTable = walk->pBuckets[i]->pTable; - for ( ; uCPUindex < uCPUlimit; uCPUindex += uCPUstep) - { - HHANDLETABLE hTable = pTable[uCPUindex]; - if (hTable) - HndVerifyTable(hTable, types, ARRAY_SIZE(types), condemned, maxgen, HNDGCF_NORMAL); - } - } + HHANDLETABLE hTable = pTable[uCPUindex]; + if (hTable) + HndVerifyTable(hTable, types, ARRAY_SIZE(types), condemned, maxgen, HNDGCF_NORMAL); } - walk = walk->pNext; } } From f8a05bbf84334b774cdc9fd6cc381d1eb6036089 Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Thu, 4 Jun 2026 13:04:32 -0400 Subject: [PATCH 2/2] Simplify comments per review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/gc/handletable.cpp | 2 -- src/coreclr/gc/objecthandle.cpp | 9 ++------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/coreclr/gc/handletable.cpp b/src/coreclr/gc/handletable.cpp index da68bf9db70213..130c72d733d09b 100644 --- a/src/coreclr/gc/handletable.cpp +++ b/src/coreclr/gc/handletable.cpp @@ -993,8 +993,6 @@ uint32_t HndCountAllHandles(BOOL fUseLocks) // get number of HandleTables per HandleTableBucket int n_slots = getNumberOfSlots(); - // After collapsing the legacy multi-bucket / multi-AppDomain handle-table-map - // scaffolding, there is at most a single bucket slot at index 0. HandleTableBucket* bucket = g_HandleTableMap.pBuckets != NULL ? g_HandleTableMap.pBuckets[0] : NULL; if (bucket == NULL) return 0; diff --git a/src/coreclr/gc/objecthandle.cpp b/src/coreclr/gc/objecthandle.cpp index 8270b547c21213..c8a51c7d6553f1 100644 --- a/src/coreclr/gc/objecthandle.cpp +++ b/src/coreclr/gc/objecthandle.cpp @@ -30,11 +30,8 @@ DhContext *g_pDependentHandleContexts; #ifndef DACCESS_COMPILE -// Returns the single global handle bucket if Ref_Initialize has run and -// Ref_Shutdown has not. After PR #128646, multi-AppDomain support has been -// gone since 2019 and g_HandleTableMap.pBuckets[0] is the only ever-populated -// bucket. Internal walks use this helper instead of iterating the map. -// g_HandleTableMap itself is retained for DAC layout (gcDacVars->handle_table_map). +// g_HandleTableMap.pBuckets[0] is the only ever-populated bucket; the map is +// retained for DAC layout (gcDacVars->handle_table_map). static FORCEINLINE HandleTableBucket* GetGlobalHandleTableBucket() { return g_HandleTableMap.pBuckets != NULL ? g_HandleTableMap.pBuckets[0] : NULL; @@ -843,8 +840,6 @@ void Ref_RemoveHandleTableBucket(HandleTableBucket *pBucket) { LIMITED_METHOD_CONTRACT; - // After collapsing the legacy multi-bucket / multi-AppDomain handle-table-map - // scaffolding, there is at most a single bucket slot at index 0. if (g_HandleTableMap.pBuckets != NULL && pBucket->HandleTableIndex == 0 && g_HandleTableMap.pBuckets[0] == pBucket)