On Wed, 12 Mar 2025 13:20:25 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:
>> Thomas Schatzl has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 24 additional >> commits since the last revision: >> >> - Merge branch 'master' into 8342382-card-table-instead-of-dcq >> - * optimized RISCV gen_write_ref_array_post_barrier() implementation >> contributed by @RealFYang >> - * fix card table verification crashes: in the first refinement phase, >> when switching the global card tables, we need to re-check whether we are >> still in the same sweep epoch or not. It might have changed due to a GC >> interrupting acquiring the Heap_lock. Otherwise new threads will scribble on >> the refinement table. >> Cause are last-minute changes before making the PR ready to review. >> >> Testing: without the patch, occurs fairly frequently when continuously >> (1 in 20) starting refinement. Does not afterward. >> - * ayang review 3 >> * comments >> * minor refactorings >> - * iwalulya review >> * renaming >> * fix some includes, forward declaration >> - * fix whitespace >> * additional whitespace between log tags >> * rename G1ConcurrentRefineWorkTask -> ...SweepTask to conform to the >> other similar rename >> - ayang review >> * renamings >> * refactorings >> - iwalulya review >> * comments for variables tracking to-collection-set and just dirtied >> cards after GC/refinement >> * predicate for determining whether the refinement has been disabled >> * some other typos/comment improvements >> * renamed _has_xxx_ref to _has_ref_to_xxx to be more consistent with >> naming >> - * ayang review - fix comment >> - * iwalulya review 2 >> * G1ConcurrentRefineWorkState -> G1ConcurrentRefineSweepState >> * some additional documentation >> - ... and 14 more: https://git.openjdk.org/jdk/compare/0c7b5abb...aec95051 > > src/hotspot/share/gc/g1/g1ConcurrentRefine.cpp line 217: > >> 215: >> 216: { >> 217: SuspendibleThreadSetLeaver sts_leave; > > Can you add some comment on why leaving the set is required? It's not obvious > to me why. I'd expect handshake to work out of the box... It isn't apparently. Removed. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23739#discussion_r1991999476
