Add new multithreaded concurrency configuration #5015

qkoziol · 2024-10-26T16:03:20Z

Added infrastructure support for multithreaded concurrency by adding an optional way to switch to using a non-recursive R/W lock for the global API lock. This is enabled with a new 'concurrency' configuration flag for the autotools & CMake builds, which is disabled by default.

When the 'concurrency' build option is chosen, the global API lock will use the R/W lock and all API calls currently will acquire a write lock, ensuring exclusive access by one thread. Over time, the API routines that are converted to support multithreaded concurrency will switch to acquiring a read lock instead.

Reentering the library from application callbacks is managed by the 'disable locking for this thread' (DLFTT) threadsafety protocol. This is internally handled within the H5_API_LOCK / H5_API_UNLOCK macros in H5private.h (as before), which invoke the 'dlftt' routines in H5TSint.c.

To support this change, the threadsafety configuration macros for the library have been updated:
- --enable-threadsafe now defines the H5_HAVE_THREADSAFE macro
- --enable-concurrency defines the H5_HAVE_CONCURRENCY macro
The new H5_HAVE_THREADSAFE_API macro is set if either H5_HAVE_THREADSAFE or H5_HAVE_CONCURRENCY is enabled.

New Github actions are added to include the concurrency configuration in the CI for the develop branch.

To support the new non-recursive R/W locking for API routines, some other changes are necessary:

Added macro wrappers around all callback invocations that could call an
application function, and therefore re-enter the library:
H5_BEFORE_USER_CB* / H5_AFTER_USER_CB*
Added H5_user_cb_prepare / H5_user_cb_restore routines that save the
state of the library when callback leaves the library. Includes error
stack and threadsafe reentry state currently.

There's also some small cleanups to various places in the library:

Moved the H5E_mpi_error_str / H5E_mpi_error_str_len globals to be local for
pushing MPI errors, so that multiple threads can't interfere with each
other.
Added H5TS_rwlock_trywrlock() routine to R/W lock interface.
Emulate R/W locks on MacOS because its implementation of
pthread_rwlock_wrlock() does not conform to the POSIX standard.
Don't acquire the global API lock in H5close, since it's acquired in H5_term_library, which is necessary because H5_term_library is invoked via other code paths that don't hold the global API lock.
Don't call H5Eget_auto2 API routine within H5_term_library.
Switched 'return NULL' in H5allocate_memory to HGOTO_DONE(NULL).
Switched H5Pget_file_space_strategy / H5Pset_file_space_strategy to use
internal routines instead of API routines.
Switched H5Oopen_by_addr & H5Ovisit1 to use internal routines instead of
API routines.
Fixed a few places in src/H5Odeprec.c where a major
error ID was passed as a minor ID.

Signed-off-by: Quincey Koziol <[email protected]>

…protocol

Also added more atomic types Signed-off-by: Quincey Koziol <[email protected]>

…rwlock

…tocol

Signed-off-by: Quincey Koziol <[email protected]>

And put it into a macro Signed-off-by: Quincey Koziol <[email protected]>

Signed-off-by: Quincey Koziol <[email protected]>

…rwlock

…tocol

Also switched to use H5_HAVE_THREADSAFE_API for indicating that either the --enable-threadsafe or --enable-concurrency options are enabled. Signed-off-by: Quincey Koziol <[email protected]>

Signed-off-by: Quincey Koziol <[email protected]>

@jhendersonHDF

Curtesy of @jhendersonHDF Signed-off-by: Quincey Koziol <[email protected]>

…rwlock

Signed-off-by: Quincey Koziol <[email protected]>

…tocol

Signed-off-by: Quincey Koziol <[email protected]>

qkoziol · 2024-10-30T22:49:46Z

src/H5.c

