RFR: 8305896: Alternative full GC forwarding [v16]

[Roman Kennke]

On Tue, 2 May 2023 14:16:16 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address @shipilev's review
>
> src/hotspot/share/gc/shared/slidingForwarding.hpp line 121:
> 
>> 119:   size_t           _region_size_words;
>> 120:   size_t           _region_size_words_shift;
>> 121:   HeapWord**       _bases_table;
> 
> Small nit. For clarity, I would prefer if we had a real structure here, e.g.:
> 
> struct region_forwarding { HeapWord* dest; HeapWord* alt_dest; };
> region_forwarding* _table;

Ok, I am changing it. It looks like it's introducing a branch on the decoding-path though. I am not sure if a C++ compiler would optimise it to a branch-free code, though. It's probably a very minor concern.

> src/hotspot/share/gc/shared/slidingForwarding.inline.hpp line 115:
> 
>> 113:   uintptr_t encoded = encode_forwarding(from_hw, to_hw);
>> 114:   markWord new_header = markWord((from_header.value() & ~MARK_LOWER_HALF_MASK) | encoded);
>> 115:   from->set_mark(new_header);
> 
> What happens if the header is displaced into an OM? Should we not update the displaced header instead?

When the header is displaced, it will be recorded in the preserved-marks table. Then we over-write the mark-word with the forwarding. At the end of the GC, we will restore the original mark from the preserved-marks table. This is the same mechanism that is already used in normal uncompressed forwarding.

> src/hotspot/share/gc/shared/slidingForwarding.inline.hpp line 125:
> 
>> 123:   assert(_bases_table != nullptr, "call begin() before asking for forwarding");
>> 124: 
>> 125:   markWord header = from->mark();
> 
> Could this header be displaced?

No. See above. We actually check for that in decode_forwarding():
  assert((encoded & markWord::marked_value) == markWord::marked_value, "must be marked as forwarded");

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13582#discussion_r1182827422
PR Review Comment: https://git.openjdk.org/jdk/pull/13582#discussion_r1182846767
PR Review Comment: https://git.openjdk.org/jdk/pull/13582#discussion_r1182847913