RFR(XS): JDK-8212122: Allow ReferenceProcessor to always be MT processing
rkennke at redhat.com
Mon Oct 15 13:15:53 UTC 2018
Am 13.10.18 um 11:38 schrieb Roman Kennke:
>>> 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
>>> 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. :)
> The problem is the (rather new) ergonomics code in RP that scales number
> of threads to spin up based on the amount of work to do. And if it ends
> up being only 1 thread, then it goes down the single-threaded-path.
> Otherwise I wouldn't actually need this change... (that might be another
> approach to this problem: don't actually change _mt_processing flag at
> all, and call the executor even with 1 thread. In Shenandoah, we do this
> and it works fine, and we special-case the 1-thread-path by not spinning
> up the workers and execute it in the calling thread (which is the VMThread).
In other words, this is a fix that actually removes 'cruft', and seems
less surprising API-wise: i.e. if caller asks for mt_processing, then
don't change it, and leave it to caller to execute in VMThread if it wishes.
An example how G1 can be made to strictly do the current behaviour (i.e.
execute in VMThread when ergo_workers==1):
... altough I haven't tested it at all. It works well with Shenandoah,
but I don't know if you'd actually do it/want it/ask me to do it ;-)
What do you think? Which approach do you like best (except the huge
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 833 bytes
Desc: OpenPGP digital signature
More information about the hotspot-gc-dev