[aarch64-port-dev ] RFR: JDK-8214527 AArch64: ZGC for Aarch64

[Per Liden]

Hi Stuart,

On 5/21/19 3:44 PM, Stuart Monteith wrote:
> Hi Per,
>     Thanks for taking a look at that.
> 
> I'll wait until ZGC on x86_64 isn't experimental. When that happens,
> if aarch64 isn't ready for production use, I'd expect we'd make the
> definition of "UseZGC" conditional, something like:
> 
> ----
> #if defined(AARCH64)
>    #define USEZGC_COND(type, name, flag, descr)
> experimental((type),(name),(flag),(descr))
> #else
>    #define USEZGC_COND(type, name, flag, descr)
> product((type),(name),(flag),(descr))
> #endif
> 
> USEZGC_COND(bool, UseZGC, false,                          \
>            "Use the Z garbage collector")                              \
> ----
> 
> That'll need more thought, but I'd like a general solution that could
> be applied more widely if, indeed, that were even necessary.

Sounds good.

> 
> I've done the refactoring of z_aarch64.ad as you suggested here:
>     http://cr.openjdk.java.net/~smonteith/8214527/webrev.2/

In z_aarch64.ad, you shouldn't need the "#if INCLUDE_ZGC" stuff, as this 
should never be compiled if ZGC is disabled, right?. Also it looks like 
the code inside ins_encode can be collapsed into a single line, like:

   z_load_barrier_slow_reg(_masm, $dst$$Register, $mem$$base$$Register, 
$mem$$index, $mem$$scale, $mem$$disp, false /* weak */);

> 
> Apart from the system calls, I agree with you about the deduplication.
> I presume there is no expectation for there to be a SPARC linux port,
> or other architectures were there might be a desire for a different
> implementation.

We don't need to cater for that now. If/when that day comes, we can 
adjust what's shared and what's not, if there's a need to.

A more general question. What kind of testing are you doing of this, and 
will you be continuously doing it going forward? Perhaps monitoring ZGC 
performance on aarch64 too? I don't expect a lot of aarch64-specific 
issues to pop up, but since aarch64 isn't part of the normal platforms 
Oracle tests/monitors we will likely not catch such issues.

cheers,
Per

> 
> BR,
>     Stuart
> 
> On Tue, 21 May 2019 at 12:05, Per Liden <per.liden at oracle.com> wrote:
>>
>> Hi Stuart,
>>
>> On 05/17/2019 04:28 PM, Stuart Monteith wrote:
>>> Hello,
>>>      This patch implements ZGC for AArch64. Unlike the previous set of
>>> patches, this duplicates the x86 multimapping scheme rather than using
>>> the Aarch64 TBI feature, where the top 8 bits are ignored by the
>>> hardware. I didn't measure any conclusive performance problems using
>>> multimapping. It would necessitate changes to SA, JNI, and wasn't
>>> going to be compatible with Graal or JVMCI in general until they were
>>> also able to switch to 64-bit literals dynamically. In addition, there
>>> are up and coming features in the AArch64 architecture that exploit
>>> the TBI area that we should keep open, such as MTE (Memory Tagging
>>> Extensions) that open the door to lightweight hardware ASAN. With MTE
>>> using 4 of the 8 bits, and ZGC and MTE perhaps needing more than 4
>>> bits, future conflict looks likely.
>>>
>>> JTreg testing hasn't demonstrated any significant differences from x86
>>> ZGC in my testing.
>>>
>>> The behaviour should be the same as is expected from x86_64, including
>>> for the behaviour with command line arguments. Currently ZGC requires
>>> -XX:+UnlockExperimentalVMOptions, that should be the case for AArch64
>>> this release. If x86_64 were to do have ZGC designated as not
>>> experimental, we must ensure it still applies for ZGC on AArch64.
>>
>> I don't think there's a good way to declare the a flag product on one
>> platform, and experimental on another. But I guess
>> zArguments::initialize_platform() on aarch64 could check for and
>> warn/exit if UnlockExperimentalVMOptions is false.
>>
>>>
>>> The associated bug is:
>>>       https://bugs.openjdk.java.net/browse/JDK-8214527
>>> The patchset is:
>>>         http://cr.openjdk.java.net/~smonteith/8214527/webrev.0/
>>
>> src/hotspot/cpu/aarch64/gc/z/z_aarch64.ad
>> -----------------------------------------
>> I'd suggest you break out the duplicate parts of these into a function,
>> like we have done in z_x86.ad.
>>
>>
>> src/hotspot/os_cpu/linux_aarch64/gc/z/zBackingFile_linux_aarch64.cpp
>> src/hotspot/os_cpu/linux_aarch64/gc/z/zBackingFile_linux_aarch64.hpp
>> src/hotspot/os_cpu/linux_aarch64/gc/z/zBackingPath_linux_aarch64.cpp
>> src/hotspot/os_cpu/linux_aarch64/gc/z/zBackingPath_linux_aarch64.hpp
>> src/hotspot/os_cpu/linux_aarch64/gc/z/zGlobals_linux_aarch64.cpp
>> src/hotspot/os_cpu/linux_aarch64/gc/z/zGlobals_linux_aarch64.hpp
>> src/hotspot/os_cpu/linux_aarch64/gc/z/zPhysicalMemoryBacking_linux_aarch64.cpp
>> src/hotspot/os_cpu/linux_aarch64/gc/z/zPhysicalMemoryBacking_linux_aarch64.hpp
>>
>> --------------------------------------------------------------------
>> I'd would like to deduplicate most of these at some point, but I'm ok
>> with doing that after this has been merged, as we might need/want to
>> slice this slightly differently (to cater for the different syscall
>> numbers etc).
>>
>>
>> cheers,
>> Per
>>
>>>
>>> There is a patch contributed by Roland Westerlin required for fixing
>>> memory barriers,
>>>     http://cr.openjdk.java.net/~smonteith/zgc-mm/20190430/membar/
>>> I'll follow up with a separate RFR patch to address that.
>>>
>>> Thanks,
>>>      Stuart
>>>