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

[David Holmes]

One oversight in atomic_bsd_x86.hpp:

  118 inline void Atomic::inc_ptr(volatile intptr_t* dest) {
  119   __asm__ __volatile__ (  "lock addq $1,(%0)"
  120                         :
  121                         : "r" (dest), "r" (mp)

Forgot to delete "mp" reference.

Also copyright years need updating.

Thanks,
David

On 26/04/2017 11:17 AM, David Holmes wrote:
> 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
>>