RFR: 8280917: Simplify G1ConcurrentRefineThread activation [v3]

[Stefan Johansson]

On Wed, 2 Feb 2022 00:27:40 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this change to to G1ConcurrentRefineThreads to simplify the
>> activation mechanism.
>> 
>> We split the class into two subclasses, one for the "primary" refinement
>> thread and another class for the remaining refinement threads.
>> 
>> The secondary threads are changed to use Monitor-based waits and notifications
>> with a supporting request flag.
>> 
>> The primary thread uses an atomic activation threshold that also serves as "is
>> running" state.  This activation threshold is used by the write barrier
>> support to determine whether the thread's semaphore needs to be signaled.
>> That threshold is now in the primary refinement thread rather than in the
>> dirty card queue set to simplify threshold management and somewhat reduce
>> coupling.
>> 
>> Testing:
>> mach5 tier1-3
>> 
>> Manual tests with refinement thread logging enabled to verify expected
>> activations occur.
>
> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix primary thread notification safepoint check

Looks good, just a couple of small comments.

src/hotspot/share/gc/g1/g1ConcurrentRefineThread.hpp line 64:

> 62:   // Returns true if should deactivate.
> 63:   // precondition: this is the current thread.
> 64:   virtual bool maybe_deactivate() = 0;

I know the name is pre-existing, but I've always found the naming it a bit odd. Or we usually call thing like this `should_deactivate()`. If you prefer keeping the old name I'm fine with it, just wanted to mention it since we are changing the code.

src/hotspot/share/gc/g1/g1ConcurrentRefineThread.hpp line 94:

> 92:   Semaphore _notifier;
> 93:   DEFINE_PAD_MINUS_SIZE(0, DEFAULT_CACHE_LINE_SIZE, 0);
> 94:   volatile size_t _threshold;

Might make sense to add some additional comment for the threshold, similar to the description you have in the PR summary: 

The primary thread uses an atomic activation threshold that also serves as "is
running" state. This activation threshold is used by the write barrier
support to determine whether the thread's semaphore needs to be signaled.

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

Marked as reviewed by sjohanss (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7282