RFR: Dynamic worker refactoring

Aleksey Shipilev shade at redhat.com
Thu Sep 21 07:03:35 UTC 2017


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.

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

*) 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;

*) Typo: "calculate"

  85 // Calcuate workers for Stop-the-world partial GC

Thanks,
-Aleksey



More information about the shenandoah-dev mailing list