RFR(S): 8210381: Obsolete EmitSync

[Mikael Vidstedt]

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/webrev/ <http://cr.openjdk.java.net/~mikael/webrevs/8210381/webrev.00/open/webrev/>
> 
> 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
>> 
>> 
>