RFR: 8280917: Simplify G1ConcurrentRefineThread activation [v3]

[Kim Barrett]

On Mon, 7 Feb 2022 10:40:36 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix primary thread notification safepoint check
>
> 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.

I'm not especially fond that name, but a `should_` prefix would incorrectly suggest it's "just" a predicate, but it has side effects.  I have some further changes coming that might do things a bit differently.  I'll keep the naming in mind.

> 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.

I meant to do that, and then forgot.  Thanks for point it out.  I'll add it before pushing.

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

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