Skip to content
This repository was archived by the owner on Sep 30, 2022. It is now read-only.

Commit 1c3965b

Browse files
committed
Fix performance regression caused by enabling opal thread support
This commit adds opal_using_threads() protection around the atomic operation in OBJ_RETAIN/OBJ_RELEASE. This resolves the performance issues seen when running psm with MPI_THREAD_SINGLE. To avoid issues with header dependencies opal_using_threads() has been moved to a new header (thread_usage.h). The OPAL_THREAD_ADD* and OPAL_THREAD_CMPSET* macros have also been relocated to this header. This was cherry-picked off a fix applied to v1.8 and not master. See open-mpi/ompi#1902. (cherry picked from commit ce91307) Signed-off-by: Nathan Hjelm <[email protected]>
1 parent c303db9 commit 1c3965b

File tree

4 files changed

+223
-162
lines changed

4 files changed

+223
-162
lines changed

opal/class/opal_object.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
12
/*
23
* Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana
34
* University Research and Technology
@@ -10,6 +11,10 @@
1011
* Copyright (c) 2004-2005 The Regents of the University of California.
1112
* All rights reserved.
1213
* Copyright (c) 2007-2014 Cisco Systems, Inc. All rights reserved.
14+
* Copyright (c) 2014 Research Organization for Information Science
15+
* and Technology (RIST). All rights reserved.
16+
* Copyright (c) 2015 Los Alamos National Security, LLC. All rights
17+
* reserved.
1318
* $COPYRIGHT$
1419
*
1520
* Additional copyrights may follow
@@ -120,7 +125,7 @@
120125
#include <assert.h>
121126
#include <stdlib.h>
122127

123-
#include "opal/sys/atomic.h"
128+
#include "opal/threads/thread_usage.h"
124129

125130
BEGIN_C_DECLS
126131

@@ -503,7 +508,7 @@ static inline opal_object_t *opal_obj_new(opal_class_t * cls)
503508
static inline int opal_obj_update(opal_object_t *object, int inc) __opal_attribute_always_inline__;
504509
static inline int opal_obj_update(opal_object_t *object, int inc)
505510
{
506-
return opal_atomic_add_32(&(object->obj_reference_count), inc);
511+
return OPAL_THREAD_ADD32(&object->obj_reference_count, inc);
507512
}
508513

509514
END_C_DECLS

opal/threads/Makefile.am

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ headers += \
2929
threads/mutex_unix.h \
3030
threads/threads.h \
3131
threads/tsd.h \
32-
threads/wait_sync.h
32+
threads/wait_sync.h \
33+
threads/thread_usage.h
3334

3435
lib@OPAL_LIB_PREFIX@open_pal_la_SOURCES += \
3536
threads/condition.c \

opal/threads/mutex.h

Lines changed: 1 addition & 159 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828

2929
#include "opal_config.h"
3030

31-
#include "opal/sys/atomic.h"
31+
#include "opal/threads/thread_usage.h"
3232

3333
BEGIN_C_DECLS
3434

@@ -40,13 +40,6 @@ BEGIN_C_DECLS
4040
* Functions for locking of critical sections.
4141
*/
4242

43-
#if OMPI_ENABLE_THREAD_MULTIPLE
44-
/*
45-
* declaring this here so that CL does not complain
46-
*/
47-
OPAL_DECLSPEC extern bool opal_uses_threads;
48-
#endif
49-
5043
/**
5144
* Opaque mutex object
5245
*/
@@ -108,73 +101,6 @@ END_C_DECLS
108101

109102
BEGIN_C_DECLS
110103

