[9] RFR (M): 8139595: MethodHandles::remove_dependent_nmethod is not MT safe

[Vladimir Ivanov]

http://cr.openjdk.java.net/~vlivanov/8139595/webrev.00
https://bugs.openjdk.java.net/browse/JDK-8139595

(Thanks to Stefan Karlsson who found the root cause.)

Operations on nmethod dependency contexts (linked lists of 
nmethodBucket*) are either guarded by CodeCache lock or happen during 
safepoints.

Recent GC enhancements introduced parallel code cache processing during 
GC, so parallel GC threads started to concurrently iterate over nmethod 
dependency lists, and it became not safe to remove stale elements 
(count() == 0) anymore.

The initial fix was to delay stale element purging and do a serial pass, 
but it works only for contexts rooted at InstanceKlasses, GC does a 
serial pass over InstanceKlasses afterwards and deletes all stale 
entries. But it doesn't work for call_site_target dependencies which are 
part of contexts rooted at CallSiteContext Java objects.

The problem manifests itself as a crash during dependency list traversal.

The fix I propose is to avoid purging of CallSiteContext contexts at all 
during GC, but remove stale entries during dependency list updates which 
happen under CodeCache lock (see safe_to_purge() check in 
methodHandles.cpp). It happens outside of safepoints and it guarantees 
exclusive access to the list. It is performed during 
j.l.i.CallSite.target updates, CallSite.target inlining, j.l.Class 
unloading, etc. Such approach allows to avoid unbounded memory 
consumption growth, only some delay (till next dependency list update) 
in memory deallocation.

To reduce amount of work at runtime I introduced a dependency list flag, 
which signals there are stale entries. It allows to skip purging if 
there are no stale entries in the list.

As a cleanup I decided to refactor nmethod dependency management code.

I came up with a DependencyContext to encapsulate the logic and used 
CPSlot idea (thanks for the pointer, John) to pack pointer + bool into a 
pointer.

Here's the excerpt from the DependencyContext description:
"Utility class to manipulate nmethod dependency context. The context 
consists of nmethodBucket* (a head of a linked list) and a boolean flag 
(does the list contains stale entries). The structure is encoded as an 
intptr_t: lower bit is used for the flag. It is possible since 
nmethodBucket* is aligned - the structure is malloc'ed in C heap.
Dependency context can be attached either to an InstanceKlass 
(_dep_context field) or CallSiteContext oop for call_site_target 
dependencies (see javaClasses.hpp). DependencyContext class operates on 
some location which holds a intptr_t value."

DependencyContext operates either on InstanceKlass::_dep_context or 
j.l.i.MHN$CallSiteContext::vmdependencies fields.

It allows to remove bool field from InstanceKlass and avoid header 
allocation for CallSiteContext.

Also, I experimented with non-packed version: have 2 fields as is and 
embed DependencyContext into InstanceKlass, but allocate it for 
CallSiteContext. The downsides were: (1) InstanceKlass footprint 
increase in some cases; (2) additional code for CDS case, since 
_dep_context is not shareable; (3) slight native memory footprint 
increase when there are many CallSite instances.

So, I decided to proceed with packed version.

Testing: hotspot/test/compiler/jsr292/CallSiteDepContextTest.java, 
-XX:+ExecuteInternalVMTests, JPRT

Thanks!

Best regards,
Vladimir Ivanov