RFR: 8079275: Remove CollectedHeap::use_parallel_gc_threads

[Kim Barrett]

On May 4, 2015, at 9:34 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> Hi,
> 
> Please review this patch to remove CollectedHeap::use_parallel_gc_threads.
> 
> http://cr.openjdk.java.net/~stefank/8079275/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8079275
> 
> This function is used from:
> 
> 1) CMS - CMS can't be started with -XX:ParallelGCThreads=0 anymore, so all checks against use_parallel_gc_threads will always be true.
> 
> 2) GenCollectedHeap::process_roots - This usage could be replaced with n_par_threads(), the same check the other parallel task (Threads::possibly_parallel_oops_do) uses.
> 
> This will change the executed code for CMS but not Serial.
> 
> Serial will always return false for CollectedHeap::use_parallel_gc_threads and GenCollectedHeap:n_par_threads(). See:
> void GenCollectedHeap::set_par_threads(uint t) {
>  assert(t == 0 || !UseSerialGC, "Cannot have parallel threads");
>  CollectedHeap::set_par_threads(t);
> 
> CMS will always return true for CollectedHeap::use_parallel_gc_threads, but can now return either true or false for n_par_threads() depending on how n_par_threads was setup for the currently executing task. This means that some CMS paths will now call StringTable::oops_do instead of StringTable::possibly_parallel_oops_do, when running single threaded.
> 
> Thanks,
> StefanK

Nice cleanup.

------------------------------------------------------------------------------
src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp 
 229   typedef CMSParGCThreadState* CMSParGCThreadStatePtr;

The change to eliminate the if-block changed the scope of this
typedef, extending it to the end of the function.  Is this typedef
actually needed?  I don't see any obvious reason why the use of it in
the NEW_C_HEAP_ARRAY call couldn't just use the pointer type directly,
rather than needing a typedef.

If the typedef doesn't have some useful purpose in the
NEW_C_HEAP_ARRAY call, then I'd rather see it removed, especially now
with the scope extension.  Your call.  I don't need a new webrev just
for this change, if you decide to make it.

[And I was about to send this when StefanJ's similar suggestion 
arrived in my in-box.  So two votes for removal.]

------------------------------------------------------------------------------ 
src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp 
2133   ModUnionClosure* muc = CollectedHeap::use_parallel_gc_threads() ?
2134                                                &_modUnionClosurePar
2135                                                : &_modUnionClosure;
2136   _cmsGen->gc_prologue_work(full, registerClosure, muc);
=>
2130   _cmsGen->gc_prologue_work(full, registerClosure, &_modUnionClosurePar);

It looks like this change eliminated the only reference to _modUnionClosure.

------------------------------------------------------------------------------