RFR: 8220606: Move ScavengeNMethods unlinking to unregister_nmethod

[Per Liden]

On 3/14/19 10:13 AM, Stefan Karlsson wrote:
> Hi Per,
> 
> On 2019-03-14 09:59, Per Liden wrote:
>> Looks good,
> 
> Thanks.
> 
>   but it looks like we could remove
>> ScavengableNMethods::flush_nmethod() completely now. Don't need a new 
>> webrev if you decide to do that.
> 
> As discussed offline.
> 
> My intention with the code was to view ScavengableNMethods as the 
> implementation of the "virtual" interface needed to deal with nmethods 
> from a GC's point-of-view. The GCs using ScavengableNMethods would 
> simply delegate to this implementation, and not have to know if it 
> needed to call flush (or any of the other functions) or not.
> 
> Your proposal changes ScavengableNMethods from being _the_ 
> implementation of nmethod handling for GenCollectedHeap and 
> ParallelScavengeHeap, and conceptually change it to be a utility class 
> used by these GCs to implement their handling of nmethods.
> 
> I prefer my interpretation of this, but I also see that we already 
> decide to skip implementing verify_nmethods in ZCollectedHeap instead of 
> delegating to ZNMethod. So, there's already a precedence in that code. :)
> 
> Updated with your proposal:
>   http://cr.openjdk.java.net/~stefank/8220606/webrev.02.delta
>   http://cr.openjdk.java.net/~stefank/8220606/webrev.02

Thanks, looks good!

/Per

> 
> StefanK
> 
> 
>>
>> /Per
>>
>> On 3/14/19 9:33 AM, Stefan Karlsson wrote:
>>> Hi all,
>>>
>>> Please review this patch to move the removal of non-alive nmethods 
>>> from the scavengable nmethods list from the flush/purge phase into 
>>> the unregister_nmethod call.
>>>
>>> This will prevent the GCs from scavenging dead nmethods.
>>>
>>> http://cr.openjdk.java.net/~stefank/8220606/webrev.01/
>>> https://bugs.openjdk.java.net/browse/JDK-8220606
>>>
>>> Tested with tier1-3.
>>>
>>> Thanks,
>>> StefanK