Try making CHPL_ATOMICS=cstdlib the default again

[ronawho]

We tried making cstdlib the default for gcc in #4443 but reverted in #4456 because of performance regressions. We tried making cstdlib the default for clang in #4480 but reverted in #4536 because of correctness issues (broken system headers)

That was ~2 years ago, so we should revisit this and see if things have changed.

Happily, it also looks like both Intel and Cray Compiler have added support for C11 atomics!

• Intel for sure has support in version 18.0 and possibly in 17.0 -- https://software.intel.com/en-us/articles/c11-support-in-intel-c-compiler
• Cray added support cce 8.7 -- https://pubs.cray.com/content/S-5212/8.7/cray-compiling-environment-release-overview-and-installation-guide/cray-compiling-environment-87-software-enhancements

We did some recent performance testing (mid July 2018) with gcc 8.1 for intrinsics vs. cstdlib and still saw some performance regressions there.

edit current status (March 2019):

We've made good progress with making cstdlib atomics the default:

• default to cstdlib for gcc (perf regressions resolved in Convert module testAndSet locks to use acquire/release orders #11851, made default in Default to cstdlib atomics for gcc >= 5 #11963)
• default to cstdlib for clang (bad headers worked around in Work around a bug in some system <atomic> headers under clang #12040, made default in Default to cstdlib atomics for clang when supported #12042)
• default to cstdlib for llvm (llvm support was fixed in Fix support for cstdlib atomics under --llvm (round 2) #12100, made default in Default to cstdlib atomics for --llvm #12079)
• default to cstdlib for cce (investigated in Investigate issues with cstdlib atomics under cce #11953, tracking in Default to cstdlib atomics for cce #12051, made default in Switch from supporting cce-classic to cce-clang #15495)
• default to cstdlib for intel (waiting for gen compiler, investigated in Spike: Investigate cstdlib atomic build failures under intel #11954, tracking in Default to cstdlib atomics for intel #12050)

Under x86, cstdlib atomics reads are significantly faster. This gist has additional details, but on a 24-core machine, using cstdlib results in a 20x serial speedup, and an 800x parallel speedup.

[dmk42]

CCE 8.6.5 is still our default for the module build, but after the 1.18 release, we should explore bumping that up to 8.7 to get the standard atomics.

[LouisJenkinsCS]

I'm pleased to see this!

For N-Body, what do you think is the bottleneck there, just as a guess. It doesn't contain a spin lock, does it?

[ronawho]

Here's a smaller reproducer for the performance differences for array-view creation (which I believe is the source of overhead for MiniMD and Nbody).

It just does a slice of a 1D array 25 million times:

use Time;

config const n = 100,
             numTrials = 25_000_000;

proc main() {
  var A : [1..n] int;

  var t : Timer; t.start();
  for 1..numTrials {
    ref s = A[1..n/2];
    if s.size > A.size then halt();
  }
  t.stop();

  writeln(t.elapsed());
}

config	time
intrinsics	1.9s
cstdlib	3.0s

[LouisJenkinsCS]

Could it be the creation of an array of atomic Booleans in LocalRADCache?

chapel/modules/internal/DefaultRectangular.chpl

Line 885 in 59eb20c

var RADLocks: [targetLocDom] atomicbool; // only accessed locally

chapel/modules/internal/DefaultRectangular.chpl

Line 967 in 72bc37a

var RADLocks: [targetLocDom] chpl__processorAtomicType(bool); // only accessed locally

But that'd be weird since its only used in distributions. Can't find any other atomics in the Chapel array implementation.

[ronawho]

Array and array views are split across several files -- I'm still digging into this, but the RadLocks are not created/used for local (default rectangular) arrays

[ronawho]

I've identified the performance difference to be from locking in the base domain and distribution classes. The locking shouldn't be needed for array-views so we should be able to eliminate it. I created #11619 to optimize out the locking (edit that's not true, the locking is needed after the slice is returned, just not as we're creating the slice.)

I also created #11620 to track adding a more comprehensive atomic test suite so we can more easily see which operations will be slower/fast under cstdlib