FastSyncRoots and monitor deflation question

[Roman Kennke]

> I see this looks odd:
> 
>     if (ShenandoahFastSyncRoots && MonitorInUseLists) {
>       if
> (_process_strong_tasks->try_claim_task(SHENANDOAH_RP_PS_ObjectSynchronizer_oops_do))
> {
>         ObjectSynchronizer::oops_do(strong_roots);
>       }
>     } else {
>       while(_om_iterator.parallel_oops_do(strong_roots));
>     }
> 
> ShenandoahFastSyncRoot is true by default, but it turns on the
> *single-threaded* path? Is that intentional? If this is really better
> than the parallel stuff, then maybe remove the parallel stuff?

I think I can answer this question. If MonitorInUseList is on (it's on
by default), the monitors are kept in thread-local-lists and scanned
while scanning thread stacks. This parallelizes naturally, and we don't
need the parallel monitor iterator. If that is the intention, the
condition is wrong though and should be 'if (!ShenandoahFastSyncRoots ||
MonitorInUseLists)'. Also MonitorInUseLists is deprecated and not using
thread-local-monitors is not really useful. So maybe scratch the whole
parallel OM iterator stuff? Seems like unnecessary, unused and untested
upstreaming burden.

Also...:

> I also see this stuff:
> 
>   experimental(bool, ShenandoahMergeSafepointCleanup, false,
>    \
>               "Do safepoint cleanup piggy-backed on thread scans")
>    \
> 
>    \
>   experimental(uint, ParallelSafepointCleanupThreads, 0,
>    \
>           "Number of parallel threads used for safepoint cleanup")
>    \
> 
>    \
> 
> 
> I.e. our improvements (if they are) to safepoint cleanup are turned off
> by default, and thus probably totally untested. Do we know if it
> actually helps? Would it be worth turning on by default so that it gets
> testing? Otherwise remove/revert the corresponding code changes and
> wait/work-on concurrent monitor deflation?

ParallelSafepointCleanupThreads is something that is basically
upstreamed, at least the API, and used by other collectors (G1 and Z).
We may just want to enable it.

About piggy-backing SP cleanup, I dunno if it's useful. It seems mostly
untested though, and also introduces some significant upstreaming burden
in very hairy code (synchronizer...). Maybe remove that stuff and wait
for conc monitor deflation?

Roman