RFR(m): 8221734: Deoptimize with handshakes

[dean.long at oracle.com]

On 4/26/19 6:11 AM, Robbin Ehn wrote:
> Hi Dean,
>
> On 4/25/19 9:39 PM, dean.long at oracle.com wrote:
>> I don't pretend to understand the biased locking changing :-)  Could 
>> you explain the need for CompiledMethod_lock, and the other lock rank 
>> changes?
>
> To iterate the compiled methods we need to hold CodeCache_lock.
> (since we do this outside of a safepoint)
> To be able to make a compiled method not reentrant it must be 
> protected by a
> lower ranked lock. So we protect the compiled method with this new 
> lock instead
> of Patching_lock (higher ranked).
>

OK.

>>
>> Replacing
>>
>> 1089 if (_method->code() == this) {
>> 1090 _method->clear_code(); // Break a cycle
>> 1091 }
>>
>>
>> with
>>
>>
>> 1087   Method::unlink_code(_method, this); // Break a cycle
>>
>> doesn't look equivalent.  The new unlink_code has a stronger check, 
>> also checking the entry point.  I'm not sure that's OK here.
>
> Thanks, yes. I had a slightly different version before.
> I'll go over this again.
>
>>
>> In Method::Method(), don't we want to be able to initialize these 
>> fields without grabbing CompiledMethod_lock?
>
> This should just be an uncontended cas.

I don't see why the cas on a global lock would be uncontended.

> So I went with simplicity, disregarding this small overhead.
> Let me know what you feel about that.
>

I'd rather there be no locking here, like the original code.

> I'll send out a new updated and tested version next week.
>

Sounds good.

dl

> Thanks!
>
> /Robbin
>
>>
>> dl
>>
>> On 4/25/19 5:05 AM, Robbin Ehn wrote:
>>> Hi all, please review.
>>>
>>> Let's deopt with handshakes.
>>> Removed VM op Deoptimize, instead we handshake.
>>> Locks needs to be inflate since we are not in a safepoint.
>>>
>>> Goes on top of:
>>> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-April/033491.html 
>>>
>>>
>>> Code:
>>> http://cr.openjdk.java.net/~rehn/8221734/v1/webrev/index.html
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8221734
>>>
>>> Passes t1-7 and multiple t1-5 runs.
>>>
>>> A few startup benchmark see a small speedup.
>>>
>>> Thanks, Robbin
>>