RFR[13]: 8224674: NMethod state machine is not monotonic

[dean.long at oracle.com]

On 7/17/19 9:59 AM, Erik Österlund wrote:
> Hi Dean,
>
> You are correct that the winner of the race will already have made 
> sure the operations you listed have run. So by not returning you would 
> essentially just continue performing a bunch of unnecessary operations 
> that won't do anything.
>
> I would prefer though to stay as close to my original fix as possible 
> as that one has gone through extensive testing, and I would like to 
> push this before the cut-off for P3 bugs to 13 tomorrow.
>
> Here is my latest attempt:
> http://cr.openjdk.java.net/~eosterlund/8224674/webrev.03/
>
> Here is an incremental webrev to the original proposition:
> http://cr.openjdk.java.net/~eosterlund/8224674/webrev.00_03/
>
> As you can see I have only changed the guarantee to assert as 
> requested by Coleen, and added a bunch of commentary to explain why 
> e.g. a failing transition does not need to worry about the 3 side 
> effects before the failure. Hope my comments make sense.
>
> I hope you think this is okay. If there is more clarification or 
> cleanups you would like to see, I am more than happy to file such RFEs 
> for 14.
>

I'm OK with this, but yes, please file an RFE for cleanup in 14. It's 
not obvious to me that make_not_entrant and make_unloaded are doing the 
same operations.  Some refactoring here should help a lot.

dl

> Thanks,
> /Erik
>
> On 2019-07-17 09:17, dean.long at oracle.com wrote:
>> On 7/16/19 10:51 AM, dean.long at oracle.com wrote:
>>> Back to the make_not_entrant / make_unloaded race.  If 
>>> make_not_entrant bails out half-way through because make_unloaded 
>>> won the race, doesn't that mean that make_unloaded needs to have 
>>> already done all the work that make_not_entrant is not doing? 
>>> unlink_from_method, invalidate_nmethod_mirror, remove_osr_nmethod, 
>>> unregister_nmethod, etc. 
>>
>> What I'm thinking is, what happens if instead of this:
>>
>> 1365     // Change state
>> 1366 if (!try_transition(state)) {
>> 1367 return false;
>> 1368 }
>> we do this: 1365     // Maybe change state
>> 1366 if (!try_transition(state)) {
>> 1367 // fall through 1368 }
>>
>> dl
>>