RFR: 8289822: G1: Make concurrent mark code owner of TAMSes [v5]

[Ivan Walulya]

On Sat, 16 Mar 2024 22:20:20 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   please review this change that moves TAMS ownership and handling out of `HeapRegion` into `G1ConcurrentMark`.
>> 
>> The rationale is that it's intrinsically a data structure only used by the marking, and only interesting during marking, so attaching it to the always present `HeapRegion` isn't fitting.
>> 
>> This also moves some code that is about marking decisions out of `HeapRegion`.
>> 
>> In this change the TAMSes are still maintained and kept up to date as long as the `HeapRegion` exists (basically always) as the snapshotting of the marking does not really snapshot ("copy over the `HeapRegion`s that existed at the time of the snapshot, only ever touching them), but assumes that they exist even for uncommitted (or free) regions due to the way it advances the global `finger` pointer.
>> I did not want to change this here.
>> 
>> This is also why `HeapRegion` still tells `G1ConcurrentMark` to update the TAMS in two places (when clearing a region and when resetting during full gc).
>> 
>> Another option I considered when implementing this is clearing all marking statistics (`G1ConcurrentMark::clear_statistics`) at these places in `HeapRegion` because it decreases the amount of places where this needs to be done (maybe one or the other place isn't really necessary already). I rejected it because this would add some work that is dependent on the number of worker threads (particularly) into `HeapRegion::hr_clear()`. This change can be looked at by viewing at all but the last commit.
>> 
>> Testing: tier1-4, gha
>> 
>> Thanks,
>>   Thomas
>
> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
> 
>   ayang review2

LGTM!

src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 1508:

> 1506:   //     * Reference discovery will not need a barrier.
> 1507: 
> 1508:   // Concurrent Mark ref processor

The comment now seems misplaced

-------------

Marked as reviewed by iwalulya (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18150#pullrequestreview-1943423814
PR Review Comment: https://git.openjdk.org/jdk/pull/18150#discussion_r1528778112