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

[Erik Österlund]

Hi David,

Sounds like we have a winner (process) unless anyone else has other 
suggestions?

Thanks,
/Erik

On 2020-09-02 08:56, David Holmes wrote:
> 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
>>