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

[coleen.phillimore at oracle.com]

Erik,

It looks like you've removed the only use of VerifyMutexLocker.  I was 
going to ask if you ran with logging that called nmethod::print_calls() 
but I don't see any callers. Maybe nmethod::print_nmethod() should call 
it.  Can you experiment with calling this print_calls to see if you can 
remove VerifyMutexLocker?

http://cr.openjdk.java.net/~eosterlund/8212681/webrev.00/src/hotspot/share/runtime/sweeper.cpp.frames.html

676 MutexLockerEx mex(CompiledIC_lock);


Why isn't this CompiledICLocker (nm)?  Can youy add a comment why it's 
different?

http://cr.openjdk.java.net/~eosterlund/8212681/webrev.00/src/hotspot/share/runtime/vmBehaviours.new.hpp.html

I see you've added more (funny spelling) behaviours.  I think this one 
should go in the code directory with the user of it and be called 
compiledICBehaviours.hpp or even codeBehaviours.hpp.

thanks,
Coleen

On 10/23/18 9:03 AM, Erik Österlund wrote:
> Hi Robbin,
>
> Thanks for the review.
>
> /Erik
>
> On 2018-10-23 14:50, Robbin Ehn wrote:
>> Hi Erik,
>>
>> Looks better than before :), don't forget copyright years before push.
>>
>> /Robbin
>>
>> On 10/22/18 12: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
>