Replace GCX_COOP with EBR for EEHashTable bucket reclamation#124822
Replace GCX_COOP with EBR for EEHashTable bucket reclamation#124822AaronRobinsonMSFT merged 5 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @agocke |
There was a problem hiding this comment.
Pull request overview
This PR replaces the prior GC cooperative-mode based scheme for safely reclaiming obsolete EEHashTable bucket arrays with Epoch-Based Reclamation (EBR), aligning EEHashTable resizing behavior with the earlier HashMap EBR work.
Changes:
- Add a dedicated global EBR collector (
g_EEHashEbr) and initialize it during EE startup; trigger cleanup from the finalizer thread’s “extra work” loop. - Refactor EEHashTable bucket allocation/freeing into helpers and use EBR critical regions + deferred deletion during table growth instead of
GCX_COOP_NO_THREAD_BROKEN+SyncClean. - Simplify
SyncCleanby removing the EEHashTable-specific cleanup list and related APIs.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/syncclean.hpp | Removes EEHashTable cleanup list API; leaves SyncClean::CleanUp() only. |
| src/coreclr/vm/syncclean.cpp | Removes EEHashTable cleanup list storage and freeing logic. |
| src/coreclr/vm/finalizerthread.cpp | Adds g_EEHashEbr cleanup request detection and processing in finalizer extra-work path. |
| src/coreclr/vm/eehash.inl | Replaces COOP transitions with EBR critical regions; defers old bucket array deletion via EBR during growth. |
| src/coreclr/vm/eehash.h | Declares EEHashTable bucket allocation/free helpers. |
| src/coreclr/vm/eehash.cpp | Implements AllocateEEHashBuckets / FreeEEHashBuckets. |
| src/coreclr/vm/ebr.h | Declares extern EbrCollector g_EEHashEbr. |
| src/coreclr/vm/ebr.cpp | Defines global g_EEHashEbr. |
| src/coreclr/vm/ceemain.cpp | Initializes g_EEHashEbr during EE startup. |
Replace cooperative GC mode transitions (GCX_COOP_NO_THREAD_BROKEN) with Epoch-Based Reclamation for safe deferred deletion of old EEHashTable bucket arrays during resize. - Add dedicated g_EEHashEbr collector (init, cleanup, critical regions) - Add AllocateEEHashBuckets/FreeEEHashBuckets helpers in eehash.cpp - Remove SyncClean::AddEEHashTable and associated cleanup infrastructure - Remove redundant MemoryBarrier before VolatilePtr::Store (release semantics) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b4f6cb0 to
c8e7d2b
Compare
The per-thread EbrThreadData is a single thread_local with one m_pCollector field, so it can only be associated with one collector at a time. Having two collectors caused the same TLS node to be linked into both thread lists, corrupting epoch tracking and thread detach cleanup. Both HashMap and EEHashTable use generic byte-array reclamation, so a single shared collector is sufficient. Added an assertion in GetOrCreateThreadData to guard against future reintroduction of multiple collectors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace CrstLeafLock usage in AsyncContinuationsManager with a dedicated CrstAsyncContinuations crst type that has proper lock ordering (AcquiredBefore LeafLock). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Can this be replaced too: runtime/src/coreclr/vm/stubmgr.cpp Line 636 in 7a5e8fc GCX_COOP_NO_THREAD_BROKEN definition from src/coreclr/vm/util.hpp.
|
Excellent observation, that is a great follow-up PR to verify this is still needed. |
Remove the g_fEEStarted conditional that immediately freed old bucket arrays during startup. Other threads (finalizer, debugger, tracing) are created before g_fEEStarted is set, so the single-threaded assumption was incorrect. Now always queue old buckets via EBR deferred deletion and always enter EBR critical regions, matching the safety guarantees that HashMap provides for async mode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jkotas
left a comment
There was a problem hiding this comment.
LGTM, assuming the tests are green
Replace cooperative GC mode transitions (
GCX_COOP_NO_THREAD_BROKEN) with Epoch-Based Reclamation for safe deferred deletion of oldEEHashTablebucket arrays during resize.Changes
g_EbrCollectorcollector with initialization inceemain.cppand cleanup infinalizerthread.cppAllocateEEHashBucketsFreeEEHashBucketshelpers ineehash.cpp, following the same pattern asHashMap'sAllocateBucketsFreeBucketsSyncClean::AddEEHashTableand associated cleanup infrastructure fromsyncclean.cpp/.hppMemoryBarrier()beforeVolatilePtr::Store()— release semantics already provide the ordering guaranteeFollow-up to #124307 (Replace
HashMapCOOP transitions with EBR). Specifically see #124307 (comment).