RFR 8209624: [JVMCI] Invalidate nmethods instead of directly unloading them when the InstalledCode is dropped

[Vladimir Kozlov]

Thank you, Tom

Looks good.

Vladimir

On 8/20/18 11:03 AM, Tom Rodriguez wrote:
> The NoSafepointVerifier in make_not_entrant_or_zombie was weakened slightly as noted below. 
> http://cr.openjdk.java.net/~never/8209624/webrev/src/hotspot/share/code/nmethod.cpp.udiff.html.  NoSafepointVerifier 
> checks that no new safepoints occur during its lifetime but since it inherits from NoGCVerifier it also checks that you 
> aren't in the middle of a GC which is too strict for this case.  There's nothing GC unsafe about calling 
> make_not_entrant during a GC.  There are at least a couple calls elsewhere in HotSpot that do a similar weakening.
> 
> tom
> 
> 
> Tom Rodriguez wrote on 8/17/18 1:46 PM:
>> The NoSafepointVerifier in make_not_entrant_or_zombie is unhappy because we're in the middle of a GC.  I think it's 
>> fine to call it in that case.   We just don't want a new one starting in the middle of that code so I'm going to relax 
>> the GC part of the assert.
>>
>>    // This can be called while the system is already at a safepoint which is ok
>>    NoSafepointVerifier nsv(true, !SafepointSynchronize::is_at_safepoint());
>>
>> I'll send out an email if the next run is clean.
>>
>> tom
>>
>> Vladimir Kozlov wrote on 8/17/18 12:24 PM:
>>> Tom, you have tests failed.
>>>
>>> Vladimir
>>>
>>> On 8/17/18 11:52 AM, Tom Rodriguez wrote:
>>>> http://cr.openjdk.java.net/~never/8209624/webrev
>>>> https://bugs.openjdk.java.net/browse/JDK-8209624
>>>>
>>>> When the lifetime of an InstalledCode instance is used to control the lifetime of the corresponding nmethod it's 
>>>> possible for the nmethod still have live activations when the nmethod is unloaded.  This is because the weak 
>>>> semantics of the installed_code means it isn't visited in the nmethod::oops_do phase.  We would have to sometimes 
>>>> maintain a second strong reference which was visited in nmethod::oops_do to make that work.  Instead simply 
>>>> invalidate the nmethod and let the sweeper unload the nmethod once it no longer has activations.  Note that this 
>>>> issue doesn't affect normal compiles requested the CompilerBroker since the InstalledCode reference is weak in that 
>>>> case.  The fix was tested with a Truffle program that originally exposed the issue.  mach5 runs are in progress.
>>>>
>>>> tom