Added template specializaton for OFGlobal<OFBool>.

Added template specialization for OFGlobal<OFBool> that uses std::atomic (C++11) instead of a read-write lock and is lock free on some platforms, thus offering a performance advantage if many threads concurrently access the global flag (e.g. while generating UIDs). The specialization is enabled only if DCMTK is compiled with STL, and C++11 is available. Thanks to Jesper Alf Dam <[email protected]> for the report, the performance analysis and the original patch.
GerHobbelt · Nov 10, 2024 · 3e348c4 · 3e348c4
1 parent 74e5d54
commit 3e348c4
Show file tree

Hide file tree

Showing 10 changed files with 172 additions and 86 deletions.
diff --git a/CMake/DCMTKConfig.cmake.in b/CMake/DCMTKConfig.cmake.in
@@ -48,6 +48,7 @@ set(DCMTK_ENABLE_STL_SYSTEM_ERROR @DCMTK_ENABLE_STL_SYSTEM_ERROR@)
 set(DCMTK_ENABLE_STL_TUPLE @DCMTK_ENABLE_STL_TUPLE@)
 set(DCMTK_ENABLE_STL_TYPE_TRAITS @DCMTK_ENABLE_STL_TYPE_TRAITS@)
 set(DCMTK_ENABLE_STL_VECTOR @DCMTK_ENABLE_STL_VECTOR@)
+set(DCMTK_ENABLE_STL_ATOMIC @DCMTK_ENABLE_STL_ATOMIC@)
 
 set(DCMTK_FORCE_FPIC_ON_UNIX @DCMTK_FORCE_FPIC_ON_UNIX@)
 

diff --git a/CMake/DCMTKConfig.old_cmake.in b/CMake/DCMTKConfig.old_cmake.in
diff --git a/CMake/GenerateDCMTKConfigure.cmake b/CMake/GenerateDCMTKConfigure.cmake
@@ -340,6 +340,7 @@ endif()
   CHECK_INCLUDE_FILE_CXX("system_error" HAVE_SYSTEM_ERROR)
   CHECK_INCLUDE_FILE_CXX("tuple" HAVE_TUPLE)
   CHECK_INCLUDE_FILE_CXX("type_traits" HAVE_TYPE_TRAITS)
+  CHECK_INCLUDE_FILE_CXX("atomic" HAVE_ATOMIC)
 
 if(NOT APPLE)
   # poll on macOS is unreliable, it first did not exist, then was broken until
@@ -1333,6 +1334,7 @@ DCMTK_ENABLE_STL98_FEATURE("vector")
 DCMTK_ENABLE_STL11_FEATURE("type_traits")
 DCMTK_ENABLE_STL11_FEATURE("tuple")
 DCMTK_ENABLE_STL11_FEATURE("system_error")
+DCMTK_ENABLE_STL11_FEATURE("atomic")
 
 # if at least one modern C++ standard should be supported,
 # enforce setting of __cplusplus macro in VS 2017 and above

diff --git a/CMake/dcmtkMacros.cmake b/CMake/dcmtkMacros.cmake
@@ -199,39 +199,20 @@ function(DCMTK_CREATE_INSTALL_EXPORTS)
     # Only create fully-fledged CMake export files if we have the related commands
     include("${DCMTK_MACROS_DIR}/CheckCMakeCommandExists.cmake")
     include(CMakePackageConfigHelpers OPTIONAL)
-    CHECK_CMAKE_COMMAND_EXISTS("CONFIGURE_PACKAGE_CONFIG_FILE")
-    CHECK_CMAKE_COMMAND_EXISTS("WRITE_BASIC_PACKAGE_VERSION_FILE")
 
-    if(HAVE_CONFIGURE_PACKAGE_CONFIG_FILE AND HAVE_WRITE_BASIC_PACKAGE_VERSION_FILE)
+    # Create and configure CMake export files
+    include("${DCMTK_MACROS_DIR}/GenerateCMakeExports.cmake")
 
