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

[Vladimir Ivanov]

Thanks, Dean.

Best regards,
Vladimir Ivanov

On 1/31/17 12:32 AM, dean.long at oracle.com wrote:
> On 1/30/17 1:06 PM, Vladimir Ivanov wrote:
>
>> Dean,
>>
>>> Are there any other uses of next_alive() where we would want something
>>> more strict like next_entrant()?  If so then it might be worth it to add
>>> this to the iterator.
>>
>> I briefly looked through next_alive() usages and didn't find evident
>> candidates.
>>
>> Also, is_marked_for_deoptimization() is cheap and filters out most of
>> the compiled methods, so doing it first may be better.
>>
>
> OK, then webrev.03 looks good to me.
>
> dl
>
>> Best regards,
>> Vladimir Ivanov
>>
>>> On 1/30/17 5:10 AM, Vladimir Ivanov wrote:
>>>> Are people fine with the following version?
>>>>
>>>>   http://cr.openjdk.java.net/~vlivanov/8173338/webrev.03/
>>>>
>>>> Otherwise, I'll file a separate RFE for
>>>> CodeCache::make_marked_nmethods_not_entrant() part.
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> On 1/27/17 11:11 PM, Vladimir Ivanov wrote:
>>>>>>> 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.
>>>>>
>>>>> Ok, I can leave the pre-lock check only for nmethods.
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov
>>>>>
>>>>>>>> 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
>>>>>>
>>>
>