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

[Thomas Schatzl]

On Tue, 5 Dec 2023 17:40:11 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.

Initial comments

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

> 114:   size_t initial_chunk_capacity = align_up(initial_capacity, capacity_alignment()) / TaskEntryChunkSizeInVoidStar;
> 115: 
> 116:   initial_chunk_capacity = round_up_power_of_2(initial_chunk_capacity);

This calculation does not take into account that `MarkStackSizeMax` may not be a power of 2, so rounding up will make initial mark stack size exceed maximum mark stack size, failing in the next line.
Example:
`java -Xlog:gc=debug,gc+marking=debug -XX:MarkStackSize=10240 -XX:MarkStackSizeMax=10240 -version`

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

> 115: 
> 116:   initial_chunk_capacity = round_up_power_of_2(initial_chunk_capacity);
> 117: 

Pre-existing: it may be useful to update `MarkStackSize` and `MarkStackSizeMax` with the final values.

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

> 192: 
> 193: void G1CMMarkStack::ChunkAllocator::expand() {
> 194:   size_t old_capacity = _capacity;

Not a bug, but for uniformity I would lock the `MarkStackChunkList_lock` for all (`reserve()`) operations. Feel free to ignore.

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

Changes requested by tschatzl (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16979#pullrequestreview-1767278151
PR Review Comment: https://git.openjdk.org/jdk/pull/16979#discussion_r1417109128
PR Review Comment: https://git.openjdk.org/jdk/pull/16979#discussion_r1417110139
PR Review Comment: https://git.openjdk.org/jdk/pull/16979#discussion_r1417128272