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

[Vladimir Kozlov]

Looks good to me too.

Thanks,
Vladimir

On 10/31/18 6:16 AM, Erik Österlund wrote:
> Hi Coleen,
> 
> Thanks for the review.
> 
> New full webrev:
> http://cr.openjdk.java.net/~eosterlund/8212681/webrev.01/
> 
> Incremental:
> http://cr.openjdk.java.net/~eosterlund/8212681/webrev.00_01/
> 
> Fixed Robbin's copyright headers, Aleksey's AArch64 build aaaaand...
> 
> On 2018-10-30 23:32, coleen.phillimore at oracle.com wrote:
>>
>> 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?
> 
> Removed.
> 
>> 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?
> 
> Good catch. Should really use the CompiledICLocker, so I changed it so it does that.
> 
>> 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.
> 
> Changed to code/codeBehaviours.hpp as requested.
> 
> Thanks,
> /Erik
> 
>> 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
>>>
>>
>