-      # Create and configure CMake export files
-      include("${DCMTK_MACROS_DIR}/GenerateCMakeExports.cmake")
+    # ${DCMTK_INSTALL_CONFIG} and ${DCMTK_CONFIG_VERSION} are
+    # defined within CMake/GenerateCMakeExports.cmake.
+    # Install DCMTKTargets.cmake to install tree
+    install(EXPORT DCMTKTargets FILE DCMTKTargets.cmake NAMESPACE DCMTK::
+            DESTINATION "${DCMTK_INSTALL_CMKDIR}" COMPONENT cmake)
 
-      # ${DCMTK_INSTALL_CONFIG} and ${DCMTK_CONFIG_VERSION} are
-      # defined within CMake/GenerateCMakeExports.cmake.
-      # Install DCMTKTargets.cmake to install tree
-      install(EXPORT DCMTKTargets FILE DCMTKTargets.cmake NAMESPACE DCMTK::
-              DESTINATION "${DCMTK_INSTALL_CMKDIR}" COMPONENT cmake)
+    # Install DCMTKConfig.cmake and DCMTKConfigVersion.cmake
+    install(FILES "${DCMTK_INSTALL_CONFIG}" "${DCMTK_CONFIG_VERSION}"
+            DESTINATION "${DCMTK_INSTALL_CMKDIR}" COMPONENT cmake)
 
-      # Install DCMTKConfig.cmake and DCMTKConfigVersion.cmake
-      install(FILES "${DCMTK_INSTALL_CONFIG}" "${DCMTK_CONFIG_VERSION}"
-              DESTINATION "${DCMTK_INSTALL_CMKDIR}" COMPONENT cmake)
-
-    else()
-
-      # Warning that we use old "configure_file" command
-      message(STATUS "Warning: Using old configure_file() mechanism to produce DCMTKConfig.cmake")
-
-      # Actually configure file
-      configure_file("${DCMTK_MACROS_DIR}/DCMTKConfig.old_cmake.in"
-                     "${DCMTK_BINARY_DIR}/DCMTKConfig.cmake" @ONLY)
-
-      # Install DCMTKConfig.cmake and DCMTKConfigVersion.cmake
-      install(FILES "${DCMTK_BINARY_DIR}/DCMTKConfig.cmake" "${DCMTK_BINARY_DIR}/DCMTKConfigVersion.cmake"
-              DESTINATION "${DCMTK_INSTALL_CMKDIR}"
-              COMPONENT cmake)
-
-    endif()
 endfunction()
 
 set(DCMTK_MACROS_DIR "${CMAKE_CURRENT_LIST_DIR}" CACHE INTERNAL "")
diff --git a/CMake/dcmtkPrepare.cmake b/CMake/dcmtkPrepare.cmake
@@ -188,6 +188,7 @@ DCMTK_INFERABLE_OPTION(DCMTK_ENABLE_STL_STRING "Enable use of STL string.")
 DCMTK_INFERABLE_OPTION(DCMTK_ENABLE_STL_TYPE_TRAITS "Enable use of STL type traits.")
 DCMTK_INFERABLE_OPTION(DCMTK_ENABLE_STL_TUPLE "Enable use of STL tuple.")
 DCMTK_INFERABLE_OPTION(DCMTK_ENABLE_STL_SYSTEM_ERROR "Enable use of STL system_error.")
+DCMTK_INFERABLE_OPTION(DCMTK_ENABLE_STL_ATOMIC "Enable use of STL atomic.")
 
 # On Windows, the built-in dictionary is default, on Unix the external one.
 # It is not possible to use both, built-in plus external default dictionary.

diff --git a/CMake/osconfig.h.in b/CMake/osconfig.h.in
@@ -864,6 +864,9 @@ DCMTK was configured to use C++20 features, but your compiler does not or was no
 /* Define if we are supposed to use STL's system_error */
 #cmakedefine HAVE_STL_SYSTEM_ERROR @HAVE_STL_SYSTEM_ERROR@
 
+/* Define if we are supposed to use STL's atomic */
+#cmakedefine HAVE_STL_ATOMIC @HAVE_STL_ATOMIC@
+
 /* Define if the input iterator category is supported */
 #cmakedefine HAVE_CONTIGUOUS_ITERATOR_CATEGORY @HAVE_CONTIGUOUS_ITERATOR_CATEGORY@
 

