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

Patricio Chilano Mateo pchilanomate at openjdk.java.net
Wed Nov 4 14:08:55 UTC 2020


On Wed, 4 Nov 2020 11:46:30 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>>> > > In `JavaThread::check_and_handle_async_exceptions()` the block depending on is_at_poll_safepoint() looks like dead code now. I wonder if `ThreadSafepointState::_at_poll_safepoint` could even be DEBUG_ONLY?
>>> > 
>>> > 
>>> > Yes, I actually thought about doing that in the first version but then I
>>> > realized that code was already dead even before this change. We only
>>> > call set_at_poll_safepoint() in handle_polling_page_exception() and the
>>> > handle_special_runtime_exit_condition() call in SS::block() already
>>> > excludes checking async exceptions for that case. The call I removed
>>> > from ~TIVMFH was exactly the same. So I don't see a path where it could
>>> > be called where is_at_poll_safepoint() returned true.
>>> > I agree that _at_poll_safepoint should probably be DEBUG_ONLY. Then we
>>> > should add an assert in check_and_handle_async_exceptions(). Do you
>>> > think I should do that here or in another bug?
>>> 
>>> I'd think you can do it in another bug also. I'm ok either way actually.
>> Ok, I filed 8255849 to track that. 
>> Thanks Richard!
>> 
>> Patricio
>
>> > 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?
> 
> 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

-------------

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


More information about the hotspot-runtime-dev mailing list