RFR (M) 8169061: Drop os::is_MP checks from Atomics

[David Holmes]

Hi Aleksey,

This looks good to me. I agree with assuming always MP for these atomic 
ops. We should also look at what other os::is_MP() checks should be 
deleted (different CR of course).

Small nit: atomic_solaris_x86.hpp - when you deleted text the 
indentation of some declarations changed eg:

jbyte _Atomic_cmpxchg_byte(jbyte exchange_value, volatile jbyte* dest,
                      jbyte compare_value);

should be

jbyte _Atomic_cmpxchg_byte(jbyte exchange_value, volatile jbyte* dest,
                            jbyte compare_value);

Running it through JPRT.

Thanks,
David

On 24/04/2017 10:53 PM, Aleksey Shipilev wrote:
> Hi,
>
> See https://bugs.openjdk.java.net/browse/JDK-8169061.
>
> Our current Atomic code looks like this:
>
> // Adding a lock prefix to an instruction on MP machine
> #define LOCK_IF_MP(mp) "cmp $0, " #mp "; je 1f; lock; 1: "
>
> inline jint Atomic::add (jint add_value, volatile jint* dest) {
>   jint addend = add_value;
>   int mp = os::is_MP();
>   __asm__ volatile ( LOCK_IF_MP(%3) "xaddl %0,(%2)"
>                     : "=r" (addend)
>                     : "0" (addend), "r" (dest), "r" (mp)
>                     : "cc", "memory");
>   return addend + add_value;
> }
>
> Notice the LOCK_IF_MP part, which means we have flag reads, additional branch,
> etc. on the hot path. I would like us to consider dropping runtime os::is_MP
> checks from Atomics. The history of the original change predates OpenJDK, but I
> think it was deemed a plausible optimization in mostly-uniprocessor world.
> Today, this penalizes multi-core platforms without a good reason.
>
> Proposed change for jdk10/hs:
>   http://cr.openjdk.java.net/~shade/8169061/webrev.01/
>
> It unconditionally does lock prefix on all atomics.
>
> Our interest for doing this is GC performance work. For example, in Shenandoah,
> native CASes are very frequently used for bitmap manipulation during marking,
> updating forwarding pointers for evacuating objects, updating references in the
> heap. We have measured around 1-2% GC time improvement with this change. I fully
> expect it to benefit G1 too, because it also does CASes on bitmaps.
>
> Testing: shenandoah tests, hotspot_runtime on Linux x86_64
>
> Putting this via the build-test system on other platforms would be appreciated!
>
> Thanks,
> -Aleksey
>