RFR: 8310031: Parallel: Implement better work distribution for large object arrays in old gen [v19]

[Richard Reingruber]

On Thu, 12 Oct 2023 11:06:16 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Richard Reingruber has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Re-cleanup (was accidentally reverted)
>
> src/hotspot/share/gc/parallel/psCardTable.cpp line 156:
> 
>> 154: 
>> 155:   // Helper struct to keep the following code compact.
>> 156:   struct Obj {
> 
> The scan-dirty-chunk code below is already quite compact; this struct-def is bulky in contrast. Maybe using plain variables inside while-loop is sufficient. Not sure either is definitely superior though.

Ok. Done.

> src/hotspot/share/gc/parallel/psCardTable.cpp line 165:
> 
>> 163:                             is_obj_array(obj->is_objArray()),
>> 164:                             end_addr(addr + obj->size()) {}
>> 165:     void next() {
> 
> Maybe `move_to_next`  so that it's clear that it's an action not a accessor (noun)?

I removed the helper struct.

> src/hotspot/share/gc/parallel/psCardTable.cpp line 307:
> 
>> 305:     HeapWord* start_addr;
>> 306:     HeapWord* end_addr;
>> 307:     DEBUG_ONLY(HeapWord* _prev_query);
> 
> This debug-field clutters the real logic, IMO.

I've removed it.

> src/hotspot/share/gc/parallel/psCardTable.hpp line 52:
> 
>> 50: #ifdef ASSERT
>> 51:       , _table_end((const CardValue*)(uintptr_t(_table) + (align_up(stripe.byte_size(), _card_size) >> _card_shift)))
>> 52: #endif
> 
> Not sure about its usefulness; the logic in the caller is super clear while this assertion logic obstructs the flow.

I've reduce the code for the bounds check.

> src/hotspot/share/gc/parallel/psCardTable.hpp line 82:
> 
>> 80:                                            const CardValue* const end) {
>> 81:       for (const CardValue* i = start; i < end; ++i) {
>> 82:         if (!is_clean(i)) {
> 
> Better to use `is_dirty` to match the method name.

Done

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14846#discussion_r1357067138
PR Review Comment: https://git.openjdk.org/jdk/pull/14846#discussion_r1357065828
PR Review Comment: https://git.openjdk.org/jdk/pull/14846#discussion_r1357065987
PR Review Comment: https://git.openjdk.org/jdk/pull/14846#discussion_r1357064658
PR Review Comment: https://git.openjdk.org/jdk/pull/14846#discussion_r1357063772