RFR(L) 8228665: Shenandoah: Implementation of Concurrent nmethod evacuation

[Roman Kennke]

Ok, looks good to me now. Thanks!

Roman

> On 7/29/19 12:25 PM, Roman Kennke wrote:
>> Hi Zhengyu,
>>
>> First glance (will need to do another review pass later):
>>
>> - Does it not also need an aarch64 port?
>> - Does it also work on x86_32?
> 
> Yes, but I guess we have to deal with them later. Let's get Linux x86_64
> and Windows 64 in and stabilize them first.
> 
> 
>> - src/hotspot/share/gc/shenandoah/shenandoahArguments.cpp:
>>
>> Why is that change there?
>>
>> -  if (!ClassUnloading ||
>> !FLAG_IS_CMDLINE(ClassUnloadingWithConcurrentMark)) {
>> +  if (!ClassUnloading) {
>>       log_info(gc)("Consider -XX:+ClassUnloadingWithConcurrentMark if
>> large pause times "
>>                    "are observed on class-unloading sensitive
>> workloads");
> 
> We do want to honor ClassUnloadingWithConcurrentMark's default value, if
> we can do concurrent class unloading.
> 
> However, I misplaced this change here, should belong to concurrent class
> unloading changeset.
> 
>>
>> - src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp:
>>
>> +  BarrierSetNMethod* _bs;
>>
>> Make this BarrierSetNMethod* const _bs. Not sure if it makes a
>> difference perf-wise, but it doesn't hurt and might give the compiler a
>> hint.
>>
> Okay.
> 
>> - I guess this partly answers the questions above:
>>
>> +bool ShenandoahConcurrentRoots::can_do_concurrent_nmethods() {
>> +#if defined(_LP64) && defined(X86) && (defined(LINUX) ||
>> defined(_WINDOWS))
>>
>> But why do we deal with LINUX and WINDOWS there? Is there anything OS
>> specific in this codes?
> 
> I guess MacOS X and Solaris X86?
> 
>>
>> - This change will enable class-unloading on every cycle, is that right?
>>
> 
> Right. Again, It should belong to next changeset.
> 
> Updated: http://cr.openjdk.java.net/~zgu/JDK-8228665/webrev.01/
> 
> Reran hotspot_gc_shenandoah tests
> 
> Thanks,
> 
> -Zhengyu
> 
> 
> 
>>
>> Thanks a lot for doing this! This is huge!
>> Roman
>>
>>
>>> This patch moves nmethod evacuation and disarming into concurrent phase.
>>> However, nmethod's liveness is still checked at a pause.
>>>
>>> This is the last step before we complete concurrent class unloading,
>>> where we move class loader and nmethod's liveness check into concurrent
>>> evacuation phase.
>>>
>>> Given the scope of this changes, let's bake it in shenandoah/jdk first,
>>> before upstream to jdk/jdk.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8228665
>>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8228665/webrev.00/
>>>
>>> Test:
>>>    hotspot_gc_shenandoah (fastdebug and release)
>>>    specjvm (fastdebug and release)
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>