RFR: 8254315: Shenandoah: Concurrent weak reference processing [v12]

[Roman Kennke]

On Tue, 27 Oct 2020 10:36:27 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Rename native argument to maybe_narrow_oop for more clarity
>
> src/hotspot/cpu/aarch64/gc/shenandoah/shenandoahBarrierSetAssembler_aarch64.cpp line 360:
> 
>> 358: 
>> 359:     if (ShenandoahBarrierSet::use_load_reference_barrier_native(decorators, type)) {
>> 360:       load_reference_barrier_native(masm, dst, src, (decorators & IN_NATIVE) == 0);
> 
> I am a bit confused. If we introduce the local variable, would it be `maybe_narrow_oop`? How does it relate to `IS_NATIVE`? Also, see that `use_load_reference_barrier_native` already tests `IS_NATIVE`.

Yeah, this is confusing. Zhengyu also stumbled over this. Notice that (decorators & IN_NATIVE) == 0 tests for 'is *not* native'. The point is that native-access is *always* uncompressed-oops, while accessing a referent is narrowOop or oop depending on UseCompressedOops. Hence the distinction. If you have a good suggestion on how to make this less confusing, I'd appreciate it.

> src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp line 64:
> 
>> 62:   static bool use_load_reference_barrier_native(DecoratorSet decorators, BasicType type);
>> 63:   static bool need_keep_alive_barrier(DecoratorSet decorators, BasicType type);
>> 64:   static ShenandoahLRBKind access_kind(DecoratorSet decorators, BasicType type);
> 
> ...or in fact, `ShenandoahBarrierSet::AccessKind` to match the `access_kind` here? Or does it clash with something else? Or rename this to `lrb_kind`?

AccessKind seems sensible. I'll try it.

> src/hotspot/share/gc/shenandoah/shenandoahThreadLocalData.hpp line 55:
> 
>> 53:   int  _disarmed_value;
>> 54:   double _paced_time;
>> 55:   ShenandoahMarkRefsSuperClosure* _mark_closure;
> 
> This rubs me the wrong way. Closures are usually stack-allocated, so we are exposing the stack pointer here.

Yeah we need to pass it between the mark-loop and the reference-processor. It's still thread-local.

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

PR: https://git.openjdk.java.net/jdk/pull/505