[15] RFR 8228818: Shenandoah: Processing weak roots in concurrent phase when possible

[Roman Kennke]

Hi Zhengyu,

the latest patch looks good to me.

Thanks,
Roman


> On 1/8/20 12:21 PM, Zhengyu Gu wrote:
>>
>>
>> On 1/8/20 11:39 AM, Roman Kennke wrote:
>>> Hi Zhengyu,
>>>
>>> src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp:
>>> +class ShenandoahEvacUpdateCleanupOopStorageRootsClosure : public
>>> BasicOopIterateClosure {
>>>
>>> Why can't this go in shenandoahHeap.cpp (only place where it's used)?
>>
>> Sure,  updated: http://cr.openjdk.java.net/~zgu/JDK-8228818/webrev.01/
>>
>>>
>>> src/hotspot/share/gc/shenandoah/shenandoahControlThread.cpp:
>>> +  // Cleanup/Evacuate weak roots
>>> +  if (heap->marking_context()->is_complete() &&
>>> ShenandoahConcurrentRoots::should_do_concurrent_class_unloading()) {
>>> +    heap->entry_weak_roots();
>>>
>>> Are you sure that we only want to do cleanup when we do
>>> conc-class-unloading? Originally, we hoooked this up in the
>>> entry_roots(), why's that not good enough?
>>
>> Yes, otherwise, weak roots are still processed at final mark/init evac
>> pause.
>>
>> I split into 2 phases, because I think it is logically simpler: when
>> weak roots processing failed, degenerated GC simply re-executes
>> related logic (parallel cleaning) and disarm nmethods, which is
>> equivalent to STW version.
> 
> Took another look, it does not seem to be a good idea, as it duplicates
> the work of update_roots(), that is called at the end of degenerated GC
> cycle to fix the roots.
> 
> Merged weak_roots phase into concurrent roots phase, and removed
> weak_roots degenerated point, also simplified the patch.
> 
> Updated webrev: http://cr.openjdk.java.net/~zgu/JDK-8228818/webrev.02/
> 
> Test:
>   Reran hotspot_gc_shenandoah (fastdebug and release)
>   on x86_64 and x86_32 Linux.
> 
> Thanks,
> 
> -Zhengyu
> 
> 
>>
>>>
>>> src/hotspot/share/gc/shenandoah/shenandoahRootVerifier.cpp:
>>>
>>> What's that change?
>>> -  return (_types & type) != 0;
>>> +  return (_types & type) == type;
>>>
>>
>> Because WeakRoots is the combination of SerialWeakRoots and
>> ConcurrentWeakRoots now, when we test WeakRoots, expect both bits are
>> set.
>>
>>
>> Thanks,
>>
>> -Zhengyu
>>
>>
>>> Thanks,
>>> Roman
>>>
>>>> Please review this enhancement that moves some of weak root processing
>>>> into concurrent phase whenever possible.
>>>>
>>>> When concurrent class unloading is enabled, the weak roots that backed
>>>> by OopStorage can be processed in concurrent phase, as Shenandoah
>>>> native
>>>> LRB can properly resolve the object and hide dead oops from mutators.
>>>>
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8228818
>>>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8228818/webrev.00/
>>>>
>>>> Test:
>>>>    hotspot_gc_shenandoah (fastdebug and release)
>>>>    on x86_64 and x86_32 Linux
>>>>    specjvm on x86_64 Linux
>>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu
>>>>
>>>
>