RFR: 8306738: Select num workers for safepoint ParallelCleanupTask

[Thomas Schatzl]

On Tue, 25 Apr 2023 11:07:48 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Now that I am looking at it, I wonder if we have a pre-existing performance issue here: what if active GC workers is very low for some GC-specific reason? We then run all safepoint cleanups, even for non-GC safepoints, using that small worker count? This might be the accidental reliance on the number of GC workers to match the static fraction of the number of CPUs allocated to the JVM, which is often true, but this would not hold exactly under `UseDynamicNumberOfGCThreads`.
>
> It is a "known" issue that if the particular task does not select its number of threads properly before executing the task (or not at all like this task) that there may be perf issues.
> 
> If this is what you want I would suggest that `active_workers()` as limit preserves the current behavior best. Otherwise the task should calculate and set its optimal number of tasks itself as it does in this change.
> 
> Not sure if setting it to `SAFEPOINT_CLEANUP_NUM_TASKS` is a good idea if some/most of this work is trivial/empty but it's probably a good first guess.
> 
> The only GC that does not seem to set safepoint_workers to something non-default seems to be Shenandoah (the stw collectors use the "main" workerthreads they also use for evacuation, so this should be maxed out mostly, and ZGC explicitly sets them to max_workers at initialization). (via code inspection, so might be wrong)

@xmas92: what is the reason for this change, did you see any situation where an inappropriate amount (too few) number of threads were used? Just curious.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13616#discussion_r1176396353