[aarch64-port-dev ] RFR 8135018: AARCH64: Missing memory barriers for CMS collector

[Liu Xin]

hi, Martin,

Thanks for the good advice. My stand is this webrev.
https://cr.openjdk.java.net/~xliu/8135018/01/

My arguments are:
1. JDK-8078438 probably never land to jdk8u because Andrew said it isn't
worth doing. I understand your concern. I can leave a comment to
JDK-8078438 as reminder, in case it happens.
2. For me, I'd like to solve a bug once a time. It's very easy to have a
mushroom-effect if we backport everything verbatim. A bug may creep into
those vertigo patches.
3. I agree with Felix's comment that the patch causes a conflict between
jdk8u and  jdk8u-shenandoah. I propose to have another webrev of
c1_LIRGenerator.cpp in jdk8u.

thanks,
--lx



On Mon, Dec 16, 2019 at 2:28 AM Doerr, Martin <martin.doerr at sap.com> wrote:

> Hi Ix,
>
>
>
> I would only recommend pushing your new webrev if it’s clear that
> JDK-8078438 will never get backported.
>
> Otherwise, the membar in MacroAssembler::store_check will be missing if it
> doesn’t get added back manually.
>
> If we go this way, I suggest to add a comment to JDK-8078438 as reminder.
>
>
>
> I don’t think performance measurements were needed because the membar is
> required for correctness. And, as you said, the feature is off by default
> and I wonder who would ever turn it on.
>
>
>
> Best regards,
>
> Martin
>
>
>
>
>
> *From:* Liu Xin <navy.xliu at gmail.com>
> *Sent:* Montag, 16. Dezember 2019 10:48
> *To:* Andrew Haley <aph at redhat.com>
> *Cc:* Doerr, Martin <martin.doerr at sap.com>;
> aarch64-port-dev at openjdk.java.net; Hohensee, Paul <hohensee at amazon.com>
> *Subject:* Re: RFR 8135018: AARCH64: Missing memory barriers for CMS
> collector
>
>
>
> Hi, Andrew and Felix,
>
>
>
> I did backport as Felix suggested. To get UseCondCardMark, we need to
> backport other 3 patches.
>
>
>
> hg qseries as follows ( applied in order)
> 8076987: https://cr.openjdk.java.net/~xliu/8076987/00/webrev/
> 8078438: https://cr.openjdk.java.net/~xliu/8078438/00/webrev/
> 8079315: https://cr.openjdk.java.net/~xliu/8079315/00/webrev/
> 8135018: https://cr.openjdk.java.net/~xliu/8135018/02/webrev/
>
>
>
> As Andrew pointed out, I also notice that it introduces a storeLoad membar
> for interpreter if UseCondCardMark is on.
>
> The reason I didn't backport UseCondCardMark at first place because it's
> off by default. I need to run all tests again with the new parameter
> -XX:+UseCondCardMark to validate it.  Another reason is that all
> performance evaluation was done on x86_64.
>
> It's really time consuming to evaluate performance impact on aarch64.  So
> I shortcut to fix my primary bug.
>
>
>
> Could you sponsor my previous webrev?
>
> https://cr.openjdk.java.net/~xliu/8135018/01/webrev/
>
>
>
> It does have a conflict with c1_LIRGenerator.cpp. How about I send another
> webrev to jdk8u and update it as well?
>
>
>
> thanks,
>
> --lx
>
>
>
>
>
> On Mon, Dec 16, 2019 at 1:00 AM Andrew Haley <aph at redhat.com> wrote:
>
> On 12/14/19 4:47 AM, Liu Xin wrote:
> > I trace the feature UseCondCardMark. It was
> > introduced by the following changes.
> > https://bugs.openjdk.java.net/browse/JDK-8076987 C1
> > https://bugs.openjdk.java.net/browse/JDK-8078438 interpreter
> > UseCondCardMark is only supported by C2 currently.
>
> UseCondCardMark was supposed to be a performance enhancement for
> very large multi-core systems, but we realized that it needed a
> StoreLoad barrier. Because StoreLoad is so expensive there is no
> performance advantage, therefore as far as I can see the whole
> UseCondCardMark exercise is a waste of time. There is no point
> back-porting changes to the interpreter and C1.
>
> Unless someone does some performance measurements to prove that
> there is some performance gain in C1 and the interpreter,
> UseCondCardMark changes should not be back-ported.
>
> --
> Andrew Haley  (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> https://keybase.io/andrewhaley
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>
>