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 zgc-dev
mailing list