RFR: 8212681: Refactor IC locking to use a fine grained CompiledICLocker

[Erik Österlund]

Hi David,

With concurrent class unloading, there is a possibility that two 
JavaThreads both miss an IC, run the 
SharedRuntime::handle_ic_miss_helper, lock disjoint fine grained IC 
locks, both ask if a CompiledIC::is_megamorphic(), causing the 
VtableStubs_lock in VtableStubs::entry_point to become contended, and 
hence possibly rely on the VM thread using lock sneaking to not cause a 
deadlock.

If we want to get rid of sneaky locking, it seems like we should use 
MutexLockerEx and conditionally lock VtableStubs_lock only when we are 
not the VM thread in a safepoint. It seems kind of evil to build the 
correctness of a solution based on redundant locking on a higher level 
that removes the contention on another lower level lock that would if 
contended cause a deadlock.

I think the InlineCacheBuffer_lock should be okay, because in the paths 
that grab that lock, I make sure to take the CompiledIC_lock first, with 
or without fine grained IC locking.

Thanks,
/Erik

On 2018-10-22 13:22, David Holmes wrote:
> Hi Erik,
>
> The CompiledIC_lock also "guards" use of other nested locks and 
> thereby avoids the possibility of lock sneaking on those nested locks 
> [1]. Will your concurrent unloading changes change this?
>
> Thanks,
> David
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8210832
>
> On 22/10/2018 8:59 PM, Erik Österlund wrote:
>> Hi,
>>
>> Today, one must acquire the global CompiledIC_lock to have permission 
>> to perform certain IC updating activities. The lock is used outside 
>> of safepoints, but for example during nmethod unloading due to GC, 
>> lock free IC cache cleaning is performed, which is safe due to being 
>> in a safepoint.
>>
>> With concurrent class unloading introduced, the global lock is too 
>> coarse grained, and a per-nmethod mechanism is needed instead. In 
>> order to allow this, an abstract CompiledICLocker class could help 
>> ensuring the safety of IC caches, and allow both the fine grained 
>> (but density costly) approach for users of concurrent class 
>> unloading, and resort to the default global locking approach otherwise.
>>
>> The idea is to retain the coarse grained implementation of the 
>> synchronization when concurrent class unloading is not being used, as 
>> supporting fine grained locking could be more costly in memory, and 
>> there does not seem to be any use in doing this unless concurrent 
>> class unloading is being used.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8212681/webrev.00/
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8212681
>>
>> Thanks,
>> /Erik