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

[coleen.phillimore at oracle.com]

Looks good!
Coleen

On 10/31/18 9: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
>>>
>>
>