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

[Erik Österlund]

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
>>
>