RFR: 8255384: Remove special_runtime_exit_condition() check from SS::block() [v3]

[David Holmes]

On 5/11/2020 12:26 am, Robbin Ehn wrote:
> On Wed, 4 Nov 2020 14:05:52 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:
> 
>> Hi Robbin,
>>
>> Thanks, for looking at this.
>>
>>>>> How about SafepointMechanism::process_if_requested_with_exit_check(bool check_asyncs)?
>>>
>>>
>>> But _suspend_flag have nothing to do with safepoint polling, handling it with SafepointMechanism doesn't seem right?
>>> Yes, but today we are already checking the _suspend_flag cases inside SS::block(), so this would only add a method to explicitly do that if you want, i.e. make it visible. In any case, I'm fine with both ways of doing it. I can leave as it is right now and see if David is fine with that.
>>
>> Patricio
>>
>>> Thanks, Robbin
>>>> That works for me.
>>>> ## Thanks,
>>>> David
> 
> Skimming a bit, as far as I can see void ThreadSafepointState::handle_polling_page_exception() should use ThreadInVMfromJava and in the other path ThreadInVMfromJavaNoAsyncException.
> No?

No :) I strongly object to using dummy thread-state transitions just to 
get the side-effects that are part of real thread-state transitions. If 
you need that side-effect in this code then it should be explicit in 
this code.

And when we have a recurring pattern like:

SafepointMechanism::process_if_requested(THREAD);
if (THREAD->has_special_runtime_exit_condition()) {
      THREAD->handle_special_runtime_exit_condition(checkAsyncs);
}

it says to me that we are missing an appropriate API abstraction to 
capture why we need these checks in the calling code.

Whether there are things presently inside 
handle_special_runtime_exit_condition that are not relevant to these 
calls sites (eg. suspend flag) is a different issue.

Cheers,
David

> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/913
>