RFR(S) 8225813: Shenandoah: Obsolete ShenandoahSuspendibleWorkers flag, as suspendible workers are required

[Zhengyu Gu]

On 6/16/19 7:27 AM, Roman Kennke wrote:
> Hi Zhengyu,
> 
> So as far as I can see, we are looking at 2 potential issues:
> 
> - Non-GC safepoint work might use our GC workers. This would only
> possibly be heap-iteration, and as far as I remember, heap-iteration is
> single-threaded and uses the VMThread. Besides: if this would happen,
> we'd already have the problem. I don't think this is the case.
> (Safepoint cleanup has this problem, and we use a separate thread pool
> for precisely this reason.)

Right. It is single-threaded.

> 
> - Non-GC safepoint work needs to look at GC roots, and concurrent
> processing of GC roots might conflict with that.
> 
> As far as I see it, this problem could also be solved by employing the
> correct barriers. I believe this only affects heap-iteration. So when
> heap-iteration scans the GC roots (during conc-evac), it needs to do the
> LRB to avoid exposing from-space-objects in the iteration. Later on,
> when we also want to do concurrent cleanup, it also needs to filter
> unreachable objects by not seeding the iteration with objects that are
> unreachable (as per the marking bitmap, not the aux bitmap!).
> 
Our current solution is to use the same roots for heap iteration, as 
current GC cycle, so both see the same live object graph.

At this phase, the problems I saw, I think, can be solved with 
additional cares to avoid suspending workers, e.g. not clear claimed 
flag for CLDG and using _claim_none to for heap iteration, etc. but I 
don't think it worth the efforts.

This change is inavoidable for concurrent class unloading and concurrent 
code root processing:

- SystemDictionary::do_unloading(), by design, can not run through 
safepoints (JDK-8222223),

- Some CLDG related VM operations, such as ClassLoaderStatsOperation and
ClassLoaderHierarchyOperation are no longer safe, if class unloading run 
through them.

- I suspect some IC related also no longer safe.

-Zhengyu




> I might be missing something there?
> 
> The reason why I'm looking at avoiding stopping GC threads at safepoints
> is that it may introduce additional latencies when bringing Java threads
> to (non-GC) safepoints like biased locking or deopt, by having them wait
> for GC work (especially when we have non-interruptible not-quite-short
> GC work like GC roots processing).
> 
> Roman
> 
>> On 6/15/19 9:26 PM, Zhengyu Gu wrote:
>>> On 6/15/19 3:35 PM, Roman Kennke wrote:
>>>> Can you give a little bit of explanation why workers need to be
>>>> suspended at safepoints for concurrent root processing?
>>>
>>> Concurrent processing uses workers and safepoint parallel task also
>>> utilizes workers, and shenandoah shares workers, so they can not overlap.
>>
>> Should be more articulate. I don't mean GC safepoint parallel tasks,
>> which can not overlap. But the tasks, such as, heap iteration that might
>> utilize workers.
>>
>> Also, heap iteration walks CLDG at a safepoint, that may interfere CLDG
>> walking during concurrent CLDG evacuation.
>>
>> -Zhengyu
>>
>>>
>>>
>>> -Zhengyu
>>>
>>>>
>>>> Other than that, the patch looks good.
>>>>
>>>> Roman
>>>>
>>>>> Please review this patch that makes suspendible workers required,
>>>>> instead of optional, and obsoletes the corresponding flag.
>>>>>
>>>>> Suspendible workers are required for concurrent root processing.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8225813
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~zgu/JDK-8225813/webrev.00/index.html
>>>>>
>>>>> Test:
>>>>>     hotspot_gc_shenandoah (fastdebug and release)
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Zhengyu
>>>>
>