111-
/**
112-
* Check and see if the process is using multiple threads.
113-
*
114-
* @retval true If the process may have more than one thread.
115-
* @retval false If the process only has a single thread.
116-
*
117-
* The value that this function returns is influenced by:
118-
*
119-
* - how MPI_INIT or MPI_INIT_THREAD was invoked,
120-
* - what the final MPI thread level was determined to be,
121-
* - whether the OMPI or MPI libraries are multi-threaded (Jan 2003:
122-
* they're not),
123-
* - whether configure determined if we have thread support or not
124-
*
125-
* MPI_INIT and MPI_INIT_THREAD (specifically, back-end OMPI startup
126-
* functions) invoke opal_set_using_threads() to influence the value of
127-
* this function, depending on their situation. Some examples:
128-
*
129-
* - if configure determined that we do not have threads, then this
130-
* value will always be false.
131-
*
132-
* - if MPI_INIT is invoked, and the ompi libraries are [still]
133-
* single-threaded, this value will be false.
134-
*
135-
* - if MPI_INIT_THREAD is invoked with MPI_THREAD_MULTIPLE, we have
136-
* thread support, and the final thread level is determined to be
137-
* MPI_THREAD_MULTIPLE, this value will be true.
138-
*
139-
* - if the process is a single-threaded OMPI executable (e.g., mpicc),
140-
* this value will be false.
141-
*
142-
* Hence, this function will return false if there is guaranteed to
143-
* only be one thread in the process. If there is even the
144-
* possibility that we may have multiple threads, true will be
145-
* returned.
146-
*/
147-
#if OMPI_ENABLE_THREAD_MULTIPLE
148-
#define opal_using_threads() opal_uses_threads
149-
#else
150-
#define opal_using_threads() 0
151-
#endif
152-
153-
/**
154-
* Set whether the process is using multiple threads or not.
155-
*
156-
* @param have Boolean indicating whether the process is using
157-
* multiple threads or not.
158-
*
159-
* @retval opal_using_threads The new return value from
160-
* opal_using_threads().
161-
*
162-
* This function is used to influence the return value of
163-
* opal_using_threads(). If configure detected that we have thread
164-
* support, the return value of future invocations of
165-
* opal_using_threads() will be the parameter's value. If configure
166-
* detected that we have no thread support, then the retuen from
167-
* opal_using_threads() will always be false.
168-
*/
169-
static inline bool opal_set_using_threads(bool have)
170-
{
171-
#if OMPI_ENABLE_THREAD_MULTIPLE
172-
opal_uses_threads = have;
173-
#endif
174-
return opal_using_threads();
175-
}
176-
177-
178104
/**
179105
* Lock a mutex if opal_using_threads() says that multiple threads may
180106
* be active in the process.
@@ -261,90 +187,6 @@ static inline bool opal_set_using_threads(bool have)
261187
} \
262188
} while (0)
263189

264-
/**
265-
* Use an atomic operation for increment/decrement if opal_using_threads()
266-
* indicates that threads are in use by the application or library.
267-
*/
268-
269-
static inline int32_t
270-
OPAL_THREAD_ADD32(volatile int32_t *addr, int delta)
271-
{
272-
int32_t ret;
273-
274-
if (opal_using_threads()) {
275-
ret = opal_atomic_add_32(addr, delta);
276-
} else {
277-
ret = (*addr += delta);
278-
}
279-
280-
return ret;
281-
}
282-
283-
#if OPAL_HAVE_ATOMIC_MATH_64
284-
static inline int64_t
285-
OPAL_THREAD_ADD64(volatile int64_t *addr, int delta)
286-
{
287-
int64_t ret;
288-
289-
if (opal_using_threads()) {
290-
ret = opal_atomic_add_64(addr, delta);
291-
} else {
292-
ret = (*addr += delta);
293-
}
294-
295-
return ret;
296-
}
297-
#endif
298-
299-
static inline size_t
300-
OPAL_THREAD_ADD_SIZE_T(volatile size_t *addr, int delta)
301-
{
302-
size_t ret;
303-
304-
if (opal_using_threads()) {
305-
ret = opal_atomic_add_size_t(addr, delta);
306-
} else {
307-
ret = (*addr += delta);
308-
}
309-
310-
return ret;
311-
}
312-
313-
/* BWB: FIX ME: remove if possible */
314-
#define OPAL_CMPSET(x, y, z) ((*(x) == (y)) ? ((*(x) = (z)), 1) : 0)
315-
316-
#if OPAL_HAVE_ATOMIC_CMPSET_32
317-
#define OPAL_ATOMIC_CMPSET_32(x, y, z) \
318-
(opal_using_threads() ? opal_atomic_cmpset_32(x, y, z) : OPAL_CMPSET(x, y, z))
319-
#endif
320-
#if OPAL_HAVE_ATOMIC_CMPSET_64
321-
#define OPAL_ATOMIC_CMPSET_64(x, y, z) \
322-
(opal_using_threads() ? opal_atomic_cmpset_64(x, y, z) : OPAL_CMPSET(x, y, z))
323-
#endif
324-
#if OPAL_HAVE_ATOMIC_CMPSET_32 || OPAL_HAVE_ATOMIC_CMPSET_64
325-
#define OPAL_ATOMIC_CMPSET(x, y, z) \
326-
(opal_using_threads() ? opal_atomic_cmpset(x, y, z) : OPAL_CMPSET(x, y, z))
327-
#endif
328-
#if OPAL_HAVE_ATOMIC_CMPSET_32 || OPAL_HAVE_ATOMIC_CMPSET_64
329-
#define OPAL_ATOMIC_CMPSET_PTR(x, y, z) \
330-
(opal_using_threads() ? opal_atomic_cmpset_ptr(x, y, z) : OPAL_CMPSET(x, y, z))
331-
#endif
332-
333-
334-
static inline void *opal_thread_swap_ptr (volatile void *ptr, void *newvalue)
335-
{
336-
if (opal_using_threads ()) {
337-
return opal_atomic_swap_ptr (ptr, newvalue);
338-
}
339-
340-
void *old = ((void **) ptr)[0];
341-
((void **) ptr)[0] = newvalue;
342-
343-
return old;
344-
}
345-
346-
#define OPAL_ATOMIC_SWAP_PTR(x, y) opal_thread_swap_ptr (x, y)
347-
348190
END_C_DECLS
349191

350192
#endif /* OPAL_MUTEX_H */

0 commit comments

Comments
 (0)