RFR(S): 8210381: Obsolete EmitSync

[Baesken, Matthias]

Hello  ,   I had a short look  at the ppc / s390x changes ,   looks OK .
To be on the safe side I put  it into  our  build/test queue,  so we see results  for ppc64(le) / s390x as well tomorrow .

Best regards, Matthias

> -----Original Message-----
> From: ppc-aix-port-dev <ppc-aix-port-dev-bounces at openjdk.java.net> On
> Behalf Of Mikael Vidstedt
> Sent: Mittwoch, 5. September 2018 03:38
> To: daniel.daugherty at oracle.com
> Cc: s390x-port-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net;
> HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>;
> aarch64-port-dev at openjdk.java.net
> Subject: Re: RFR(S): 8210381: Obsolete EmitSync
> 
> 
> Dan/Vladimir, thanks for the reviews.
> 
> FWIW, in addition to the build&test job I also did some manual work to
> compare the disassembly of the resulting libjvm; the C++ compiler seems to
> agree with my “manual” dead code elimination (yay!).
> 
> Cheers,
> Mikael
> 
> > On Sep 4, 2018, at 1:40 PM, Daniel D. Daugherty
> <daniel.daugherty at oracle.com> wrote:
> >
> > On 9/4/18 3:36 PM, Mikael Vidstedt wrote:
> >> Please review this change which obsoletes the EmitSync flag. In particular,
> I could use some help from ppc, aarch64, and s390 maintainers to verify that
> the change actually builds and (still) works.
> >>
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8210381
> <https://bugs.openjdk.java.net/browse/JDK-8210381>
> >> Webrev:
> http://cr.openjdk.java.net/~mikael/webrevs/8210381/webrev.00/open/we
> brev/
> <http://cr.openjdk.java.net/~mikael/webrevs/8210381/webrev.00/open/w
> ebrev/>
> >
> > src/hotspot/cpu/aarch64/aarch64.ad
> >     No comments.
> >
> > src/hotspot/cpu/ppc/macroAssembler_ppc.cpp
> >     No comments.
> >
> > src/hotspot/cpu/s390/macroAssembler_s390.cpp
> >     No comments.
> >
> > src/hotspot/cpu/sparc/macroAssembler_sparc.cpp
> >     No comments.
> >
> > src/hotspot/cpu/x86/macroAssembler_x86.cpp
> >     No comments.
> >
> > src/hotspot/share/runtime/arguments.cpp
> >     No comments.
> >
> > src/hotspot/share/runtime/globals.hpp
> >     No comments.
> >
> >
> > General comments:
> >
> >   - The 'if (EmitSync & 0x01) {' checks are how we disable the
> >     use of compiled code for monitors. We use -XX:EmitSync=1
> >     to diagnose things when we wonder if the compiled monitor
> >     code is broken.
> >   - The 'if (EmitSync & 4)' checks are how we disable the use
> >     of compiled code for unlocking of monitors; we still use
> >     compiled code for locking of monitors. We use -XX:EmitSync=4
> >     to diagnose things when we wonder if the compiled monitor
> >     unlock code is broken.
> >   - BTW, I think s390 is using (EmitSync & 0x01) differently
> >     than other platforms. No, I didn't try to figure out exactly
> >     how s390 was using that option.
> >
> > This looks like a clean obsoletion of the '-XX:EmitSync=<val>'
> > option so thumbs up from that POV.
> >
> > Use of '-XX:EmitSync=1' for diagnostic purposes is probably
> > limited to a handful of people including me. Use of the
> > '-XX:EmitSync=4' is probably even more rare; I can't remember
> > using it myself. So while I have used the '-XX:EmitSync=<val>'
> > option to debug Java monitor stuff, it has been a long time
> > time...
> >
> > Dan
> >
> >>
> >> * Background (from bug)
> >>
> >> The experimental EmitSync flag can in theory be used to select which
> implementation of the synchronization primitives to use. The flag was
> convenient when the various implementations were compared a long time
> ago.
> >>
> >> In practice the only implementation that is used and tested today is the
> default one. The EmitSync flag no longer serves the purpose it used to, and is
> "Unsafe, Unstable" (the documentation of the flag says so explicitly). It
> should be obsoleted and later removed.
> >>
> >> Cheers,
> >> Mikael
> >>
> >>
> >