RFR: 8280087: G1: Handle out-of-mark stack situations during reference processing more gracefully [v5]

[Thomas Schatzl]

On Thu, 7 Dec 2023 20:03:36 GMT, Ivan Walulya <iwalulya at openjdk.org> wrote:

>> Hi all,
>> 
>> Please review this change to modify how we grow the global marking stack used by Concurrent Marking. This patch allows for growing the marking stack without the need to copy over elements. Consequently, we can grow the marking stack even during the reference processing phase without the need to restart the marking cycle.
>> 
>> This mainly addresses the issue where object marking work created during reference processing may overflow the global marking stack. Currently G1 just bails out with a fatal error  as expanding the marking stack would require a restart which is not valid during the reference processing phase.
>> 
>> We have decided to maintain the restart concurrent marking when the global mark stack overflows during the marking phase of the concurrent cycle as this offers better memory utilisation.
>> 
>> Testing: Tier 1-5.
>
> Ivan Walulya has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Albert Review

lgtm.

This is a pre-existing issue, but there should be some note somewhere in which situations what action are taken in case of overflow. Maybe you can find some appropriate place.

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 155:

> 153:     MutexLocker x(MarkStackChunkList_lock, Mutex::_no_safepoint_check_flag);
> 154:     if (Atomic::load_acquire(&_buckets[bucket]) == nullptr) {
> 155:       // Double capacity if possible

Suggestion:

      // Double capacity if possible.

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 244:

> 242:     size_t bucket_capacity = bucket_size(i);
> 243: 
> 244:     // Trim bucket size so that we do not exceed the _max_capacity

Suggestion:

    // Trim bucket size so that we do not exceed _max_capacity.

src/hotspot/share/gc/g1/g1ConcurrentMark.hpp line 152:

> 150:     //        +----+        +----+----+
> 151:     //        |    |------->|    |    |
> 152:     //        +----+        +----+----+

Suggestion:

    //        +----+
    //        |    |        +----+----+
    //        |    |------->|    |    |
    //        +----+        +----+----+

(Not sure why the first bucket has smaller height than the others)

src/hotspot/share/gc/g1/g1ConcurrentMark.hpp line 235:

> 233:   // Allocate a new chunk from the reserved memory, using the high water mark. Returns
> 234:   // null if out of memory.
> 235:   TaskQueueEntryChunk* allocate_new_chunk();

please remove :)

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

Marked as reviewed by tschatzl (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16979#pullrequestreview-1781875434
PR Review Comment: https://git.openjdk.org/jdk/pull/16979#discussion_r1426755979
PR Review Comment: https://git.openjdk.org/jdk/pull/16979#discussion_r1426756709
PR Review Comment: https://git.openjdk.org/jdk/pull/16979#discussion_r1426763338
PR Review Comment: https://git.openjdk.org/jdk/pull/16979#discussion_r1426767774