@@ -214,14 +215,14 @@ H5_init_library(void)
     */
    if (!H5_dont_atexit_g) {

-#if defined(H5_HAVE_THREADSAFE)
+#ifdef H5_HAVE_THREADSAFE_API


Example of macro name change. H5_HAVE_THREADSAFE_API if either H5_HAVE_THREADSAFE or H5_HAVE_CONCURRENCY is enabled

qkoziol · 2024-10-30T22:50:18Z

src/H5.c

@@ -321,7 +322,7 @@ H5_term_library(void)
    H5CX_push(&api_ctx);

    /* Check if we should display error output */
-    (void)H5Eget_auto2(H5E_DEFAULT, &func, NULL);


Remove API call within the library.

qkoziol · 2024-10-30T22:51:12Z

src/H5.c

@@ -332,8 +333,13 @@ H5_term_library(void)
        while (curr_atclose) {
            H5_atclose_node_t *tmp_atclose; /* Temporary pointer to 'atclose' node */

-            /* Invoke callback, providing context */
-            (*curr_atclose->func)(curr_atclose->ctx);
+            /* Prepare & restore library for user callback */


Example of macro wrappers around callback invocations that could call an application function, and therefore re-enter the library.

qkoziol · 2024-10-30T22:52:22Z

src/H5.c

@@ -1097,13 +1103,14 @@ H5allocate_memory(size_t size, bool clear)
    FUNC_ENTER_API_NOINIT

    if (0 == size)
-        return NULL;


Have to leave via FUNC_LEAVE, so the API lock can be dropped.

qkoziol · 2024-10-30T22:53:18Z

src/H5.c

@@ -1226,3 +1233,63 @@ H5is_library_terminating(bool *is_terminating /*out*/)

    FUNC_LEAVE_API_NOINIT(ret_value)
 } /* end H5is_library_terminating() */
+
+/*-------------------------------------------------------------------------
+ * Function:    H5_user_cb_prepare


Save the state of the library when callback leaves the library. Includes error stack and threadsafe reentry state currently

qkoziol · 2024-10-30T22:55:19Z

src/H5Dfill.c

Some error cleanups along with the callback wrappers.

qkoziol · 2024-10-30T22:57:20Z

src/H5Eint.c

@@ -363,6 +355,82 @@ H5E_term_package(void)
    FUNC_LEAVE_NOAPI(n)
 } /* end H5E_term_package() */

+/*-------------------------------------------------------------------------
+ * Function:    H5E_user_cb_prepare


Save and restore the default error reporting settings, to prevent an application from changing them in a callback.

qkoziol · 2024-10-30T22:58:32Z

src/H5Eint.c

@@ -1369,6 +1458,40 @@ H5E__get_auto(const H5E_stack_t *estack, H5E_auto_op_t *op, void **client_data)
    FUNC_LEAVE_NOAPI(SUCCEED)
 } /* end H5E__get_auto() */

+/*-------------------------------------------------------------------------
+ * Function:    H5E_get_default_auto_func


Used when terminating the library.

qkoziol · 2024-10-30T22:59:35Z

src/H5Eprivate.h

@@ -181,29 +186,58 @@
 /*
 * MPI error handling macros.
 */
-
-extern char H5E_mpi_error_str[MPI_MAX_ERROR_STRING];


Moved these from global to local, so that multiple threads won't stomp on the values.

qkoziol · 2024-10-30T23:02:39Z

src/H5FDint.c

Some style cleanups along with the callback warappers.

qkoziol · 2024-10-30T23:04:48Z

src/H5FO.c

Added missing H5private.h

qkoziol · 2024-10-30T23:06:35Z

src/H5Odeprec.c

            /* Deserialize VOL object token into object address */
-            if (H5VLnative_token_to_addr(obj_id, oinfo2->token, &oinfo.addr) < 0)


Don't call API routines within the library.

qkoziol · 2024-10-30T23:07:07Z

src/H5Odeprec.c

        /* This is a native-specific routine that requires serialization of the token */
-        if (H5VLnative_addr_to_token(loc_id, addr, &obj_token) < 0)


Don't call API routines within the library.

qkoziol · 2024-10-30T23:07:29Z

src/H5Odeprec.c

@@ -423,7 +441,7 @@ H5Oget_info1(hid_t loc_id, H5O_info1_t *oinfo /*out*/)

    /* Must use native VOL connector for this operation */
    if (!is_native_vol_obj)
-        HGOTO_ERROR(H5E_OHDR, H5E_VOL, FAIL,


Don't use major error codes as minor codes.

qkoziol · 2024-10-30T23:08:02Z

src/H5Pdeprec.c

@@ -543,9 +551,11 @@ H5Pset_file_space(hid_t plist_id, H5F_file_space_type_t strategy, hsize_t thresh
     *      the existing threshold is retained.
     */
    if (!in_strategy)
-        H5Pget_file_space(plist_id, &in_strategy, NULL);


Don't call API routines within the library.

qkoziol · 2024-10-30T23:10:04Z

src/H5Pfcpl.c

@@ -1165,32 +1165,21 @@ H5Pget_shared_mesg_phase_change(hid_t plist_id, unsigned *max_list /*out*/, unsi
 } /* end H5Pget_shared_mesg_phase_change() */

 /*-------------------------------------------------------------------------
- * Function:	H5Pset_file_space_strategy
+ * Function:	H5P__set_file_space_strategy


Made package routines for these calls, to support H5Pget/set_file_space changes.

qkoziol · 2024-10-30T23:11:13Z

src/H5SMcache.c

Added missing H5private.h

qkoziol · 2024-10-30T23:13:19Z

src/H5TSint.c

@@ -74,6 +77,12 @@ typedef struct H5TS_tinfo_node_t {
 /* Local Prototypes */
 /********************/
 static H5TS_tinfo_node_t *H5TS__tinfo_create(void);
+#ifdef H5_HAVE_CONCURRENCY


Routines to support the "disable locking for this thread" ("DLFTT") protocol when making user callbacks.

qkoziol · 2024-10-30T23:14:04Z

src/H5TSint.c

    if (H5_UNLIKELY(H5TS_mutex_init(&H5TS_api_info_p.api_mutex, H5TS_MUTEX_TYPE_RECURSIVE) < 0))
        HGOTO_DONE(FAIL);
    H5TS_api_info_p.lock_count = 0;
+#else /* H5_HAVE_CONCURRENCY */


Use non-recursive R/W lock for global API lock when concurrency configure option is enabled.

qkoziol · 2024-10-30T23:15:13Z

src/H5TSint.c

    if (*acquired) {
        for (unsigned u = 0; u < (lock_count - 1); u++)
            if (H5_UNLIKELY(H5TS_mutex_lock(&H5TS_api_info_p.api_mutex) < 0))
                HGOTO_DONE(FAIL);
        H5TS_api_info_p.lock_count += lock_count;
    }
+#else /* H5_HAVE_CONCURRENCY */


Implement the DLFTT protocol - don't reacquire global lock when locking is disabled for a thread.

qkoziol · 2024-10-30T23:16:50Z

src/H5TSprivate.h

+/*
+ * Emulated pthread rwlock for MacOS
+ *
+ * Can't use pthread rwlock on MacOS due to: "The results [of calling


qkoziol · 2024-10-30T23:17:41Z

src/H5TSprivate.h

@@ -304,6 +341,8 @@ H5_DLL herr_t H5TS_rwlock_init(H5TS_rwlock_t *lock);
 static inline herr_t H5TS_rwlock_rdlock(H5TS_rwlock_t *lock);
 static inline herr_t H5TS_rwlock_rdunlock(H5TS_rwlock_t *lock);
 static inline herr_t H5TS_rwlock_wrlock(H5TS_rwlock_t *lock);
+static inline herr_t H5TS_rwlock_trywrlock(H5TS_rwlock_t *lock, bool *acquired)


New non-recursive R/W lock routine

qkoziol · 2024-10-30T23:20:38Z

src/H5UC.c

Missing H5private.h

qkoziol · 2024-10-30T23:23:47Z

src/H5private.h

+    H5E_user_cb_state_t h5e_state; /* State for H5E package */
+} H5_user_cb_state_t;
+
+#define H5_BEFORE_USER_CB(err)                                                                               \


Macros to wrap user callbacks.

qkoziol · 2024-10-30T23:24:32Z

src/H5private.h

+/* Both the 'threadsafe' and 'concurrency' options provide threadsafely for
+ * API calls.
+ */
+#if defined(H5_HAVE_THREADSAFE) || defined(H5_HAVE_CONCURRENCY)


Set H5_HAVE_THREADSAFE_API if either of the macros that indicate threadsafe APIs are set.

qkoziol · 2024-10-30T23:24:56Z

src/H5private.h

-#else                 /* H5_HAVE_THREADSAFE */
+#else /* H5_HAVE_CONCURRENCY */
+/* Local variable for 'disable locking for this thread' (DLFTT) state */
+#define H5DLFTT_DECL unsigned dlftt = 0;


Updates API locking for the DLFTT protocol

qkoziol · 2024-10-30T23:26:19Z

test/hdfs.c

clang-format

qkoziol · 2024-10-30T23:28:25Z

test/ttsafe_develop.c

+    is_native = H5VL__is_native_connector_test(vol_id);
+    CHECK(is_native, FAIL, "H5VL__is_native_connector_test");
+
+    if (is_native) {


Pass-through VOL connectors increment the API counter by various amounts, so just skip this portion of the test.

qkoziol and others added 30 commits May 24, 2024 20:18

Add 'THREADS' options

74f57ec

Signed-off-by: Quincey Koziol <[email protected]>

Merge remote-tracking branch 'origin/develop' into threading_updates

ef0e23a

Remove H5TS_NO_THREAD_SAFETY_ANALYSIS macro

138f8a1

Signed-off-by: Quincey Koziol <[email protected]>

Merge remote-tracking branch 'origin/threading_updates' into locking_…

f2fe9d0

…protocol

Committing clang-format changes

450f978

Committing clang-format changes

275c8a9

Merge remote-tracking branch 'origin/develop' into threading_updates

a18aac4

Add FFS & "simple" R/W locks and fix thread barrier & pool

9ae5be7

Also added more atomic types Signed-off-by: Quincey Koziol <[email protected]>

Merge remote-tracking branch 'origin/threading_updates' into non_rec_…

9b303ad

…rwlock

Merge remote-tracking branch 'origin/non_rec_rwlock' into locking_pro…

c28c18b

…tocol

Committing clang-format changes

cba6c74

Committing clang-format changes

f761c3b

Fix spelling

fc22454

Signed-off-by: Quincey Koziol <[email protected]>

Split recursive R/W lock's unlock routine into _rdunlock & _wrunlock

295aa33

Signed-off-by: Quincey Koziol <[email protected]>

Committing clang-format changes

f26d0a9

Only set thread cancellation state w/pthreads

b07555f

And put it into a macro Signed-off-by: Quincey Koziol <[email protected]>

Committing clang-format changes

9232548

Fix subfiling configuratio w/CMake

d8e9243

Signed-off-by: Quincey Koziol <[email protected]>

Merge remote-tracking branch 'origin/threading_updates' into non_rec_…

f0ed1c5

…rwlock

Merge remote-tracking branch 'origin/non_rec_rwlock' into locking_pro…

da16787

…tocol

Add --enable-concurrency option for autotools and CMake

a6a9c7f

Also switched to use H5_HAVE_THREADSAFE_API for indicating that either the --enable-threadsafe or --enable-concurrency options are enabled. Signed-off-by: Quincey Koziol <[email protected]>

Make H5TS_ffs_rwlock_t atomic

f5e83a7

Signed-off-by: Quincey Koziol <[email protected]>

Committing clang-format changes

c0cf43b

Protect compilation

8c7e286

Signed-off-by: Quincey Koziol <[email protected]>

Fix missing variable declaration

a288273

Signed-off-by: Quincey Koziol <[email protected]>

Move check for MPI_Comm_split_type earlier

d5f7b9e

Curtesy of @jhendersonHDF Signed-off-by: Quincey Koziol <[email protected]>

Merge remote-tracking branch 'origin/threading_updates' into non_rec_…

9ee565c

…rwlock

Make H5TS_ffs_rwlock_t atomic

17b923e

Signed-off-by: Quincey Koziol <[email protected]>

Merge remote-tracking branch 'origin/non_rec_rwlock' into locking_pro…

3676b3d

…tocol

Fix warning for clang builds

e7d2e4e

Signed-off-by: Quincey Koziol <[email protected]>