RFR: 8342382: Implementation of JEP G1: Improve Application Throughput with a More Efficient Write-Barrier [v17]
Thomas Schatzl
tschatzl at openjdk.org
Wed Mar 12 17:44:01 UTC 2025
On Wed, 12 Mar 2025 13:20:25 GMT, Albert Mingkun Yang <ayang at 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
More information about the core-libs-dev
mailing list