RFR: 8137022: Concurrent refinement thread adjustment and (de-)activation suboptimal [v3]

[Stefan Johansson]

On Tue, 27 Sep 2022 15:48:13 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1ConcurrentRefineThread.cpp line 61:
>> 
>>> 59:     SuspendibleThreadSetJoiner sts_join;
>>> 60:     G1ConcurrentRefineStats active_stats_start = _refinement_stats;
>>> 61:     report_active("Activated");
>> 
>> Because `G1ConcurrentRefineThread::run_service()` is shared between the primary and secondary thread there will be an "Activated" log message for each periodic wake of the primary refinement thread. This is on the trace level, but I think we should only call `report_active()` when we know there is work to be done by the thread, not just checks.
>
> The point of this message is mostly to see when these threads are
> (potentially) competing with application threads for processor time, which
> occurs regardless of whether they do anything or just quickly go back to
> waiting.
> 
> At this point the primary thread doesn't know whether there is work to be
> done, since it hasn't (successfully) called update_threads_periodically yet.
> (It can't call that until inside the STS context, since it isn't thread-safe
> against a GC. It can't usefully call it between the STS entry and the report,
> since it might not succeed and need to be retried, which we deal with by
> trying to do some work before trying again, instead of useless spinning.)
> The same is true when a thread is resumed after a safepoint (even more so
> there, since a secondary thread might already be unwanted).
> 
> Conditionally reporting activation would need to be mirrored when reporting
> deactivation.
> 
> For all those reasons, I think it should be left as is.

I agree that fixing it would come with some additional logic, the current approach is very nice and simple. Given that this is on trace level I can live with the additional messages =)

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

PR: https://git.openjdk.org/jdk/pull/10256