RFR: Cleanup UseShenandoahOWST blocks

[Zhengyu Gu]

On 07/09/2018 08:29 AM, Roman Kennke wrote:
>>
>> On 07/09/2018 05:12 AM, Roman Kennke wrote:
>>> Looks good, thank you!
>>>
>>> We should soon consider to go for OWST by default. It's baked long
>>> enough. The only thing to wait for is current upstreaming efforts by
>>> Zhengyu, at which point we can just use upstreams TQ stuff.
>>
>> Looks like that Thomas Schatzl is playing it. He mentioned that he ran
>> into unexplainable hung with source code we probably don't know about.
> 
> Hmm. That might be related to TQs stuff, or maybe not. 1. I've seen an
> upstream bug (but don't remember what it was) about a freeze. 
Humm ... let me search up ...

2. What
> you're upstreaming is not what we have in Shenandoah, right?
I am upstreaming ParallelCleaningTask, so, yes, we have it in Shenandoah.

I would like to study to bit why Shenandoah has *much better* 
termination characteristics vs. upstream, potential some good or bad 
hiding behind it :-)

-Zhengyu

> 
> Thanks, Roman
> 
>>
>> -Zhengyu
>>
>>>
>>> Roman
>>>
>>>
>>>> This applies a few trivial cleanups around UseShenandoahOWST blocks.
>>>> Mostly renames "*_task" ->
>>>> task, and cleaning up useless UseShenandoahOWST in
>>>> ShenandoahTraversalGC::init_traversal_collection:
>>>>
>>>> diff -r fde9a873d896
>>>> src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
>>>> --- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
>>>> Mon Jul 09 10:29:38 2018 +0200
>>>> +++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
>>>> Mon Jul 09 10:34:32 2018 +0200
>>>> @@ -419,16 +419,16 @@
>>>>
>>>>      task_queues()->reserve(nworkers);
>>>>
>>>>      if (UseShenandoahOWST) {
>>>>        ShenandoahTaskTerminator terminator(nworkers, task_queues());
>>>> -    ShenandoahConcurrentMarkingTask markingTask =
>>>> ShenandoahConcurrentMarkingTask(this,
>>>> &terminator, update_refs);
>>>> -    workers->run_task(&markingTask);
>>>> +    ShenandoahConcurrentMarkingTask task(this, &terminator,
>>>> update_refs);
>>>> +    workers->run_task(&task);
>>>>      } else {
>>>>        ParallelTaskTerminator terminator(nworkers, task_queues());
>>>> -    ShenandoahConcurrentMarkingTask markingTask =
>>>> ShenandoahConcurrentMarkingTask(this,
>>>> &terminator, update_refs);
>>>> -    workers->run_task(&markingTask);
>>>> +    ShenandoahConcurrentMarkingTask task(this, &terminator,
>>>> update_refs);
>>>> +    workers->run_task(&task);
>>>>      }
>>>>
>>>>      assert(task_queues()->is_empty() || sh->cancelled_gc(), "Should
>>>> be empty when not cancelled");
>>>>    }
>>>>
>>>> diff -r fde9a873d896
>>>> src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
>>>> --- a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
>>>> Mon Jul 09 10:29:38 2018 +0200
>>>> +++ b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
>>>> Mon Jul 09 10:34:32 2018 +0200
>>>> @@ -417,19 +417,12 @@
>>>>        {
>>>>          uint nworkers = _heap->workers()->active_workers();
>>>>          task_queues()->reserve(nworkers);
>>>>          ShenandoahRootProcessor rp(_heap, nworkers,
>>>> ShenandoahPhaseTimings::init_traversal_gc_work);
>>>>
>>>> -      if (UseShenandoahOWST) {
>>>> -        ShenandoahTaskTerminator terminator(nworkers, task_queues());
>>>> -        ShenandoahInitTraversalCollectionTask traversal_task(&rp);
>>>> -        _heap->workers()->run_task(&traversal_task);
>>>> -      } else {
>>>> -        ParallelTaskTerminator terminator(nworkers, task_queues());
>>>> -        ShenandoahInitTraversalCollectionTask traversal_task(&rp);
>>>> -        _heap->workers()->run_task(&traversal_task);
>>>> -      }
>>>> +      ShenandoahInitTraversalCollectionTask traversal_task(&rp);
>>>> +      _heap->workers()->run_task(&traversal_task);
>>>>        }
>>>>
>>>>    #if defined(COMPILER2) || INCLUDE_JVMCI
>>>>        DerivedPointerTable::update_pointers();
>>>>    #endif
>>>> @@ -641,16 +634,16 @@
>>>>      if (!_heap->cancelled_gc()) {
>>>>        uint nworkers = _heap->workers()->active_workers();
>>>>        task_queues()->reserve(nworkers);
>>>>        if (UseShenandoahOWST) {
>>>>          ShenandoahTaskTerminator terminator(nworkers, task_queues());
>>>> -      ShenandoahConcurrentTraversalCollectionTask
>>>> traversal_task(&terminator);
>>>> -      _heap->workers()->run_task(&traversal_task);
>>>> +      ShenandoahConcurrentTraversalCollectionTask task(&terminator);
>>>> +      _heap->workers()->run_task(&task);
>>>>        } else {
>>>>          ParallelTaskTerminator terminator(nworkers, task_queues());
>>>> -      ShenandoahConcurrentTraversalCollectionTask
>>>> traversal_task(&terminator);
>>>> -      _heap->workers()->run_task(&traversal_task);
>>>> +      ShenandoahConcurrentTraversalCollectionTask task(&terminator);
>>>> +      _heap->workers()->run_task(&task);
>>>>        }
>>>>      }
>>>>
>>>>      if (!_heap->cancelled_gc() && ShenandoahPreclean &&
>>>> _heap->process_references()) {
>>>>        ShenandoahEvacOOMScope oom_evac_scope;
>>>> @@ -672,16 +665,16 @@
>>>>
>>>>        // Finish traversal
>>>>        ShenandoahRootProcessor rp(_heap, nworkers,
>>>> ShenandoahPhaseTimings::final_traversal_gc_work);
>>>>        if (UseShenandoahOWST) {
>>>>          ShenandoahTaskTerminator terminator(nworkers, task_queues());
>>>> -      ShenandoahFinalTraversalCollectionTask traversal_task(&rp,
>>>> &terminator);
>>>> -      _heap->workers()->run_task(&traversal_task);
>>>> +      ShenandoahFinalTraversalCollectionTask task(&rp, &terminator);
>>>> +      _heap->workers()->run_task(&task);
>>>>        } else {
>>>>          ParallelTaskTerminator terminator(nworkers, task_queues());
>>>> -      ShenandoahFinalTraversalCollectionTask traversal_task(&rp,
>>>> &terminator);
>>>> -      _heap->workers()->run_task(&traversal_task);
>>>> +      ShenandoahFinalTraversalCollectionTask task(&rp, &terminator);
>>>> +      _heap->workers()->run_task(&task);
>>>>        }
>>>>    #if defined(COMPILER2) || INCLUDE_JVMCI
>>>>        DerivedPointerTable::update_pointers();
>>>>    #endif
>>>>      }
>>>>
>>>>
>>>> Testing: tier1_gc_shenandoah
>>>>
>>>> Thanks,
>>>> -Aleksey
>>>>
>>>
>>>
> 
>