diff --git a/config/configure b/config/configure
@@ -755,6 +755,7 @@ enable_stl_string
 enable_stl_type_traits
 enable_stl_tuple
 enable_stl_system_error
+enable_stl_atomic
 '
       ac_precious_vars='build_alias
 host_alias
@@ -1432,6 +1433,10 @@ Optional Features:
                           use C++ STL system_error
   --disable-stl-system_error
                           do not use C++ STL system_error
+  --enable-stl-atomic
+                          use C++ STL atomic
+  --disable-stl-atomic
+                          do not use C++ STL atomic
 
 Optional Packages:
   --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
@@ -7346,6 +7351,18 @@ fi
 
 done
 
+for ac_header in atomic
+do :
+  ac_fn_cxx_check_header_mongrel "$LINENO" "atomic" "ac_cv_header_atomic" "$ac_includes_default"
+if test "x$ac_cv_header_atomic" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_ATOMIC 1
+_ACEOF
+
+fi
+
+done
+
 for ac_header in arpa/inet.h
 do :
   ac_fn_cxx_check_header_mongrel "$LINENO" "arpa/inet.h" "ac_cv_header_arpa_inet_h" "$ac_includes_default"
@@ -10401,7 +10418,7 @@ if ${ac_cv_check_std_namespace+:} false; then :
 else
   ac_link_o='${CXX-g++} -o conftest $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.o $LIBS 1>&5'
 cat > conftest.$ac_ext <<EOF
-#line 10404 "configure"
+#line 10421 "configure"
 #include "confdefs.h"
 
 #include <iostream>
@@ -13349,6 +13366,38 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_enable_stl_system_error" >&5
 $as_echo "$ac_enable_stl_system_error" >&6; }
 
+ac_enable_stl_atomic="auto"
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to enable STL atomic support" >&5
+$as_echo_n "checking whether to enable STL atomic support... " >&6; }
+# Check whether --enable-stl-atomic was given.
+if test "${enable_stl_atomic+set}" = set; then :
+  enableval=$enable_stl_atomic;  case "$enableval" in
+  yes)
+   ac_enable_stl_atomic="yes"
+   ;;
+
+  *)
+    ac_enable_stl_atomic="no"
+    ;;
+  esac
+
+fi
+
+if test "$ac_enable_stl_atomic" = "auto"; then
+  ac_enable_stl_atomic="$ac_enable_stl"
+fi
+if test "$ac_enable_stl_atomic" = "yes"; then
+  if test $ac_cv_header_atomic = yes ; then
+
+$as_echo "#define HAVE_STL_ATOMIC 1" >>confdefs.h
+
+  else
+    ac_enable_stl_atomic="unsupported -> no"
+  fi
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_enable_stl_atomic" >&5
+$as_echo "$ac_enable_stl_atomic" >&6; }
+
 
 
 

diff --git a/config/configure.in b/config/configure.in
@@ -474,6 +474,7 @@ AC_HEADER_SYS_WAIT
 AC_HEADER_DIRENT
 AC_CHECK_TCP_H
 AC_CHECK_HEADERS(alloca.h)
+AC_CHECK_HEADERS(atomic)
 AC_CHECK_HEADERS(arpa/inet.h)
 AC_CHECK_HEADERS(fcntl.h)
 AC_CHECK_HEADERS(fnmatch.h)
@@ -1712,6 +1713,35 @@ if test "$ac_enable_stl_system_error" = "yes"; then
 fi
 AC_MSG_RESULT($ac_enable_stl_system_error)
 
