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

[Robbin Ehn]

On Wed, 4 Nov 2020 14:24:00 GMT, Robbin Ehn <rehn 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
>
>> 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?

> @robehn are you okay with SafepointMechanism::process_if_requested_with_exit_check()?
> 
> Patricio

Yes, with a caveat; I may try to move it later.

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

Yes in this is what interfaceSupport.xxx gives us.
So I would had preferred a method in there or use transition RAII thingies.
(I argue we would not be using them for side effect, because you should to be in VM to be blocked.
E.g. blocked in native makes no sense and we have no 'blocked' in java.
So to me this is just a place where cheat transition because we can get away with it.)

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