RFR: Dynamic worker refactoring

[Zhengyu Gu]

On 09/21/2017 03:03 AM, Aleksey Shipilev wrote:
> On 09/20/2017 11:27 PM, Zhengyu Gu wrote:
>> This patch refactoring dynamic GC worker code out of ShenandoahCollectorPolicy, which is overcrowd.
>>
>> My tests never show any benefits of dynamic workers so far, and our customized code does not seem to
>> outperform shared implementation, so lets revert back to shared implementation.
>>
>> Webrev: http://cr.openjdk.java.net/~zgu/shenandoah/dwg_refactor/webrev.00/
> 
> Looks very nice.
> 
> *) Looking at shenandoahWorkerPolicy.hpp, it would seem the calc_worker stuff for update-refs is
> missing? Should be *_for_conc_update_refs(), *_for_final_update_refs()? Init update-refs does not
> require workers, IIRC.
> 
Yep, thanks for pointing out.


> *) Stylistic: maybe _prev_workers_for_marking is too excessive for the name? Suggestion: _prev_marking.
> 
shorten all variables.

> *) Stylistic: indenting for arguments seems better readable like this:
> 
>    _prev_marking = AdaptiveSizePolicy::calc_active_workers(ParallelGCThreads,
>                                                            active_workers,
>                                                            Threads::number_of_non_daemon_threads());
> 
> *) Stylistic: ternary statements should probably get farther indenting, like this (or maybe after
> shortening the variable names, they are fine with single line?):
> 
>    uint active_workers = (_prev_workers_for_conc_marking == 0) ?
>                             ConcGCThreads : _prev_workers_for_conc_marking;
> 
Fixed.

> *) Typo: "calculate"
> 
>    85 // Calcuate workers for Stop-the-world partial GC
> 
Fixed.

Updated webrev: 
http://cr.openjdk.java.net/~zgu/shenandoah/dwg_refactor/webrev.01/

Thanks,

-Zhengyu
> Thanks,
> -Aleksey
>