+ac_enable_stl_atomic="auto"
+AC_MSG_CHECKING(whether to enable STL atomic support)
+AC_ARG_ENABLE(stl-atomic,
+[  --enable-stl-atomic
+                          use C++ STL atomic
+  --disable-stl-atomic
+                          do not use C++ STL atomic],
+[ case "$enableval" in
+  yes)
+   ac_enable_stl_atomic="yes"
+   ;;
+
+  *)
+    ac_enable_stl_atomic="no"
+    ;;
+  esac ]
+)
+if test "$ac_enable_stl_atomic" = "auto"; then
+  ac_enable_stl_atomic="$ac_enable_stl"
+fi
+if test "$ac_enable_stl_atomic" = "yes"; then
+  if test $ac_cv_header_atomic = yes ; then
+    AC_DEFINE(HAVE_STL_ATOMIC, 1, [Define if STL's atomic should be used.])
+  else
+    ac_enable_stl_atomic="unsupported -> no"
+  fi
+fi
+AC_MSG_RESULT($ac_enable_stl_atomic)
+
 
 dnl -------------------------------------------------------
 dnl Test for some additional functions and keywords

diff --git a/config/include/dcmtk/config/osconfig.h.in b/config/include/dcmtk/config/osconfig.h.in
@@ -100,6 +100,9 @@
 /* Define to 1 if you have the `atoll' function. */
 #undef HAVE_ATOLL
 
+/* Define to 1 if you have the <atomic> header file. */
+#undef HAVE_ATOMIC
+
 /* Define if __attribute__((aligned)) is available. */
 #undef HAVE_ATTRIBUTE_ALIGNED
 
@@ -600,6 +603,9 @@ typedef unsigned short ushort;
 /* Define if STL's algorithm should be used. */
 #undef HAVE_STL_ALGORITHM
 
+/* Define if STL's atomic should be used. */
+#undef HAVE_STL_ATOMIC
+
 /* Define if STL's list should be used. */
 #undef HAVE_STL_LIST
 

diff --git a/ofstd/include/dcmtk/ofstd/ofglobal.h b/ofstd/include/dcmtk/ofstd/ofglobal.h
@@ -20,16 +20,18 @@
  *           Read/Write Lock for multi-thread applications.
  *           class T must have copy constructor and assignment operator.
  *
- *
  */
 
-
 #ifndef OFGLOBAL_H
 #define OFGLOBAL_H
 
 #include "dcmtk/config/osconfig.h"
 #include "dcmtk/ofstd/ofthread.h"  /* for class OFBool */
 
+#if defined(HAVE_STL_ATOMIC) && defined(WITH_THREADS)
+#include <atomic>
+#endif
+
 /** Template class which allows to declare global objects that are
  *  protected by a Read/Write Lock if used in multi-thread applications.
  *  Must be compiled with -DWITH_THREADS for multi-thread-operation.
@@ -125,4 +127,69 @@ template <class T> class OFGlobal
 
 };
 
+#if defined(HAVE_STL_ATOMIC) && defined(WITH_THREADS)
+/** template specialization for OFGlobal<OFBool> that uses a std::atomic flag
+ *  without read-write lock and is lock-free (and thus much more efficient) on many platforms.
+ */
+template <> class OFGlobal<OFBool>
+{
+public:
+
+  /** constructor.
+   *  @param arg value to which this object is initialised
+   */
+  OFGlobal(const OFBool &arg)
+  : val(arg)
+  {
+  }
+
+  /** destructor.
+   */
+  virtual ~OFGlobal() { }
+
+  /** assigns new value to this object.
+   *  @param arg new value
+   */
+  void set(const OFBool& arg)
+  {
+    val = arg;
+  }
+
+  /** gets the value of this object.
+   *  @param arg return value is assigned to this parameter.
+   */
+  void xget(bool& arg)
+  {
+    arg = val;
+  }
+
+
+  /** returns the value of this object. The result
+   *  is returned by value, not by reference.
+   *  @return value of this object.
+   */
+  OFBool get()
+  {
+    OFBool result(val);
+    return result;
+  }
+
+private:
+
+  /** value of this object
+   */
+  std::atomic<OFBool> val;
+
+  /** unimplemented private default constructor */
+  OFGlobal();
+
+  /** unimplemented private copy constructor */
+  OFGlobal(const OFGlobal<OFBool>& arg);
+
+  /** unimplemented private assignment operator */
+  const OFGlobal<OFBool>& operator=(const OFGlobal<OFBool>& arg);
+
+};
+#endif
+
 #endif