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

[David Holmes]

On 2/09/2020 4:37 pm, Erik Österlund wrote:
> 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?

Sorry yield => Thread.yield => sched_yield  - scheduling!

yield sounds very much like block as well.

So I'd vote for process over yield.

Cheers,
David
-----

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