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

[Zhengyu Gu]

On 6/16/19 3:20 PM, Roman Kennke wrote:
> BTW, *if* we claim CLDs in heap-iteration we must change/fix it, because
> it'd be a bug: if heap-iteration would happen during marking, and it
> claims all the CLDs, then subsequent marking would fail to mark through
> those CLDs.

Yes, I intent to fix this, so that we don't have to enforce suspendible 
workers for pre-JDK13 releases. Also, I moved this subtask under 
concurrent class unloading (JDK-8226241) and will file another CR to 
address this and concurrent CLDG evacuation, as I narrowed JDK-8225534 
just for JNIHandles and infrastructure.

> 
> Also, we should not neglect Traversal GC too much. We want to stabilize
> Traversal GC and get it to feature parity with conc-mark-GC. Ideally we
> would mirror whatever we do in conc-mark to traversal GC ASAP. It's ok
> to let it slip for a little now, I will take care of it when I'm back
> from PTO.

This CR does not address traversal, does not mean will not. We do want 
changeset relative small, right?

-Zhengyu




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