RFR: Parallelize safepoint cleanup

[David Holmes]

Hi Roman,

On 29/05/2017 6:23 PM, Roman Kennke wrote:
> Hi David,
> 
> the patch applied cleanly, i.e. was not affected by any hotspot changes.
> Most of your suggestions applied to the GC part that I excluded from the
> scope of this change, except for the following:
> 
>>> I introduced a dedicated worker thread pool in SafepointSynchronize.
>>> This is only initialized when -XX:+ParallelSafepointCleanup is enabled,
>>> and uses -XX:ParallelSafepointCleanupThreads=X threads, defaulting to 8
>>> threads (just a wild guess, open for discussion. With
>>> -XX:-ParallelSafepointCleanup turned off (the default) it will use the
>>> old serial safepoint cleanup processing (with optional GC hooks, see
>>> below).
>>
>> I think the default needs to be ergonomically set, as for other
>> "thread pools". Both flags should be experimental at this stage.
> 
> I implemented this using the following:
> 
>    if (ParallelSafepointCleanup &&
> FLAG_IS_DEFAULT(ParallelSafepointCleanupThreads)) {
>      // This will pick up ParallelGCThreads if set...
>      FLAG_SET_DEFAULT(ParallelSafepointCleanupThreads,
> Abstract_VM_Version::parallel_worker_threads());
>    }
> 
> As noted, it will pick up ParallelGCThreads, which might be a reasonable
> number. Otherwise we'd need to re-work the logic in vm_version.cpp to
> calculate the number of worker threads without considering
> ParallelGCThreads.

I think this is way too big a number of threads to consider for this 
situation. GC has a huge amount of work to do, but the safepoint cleanup 
should be much less work.

I'm more inclined now to set a small hard-wired default initially - say 
4 - and for future work look at generalising the existing fixed GC 
thread pool into a more general purpose, dynamic thread pool (something 
similar to java.util.concurrent.ThreadPoolExecutor.) I note others 
object to a dedicated thread pool for this, so generalising the existing 
one seems to be a reasonable future direction, whilst avoiding tying 
this to specific GCs.

Thanks,
David
-----

> http://cr.openjdk.java.net/~rkennke/8180932/webrev.02/
> <http://cr.openjdk.java.net/%7Erkennke/8180932/webrev.02/>
> 
> What do you think?
> 
> Roman
>