RFR: 8338440: Parallel: Improve fragmentation mitigation in Full GC

[Albert Mingkun Yang]

On Thu, 22 Aug 2024 11:27:03 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> Extend `SplitInfo` to support more fine-grained splitting to mitigate the fragmentation issue during full GC. Added comments and diagrams in the process.
>> 
>> For easier review, it's best to start with `SplitInfo` and then proceed to see how it is constructed in `summarize_split_space` and consumed in `first_src_addr`. The accompanying diagrams should help create a clear mental image.
>> 
>> With this patch, the exec time of `runtime/ClassInitErrors/TestOutOfMemoryDuringInit.java` using Parallel drops from ~30s to ~8s, the same as other GCs, and gc-log shows similar number of GC cycles as well.
>> 
>> Test: tier1-8, systemgc micro bm, CacheStress, dacapo, specjbb2005, specjvm2008
>
> src/hotspot/share/gc/parallel/psParallelCompact.cpp line 482:
> 
>> 480:       assert(source_next != nullptr, "source_next is null when splitting");
>> 481:       *source_next = summarize_split_space(cur_region, split_info, dest_addr,
>> 482:                                            target_end, target_next);
> 
> Actually, the last parameter `target_next` of the method `summarize_split_space` won't be used at any other places. The bottom of the new space will be used instead (see `PSParallelCompact::summary_phase`). So I think the last parameter `target_next` of the method `summarize_split_space` can be **removed**. Then in `PSParallelCompact::summary_phase`, we can pass a null pointer in this situation (See code below) so that the meaning becomes clearer.
> 
> 
> // method `PSParallelCompact::summary_phase`
> } else if (live > 0) {
>       // Attempt to fit part of the source space into the target space.
>       HeapWord* next_src_addr = nullptr;
>       bool done = _summary_data.summarize(_space_info[id].split_info(),
>                                           space->bottom(), space->top(),
>                                           &next_src_addr,
>                                           *new_top_addr, dst_space_end,
>                                           new_top_addr);   // <--- here, can pass `nullptr`
>       assert(!done, "space should not fit into old gen");
>       assert(next_src_addr != nullptr, "sanity");

`target_next` is used in the callee to set up the new-top.

In `summarize_split_space`:


    // Update new top of target space
    *target_next = new_top;

> src/hotspot/share/gc/parallel/psParallelCompact.hpp line 152:
> 
>> 150:   HeapWord*    _split_point;
>> 151:   size_t       _preceding_live_words;
>> 152:   uint         _split_destination_count;
> 
> The names `_split_destination` and `_split_destination_count` may be ambiguous. In `_split_destination`, the prefix `split` means two parts whose destinations locate in different spaces. But in `_split_destination_count`, the prefix `split` means two parts whose destinations locate in different regions but in the same space. I suggest to change the field `_split_destination_count` to `_preceding_destination_count` (or other more appropriate name).

I tried to make fields of `class SplitInfo` to match their counterpart in `class RegionData`, except the additional `_split_` prefix, in order to signify these fields are closely related.

> But in _split_destination_count, the prefix split means two parts whose destinations locate in different regions but in the same space.

Why are they in the "same" space? The purpose of having "split" to support space-boundary so that the first part and the second part are in two spaces.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20590#discussion_r1728631650
PR Review Comment: https://git.openjdk.org/jdk/pull/20590#discussion_r1728628594