[9] RFR (XS): C2: continuous CallSite relinkage eventually disables compilation for a method

[dean.long at oracle.com]

On 1/27/17 4:29 AM, Vladimir Ivanov wrote:

> Vladimir, John, thanks for the feedback.
>
> I incorporated your suggestions:
>
>   http://cr.openjdk.java.net/~vlivanov/8173338/webrev.02/
>
>> It could be unsafe to check outside lock. method::_code is cleaned up
>> under the same lock but after the state change. So if a caller access
>> _code field it would be wrong.
>
> I don't see how method::_code is relevant to compiled method state 
> checks. It's just a double-checked locking idiom. Should be safe since 
> state transitions are monotonic.
>

I think there is a path where AOT methods can become in-use again, so 
it's not completely monotonic.

dl

>> But, I think, it should be safe with your original change in
>> CodeCache::make_marked_nmethods_not_entrant() because it does not access
>> method::_code. You should use !is_not_entrant() instead of is_alive() -
>> it works for AOT too.
>
> I decided to keep state checks both in 
> CodeCache::make_marked_nmethods_not_entrant() and 
> nmethod::make_not_entrant_or_zombie(). !is_not_entrant() isn't precise 
> enough, since it passes for zombie nmethods as well.
>
> Also, added equivalent pre-lock check in 
> AOTCompiledMethod::make_not_entrant_helper(). Let me know if you see 
> any problems with that.
>
> Best regards,
> Vladimir Ivanov
>
>> On 1/26/17 12:44 PM, Vladimir Ivanov wrote:
>>> What about moving the check to nmethod::make_not_entrant()?
>>>
>>> http://cr.openjdk.java.net/~vlivanov/8173338/webrev.01
>>>
>>> Or I can fix it separately.
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 1/26/17 11:30 PM, Vladimir Kozlov wrote:
>>>> It will not work for AOT nmethods which have not_used state.
>>>>
>>>> Vladimir
>>>>
>>>> On 1/26/17 8:47 AM, Vladimir Ivanov wrote:
>>>>> http://cr.openjdk.java.net/~vlivanov/8173338/webrev.00/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8173338
>>>>>
>>>>> The fix for JDK-7177745 disabled deoptimization count update when it
>>>>> happens due to CallSite.target change.
>>>>>
>>>>> There was one case missed though: the count is still updated when
>>>>> compilation fails due to a dependency becoming invalid
>>>>> during the compilation.
>>>>>
>>>>> The fix is to avoid the update when recompilation fails due to
>>>>> call_site_target dependency failure.
>>>>>
>>>>> Also, added a check in CodeCache::make_marked_nmethods_not_entrant()
>>>>> to avoid repeated nmethod::make_not_entrant() calls
>>>>> on nmethods which are already not entrant. Otherwise, the check is
>>>>> performed under Patching_lock inside
>>>>> nmethod::make_not_entrant_or_zombie() for every non-entrant method in
>>>>> the code cache.
>>>>>
>>>>> Testing: regression test, RBT (in progress)
>>>>>
>>>>> Thanks!
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov