From 2dcbacbd13d5ae95fa4b4c732a8261aec67be5c5 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Wed, 28 Aug 2019 05:55:19 +0200 Subject: [PATCH] Use MONOTONIC clock to set Garbage Collector pthread_cond_timedwait. (#733) * Added option for monotonic clock --- CMakeLists.txt | 23 ++++++++++++++++++++++ configure-data.tcl | 18 ++++++++++++----- scripts/haiUtil.cmake | 46 +++++++++++++++++++++++++++++++++++++++++++ srtcore/api.cpp | 16 +++++++++++++-- 4 files changed, 96 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ec91d0853..e48e493e3 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -109,6 +109,16 @@ option(USE_GNUTLS "DEPRECATED. Use USE_ENCLIB=openssl|gnutls|mbedtls instead" OF option(ENABLE_CXX_DEPS "Extra library dependencies in srt.pc for the CXX libraries useful with C language" ON) option(USE_STATIC_LIBSTDCXX "Should use static rather than shared libstdc++" OFF) option(ENABLE_INET_PTON "Set to OFF to prevent usage of inet_pton when building against modern SDKs while still requiring compatibility with older Windows versions, such as Windows XP, Windows Server 2003 etc." ON) + +# ENABLE_MONOTONIC_CLOCK: enforces the use of clock_gettime to get the current +# time, instead of gettimeofday. This function allows to force a monotonic +# clock, which is independent on the currently set time in the system. The CV, +# for which the *_timedwait() functions are used with so obtained current time, +# must be appropriately configured. The consequence of enabling this option, +# however, may be portability issues around the clock_gettime() function, which +# is not available on every SDK, or extra -lrt option is sometimes required +# (this requirement will be autodetected). +option(ENABLE_MONOTONIC_CLOCK "Enforced clock_gettime with monotonic clock on GC CV /temporary fix for #729/" OFF) option(USE_OPENSSL_PC "Use pkg-config to find OpenSSL libraries" ON) option(USE_BUSY_WAITING "Enable more accurate sending times at a cost of potentially higher CPU load" OFF) option(USE_GNUSTL "Get c++ library/headers from the gnustl.pc" OFF) @@ -204,6 +214,10 @@ if (DEFINED HAVE_INET_PTON) add_definitions(-DHAVE_INET_PTON=1) endif() +if (ENABLE_MONOTONIC_CLOCK) + add_definitions(-DENABLE_MONOTONIC_CLOCK=1) +endif() + if (ENABLE_ENCRYPTION) if ("${USE_ENCLIB}" STREQUAL "gnutls") set (SSL_REQUIRED_MODULES "gnutls nettle") @@ -332,6 +346,15 @@ if (USE_STATIC_LIBSTDCXX) endif() endif() +# We need clock_gettime, but on some systems this is only provided +# by librt. Check if librt is required. +if (ENABLE_MONOTONIC_CLOCK AND LINUX) + # "requires" - exits on FATAL_ERROR when clock_gettime not available + test_requires_clock_gettime(NEED_CLOCK_GETTIME) + set (WITH_EXTRALIBS "${WITH_EXTRALIBS} ${NEED_CLOCK_GETTIME}") +endif() + + # This options is necessary on some systems; on a cross-ARM compiler it # has been detected, for example, that -lrt is necessary for some applications # because clock_gettime is needed by some functions and it is alternatively diff --git a/configure-data.tcl b/configure-data.tcl index 5850c7bfc..69ba51e96 100644 --- a/configure-data.tcl +++ b/configure-data.tcl @@ -29,7 +29,6 @@ set internal_options { with-compiler-type= "compiler type: gcc(default), cc, others simply add ++ for C++" with-srt-name= "Override srt library name" with-haicrypt-name= "Override haicrypt library name (if compiled separately)" - enable-debug "turn on debug+nonoptimized build mode (if =2, debug+optimized)" } # Options that refer directly to variables used in CMakeLists.txt @@ -38,14 +37,20 @@ set cmake_options { enable-encryption "Should encryption features be enabled (default: ON)" enable-unittests "Should the unit tests be enabled (default: OFF)" enable-c++11 "Should the c++11 parts (srt-live-transmit) be enabled (default: ON)" - enable-c-deps "Extra library dependencies in srt.pc for C language (default: OFF)" + enable-apps "Should the Support Applications be Built? (default: ON)" + enable-testing "Should developer testing applications be built (default: OFF)" + enable-c++-deps "Extra library dependencies in srt.pc for C language (default: OFF)" enable-heavy-logging "Should heavy debug logging be enabled (default: OFF)" enable-logging "Should logging be enabled (default: ON)" + enable-debug=<0,1,2> "Enable debug mode (0=disabled, 1=debug, 2=rel-with-debug)" + enable-haicrypt-logging "Should logging in haicrypt be enabled (default: OFF)" + enable-inet-pton "Set to OFF to prevent usage of inet_pton when building against modern SDKs (default: ON)" + enable-monotonic-clock "Enforced clock_gettime with monotonic clock on GC CV /temporary fix for #729/ (default: OFF)" enable-profile "Should instrument the code for profiling. Ignored for non-GNU compiler. (default: OFF)" - enable-separate-haicrypt "Should haicrypt be built as a separate library file (default: OFF)" + enable-relative-libpath "Should applications contain relative library paths, like ../lib (default: OFF)" enable-shared "Should libsrt be built as a shared library (default: ON)" enable-static "Should libsrt be built as a static library (default: ON)" - enable-suflip "Shuld suflip tool be built (default: OFF)" + enable-suflip "Should suflip tool be built (default: OFF)" enable-getnameinfo "In-logs sockaddr-to-string should do rev-dns (default: OFF)" enable-thread-check "Enable #include that implements THREAD_* macros" openssl-crypto-library= "Path to a library." @@ -54,8 +59,11 @@ set cmake_options { pkg-config-executable= "pkg-config executable" pthread-include-dir= "Path to a file." pthread-library= "Path to a library." - use-gnutls "DEPRECATED. Use --use-enclib=openssl|gnutls|mbedtls" + use-busy-waiting "Enable more accurate sending times at a cost of potentially higher CPU load (default: OFF)" + use-gnustl "Get c++ library/headers from the gnustl.pc" use-enclib "Encryption library to be used: openssl(default), gnutls, mbedtls" + use-gnutls "DEPRECATED. Use USE_ENCLIB=openssl|gnutls|mbedtls instead" + use-openssl-pc "Use pkg-config to find OpenSSL libraries (default: ON)" use-static-libstdc++ "Should use static rather than shared libstdc++ (default: OFF)" } diff --git a/scripts/haiUtil.cmake b/scripts/haiUtil.cmake index 98562482b..bd4dd1e0c 100644 --- a/scripts/haiUtil.cmake +++ b/scripts/haiUtil.cmake @@ -7,6 +7,7 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. # +include(CheckCXXSourceCompiles) # Useful for combinging paths @@ -241,3 +242,48 @@ function (getVarsWith _prefix _varResult) string (REGEX MATCHALL "(^|;)${_prefix}[A-Za-z0-9_]*" _matchedVars "${_vars}") set (${_varResult} ${_matchedVars} PARENT_SCOPE) endfunction() + +function (check_testcode_compiles testcode libraries _successful) + set (save_required_libraries ${CMAKE_REQUIRED_LIBRARIES}) + set (CMAKE_REQUIRED_LIBRARIES "${CMAKE_REQUIRED_LIBRARIES} ${libraries}") + + check_cxx_source_compiles("${testcode}" ${_successful}) + set (${_successful} ${${_successful}} PARENT_SCOPE) + set (CMAKE_REQUIRED_LIBRARIES ${save_required_libraries}) +endfunction() + +function (test_requires_clock_gettime _result) + # This function tests if clock_gettime can be used + # - at all + # - with or without librt + + # Result will be: + # rt (if librt required) + # "" (if no extra libraries required) + # -- killed by FATAL_ERROR if clock_gettime is not available + + set (code " + #include + int main() { + timespec res\; + int result = clock_gettime(CLOCK_MONOTONIC, &res)\; + return result == 0\; + } + ") + + check_testcode_compiles(${code} "" HAVE_CLOCK_GETTIME_IN) + if (HAVE_CLOCK_GETTIME_IN) + message(STATUS "Checked clock_gettime(): no extra libs needed") + set (${_result} "" PARENT_SCOPE) + return() + endif() + + check_testcode_compiles(${code} "rt" HAVE_CLOCK_GETTIME_LIBRT) + if (HAVE_CLOCK_GETTIME_LIBRT) + message(STATUS "Checked clock_gettime(): requires -lrt") + set (${_result} "-lrt" PARENT_SCOPE) + return() + endif() + + message(FATAL_ERROR "clock_gettime() is not available on this system") +endfunction() diff --git a/srtcore/api.cpp b/srtcore/api.cpp index ca613e9f8..07cc1a4b5 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -212,8 +212,14 @@ int CUDTUnited::startup() m_bClosing = false; pthread_mutex_init(&m_GCStopLock, NULL); +#if ENABLE_MONOTONIC_CLOCK + pthread_condattr_t CondAttribs; + pthread_condattr_init(&CondAttribs); + pthread_condattr_setclock(&CondAttribs, CLOCK_MONOTONIC); + pthread_cond_init(&m_GCStopCond, &CondAttribs); +#else pthread_cond_init(&m_GCStopCond, NULL); - +#endif { ThreadName tn("SRT:GC"); pthread_create(&m_GCThread, NULL, garbageCollect, this); @@ -1881,13 +1887,19 @@ void* CUDTUnited::garbageCollect(void* p) // self->checkTLSValue(); //#endif - timeval now; timespec timeout; +#if ENABLE_MONOTONIC_CLOCK + clock_gettime(CLOCK_MONOTONIC, &timeout); + timeout.tv_sec++; + HLOGC(mglog.Debug, log << "GC: sleep until " << FormatTime(uint64_t(timeout.tv_nsec)/1000 + 1000000*(timeout.tv_sec))); +#else + timeval now; gettimeofday(&now, 0); timeout.tv_sec = now.tv_sec + 1; timeout.tv_nsec = now.tv_usec * 1000; HLOGC(mglog.Debug, log << "GC: sleep until " << FormatTime(uint64_t(now.tv_usec) + 1000000*(timeout.tv_sec))); +#endif pthread_cond_timedwait( &self->m_GCStopCond, &self->m_GCStopLock, &timeout); }