RFR: 8080106: Refactor setup of parallel GC threads

[Jon Masamitsu]

On 5/13/2015 6:17 AM, Stefan Karlsson wrote:
> Hi all,
>
> Please review these patches to unify the ways we specify the number of 
> used worker threads. The main goal for these patches is to get rid of 
> CollectedHeap::set_par_threads() and CollectedHeap::n_par_threads().
>
> The RFE has been split into multiple sub-tasks:
>  8080109: Use single-threaded code in 
> Threads::possibly_parallel_oops_do when running with only one worker 
> thread
>  8080110: Remove usage of CollectedHeap::n_par_threads() from root 
> processing
>  8080111: Remove SubTaskDone::_n_threads
>  8080112: Replace and remove the last usages of 
> CollectedHeap::n_par_threads()
>  8080113: Remove CollectedHeap::set_par_threads()
>
>
>
> ---
> http://cr.openjdk.java.net/~stefank/8080110/webrev.00
> 8080110: Remove usage of CollectedHeap::n_par_threads() from root 
> processing
>
> Remove the explicit usages of CollectedHeap::n_par_threads() from the 
> root processing code.
>
> The proposal is to pass the number of worker threads via the 
> StrongRootsScope object. The StrongRootsScope object is already setup 
> from single-threaded code where we know how many worker threads are 
> going to be used.
>
> ---
http://cr.openjdk.java.net/~stefank/8080110/webrev.00/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp.frames.html

2925   CMSParInitialMarkTask(CMSCollector* collector, StrongRootsScope* strong_roots_scope, uint n_workers) :
2926       CMSParMarkTask("Scan roots and young gen for initial mark in parallel", collector, n_workers),
2927       _strong_roots_scope(strong_roots_scope) {}


CMSParInitialMarkTask and CMSParRemarkTask constructors both take a 
StrongRootsScope* (which has the number of
workers) and a n_workers.  Did you look at whether the n_workers 
parameter was needed?  If this is just one
change too many to make here, that's fine.

http://cr.openjdk.java.net/~stefank/8080110/webrev.00/src/share/vm/gc_implementation/g1/g1RootProcessor.cpp.udiff.html

Is set_num_workers() needed?

  void G1RootProcessor::set_num_workers(uint active_workers) {
+  assert(active_workers == _srs.n_threads(),
+      err_msg("Mismatch between number of worker threads. active_workers: %u and n_workers(): %u",
+              active_workers,
+              _srs.n_threads()));
    _process_strong_tasks->set_n_threads(active_workers);
  }

You already have "active_workers" from the StrongRootsScope.

http://cr.openjdk.java.net/~stefank/8080110/webrev.00/src/share/vm/memory/defNewGeneration.cpp.udiff.html

+  {
+    // SerialGC runs with n_workers == 0.
+    StrongRootsScope srs(0);

I would have thought 1 would be passed instead of 0.  The 80801009 change uses 1 for serial, right?

That's all.  The rest looks good.

Jon