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

[Zhengyu Gu]

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
>>>
>>