Replace GCX_COOP with EBR for EEHashTable bucket reclamation#124822
Replace GCX_COOP with EBR for EEHashTable bucket reclamation#124822AaronRobinsonMSFT wants to merge 1 commit 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>
AaronRobinsonMSFT
force-pushed
the
ebr-eehash
branch
from
b4f6cb0 to
c8e7d2b
Compare
|
|
||
| InterlockedExchange( (LONG *) &m_bGrowing, 0); | ||
|
|
||
| // Before EE starts we're single-threaded; delete old buckets immediately. |
There was a problem hiding this comment.
Is this a correct statement? We do create finalizer thread, debugger threads, tracing threads, ... before g_fEEStarted is set to true. Is it guaranteed that none of those threads cross the path with this?
| g_HashMapEbr.Init(); | ||
|
|
||
| // Initialize EBR for EEHashTable bucket reclamation. | ||
| g_EEHashEbr.Init(); |
There was a problem hiding this comment.
I do not see how the single copy of the thread local state works with multiple instances of the Ebr.
Should both HashMap and EEHash use the same instance? Both of them are dealing with byte[] at the end of the day.
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_EEHashEbrcollector (separate fromHashMap'sg_HashMapEbr), with initialization inceemain.cppand cleanup inf inalizerthread.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).