RFR: 8252661: Change SafepointMechanism terminology to talk less about "blocking"

[David Holmes]

Hi Erik,

Okay let's play ... :)

On 2/09/2020 1:53 am, Erik Österlund wrote:
> Hi,
> 
> The SafepointMechanism class has been used to perform safepoint 
> operations, originally. Now we also perform handshake operations, and
> soon also concurrent stack processing, using the same hooks.
> Therefore, names such as SafepointMechanism::should_block no longer
> sound right, when the real question is whether it should process a
> pending operation (be that a safepoint, handshake or whatever else).

Here's the proposed set of name changes:

block_or_handshake -> process_operation
block_if_requested -> process_operation_if_requested
block_if_requested_slow -> process_operation_if_requested_slow
should_block -> should_process_operation

So "block" is wrong when you want to process a handshake or concurrent 
stack scanning operation.

block_or_handshake is currently accurate, whereas the other block* 
methods ignore the "or handshake" part. But names that enumerate choices 
don't scale well and become unwieldy even with two choices.

So we need a verb that captures the need to do something. "process" is 
not terrible if you consider blocking to be a form of processing, but I 
personally don't like it in that context in combination with "operation".

The multiplicity of what this now does reminds me of 
has_special_runtime_exit_condition/handle_special_runtime_exit_condition.

I don't like the existing "should" naming as the subject is wrong. This 
reads well to me:

if (thread->should_block()) ...

whereas this is somewhat jarring:

if (SafepointMechanism::should_block(thread)) ...

and would be better phrased as:

if (SafepointMechanism::needs_to_block(thread)) ...

Generalising that:

if (SafepointMechanism::is_active_for(thread)) ...

when then leads me (back) to:

SafepointMechanism::process(thread);
SafepointMechanism::process_if_requested(thread);
SafepointMechanism::process_if_requested_slow(thread);

Next player please ... :)

Cheers,
David
-----

> Naming is hard, so I don't want this discussion in my concurrent stack 
> processing patch.
> I have a webrev with proposed naming changes to better reflect how this 
> is used:
> http://cr.openjdk.java.net/~eosterlund/8252661/webrev.00/
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8252661
> 
> Thanks,
> /Erik