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

[Roman Kennke]

On Tue, 27 Oct 2020 10:54:25 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/share/gc/shenandoah/shenandoahBarrierSet.hpp line 104:
> 
>> 102: 
>> 103:   template<typename T>
>> 104:   inline oop load_reference_barrier_native(oop obj, T* load_addr);
> 
> These might be forked to a separate cleanup? Not insisting... That would make backports a bit cleaner, though.

I am changing back the template arg of load_reference_barrier_mutator() instead, we are using class everywhere else.

> src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp line 81:
> 
>> 79:   _heap(ShenandoahHeap::heap()),
>> 80:   _mark_context(_heap->marking_context()),
>> 81:   _strong(true)
> 
> Do we want to turn this to yet another template parameter, like for dedup? That would also resolve passing `true` or `false` to `strong` argument without comments.

We need to switch strength in ShenandoahConcurrentMark::do_task() and we get passed-in a ready closure there. I am not sure how we could do that with template-args. Template args only make sense for things that don't change during marking.

> src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.inline.hpp line 264:
> 
>> 262:         marked = mark_context->mark_strong(obj, marked_first);
>> 263:       } else {
>> 264:         marked = mark_context->mark_final(obj, marked_first);
> 
> Is this `mark_final` actually `mark_weak`?

We could name it so, but it really means 'reachable through a FinalReference' so 'finalizably reachable' and 'marked final(izable)' seems the more correct term. It is weaker than 'strong' though, so yeah we could rename this. WDYT?

> src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.inline.hpp line 64:
> 
>> 62: }
>> 63: 
>> 64: inline bool ShenandoahMarkBitMap::mark_final(HeapWord* heap_addr, bool& marked_first) {
> 
> It looks to me that `marked_first` is always the same as the return value? If so, can we drop that out-argument?

Yes, that is true for mark_final(), but not for mark_strong(). I am changing it as you suggested.

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

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