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