RFR: 8329469: Generational ZGC: Move the methods forwarding_[index|find|insert] from zRelocate.cpp to ZForwarding [v2]
Stefan Karlsson
stefank at openjdk.org
Tue Apr 9 12:40:10 UTC 2024
On Tue, 2 Apr 2024 13:39:25 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
>
> Guoxiong Li has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>
> - Mark several methods as private. Reduce cursor.
> - Merge branch 'master' into MOVE_FORWARDING
> - Move methods about ZForwarding.
I think this looks good. I've added two whitespace suggestions below that I'd like to get incorporated. If you accept them, I'll mark this as Reviewed. Thanks.
src/hotspot/share/gc/z/zForwarding.hpp line 90:
> 88: zaddress find(zoffset from_offset, ZForwardingCursor* cursor);
> 89: zoffset insert(uintptr_t from_index, zoffset to_offset, ZForwardingCursor* cursor);
> 90: zaddress insert(zoffset from_offset, zaddress to_addr, ZForwardingCursor* cursor);
Suggestion:
uintptr_t index(zoffset from_offset);
ZForwardingEntry find(uintptr_t from_index, ZForwardingCursor* cursor) const;
zaddress find(zoffset from_offset, ZForwardingCursor* cursor);
zoffset insert(uintptr_t from_index, zoffset to_offset, ZForwardingCursor* cursor);
zaddress insert(zoffset from_offset, zaddress to_addr, ZForwardingCursor* cursor);
src/hotspot/share/gc/z/zForwarding.hpp line 152:
> 150: zaddress find(zaddress_unsafe from_addr, ZForwardingCursor* cursor);
> 151: zaddress find(zaddress_unsafe from_addr);
> 152: zaddress insert(zaddress from_addr, zaddress to_addr, ZForwardingCursor* cursor);
Suggestion:
zaddress find(zaddress from_addr, ZForwardingCursor* cursor);
zaddress find(zaddress_unsafe from_addr, ZForwardingCursor* cursor);
zaddress find(zaddress_unsafe from_addr);
zaddress insert(zaddress from_addr, zaddress to_addr, ZForwardingCursor* cursor);
-------------
Changes requested by stefank (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18579#pullrequestreview-1988924471
PR Review Comment: https://git.openjdk.org/jdk/pull/18579#discussion_r1557560277
PR Review Comment: https://git.openjdk.org/jdk/pull/18579#discussion_r1557560791
More information about the hotspot-gc-dev
mailing list