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