RFR(XS): JDK-8212122: Allow ReferenceProcessor to always be MT processing

[sangheon.kim at oracle.com]

Hi Roman,

On 10/12/18 7:56 AM, Roman Kennke wrote:
> Currently, in ReferenceProcessor, the mt_processing flag is adjusted for
> each phase depending on the number of workers: if it's 1 worker, then
> mt_processing is disabled, for more than 1 worker, mt_processing is
> enabled. Depending on the flag, ref-processing is either done through
> AbstractRefProcTaskExecutor, or directly in VMThread. However, we have a
> situation where we always want to execute via the executor, and never
> directly. Instead we decide in the executor whether or not we want to
> execute directly in the VMThread.
>
> A simple change allows a subclass of ReferenceProcessor to change this
> behavior.
>
> In case you're interested why we need to do this: Shenandoah requires
> (for each worker) a scope around reference processing. In current
> scheme, we'd need to put one scope around
> RP::process_discovered_reference() to cover the 1-threaded case, and one
> scope inside each worker to cover those. However, we cannot handle this
> sort of weirdly nested scoping.
>
> (If it was me, I'd rework this whole execution scheme to always call the
> MT config, and let RP figure out and execute directly in VMThread if
> it's only 1 worker, and not put this burden on the caller to set up both
> for 1-threaded and N-threaded execution. This is just an ugly mess.)
I agree with you.
In addition to Kim's comment which covered ParallelGC case,
1) For CMS, single vs. MT processing should be decided before calling 
RP  because of CMS specific implementation. For more detail, please 
refer comments at https://bugs.openjdk.java.net/browse/JDK-6938732
2) For G1, MT processing is set to true by default. But still caller has 
2 paths as you described. I'm not sure whether single thread case is 
still needed there or not because RP is deciding to use single or 
multiple threads. Probably we can remove it unless an user really wants 
to force using single thread only. :)

>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8212122
> Webrev:
> http://cr.openjdk.java.net/~rkennke/JDK-8212122/webrev.00/
Looks good to me.

Thanks,
Sangheon

>
> Testing: passes hotspot/tier1 locally. Works well with Shenandoah too.
>
> Ok?
>
> Roman
>