RFR(M): 8155949: Support relaxed semantics in cmpxchg

[Doerr, Martin]

Hello everybody,

thanks for finding this issue. New webrev is here:
http://cr.openjdk.java.net/~mdoerr/8155949_relaxed_cas/webrev.02/

Best regards,
Martin

From: Hiroshi H Horii [mailto:HORII at jp.ibm.com]
Sent: Dienstag, 10. Mai 2016 15:18
To: David Holmes <david.holmes at oracle.com>
Cc: hotspot-gc-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net; Tim Ellison <Tim_Ellison at uk.ibm.com>; Doerr, Martin <martin.doerr at sap.com>
Subject: Re: RFR(M): 8155949: Support relaxed semantics in cmpxchg

Hi David,

> Just need another reviewer to chime in - given you and Martin are both
> contributors. Or are you the main contributor with Martin being a reviewer?

Martin and I are contributors of this change.

> Still a problem on Solaris sparc:

Martin, could you create a new change in webrev with the patch that David sent?

Regards,
Hiroshi
-----------------------
Hiroshi Horii, Ph.D.
IBM Research - Tokyo


David Holmes <david.holmes at oracle.com<mailto:david.holmes at oracle.com>> wrote on 05/10/2016 21:29:53:

> From: David Holmes <david.holmes at oracle.com<mailto:david.holmes at oracle.com>>
> To: Hiroshi H Horii/Japan/IBM at IBMJP, "hotspot-runtime-
> dev at openjdk.java.net<mailto:dev at openjdk.java.net>" <hotspot-runtime-dev at openjdk.java.net<mailto:hotspot-runtime-dev at openjdk.java.net>>
> Cc: Tim Ellison <Tim_Ellison at uk.ibm.com<mailto:Tim_Ellison at uk.ibm.com>>, "ppc-aix-port-
> dev at openjdk.java.net<mailto:dev at openjdk.java.net>" <ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>>, "hotspot-
> gc-dev at openjdk.java.net<mailto:gc-dev at openjdk.java.net>" <hotspot-gc-dev at openjdk.java.net<mailto:hotspot-gc-dev at openjdk.java.net>>
> Date: 05/10/2016 21:31
> Subject: Re: RFR(M): 8155949: Support relaxed semantics in cmpxchg
>
> On 10/05/2016 9:04 PM, David Holmes wrote:
> > Hi Hiroshi,
> >
> > On 10/05/2016 8:44 PM, Hiroshi H Horii wrote:
> >> Hi All,
> >>
> >> Can I please request reviews for the following change?
> >>
> >> Code change:
> >> http://cr.openjdk.java.net/~mdoerr/8155949_relaxed_cas/webrev.01/
> >
> > Changes look good. I'm currently running them through our internal build
> > system. I will sponsor this and push the change through JPRT.
>
> Still a problem on Solaris sparc:
>
> "/opt/jprt/T/P1/102505.daholme/s/hotspot/src/share/vm/runtime/
> atomic.inline.hpp",
> line 96: Error: Could not find a match for static Atomic::cmpxchg(signed
> char, volatile signed char*, signed char).
> 1 Error(s) detected.
>
> Needs this patch:
>
> diff -r 68853ef19be9 src/share/vm/runtime/atomic.inline.hpp
> --- a/src/share/vm/runtime/atomic.inline.hpp
> +++ b/src/share/vm/runtime/atomic.inline.hpp
> @@ -92,7 +92,7 @@
>
>   #ifndef VM_HAS_SPECIALIZED_CMPXCHG_BYTE
>   // See comment in atomic.cpp how to override.
> -inline jbyte Atomic::cmpxchg(jbyte exchange_value, volatile jbyte
> *dest, jbyte comparand)
> +inline jbyte Atomic::cmpxchg(jbyte exchange_value, volatile jbyte
> *dest, jbyte comparand, cmpxchg_memory_order order)
>   {
>     return cmpxchg_general(exchange_value, dest, comparand);
>   }
>
> David
> -----
>
> > Just need another reviewer to chime in - given you and Martin are both
> > contributors. Or are you the main contributor with Martin being a reviewer?
> >
> > Thanks,
> > David
> >
> > PS. It's my night now so I'll be signing off and will pick this up in
> > the morning.
> >
> >> This change follows the discussion started from these mails.
> >> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-
> April/018960.html
> >>
> >> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-
> April/019148.html
> >>
> >> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-
> May/019320.html
> >>
> >>
> >> Description:
> >> This change provides relaxed compare-and-exchange by introducing
> >> relaxed memory order. As described in atomic_linux_ppc.inline.hpp,
> >> the current implementation of cmpxchg is fence_cmpxchg_acquire.
> >> This implementation is useful for general purposes because twice calls of
> >> sync before and after cmpxchg will provide strict consistency.
> >> However, they sometimes cause overheads because sync instructions are
> >> very expensive in the current POWER chip design.
> >>
> >> We confirmed this change improves performance of copy_to_survivor
> >> in the parallel GC. However, we will need more investigation of GC
> >> by more experts. So, We would like to request a review of the change
> >> of cmpxchg first (as Martin requested).
> >> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-
> April/019188.html
> >>
> >>
> >> Summary of source code changes:
> >>
> >> * src/share/vm/runtime/atomic.hpp
> >>      - Defines enum memory_order and adds a parameter to cmpxchg.
> >>
> >> * src/share/vm/runtime/atomic.cpp
> >> * src/os_cpu/bsd_x86/vm/atomic_bsd_x86.inline.hpp
> >> * src/os_cpu/bsd_zero/vm/atomic_bsd_zero.inline.hpp
> >> * src/os_cpu/linux_aarch64/vm/atomic_linux_aarch64.inline.hpp
> >> * src/os_cpu/linux_sparc/vm/atomic_linux_sparc.inline.hpp
> >> * src/os_cpu/linux_x86/vm/atomic_linux_x86.inline.hpp
> >> * src/os_cpu/linux_zero/vm/atomic_linux_zero.inline.hpp
> >> * src/os_cpu/solaris_sparc/vm/atomic_solaris_sparc.inline.hpp
> >> * src/os_cpu/solaris_x86/vm/atomic_solaris_x86.inline.hpp
> >> * src/os_cpu/windows_x86/vm/atomic_windows_x86.inline.hpp
> >>      - Added a parameter for each cmpxchg function to follow
> >>         the change of atomic.hpp. Their implementations are not changed.
> >>
> >> * src/os_cpu/aix_ppc/vm/atomic_aix_ppc.inline.hpp
> >> * src/os_cpu/linux_ppc/vm/atomic_linux_ppc.inline.hpp
> >>      - Added a parameter for each cmpxchg function to follow
> >>         the change of atomic.hpp. In addition, implementations
> >>         are changed corresponding to the specified memory_order.
> >>
> >> Regards,
> >> Hiroshi
> >> -----------------------
> >> Hiroshi Horii, Ph.D.
> >> IBM Research - Tokyo
> >>
>