RFR: 8327387: G1: Refactor region liveness processing after completion of concurrent marking [v2]

[Thomas Schatzl]

On Wed, 6 Mar 2024 14:33:14 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> The diff is not quite easy to follow. Instead, I'd recommend to read the new code directly, and the entry-point (and the gist) is in `G1OnRegionClosure::do_heap_region`.
>> 
>> While running BigRamTester, h2, and specjbb2015-fixed, a slight reduction in subphase time (approximately 0.01ms) was observed. However, it's important to note that these subphases are already quite brief in baseline.
>> 
>> Test: tier1-6
>
> Albert Mingkun Yang 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 three additional commits since the last revision:
> 
>  - review
>  - Merge branch 'master' into g1-merge-remark
>  - g1-merge-remark

Lgtm.

Please fix the phase name before pushing, the other nit is up to you.

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

> 1221:         _num_humongous_regions_removed++;
> 1222:         _freed_bytes += hr->used();
> 1223:         hr->set_containing_set(nullptr);

Suggestion:

        assert(hr->used() > 0, "precondition");
        assert(!hr->has_pinned_objects(), "precondition");
        assert(hr->is_humongous(), "precondition");

        _num_humongous_regions_removed++;

        _freed_bytes += hr->used();
        hr->set_containing_set(nullptr);

Just a nit to make the `reclaim_empty*` methods look more equal. The pinned check can now be added because it means something different than before when humongous regions were implicitly pinned.

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

> 1405: 
> 1406:     {
> 1407:       GCTraceTime(Debug, gc, phases) debug("Select For Rebuild & Reclaim Empty Regions", _gc_timer_cm);

Maybe not use the `&` as we typically write out the "and".

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

Marked as reviewed by tschatzl (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18127#pullrequestreview-1924466831
PR Review Comment: https://git.openjdk.org/jdk/pull/18127#discussion_r1517473790
PR Review Comment: https://git.openjdk.org/jdk/pull/18127#discussion_r1517477044