RFR: 8216557 Aarch64: Add support for Concurrent Class Unloading

[Stuart Monteith]

Hi,
	If anyone has bandwidth, would the be able to review this patch? It
addresses Andrew, Per and Erik's comments:
        http://cr.openjdk.java.net/~smonteith/8216557/webrev.1/

Thanks,
	Stuart


On 27/03/2020 09:47, Erik Österlund wrote:
> Hi Stuart,
> 
> Thanks for sorting this out on AArch64. It is nice to see thatyou can
> implement these
> barriers on platforms that do not have instruction cache coherency.
> 
> One small change request:
> It looks like in C1 you inject the entry barrier right after build_frame
> is done:
> 
>  629       build_frame();
>  630       {
>  631         // Insert nmethod entry barrier into frame.
>  632         BarrierSetAssembler* bs =
> BarrierSet::barrier_set()->barrier_set_assembler();
>  633         bs->nmethod_entry_barrier(_masm);
>  634       }
> 
> Unfortunately, this is in the platform independent part of the LIR
> assembler. In the x86 version
> we inject it at the very end of build_frame() instead, which is a
> platform-specific function.
> The platform-specific function is in the C1 macro assembler file for
> that platform.
> 
> We intentionally put it in the platform-specific path as it is a
> platform-specific feature.
> Now on x86, the barrier code will be emitted once in build_frame() and
> once after returning
> from build_frame, resulting in two nmethod entry barriers, and only the
> first one will get
> patched, causing the second one to mostly take slow paths, which isn't
> necessarily wrong,
> but will cause regressions.
> 
> I would propose you just move those lines into the very end of the
> AArch64-specific part of
> build_frame().
> 
> I don't need to see another webrev for that trivial code motion. This
> looks good to me.
> Agan, thanks a lot for fixing this! It will allow me to go forward with
> concurrent stack
> scanning on AArch64 as well.
> 
> Thanks,
> /Erik
> 
> 
> On 2020-03-26 23:42, Stuart Monteith wrote:
>> Hello,
>>          Please review this change to implement nmethod entry barriers on
>> aarch64, and hence concurrent class unloading with ZGC. Shenandoah will
>> need to be separately tested and enabled - there are problems with this
>> on Shenandoah.
>>
>> It has been tested with JTreg, runs with SPECjbb, gcbench, and Lucene as
>> well as Netbeans.
>>
>> In terms of interesting features:
>>           With nmethod entry barriers,  immediate oops are removed by:
>>                  LIR_Assembler::jobject2reg  and  MacroAssembler::movoop
>>          This is to ensure consistency with the entry barrier, as
>> otherwise with
>> an immediate we'd otherwise need an ISB.
>>
>>          I've added "-XX:DeoptNMethodBarrierALot". I found this
>> functionality
>> useful in testing as deoptimisation is very infrequent. I've written it
>> as an atomic to avoid it happening too frequently. As it is a new
>> option, I'm not sure whether any more is needed than this review. A new
>> test has been added
>> "test/hotspot/jtreg/gc/stress/gcbasher/TestGCBasherDeoptWithZ.java" to
>> test GC with that option enabled.
>>
>>          BarrierSetAssembler::nmethod_entry_barrier
>>          This method emits the barrier code. In internal review it was
>> suggested
>> the "dmb( ISHLD )" should be replaced by "membar(LoadLoad)". I've not
>> done this as the BarrierSetNMethod code checks the exact instruction
>> sequence, and I prefer to be explicit.
>>
>>          Benchmarking method entry shows an increase of around 6ns
>> with the
>> nmethod entry barrier.
>>
>>
>> The deoptimisation code was contributed by Andrew Haley.
>>
>> The bug:
>>          https://bugs.openjdk.java.net/browse/JDK-8216557
>>
>> The webrev:
>>          http://cr.openjdk.java.net/~smonteith/8216557/webrev.0/
>>
>>
>> BR,
>>          Stuart