RFR: 8272138: ZGC: Adopt release ordering for self-healing [v3]
Per Liden
pliden at openjdk.java.net
Tue Aug 10 14:21:32 UTC 2021
On Tue, 10 Aug 2021 14:12:30 GMT, Per Liden <pliden at openjdk.org> wrote:
>> Xiaowei Lu has updated the pull request incrementally with one additional commit since the last revision:
>>
>> 8272138: ZGC: Adopt release ordering for self-healing
>
> src/hotspot/share/gc/z/zRelocate.cpp line 77:
>
>> 75: ZUtils::object_copy_disjoint(from_addr, to_addr, size);
>> 76:
>> 77: OrderAccess::release();
>
> Could you please replace the calls to `OrderAccess::release()` with a single call here instead:
>
> ```--- a/src/hotspot/share/gc/z/zForwarding.inline.hpp
> +++ b/src/hotspot/share/gc/z/zForwarding.inline.hpp
> @@ -136,6 +136,8 @@ inline uintptr_t ZForwarding::insert(uintptr_t from_index, uintptr_t to_offset,
> const ZForwardingEntry new_entry(from_index, to_offset);
> const ZForwardingEntry old_entry; // Empty
>
> + OrderAccess::release();
> +
> for (;;) {
> const ZForwardingEntry prev_entry = Atomic::cmpxchg(entries() + *cursor, old_entry, new_entry, memory_order_relaxed);
> if (!prev_entry.populated()) {
>
>
> While keeping the calls close to the copy code has some value, I think I'd prefer to have it in a single place, and close to the corresponding acquire in `at()`.
I updated my comment above, since it had a typo (used `memory_order_release` instead of `memory_order_relaxed`).
-------------
PR: https://git.openjdk.java.net/jdk/pull/5046
More information about the hotspot-gc-dev
mailing list