Skip to content

Commit

Permalink
Merge pull request #17419 from ronawho/serialize-ucx-polling
Browse files Browse the repository at this point in the history
Serialize gasnet polling calls for UCX too

[reviewed by @gbtitus]

Similar to #14912, but for UCX. Concurrent polling can cause contention
and performance regressions so serialize the calls. This improves
performance for some of the same benchmarks we saw in #14912 but the
difference is not as dramatic as it was for IBV.

Some performance results on 48-core Cascade Lake nodes with HDR IB:

| config | RA-on        | Indexgather    |
| ------ | -----------: | -------------: |
| before | 0.00245 GUPS | 1010 MB/s/node |
| after  | 0.00275 GUPS | 1120 MB/s/node |

And 128-core Rome nodes with HDR IB:

| config | RA-on        | Indexgather    |
| ------ | -----------: | -------------: |
| before | 0.00150 GUPS |  670 MB/s/node |
| after  | 0.00175 GUPS | 1060 MB/s/node |
ronawho authored Mar 17, 2021
2 parents 3f5ec1f + dcb5451 commit 51a10b4
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions runtime/src/comm/gasnet/comm-gasnet.c
Original file line number Diff line number Diff line change
@@ -729,10 +729,10 @@ static chpl_bool pollingRequired;
static atomic_bool pollingLock;

static inline void am_poll_try(void) {
// Serialize polling for IBV and Aries. Concurrent polling causes contention
// in these configurations. For other configurations that are AM-based
// (udp/amudp, mpi/ammpi) serializing can hurt performance.
#if defined(GASNET_CONDUIT_IBV) || defined(GASNET_CONDUIT_ARIES)
// Serialize polling for IBV, UCX, and Aries. Concurrent polling causes
// contention in these configurations. For other configurations that are
// AM-based (udp/amudp, mpi/ammpi) serializing can hurt performance.
#if defined(GASNET_CONDUIT_IBV) || defined(GASNET_CONDUIT_UCX) || defined(GASNET_CONDUIT_ARIES)
if (!atomic_load_explicit_bool(&pollingLock, memory_order_acquire) &&
!atomic_exchange_explicit_bool(&pollingLock, true, memory_order_acquire)) {
(void) gasnet_AMPoll();

0 comments on commit 51a10b4

Please sign in to comment.