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

[Yangfei (Felix)]

Hi Liu Xin,

  Based on what Andrew said about UseCondCardMark, I agree with Martin that we go with your new backport webrev for 8135018 first.  
  My only concern is that we changed to store CardTableModRefBS::dirty_card_val() instead of constant zero to card_addr in c1_LIRGenerator.cpp.  
  It looks to me unnecessary to add this change when you are adding the missing memory bars.  
  So I suggest we do: 

  diff -r 09d4b646f756 src/share/vm/c1/c1_LIRGenerator.cpp
--- a/src/share/vm/c1/c1_LIRGenerator.cpp       Tue Nov 12 17:54:52 2019 +0800
+++ b/src/share/vm/c1/c1_LIRGenerator.cpp       Mon Dec 16 19:17:51 2019 +0800
@@ -1622,10 +1622,16 @@
   if (can_inline_as_constant(card_table_base)) {
     __ move(LIR_OprFact::intConst(0),
               new LIR_Address(tmp, card_table_base->as_jint(), T_BYTE));
+    if (UseConcMarkSweepGC && CMSPrecleaningEnabled) {
+      __ membar_storestore();
+    }
   } else {
     __ move(LIR_OprFact::intConst(0),
               new LIR_Address(tmp, load_constant(card_table_base),
                               T_BYTE));
+    if (UseConcMarkSweepGC && CMSPrecleaningEnabled) {
+      __ membar_storestore();
+    }
   }
 #endif
 }


Thanks,
Felix

> 
> 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<mailto: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