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

Tue May 10 20:56:09 UTC 2016

On 11/05/2016 12:27 AM, Doerr, Martin wrote:
> Hello everybody,
>
> thanks for finding this issue. New webrev is here:
>
> http://cr.openjdk.java.net/~mdoerr/8155949_relaxed_cas/webrev.02/

Unfortunately my test run hit a crash on Solaris sparc:

# Problematic frame:
# V  [libjvm.so+0xcc35c4] 
markOopDesc*markOopDesc::displaced_mark_helper()const+0x64

I'm going to have to do some more testing to see if that is actually 
related to the change. I know it should not be, but given we CAS marks I 
have to wonder if there's some subtle bad interaction here. :(

David
-----

>
>
> 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
>> >>
>>
>