RFR: 8216557 Aarch64: Add support for Concurrent Class Unloading

Stuart Monteith stuart.monteith at arm.com
Mon Apr 27 16:34:49 UTC 2020


Thanks Erik, Per, Andrew,
	I've fixed up the testcase and retested.

Uploaded here:

	http://cr.openjdk.java.net/~smonteith/8216557/webrev.2/

Would someone be able to submit this for me?


Thanks,
	Stuart

On 23/04/2020 11:51, Erik Österlund wrote:
> Hi Stuart,
> 
> This looks good to me.
> 
> Thanks,
> /Erik
> 
> On 2020-04-20 16:19, Stuart Monteith wrote:
>> 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
> 




More information about the hotspot-gc-dev mailing list