RFR 8242263: Diagnose synchronization on primitive wrappers

[Doerr, Martin]

Hi Patricio,

thanks for incorporating my fixes.

I don’t know if I can find time for measurements next week. I have a lot of other things to do.
Feel free to push it after you got enough reviews.

Best regards,
Martin


From: Patricio Chilano <patricio.chilano.mateo at oracle.com>
Sent: Freitag, 7. August 2020 17:29
To: Doerr, Martin <martin.doerr at sap.com>; daniel.daugherty at oracle.com; David Holmes <david.holmes at oracle.com>; hotspot-runtime-dev at openjdk.java.net; hotspot-jfr-dev at openjdk.java.net
Subject: Re: RFR 8242263: Diagnose synchronization on primitive wrappers

Hi Martin,
On 8/7/20 12:58 AM, Doerr, Martin wrote:

Hi Patricio,



thanks for the update.



Sorry for causing trouble related to the tbnz instructions on aarch64.

My concern was basically to use the correct number of Bytes for the load instructions.

You have fixed that in webrev v3. Thanks.



Unfortunately, I did the same mistake regarding the flag requirements on PPC64 and s390. And we have a wrong label on PPC64 (otherwise branch destination can be too far).

Test duration is still terrible with the slow debug build. I still got timeouts. (Maybe I should better use fast debug builds.)
I measured the time in my Mac (fastdebug build). The FatalTest takes ~ less than a second to run, and we have 7*8=56 of those. The LogTest takes between 1-4 seconds depending on the flag, and we have 7 of those. Overall it takes 2-3 minutes to run. Maybe you can measure those times too to see where its taking so long?



Fixes see below.
Great, added the new fixes. Here is v4:

Full:  http://cr.openjdk.java.net/~pchilanomate/8242263/v4/webrev/
Inc:   http://cr.openjdk.java.net/~pchilanomate/8242263/v4/inc/webrev/<http://cr.openjdk.java.net/~pchilanomate/8242263/v4/webrev/>



Sometimes simple changes can require quite some time ...
Yep, no problem  : )

Thanks!


Patricio


Best regards,

Martin





diff -r f629202b3862 src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp

--- a/src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp     Thu Aug 06 22:03:39 2020 +0200

+++ b/src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp     Thu Aug 06 23:26:02 2020 +0200

@@ -109,7 +109,7 @@

     load_klass(Rscratch, Roop);

     lwz(Rscratch, in_bytes(Klass::access_flags_offset()), Rscratch);

     testbitdi(CCR0, R0, Rscratch, exact_log2(JVM_ACC_IS_BOX_CLASS));

-    bne(CCR0, slow_case);

+    bne(CCR0, slow_int);

   }



   if (UseBiasedLocking) {

diff -r f629202b3862 src/hotspot/cpu/ppc/macroAssembler_ppc.cpp

--- a/src/hotspot/cpu/ppc/macroAssembler_ppc.cpp        Thu Aug 06 22:03:39 2020 +0200

+++ b/src/hotspot/cpu/ppc/macroAssembler_ppc.cpp        Thu Aug 06 23:26:02 2020 +0200

@@ -2839,8 +2839,8 @@

   if (DiagnoseSyncOnPrimitiveWrappers != 0) {

     load_klass(temp, oop);

     lwz(temp, in_bytes(Klass::access_flags_offset()), temp);

-    testbitdi(CCR0, R0, temp, exact_log2(JVM_ACC_IS_BOX_CLASS));

-    bne(CCR0, cont);

+    testbitdi(flag, R0, temp, exact_log2(JVM_ACC_IS_BOX_CLASS));

+    bne(flag, cont);

   }



   if (try_bias) {

diff -r f629202b3862 src/hotspot/cpu/s390/macroAssembler_s390.cpp

--- a/src/hotspot/cpu/s390/macroAssembler_s390.cpp      Thu Aug 06 22:03:39 2020 +0200

+++ b/src/hotspot/cpu/s390/macroAssembler_s390.cpp      Thu Aug 06 23:26:02 2020 +0200

@@ -3360,8 +3360,10 @@



   if (DiagnoseSyncOnPrimitiveWrappers != 0) {

     load_klass(Z_R1_scratch, oop);

-    testbit(Address(Z_R1_scratch, Klass::access_flags_offset()), exact_log2(JVM_ACC_IS_BOX_CLASS));

-    z_btrue(done);

+    z_l(Z_R1_scratch, Address(Z_R1_scratch, Klass::access_flags_offset()));

+    assert((JVM_ACC_IS_BOX_CLASS & 0xFFFF) == 0, "or change following instruction");

+    z_nilh(Z_R1_scratch, JVM_ACC_IS_BOX_CLASS >> 16);

+    z_brne(done);

   }



   if (try_bias) {

diff -r f629202b3862 test/hotspot/jtreg/runtime/Monitor/SyncOnPrimitiveWrapperTest.java

--- a/test/hotspot/jtreg/runtime/Monitor/SyncOnPrimitiveWrapperTest.java        Thu Aug 06 22:03:39 2020 +0200

+++ b/test/hotspot/jtreg/runtime/Monitor/SyncOnPrimitiveWrapperTest.java        Thu Aug 06 23:26:02 2020 +0200

@@ -31,7 +31,7 @@

  * @bug 8242263

  * @summary Exercise DiagnoseSyncOnPrimitiveWrappers diagnostic flag

  * @library /test/lib

- * @run driver SyncOnPrimitiveWrapperTest

+ * @run driver/timeout=180000 SyncOnPrimitiveWrapperTest

  */



 public class SyncOnPrimitiveWrapperTest {