RFR: 8325725: Parallel: Refactor PSParallelCompact::fill_dense_prefix_end [v2]

[Albert Mingkun Yang]

On Thu, 29 Feb 2024 10:55:38 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Albert Mingkun Yang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit:
>> 
>>   pgc-end-bit
>
> src/hotspot/share/gc/parallel/psParallelCompact.cpp line 1448:
> 
>> 1446:   HeapWord* const dense_prefix_end = dense_prefix(id);
>> 1447:   RegionData* const region = _summary_data.addr_to_region_ptr(dense_prefix_end);
>> 1448:   idx_t const dense_prefix_bit = _mark_bitmap.addr_to_bit(dense_prefix_end);
> 
> Could probably be moved into the scope using it.

It's used in the final `if` in this method as well.

> src/hotspot/share/gc/parallel/psParallelCompact.cpp line 2566:
> 
>> 2564:       if (!mbm->is_obj_beg(beg_bit) && !mbm->is_obj_end(beg_bit - 1)) {
>> 2565:         beg_addr = mbm->find_obj_beg(beg_addr, end_addr);
>> 2566:       }
> 
> I am not sure this refactoring improves upon the previous code. It adds an additional block level in the code, and removes the descriptive method name.
> Actually this and the previous hunk is simply inlining of `dead_space_crosses_boundary`.
> 
> Not insisting on reverting this change, but I prefer putting more complicated conditions into descriptive helpers.

I had great difficulty in understanding what `dead_space_crosses_boundary` does, so I inlined it to avoid the redundant partial-obj-size comparison  here -- it also makes the obj-bit checking more transparent.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17822#discussion_r1507460104
PR Review Comment: https://git.openjdk.org/jdk/pull/17822#discussion_r1507462602