RFR 8242263: Diagnose synchronization on primitive wrappers
Doerr, Martin
martin.doerr at sap.com
Fri Aug 7 19:33:50 UTC 2020
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 {
More information about the hotspot-runtime-dev
mailing list