RFR: 8338440: Parallel: Improve fragmentation mitigation in Full GC [v2]

[Albert Mingkun Yang]

On Fri, 23 Aug 2024 10:57:03 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> `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;
>
>> Actually, the last parameter target_next of the method summarize_split_space won't be used at any other places.
> 
>> target_next is used in the callee to set up the new-top.
> 
> I means that the `target_next` is updated in `summarize_split_space` but the updated `target_next` is never used later. The bottom of the new space will be used instead.
> 
> 
> // method PSParallelCompact::summary_phase
> 
> } else if (live > 0) {
> // other code, skip
>       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); // <-- `new_top_addr` is updated, but not unnecessary
> // other code, skip
>       new_top_addr = _space_info[id].new_top_addr(); // <-- `new_top_addr` is updated again
>       done = _summary_data.summarize(_space_info[id].split_info(),
>                                      next_src_addr, space->top(),
>                                      nullptr,
>                                      space->bottom(), dst_space_end, // <-- the bottom of the new space is used
>                                      new_top_addr);
> // other code, skip
>     }
> 
> 
> A draft diff is shown below:
> 
> 
> --- a/src/hotspot/share/gc/parallel/psParallelCompact.cpp
> +++ b/src/hotspot/share/gc/parallel/psParallelCompact.cpp
> @@ -310,8 +310,7 @@ ParallelCompactData::summarize_dense_prefix(HeapWord* beg, HeapWord* end)
>  HeapWord* ParallelCompactData::summarize_split_space(size_t src_region,
>                                                       SplitInfo& split_info,
>                                                       HeapWord* const destination,
> -                                                     HeapWord* const target_end,
> -                                                     HeapWord** target_next) {
> +                                                     HeapWord* const target_end) {
>    assert(destination <= target_end, "sanity");
>    assert(destination + _region_data[src_region].data_size() > target_end,
>      "region should not fit into target space");
> @@ -373,9 +372,6 @@ HeapWord* ParallelCompactData::summarize_split_space(size_t src_region,
>        }
>      }
>  
> -    // Update new top of target space
> -    *target_next = new_top;
> -
>      return overflowing_obj;
>    }
>  
> @@ -397,7 +393,6 @@ HeapWord* ParallelCompactData::summarize_split_space(size_t src_regio...

`gc/InfiniteList.java` (if running Parallel) fails with this patch on my box.

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

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