RFR: 8329469: Generational ZGC: Move the methods forwarding_[index|find|insert] from zRelocate.cpp to ZForwarding

[Stefan Karlsson]

On Tue, 2 Apr 2024 09:59:26 GMT, Guoxiong Li <gli at openjdk.org> wrote:

> Hi all,
> 
> This patch moves the methods `zRelocate.cpp/forwarding_[index|find|insert]`  from the file `zRelocate.cpp` to the class `ZForwarding`.  The first parameter of the methods `zRelocate.cpp/forwarding_[index|find|insert]` is always `ZForwarding*`. These methods should be placed in the class `ZForwarding` so that the first parameter `ZForwarding*` can be added by the compiler and then it can improve the encapsulation.
> 
> But the non-generational ZGC can't make such a move because the method signatures may be duplicate.
> 
> The test `make test-tier1_gc` passed locally. Thanks for taking the time to review.
> 
> Best Regards,
> -- Guoxiong

This looks like a good cleanup. Some comments below:

src/hotspot/share/gc/z/zForwarding.hpp line 153:

> 151:   zoffset insert(uintptr_t from_index, zoffset to_offset, ZForwardingCursor* cursor);
> 152:   zaddress insert(zoffset from_offset, zaddress to_addr, ZForwardingCursor* cursor);
> 153:   zaddress insert(zaddress from_addr, zaddress to_addr, ZForwardingCursor* cursor);

Could some of these functions be moved to the `private:` section where you added the `index` function?

src/hotspot/share/gc/z/zRelocate.cpp line 356:

> 354: 
> 355:   // Lookup forwarding
> 356:   zaddress to_addr = forwarding->find(from_addr, &cursor);

While looking at this, I now see that this could use the find overload that doesn't take a ZForwardingCursor. Could you update the code to do so?

src/hotspot/share/gc/z/zRelocate.cpp line 384:

> 382: zaddress ZRelocate::forward_object(ZForwarding* forwarding, zaddress_unsafe from_addr) {
> 383:   ZForwardingCursor cursor;
> 384:   const zaddress to_addr = forwarding->find(from_addr, &cursor);

Same here. The cursor is not necessary.

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

Changes requested by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18579#pullrequestreview-1973411085
PR Review Comment: https://git.openjdk.org/jdk/pull/18579#discussion_r1547637167
PR Review Comment: https://git.openjdk.org/jdk/pull/18579#discussion_r1547646521
PR Review Comment: https://git.openjdk.org/jdk/pull/18579#discussion_r1547646836