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

[Erik Österlund]

Hi David,

Not bad! I tend to agree, and like it.

How do we feel about “yield” instead of process though? We yield the normal execution to do... something. Like this:

SafepointMechanism::yield(thread);
SafepointMechanism::yield_if_requested(thread);
SafepointMechanism::yield_if_requested_slow(thread);

It is yet a bit more abstract I think. Oh and a few characters shorter.

What do you think? Yield vs process?

Next player!

Thanks,
/Erik

> On 2 Sep 2020, at 07:54, David Holmes <david.holmes at oracle.com> wrote:
> 
> 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