Skip to content

Commit

Permalink
Add new multithreaded concurrency configuration (#5015)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
qkoziol authored Dec 19, 2024
1 parent f9d60b5 commit 331193f
Show file tree
Hide file tree
Showing 94 changed files with 4,385 additions and 1,252 deletions.
7 changes: 3 additions & 4 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ IndentCaseLabels: true
IndentGotoLabels: false
#llvm11: IndentExternBlock: AfterExternBlock
#llvm11: InsertTrailingCommas: None
MacroBlockBegin: "^BEGIN_FUNC"
MacroBlockEnd: "^END_FUNC"
MacroBlockBegin: "^H5_BEFORE_USER_CB*|^H5E_PAUSE_ERRORS"
MacroBlockEnd: "^H5_AFTER_USER_CB*|^H5E_RESUME_ERRORS"
ObjCBlockIndentWidth: 4
#llvm11: ObjCBreakBeforeNestedBlockParam: true
ReflowComments: true
Expand All @@ -81,9 +81,8 @@ StatementMacros:
- FUNC_LEAVE_NOAPI_NAMECHECK_ONLY
- FUNC_LEAVE_NOAPI_VOID_NAMECHECK_ONLY
- FUNC_LEAVE_NOAPI_NOFS
- H5E_BEGIN_TRY
- H5E_END_TRY
- H5E_PRINTF
- H5E_THROW
- H5_BEGIN_TAG
- H5_END_TAG
- H5_GCC_DIAG_OFF
Expand Down
20 changes: 20 additions & 0 deletions .github/workflows/autotools.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,31 +44,51 @@ jobs:
with:
build_mode: "production"

call-debug-concurrent-autotools:
name: "Autotools Debug Concurrency Workflows"
uses: ./.github/workflows/main-auto.yml
with:
concurrent: enable
thread_safety: disable
build_mode: "debug"

call-release-concurrent-autotools:
name: "Autotools Release Concurrency Workflows"
uses: ./.github/workflows/main-auto.yml
with:
concurrent: enable
thread_safety: disable
build_mode: "production"

call-debug-thread-autotools:
name: "Autotools Debug Thread-Safety Workflows"
uses: ./.github/workflows/main-auto.yml
with:
concurrent: disable
thread_safety: enable
build_mode: "debug"

call-release-thread-autotools:
name: "Autotools Release Thread-Safety Workflows"
uses: ./.github/workflows/main-auto.yml
with:
concurrent: disable
thread_safety: enable
build_mode: "production"

call-debug-autotools:
name: "Autotools Debug Workflows"
uses: ./.github/workflows/main-auto.yml
with:
concurrent: disable
thread_safety: disable
build_mode: "debug"

call-release-autotools:
name: "Autotools Release Workflows"
uses: ./.github/workflows/main-auto.yml
with:
concurrent: disable
thread_safety: disable
build_mode: "production"

Expand Down
20 changes: 20 additions & 0 deletions .github/workflows/cmake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,31 +28,51 @@ jobs:
name: "CMake Special Workflows"
uses: ./.github/workflows/main-cmake-spc.yml

call-debug-concurrent-cmake:
name: "CMake Debug Concurrency Workflows"
uses: ./.github/workflows/main-cmake.yml
with:
concurrent: "CC"
thread_safety: ""
build_mode: "Debug"

call-release-concurrent-cmake:
name: "CMake Release Concurrency Workflows"
uses: ./.github/workflows/main-cmake.yml
with:
concurrent: "CC"
thread_safety: ""
build_mode: "Release"

call-debug-thread-cmake:
name: "CMake Debug Thread-Safety Workflows"
uses: ./.github/workflows/main-cmake.yml
with:
concurrent: ""
thread_safety: "TS"
build_mode: "Debug"

call-release-thread-cmake:
name: "CMake Release Thread-Safety Workflows"
uses: ./.github/workflows/main-cmake.yml
with:
concurrent: ""
thread_safety: "TS"
build_mode: "Release"

call-debug-cmake:
name: "CMake Debug Workflows"
uses: ./.github/workflows/main-cmake.yml
with:
concurrent: ""
thread_safety: ""
build_mode: "Debug"

call-release-cmake:
name: "CMake Release Workflows"
uses: ./.github/workflows/main-cmake.yml
with:
concurrent: ""
thread_safety: ""
build_mode: "Release"

Expand Down
31 changes: 28 additions & 3 deletions .github/workflows/main-auto.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ on:
description: "thread-safety enable/disable"
required: true
type: string
concurrent:
description: "concurrency enable/disable"
required: true
type: string
build_mode:
description: "release vs. debug build"
required: true
Expand All @@ -22,7 +26,7 @@ jobs:
# Linux (Ubuntu) w/ gcc + Autotools
#
Autotools_build_and_test:
name: "GCC-${{ inputs.build_mode }}-TS=${{ inputs.thread_safety }}d"
name: "GCC-${{ inputs.build_mode }}-TS=${{ inputs.thread_safety }}d-CC=${{ inputs.concurrent }}d"
# Don't run the action if the commit message says to skip CI
if: "!contains(github.event.head_commit.message, 'skip-ci')"

Expand Down Expand Up @@ -60,6 +64,7 @@ jobs:
--enable-shared \
--disable-parallel \
--${{ inputs.thread_safety }}-threadsafe \
--${{ inputs.concurrent }}-concurrency \
--enable-cxx \
--enable-fortran \
--enable-java \
Expand All @@ -68,7 +73,7 @@ jobs:
--enable-ros3-vfd \
--with-szlib=yes
shell: bash
if: ${{ inputs.thread_safety == 'disable' }}
if: ${{ inputs.thread_safety == 'disable' && inputs.concurrent == 'disable'}}

- name: Autotools Configure (Thread-Safe)
run: |
Expand All @@ -79,14 +84,34 @@ jobs:
--enable-build-mode=${{ inputs.build_mode }} \
--enable-shared \
--${{ inputs.thread_safety }}-threadsafe \
--${{ inputs.concurrent }}-concurrency \
--disable-hl \
--disable-parallel \
--enable-mirror-vfd \
--enable-direct-vfd \
--enable-ros3-vfd \
--with-szlib=yes
shell: bash
if: ${{ inputs.thread_safety == 'enable' }}
if: ${{ inputs.thread_safety == 'enable' && inputs.concurrent == 'disable' }}

- name: Autotools Configure (Concurrency)
run: |
sh ./autogen.sh
mkdir "${{ runner.workspace }}/build"
cd "${{ runner.workspace }}/build"
$GITHUB_WORKSPACE/configure \
--enable-build-mode=${{ inputs.build_mode }} \
--enable-shared \
--${{ inputs.thread_safety }}-threadsafe \
--${{ inputs.concurrent }}-concurrency \
--disable-hl \
--disable-parallel \
--enable-mirror-vfd \
--enable-direct-vfd \
--enable-ros3-vfd \
--with-szlib=yes
shell: bash
if: ${{ inputs.thread_safety == 'disable' && inputs.concurrent == 'enable' }}

- name: Autotools Build
run: make -j3
Expand Down
53 changes: 40 additions & 13 deletions .github/workflows/main-cmake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ on:
description: "TS or empty"
required: true
type: string
concurrent:
description: "CC or empty"
required: true
type: string
build_mode:
description: "release vs. debug build"
required: true
Expand Down Expand Up @@ -103,7 +107,7 @@ jobs:
run_tests: true

# Sets the job's name from the properties
name: "${{ matrix.name }}-${{ inputs.build_mode }}-${{ inputs.thread_safety }}"
name: "${{ matrix.name }}-${{ inputs.build_mode }}-${{ inputs.thread_safety }}-${{ inputs.concurrent }}"

# Don't run the action if the commit message says to skip CI
if: "!contains(github.event.head_commit.message, 'skip-ci')"
Expand Down Expand Up @@ -186,7 +190,7 @@ jobs:
-DHDF5_PACK_MACOSX_DMG:BOOL=OFF \
$GITHUB_WORKSPACE
shell: bash
if: ${{ inputs.thread_safety != 'TS' }}
if: ${{ inputs.thread_safety != 'TS' && inputs.concurrent != 'CC'}}

- name: CMake Configure (Thread-Safe)
run: |
Expand All @@ -199,6 +203,7 @@ jobs:
-DBUILD_STATIC_LIBS:BOOL=${{ (matrix.os != 'windows-latest') }} \
-DHDF5_ENABLE_ALL_WARNINGS:BOOL=ON \
-DHDF5_ENABLE_THREADSAFE:BOOL=ON \
-DHDF5_ENABLE_CONCURRENCY:BOOL=OFF \
-DHDF5_ENABLE_PARALLEL:BOOL=${{ matrix.parallel }} \
-DHDF5_BUILD_CPP_LIB:BOOL=OFF \
-DHDF5_BUILD_FORTRAN:BOOL=OFF \
Expand All @@ -214,7 +219,35 @@ jobs:
-DHDF5_PACK_MACOSX_DMG:BOOL=OFF \
$GITHUB_WORKSPACE
shell: bash
if: ${{ inputs.thread_safety == 'TS' }}
if: ${{ inputs.thread_safety == 'TS' && inputs.concurrent != 'CC'}}

- name: CMake Configure (Concurrency)
run: |
mkdir "${{ runner.workspace }}/build"
cd "${{ runner.workspace }}/build"
cmake -C $GITHUB_WORKSPACE/config/cmake/cacheinit.cmake \
${{ matrix.generator }} \
-DCMAKE_BUILD_TYPE=${{ inputs.build_mode }} \
-DBUILD_SHARED_LIBS=ON \
-DBUILD_STATIC_LIBS=${{ (matrix.os != 'windows-latest') }} \
-DHDF5_ENABLE_ALL_WARNINGS=ON \
-DHDF5_ENABLE_THREADSAFE:BOOL=OFF \
-DHDF5_ENABLE_CONCURRENCY:BOOL=ON \
-DHDF5_ENABLE_PARALLEL:BOOL=${{ matrix.parallel }} \
-DHDF5_BUILD_CPP_LIB:BOOL=OFF \
-DHDF5_BUILD_FORTRAN:BOOL=OFF \
-DHDF5_BUILD_JAVA:BOOL=OFF \
-DHDF5_BUILD_HL_LIB:BOOL=OFF \
-DHDF5_BUILD_DOC=OFF \
-DLIBAEC_USE_LOCALCONTENT=${{ matrix.localaec }} \
-DZLIB_USE_LOCALCONTENT=${{ matrix.localzlib }} \
-DHDF5_ENABLE_MIRROR_VFD:BOOL=${{ matrix.mirror_vfd }} \
-DHDF5_ENABLE_DIRECT_VFD:BOOL=${{ matrix.direct_vfd }} \
-DHDF5_ENABLE_ROS3_VFD:BOOL=${{ matrix.ros3_vfd }} \
-DHDF5_PACK_EXAMPLES:BOOL=ON \
$GITHUB_WORKSPACE
shell: bash
if: ${{ inputs.thread_safety != 'TS' && inputs.concurrent == 'CC'}}

# BUILD
- name: CMake Build
Expand All @@ -225,13 +258,7 @@ jobs:
- name: CMake Run Tests
run: ctest . --parallel 2 -C ${{ inputs.build_mode }} -V
working-directory: ${{ runner.workspace }}/build
if: ${{ matrix.run_tests && (inputs.thread_safety != 'TS') }}

# THREAD-SAFE
- name: CMake Run Thread-Safe Tests
run: ctest . --parallel 2 -C ${{ inputs.build_mode }} -V -R ttsafe
working-directory: ${{ runner.workspace }}/build
if: ${{ matrix.run_tests && (inputs.thread_safety == 'TS') }}
if: ${{ matrix.run_tests }}

- name: CMake Run Package
run: cpack -C ${{ inputs.build_mode }} -V
Expand All @@ -253,20 +280,20 @@ jobs:
name: zip-vs2022_cl-${{ inputs.build_mode }}-binary
path: ${{ runner.workspace }}/build/HDF5-*-win64.zip
if-no-files-found: error # 'warn' or 'ignore' are also available, defaults to `warn`
if: ${{ (matrix.os == 'windows-latest') && (inputs.thread_safety != 'TS') && ( inputs.build_mode != 'Debug') }}
if: ${{ (matrix.os == 'windows-latest') && (inputs.thread_safety != 'TS') && (inputs.concurrent != 'CC') && ( inputs.build_mode != 'Debug') }}

- name: Save published binary (linux)
uses: actions/upload-artifact@v4
with:
name: tgz-ubuntu-2404_gcc-${{ inputs.build_mode }}-binary
path: ${{ runner.workspace }}/build/HDF5-*-Linux.tar.gz
if-no-files-found: error # 'warn' or 'ignore' are also available, defaults to `warn`
if: ${{ (matrix.os == 'ubuntu-latest') && (inputs.thread_safety != 'TS') && ( inputs.build_mode != 'Debug') }}
if: ${{ (matrix.os == 'ubuntu-latest') && (inputs.thread_safety != 'TS') && (inputs.concurrent != 'CC') && ( inputs.build_mode != 'Debug') }}

- name: Save published binary (Mac_latest)
uses: actions/upload-artifact@v4
with:
name: tgz-macos14_clang-${{ inputs.build_mode }}-binary
path: ${{ runner.workspace }}/build/HDF5-*-Darwin.tar.gz
if-no-files-found: error # 'warn' or 'ignore' are also available, defaults to `warn`
if: ${{ (matrix.os == 'macos-latest') && (inputs.thread_safety != 'TS') && ( inputs.build_mode != 'Debug') }}
if: ${{ (matrix.os == 'macos-latest') && (inputs.thread_safety != 'TS') && (inputs.concurrent != 'CC') && ( inputs.build_mode != 'Debug') }}
53 changes: 53 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,59 @@ if (HDF5_ENABLE_THREADSAFE)
set (H5_HAVE_THREADSAFE 1)
endif ()

#-----------------------------------------------------------------------------
# Option to use multi-threaded concurrency
#-----------------------------------------------------------------------------
option (HDF5_ENABLE_CONCURRENCY "Enable multi-threaded concurrency" OFF)
if (HDF5_ENABLE_CONCURRENCY)
# check for unsupported options
if (WIN32)
if (BUILD_STATIC_LIBS)
message (FATAL_ERROR " **** multi-threaded concurrency option not supported with static library **** ")
endif ()
endif ()
if (HDF_ENABLE_PARALLEL)
if (NOT ALLOW_UNSUPPORTED)
message (FATAL_ERROR " **** Parallel and multi-threaded concurrency options are not supported, override with ALLOW_UNSUPPORTED option **** ")
else ()
message (VERBOSE " **** Allowing unsupported parallel and multi-threaded concurrency options **** ")
endif ()
endif ()
if (HDF5_BUILD_FORTRAN)
if (NOT ALLOW_UNSUPPORTED)
message (FATAL_ERROR " **** Fortran and multi-threaded concurrency options are not supported, override with ALLOW_UNSUPPORTED option **** ")
else ()
message (VERBOSE " **** Allowing unsupported Fortran and multi-threaded concurrency options **** ")
endif ()
endif ()
if (HDF5_BUILD_CPP_LIB)
if (NOT ALLOW_UNSUPPORTED)
message (FATAL_ERROR " **** C++ and multi-threaded concurrency options are not supported, override with ALLOW_UNSUPPORTED option **** ")
else ()
message (VERBOSE " **** Allowing unsupported C++ and multi-threaded concurrency options **** ")
endif ()
endif ()
if (HDF5_BUILD_HL_LIB)
if (NOT ALLOW_UNSUPPORTED)
message (FATAL_ERROR " **** HL and multi-threaded concurrency options are not supported, override with ALLOW_UNSUPPORTED option **** ")
else ()
message (VERBOSE " **** Allowing unsupported HL and multi-threaded concurrency options **** ")
endif ()
endif ()

# Check for threading package
if (NOT Threads_FOUND)
message (FATAL_ERROR " **** multi-threaded concurrency option requires a threading package and none was found **** ")
endif ()

# Multi-threaded concurrency and threadsafe options are mutually exclusive
if (HDF5_ENABLE_THREADSAFE)
message (FATAL_ERROR " **** multi-threaded concurrency and threadsafe options are mutually exclusive **** ")
endif ()

set (H5_HAVE_CONCURRENCY 1)
endif ()

#-----------------------------------------------------------------------------
# Option to build the map API
#-----------------------------------------------------------------------------
Expand Down
Loading

0 comments on commit 331193f

Please sign in to comment.