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

[Zhengyu Gu]

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
>