RFR 8242263: Diagnose synchronization on primitive wrappers
Doerr, Martin
martin.doerr at sap.com
Tue Aug 4 16:35:59 UTC 2020
Hi Patricio,
> Ok, I'll change it to movl + testl and test it out before sending v3.
Thanks. I forgot to mention arm + aarch64.
Aarch64 uses ldrw + tbnz.
Arm 32 bit uses ldr_u32 + tbnz.
I remember having seen the same mistake
And nobody noticed it on little Endian platforms.
> I see that some tests use @run driver/timeout=xxxx. Maybe you can
> specify that and see if that fixes it? Let me know if that works and I
> can add it to the test.
That seems to have an effect. But now I'm not patient enough to wait for the test to finish.
Maybe the problem is that I'm using slow debug builds.
But is there a chance to make the test quicker without losing coverage significantly?
I think the test is too slow this way to run it on a regular basis. We'd need to spend dedicated machines for it.
Best regards,
Martin
> -----Original Message-----
> From: Patricio Chilano <patricio.chilano.mateo at oracle.com>
> Sent: Dienstag, 4. August 2020 17:47
> To: Doerr, Martin <martin.doerr at sap.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,
>
> Thanks for fixing PPC and taking care of s390!
>
> On 8/4/20 11:18 AM, Doerr, Martin wrote:
> > Hi Patricio,
> >
> > I suggest to use movl + testl for checking the access flag as for other access
> flags on x64.
> Ok, I'll change it to movl + testl and test it out before sending v3.
>
> > New version for PPC64 and s390 see below.
> >
> > The test SyncOnPrimitiveWrapperTest produces hs_err files as expected.
> However, I'm getting timeout issues:
> > timed out (timeout set to 120000ms, elapsed time including timeout
> handling was 122372ms)
> > Can we provide more time?
> I see that some tests use @run driver/timeout=xxxx. Maybe you can
> specify that and see if that fixes it? Let me know if that works and I
> can add it to the test.
>
> Thanks,
> Patricio
> > Best regards,
> > Martin
> >
> >
> > diff -r 77852e129401 src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp
> > --- a/src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp Tue Aug 04
> 10:03:57 2020 +0200
> > +++ b/src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp Tue Aug 04
> 16:04:31 2020 +0200
> > @@ -107,8 +107,8 @@
> >
> > if (DiagnoseSyncOnPrimitiveWrappers != 0) {
> > load_klass(Rscratch, Roop);
> > - ld(Rscratch, in_bytes(Klass::access_flags_offset()), Rscratch);
> > - andi(Rscratch, Rscratch, JVM_ACC_IS_BOX_CLASS);
> > + lwz(Rscratch, in_bytes(Klass::access_flags_offset()), Rscratch);
> > + testbitdi(CCR0, R0, Rscratch, exact_log2(JVM_ACC_IS_BOX_CLASS));
> > bne(CCR0, slow_case);
> > }
> >
> > diff -r 77852e129401 src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp
> > --- a/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp Tue Aug 04
> 10:03:57 2020 +0200
> > +++ b/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp Tue Aug 04
> 16:04:31 2020 +0200
> > @@ -912,8 +912,8 @@
> >
> > if (DiagnoseSyncOnPrimitiveWrappers != 0) {
> > load_klass(tmp, object);
> > - ld(tmp, in_bytes(Klass::access_flags_offset()), tmp);
> > - andi(tmp, tmp, JVM_ACC_IS_BOX_CLASS);
> > + lwz(tmp, in_bytes(Klass::access_flags_offset()), tmp);
> > + testbitdi(CCR0, R0, tmp, exact_log2(JVM_ACC_IS_BOX_CLASS));
> > bne(CCR0, slow_case);
> > }
> >
> > diff -r 77852e129401 src/hotspot/cpu/ppc/macroAssembler_ppc.cpp
> > --- a/src/hotspot/cpu/ppc/macroAssembler_ppc.cpp Tue Aug 04
> 10:03:57 2020 +0200
> > +++ b/src/hotspot/cpu/ppc/macroAssembler_ppc.cpp Tue Aug 04
> 16:04:31 2020 +0200
> > @@ -2838,8 +2838,8 @@
> >
> > if (DiagnoseSyncOnPrimitiveWrappers != 0) {
> > load_klass(temp, oop);
> > - ld(temp, in_bytes(Klass::access_flags_offset()), temp);
> > - andi(temp, temp, JVM_ACC_IS_BOX_CLASS);
> > + lwz(temp, in_bytes(Klass::access_flags_offset()), temp);
> > + testbitdi(CCR0, R0, temp, exact_log2(JVM_ACC_IS_BOX_CLASS));
> > bne(CCR0, cont);
> > }
> >
> > diff -r 77852e129401 src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp
> > --- a/src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp Tue Aug 04
> 10:03:57 2020 +0200
> > +++ b/src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp Tue Aug 04
> 16:04:31 2020 +0200
> > @@ -91,6 +91,12 @@
> > // Save object being locked into the BasicObjectLock...
> > z_stg(obj, Address(disp_hdr, BasicObjectLock::obj_offset_in_bytes()));
> >
> > + if (DiagnoseSyncOnPrimitiveWrappers != 0) {
> > + load_klass(Z_R1_scratch, obj);
> > + testbit(Address(Z_R1_scratch, Klass::access_flags_offset()),
> exact_log2(JVM_ACC_IS_BOX_CLASS));
> > + z_btrue(slow_case);
> > + }
> > +
> > if (UseBiasedLocking) {
> > biased_locking_enter(obj, hdr, Z_R1_scratch, Z_R0_scratch, done,
> &slow_case);
> > }
> > diff -r 77852e129401 src/hotspot/cpu/s390/interp_masm_s390.cpp
> > --- a/src/hotspot/cpu/s390/interp_masm_s390.cpp Tue Aug 04 10:03:57
> 2020 +0200
> > +++ b/src/hotspot/cpu/s390/interp_masm_s390.cpp Tue Aug 04 16:04:31
> 2020 +0200
> > @@ -1000,6 +1000,12 @@
> > // Load markWord from object into displaced_header.
> > z_lg(displaced_header, oopDesc::mark_offset_in_bytes(), object);
> >
> > + if (DiagnoseSyncOnPrimitiveWrappers != 0) {
> > + load_klass(Z_R1_scratch, object);
> > + testbit(Address(Z_R1_scratch, Klass::access_flags_offset()),
> exact_log2(JVM_ACC_IS_BOX_CLASS));
> > + z_btrue(slow_case);
> > + }
> > +
> > if (UseBiasedLocking) {
> > biased_locking_enter(object, displaced_header, Z_R1, Z_R0, done,
> &slow_case);
> > }
> > diff -r 77852e129401 src/hotspot/cpu/s390/macroAssembler_s390.cpp
> > --- a/src/hotspot/cpu/s390/macroAssembler_s390.cpp Tue Aug 04
> 10:03:57 2020 +0200
> > +++ b/src/hotspot/cpu/s390/macroAssembler_s390.cpp Tue Aug 04
> 16:04:31 2020 +0200
> > @@ -3358,6 +3358,12 @@
> > // Load markWord from oop into mark.
> > z_lg(displacedHeader, 0, oop);
> >
> > + 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);
> > + }
> > +
> > if (try_bias) {
> > biased_locking_enter(oop, displacedHeader, temp, Z_R0, done);
> > }
> >
More information about the hotspot-runtime-dev
mailing list