Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new multithreaded concurrency configuration #5015

Open
wants to merge 610 commits into
base: develop
Choose a base branch
from

Conversation

qkoziol
Copy link
Contributor

@qkoziol qkoziol commented Oct 26, 2024

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.

qkoziol and others added 30 commits May 28, 2024 19:10
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]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
github-actions bot and others added 12 commits November 18, 2024 21:35
Also clean up some warnings in new threadsafe-related macros

Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
Signed-off-by: Quincey Koziol <[email protected]>
@qkoziol
Copy link
Contributor Author

qkoziol commented Nov 21, 2024

@derobins @fortnern @jhendersonHDF @mattjala - Can I get some reviews on this, I've got a sequence of branches queued after this one.

src/H5TSprivate.h Outdated Show resolved Hide resolved
Copy link
Contributor

@mattjala mattjala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good besides a few missing blocks

@qkoziol
Copy link
Contributor Author

qkoziol commented Nov 26, 2024

Looks good besides a few missing blocks

Thanks for the review. There's no need for additional blocks, but I'll take out the definition for H5TS_init().

mattjala
mattjala previously approved these changes Dec 2, 2024
src/H5TSint.c Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Build CMake, Autotools Component - C Library Core C library issues (usually in the src directory) Component - Testing Code in test or testpar directories, GitHub workflows Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub Type - Improvement Improvements that don't add a new feature or functionality Type - New Feature Add a new API call, functionality, or tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants