RFR: ZGC ZNMethodTable changes 8219462, 8219463, 8219464, 8219466, 8219467, 8219468, 82194629

[Per Liden]

On 2/21/19 9:50 AM, Stefan Karlsson wrote:
> Hi all,
> 
> Please review these patches that mostly restructures the ZNMethodTable 
> code. The intent is to make ZNMethodTable a slimmed down table 
> implementation, while the ZGC specific class unloading code is pulled 
> out to a new ZNMethod sub component.
> 
> A webrev of all patches can be found here:
> http://cr.openjdk.java.net/~stefank/zgc/zNMethod/webrev/

Since I've seen this code before Stefan sent it out I only have a few 
minor nits.

src/hotspot/share/gc/z/zNMethodClosure.hpp
------------------------------------------
- It seems general enough to be named NMethodClosure and placed in 
iterator.hpp.

src/hotspot/share/gc/z/zNMethodData.hpp
---------------------------------------
- Include guard missing.
- immediates_begin/immediates_end alignment.

src/hotspot/share/gc/z/zNMethodAllocator.hpp
--------------------------------------------
- "public AllStatic"
- allocate/free alignment.

src/hotspot/share/gc/z/zNMethod.hpp
-----------------------------------
I'd like to avoid having "nmethod" in the function names, as it's given 
by the class context. However, calling a function "register" is 
problematic so I'm ok with leaving it as is for now.

Other than that, looks good.

/Per

> 
> ------------------------------------------------------------------------
> 8219462: ZGC: Use wait/notify in ZNMethodTable
> https://bugs.openjdk.java.net/browse/JDK-8219462
> http://cr.openjdk.java.net/~stefank/8219462/webrev.01/
> 
> The nmethod sweeper is not allowed to unregister an nmethod while the GC 
> is iterating over the ZNMethodTable. The current code spins in a loop 
> releasing the lock and sleeping for 1 ms. I propose that we replace this 
> with wait and notify.
> 
> ------------------------------------------------------------------------
> 8219463: ZGC: Remove redundant ZNMethodTable::_iter_lock
> https://bugs.openjdk.java.net/browse/JDK-8219463
> http://cr.openjdk.java.net/~stefank/8219463/webrev.01/
> 
> The ZNMethodTable lock guards accesses with both the CodeCache_lock and 
> it's own _iter_lock. This used to necessary because there was a path 
> that didn't take the CodeCache_lock. A recent bug fix changed this and 
> we always hold the CodeCache_lock when we take the _iter_lock. I propose 
> that we remove the _iter_lock.
> 
> ------------------------------------------------------------------------
> 8219464: ZGC: Move nmethod oop properties from ZNMethodTableEntry to 
> ZNMethodData
> https://bugs.openjdk.java.net/browse/JDK-8219464
> http://cr.openjdk.java.net/~stefank/8219464/webrev.01/
> 
> The ZNMethodTableEntry contains two cached values, regarding the 
> properties of an nmethod. The first is whether the nmethod has 
> "immediate oops" in the code. The second property is used to indicate if 
> the nmethod has "non immediate oop", which probably is a bad name, but 
> it tells if there are oop-derived immediates that need to be fixed when 
> Objects have been moved.
> 
> I propose that we move these properties to the ZNMethodData objects that 
> are attached to the nmethods via the gc specific gc_data. This allows 
> for a cleaner separation of the proper table code in ZNMethodTable and 
> the class ZGC class unloading code.
> 
> ------------------------------------------------------------------------
> 8219466: ZGC: Extract allocation functionality into a new 
> ZNMethodAllocator class
> https://bugs.openjdk.java.net/browse/JDK-8219466
> http://cr.openjdk.java.net/~stefank/8219466/webrev.01/
> 
> ZNMethodData delegates the responsibility to safely free it's memory to 
> ZNMethodTable. I propose we break this dependency by introducing a 
> ZNMethodAllocator class that both classes can use. ZNMethodTable will 
> notify ZNMethodAllocator when its unsafe to free data and when it's time 
> to execute the deferred frees. ZNMethodData will then simply use 
> ZNMethodAllocator to allocate and free, and ZNMethodAllocator will 
> handle the deferred frees under the hood.
> 
> ------------------------------------------------------------------------
> 8219467: ZGC: Move ZNMethodData to its own file
> https://bugs.openjdk.java.net/browse/JDK-8219467
> http://cr.openjdk.java.net/~stefank/8219467/webrev.01/
> 
> A simple move of the code.
> 
> ------------------------------------------------------------------------
> 8219468: ZGC: Extract iteration functionality into a new 
> ZNMethodTableIteration class
> https://bugs.openjdk.java.net/browse/JDK-8219468
> http://cr.openjdk.java.net/~stefank/8219468/webrev.01/
> 
> Extracts the iteration code into its own class.
> 
> This patch also changes the how tables are deleted when an iteration is 
> in progress, to use the new ZNMethodAllocator class instead.
> 
> ------------------------------------------------------------------------
> 8219469: ZGC: Extract functions out from ZNMethodTable into new ZNMethod 
> class
> https://bugs.openjdk.java.net/browse/JDK-8219469
> http://cr.openjdk.java.net/~stefank/8219469/webrev.01/
> 
> This patch moves the non-table specific ZGC nmethod handling code out to 
> a new ZNMethod class.
> 
> ------------------------------------------------------------------------
> 
> Tested with:
> tier1,tier2,tier3, gc-test-suite
> 
> Thanks,
> StefanK