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

[Stuart Monteith]

Hello,
  Thanks again Per, I've clean that up:
http://cr.openjdk.java.net/~smonteith/8214527/webrev.3/

My intention is to add a test loop for ZGC aarch64 once it has been
merged. The performance monitoring would fit in with what we have
internally just now.
I appreciate the necessity for this.

I've been testing with JTReg, JCStress, specJBB and I've been also
been using Lucene, which has been reliable at finding issues. Of
course, the more testing there is, the better.

BR,
   Stuart


On Thu, 23 May 2019 at 10:57, Per Liden <per.liden at oracle.com> wrote:
>
> 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
> >>>