* [PATCH 0/2] PR libstdc++/41861 Add full steady_clock support to condition_variable
@ 2019-07-15 16:48 Mike Crowe
2019-07-15 16:48 ` [PATCH 2/2] " Mike Crowe
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Mike Crowe @ 2019-07-15 16:48 UTC (permalink / raw)
To: libstdc++, gcc-patches; +Cc: Mike Crowe
The pthread_cond_clockwait function was recently added[1] to glibc,
and is due to be released in glibc 2.30. If this function is available
in the C library it can be used it to fix
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861 by supporting
std::chrono::steady_clock properly with std::condition_variable.
Mike Crowe (2):
Add user-defined clock to libstdc++ condition_variable tests
PR libstdc++/41861 Add full steady_clock support to condition_variable
libstdc++-v3/ChangeLog | 34 +++++++++++++++++++++++++++++-
libstdc++-v3/acinclude.m4 | 31 +++++++++++++++++++++++++++-
libstdc++-v3/config.h.in | 3 +++-
libstdc++-v3/configure | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
libstdc++-v3/configure.ac | 3 +++-
libstdc++-v3/include/std/condition_variable | 52 +++++++++++++++++++++++++++++++++++++++------
libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc | 29 ++++++++++++++++++++++---
libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc | 29 ++++++++++++++++++++++---
8 files changed, 252 insertions(+), 12 deletions(-)
base-commit: 7b0d7c79224b412ea5073593943ab1525cbbda73
--
git-series 0.9.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 2/2] PR libstdc++/41861 Add full steady_clock support to condition_variable 2019-07-15 16:48 [PATCH 0/2] PR libstdc++/41861 Add full steady_clock support to condition_variable Mike Crowe @ 2019-07-15 16:48 ` Mike Crowe 2019-09-04 13:39 ` Jonathan Wakely 2019-07-15 16:51 ` [PATCH 1/2] Add user-defined clock to libstdc++ condition_variable tests Mike Crowe 2019-08-19 16:27 ` [PATCH 0/2] PR libstdc++/41861 Add full steady_clock support to condition_variable Mike Crowe 2 siblings, 1 reply; 10+ messages in thread From: Mike Crowe @ 2019-07-15 16:48 UTC (permalink / raw) To: libstdc++, gcc-patches; +Cc: Mike Crowe The pthread_cond_clockwait function was recently added[1] to glibc, and is due to be released in glibc 2.30. If this function is available in the C library it can be used it to fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861 by supporting std::chrono::steady_clock properly with std::condition_variable. This means that code using std::condition_variable::wait_for or std::condition_variable::wait_until with std::chrono::steady_clock is no longer subject to timing out early or potentially waiting for much longer if the system clock is warped at an inopportune moment. If pthread_cond_clockwait is available then std::chrono::steady_clock is deemed to be the "best" clock available which means that it is used for the relative wait_for calls and absolute wait_until calls using user-defined clocks. Calls explicitly using std::chrono::system_clock continue to use CLOCK_REALTIME via __gthread_cond_timedwait. If pthread_cond_clockwait is not available then std::chrono::system_clock is deemed to be the "best" clock available which means that the previous suboptimal behaviour remains. [1] https://sourceware.org/git/?p=glibc.git;a=commit;h=afe4de7d283ebd88157126c5494ce1796194c16e libstdc++-v3/ * include/std/condition_variable: Add include of <bits/c++config.h> to make _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT available. (condition_variable): Split __clock_t into __system_clock_t, which is always system_clock and __best_clock_t, which is the clock that we wish to convert arbitrary other clocks to. If we know that pthread_cond_clockwait is available then it is best to convert clocks to steady_clock, otherwise it's best to use system_clock. (wait_until): If pthread_cond_clockwait is available, provide a steady_clock overload. Convert previous __clock_t overload to use __system_clock_t. Convert generic overload to convert passed clock to __best_clock_t. (wait_until_impl): Add steady_clock overload that calls pthread_cond_clockwait. Convert previous __clock_t overload to use __system_clock_t. (condition_variable_any): Use steady_clock for __clock_t if pthread_cond_clockwait is available. * acinclude.m4: Detect the availability of POSIX-proposed pthread_cond_clockwait function. * configure: Likewise. * configure.ac: Likewise. * config.h.in: Add _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT define to indicate presence of pthread_cond_clockwait function. --- libstdc++-v3/ChangeLog | 25 +++++++- libstdc++-v3/acinclude.m4 | 31 ++++++++- libstdc++-v3/config.h.in | 3 +- libstdc++-v3/configure | 83 ++++++++++++++++++++++- libstdc++-v3/configure.ac | 3 +- libstdc++-v3/include/std/condition_variable | 52 ++++++++++++-- 6 files changed, 191 insertions(+), 6 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index a52a704..597000b 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,30 @@ 2019-07-15 Mike Crowe <mac@mcrowe.com> + * include/std/condition_variable: Add include of <bits/c++config.h> + to make _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT available. + (condition_variable): Split __clock_t into __system_clock_t, which + is always system_clock and __best_clock_t, which is the clock that + we wish to convert arbitrary other clocks to. If we know that + pthread_cond_clockwait is available then it is best to convert + clocks to steady_clock, otherwise it's best to use + system_clock. (wait_until): If pthread_cond_clockwait is available, + provide a steady_clock overload. Convert previous __clock_t + overload to use __system_clock_t. Convert generic overload to + convert passed clock to __best_clock_t. (wait_until_impl): Add + steady_clock overload that calls pthread_cond_clockwait. Convert + previous __clock_t overload to use + __system_clock_t. (condition_variable_any): Use steady_clock for + __clock_t if pthread_cond_clockwait is available. + + * acinclude.m4: Detect the availability of POSIX-proposed + pthread_cond_clockwait function. + * configure: Likewise. + * configure.ac: Likewise. + * config.h.in: Add _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT define to + indicate presence of pthread_cond_clockwait function. + +2019-07-15 Mike Crowe <mac@mcrowe.com> + * testsuite/30_threads/condition_variable/members/2.cc (test01): Parameterise so that test can be run against an arbitrary clock. (main): Test using std::chrono::steady_clock and a user-defined diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4 index 24145fd..35979e7 100644 --- a/libstdc++-v3/acinclude.m4 +++ b/libstdc++-v3/acinclude.m4 @@ -4194,6 +4194,37 @@ AC_DEFUN([GLIBCXX_CHECK_PTHREADS_NUM_PROCESSORS_NP], [ ]) dnl +dnl Check whether pthread_cond_clockwait is available in <pthread.h> for std::condition_variable to use, +dnl and define _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT. +dnl +AC_DEFUN([GLIBCXX_CHECK_PTHREAD_COND_CLOCKWAIT], [ + + AC_LANG_SAVE + AC_LANG_CPLUSPLUS + ac_save_CXXFLAGS="$CXXFLAGS" + CXXFLAGS="$CXXFLAGS -fno-exceptions" + ac_save_LIBS="$LIBS" + LIBS="$LIBS -lpthread" + + AC_MSG_CHECKING([for pthread_cond_clockwait]) + AC_CACHE_VAL(glibcxx_cv_PTHREAD_COND_CLOCKWAIT, [ + GCC_TRY_COMPILE_OR_LINK( + [#include <pthread.h>], + [pthread_mutex_t mutex; pthread_cond_t cond; struct timespec ts; int n = pthread_cond_clockwait(&cond, &mutex, 0, &ts);], + [glibcxx_cv_PTHREAD_COND_CLOCKWAIT=yes], + [glibcxx_cv_PTHREAD_COND_CLOCKWAIT=no]) + ]) + if test $glibcxx_cv_PTHREAD_COND_CLOCKWAIT = yes; then + AC_DEFINE(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT, 1, [Define if pthread_cond_clockwait is available in <pthread.h>.]) + fi + AC_MSG_RESULT($glibcxx_cv_PTHREAD_COND_CLOCKWAIT) + + CXXFLAGS="$ac_save_CXXFLAGS" + LIBS="$ac_save_LIBS" + AC_LANG_RESTORE +]) + +dnl dnl Check whether sysctl is available in <pthread.h>, and define _GLIBCXX_USE_SYSCTL_HW_NCPU. dnl AC_DEFUN([GLIBCXX_CHECK_SYSCTL_HW_NCPU], [ diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in index 99286e6..3d13402 100644 --- a/libstdc++-v3/config.h.in +++ b/libstdc++-v3/config.h.in @@ -991,6 +991,9 @@ /* Define if pthreads_num_processors_np is available in <pthread.h>. */ #undef _GLIBCXX_USE_PTHREADS_NUM_PROCESSORS_NP +/* Define if pthread_cond_clockwait is available in <pthread.h>. */ +#undef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT + /* Define if POSIX read/write locks are available in <gthr.h>. */ #undef _GLIBCXX_USE_PTHREAD_RWLOCK_T diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure index ab46399..c0b98d8 100755 --- a/libstdc++-v3/configure +++ b/libstdc++-v3/configure @@ -21581,6 +21581,89 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu +# For pthread_cond_clockwait + + + + ac_ext=cpp +ac_cpp='$CXXCPP $CPPFLAGS' +ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_cxx_compiler_gnu + + ac_save_CXXFLAGS="$CXXFLAGS" + CXXFLAGS="$CXXFLAGS -fno-exceptions" + ac_save_LIBS="$LIBS" + LIBS="$LIBS -lpthread" + + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for pthread_cond_clockwait" >&5 +$as_echo_n "checking for pthread_cond_clockwait... " >&6; } + if test "${glibcxx_cv_PTHREAD_COND_CLOCKWAIT+set}" = set; then : + $as_echo_n "(cached) " >&6 +else + + if test x$gcc_no_link = xyes; then + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +#include <pthread.h> +int +main () +{ +pthread_mutex_t mutex; pthread_cond_t cond; struct timespec ts; int n = pthread_cond_clockwait(&cond, &mutex, 0, &ts); + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_compile "$LINENO"; then : + glibcxx_cv_PTHREAD_COND_CLOCKWAIT=yes +else + glibcxx_cv_PTHREAD_COND_CLOCKWAIT=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +else + if test x$gcc_no_link = xyes; then + as_fn_error "Link tests are not allowed after GCC_NO_EXECUTABLES." "$LINENO" 5 +fi +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +#include <pthread.h> +int +main () +{ +pthread_mutex_t mutex; pthread_cond_t cond; struct timespec ts; int n = pthread_cond_clockwait(&cond, &mutex, 0, &ts); + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_link "$LINENO"; then : + glibcxx_cv_PTHREAD_COND_CLOCKWAIT=yes +else + glibcxx_cv_PTHREAD_COND_CLOCKWAIT=no +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +fi + +fi + + if test $glibcxx_cv_PTHREAD_COND_CLOCKWAIT = yes; then + +$as_echo "#define _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT 1" >>confdefs.h + + fi + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $glibcxx_cv_PTHREAD_COND_CLOCKWAIT" >&5 +$as_echo "$glibcxx_cv_PTHREAD_COND_CLOCKWAIT" >&6; } + + CXXFLAGS="$ac_save_CXXFLAGS" + LIBS="$ac_save_LIBS" + ac_ext=c +ac_cpp='$CPP $CPPFLAGS' +ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_c_compiler_gnu + + + ac_fn_c_check_header_mongrel "$LINENO" "locale.h" "ac_cv_header_locale_h" "$ac_includes_default" if test "x$ac_cv_header_locale_h" = xyes; then : diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac index 80d8202..ad4ae0c 100644 --- a/libstdc++-v3/configure.ac +++ b/libstdc++-v3/configure.ac @@ -220,6 +220,9 @@ GLIBCXX_ENABLE_LIBSTDCXX_TIME # Check for tmpnam which is obsolescent in POSIX.1-2008 GLIBCXX_CHECK_TMPNAM +# For pthread_cond_clockwait +GLIBCXX_CHECK_PTHREAD_COND_CLOCKWAIT + AC_LC_MESSAGES # For hardware_concurrency diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable index a83996a..0e1bdf7 100644 --- a/libstdc++-v3/include/std/condition_variable +++ b/libstdc++-v3/include/std/condition_variable @@ -35,6 +35,7 @@ # include <bits/c++0x_warning.h> #else +#include <bits/c++config.h> #include <chrono> #include <bits/std_mutex.h> #include <bits/unique_lock.h> @@ -65,8 +66,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// condition_variable class condition_variable { - typedef chrono::system_clock __clock_t; typedef chrono::steady_clock __steady_clock_t; +#if defined(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT) + typedef chrono::steady_clock __best_clock_t; +#else + typedef chrono::system_clock __best_clock_t; +#endif + typedef chrono::system_clock __system_clock_t; typedef __gthread_cond_t __native_type; #ifdef __GTHREAD_COND_INIT @@ -101,10 +107,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION wait(__lock); } +#if defined(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT) + template<typename _Duration> + cv_status + wait_until(unique_lock<mutex>& __lock, + const chrono::time_point<__steady_clock_t, _Duration>& __atime) + { return __wait_until_impl(__lock, __atime); } +#endif + template<typename _Duration> cv_status wait_until(unique_lock<mutex>& __lock, - const chrono::time_point<__clock_t, _Duration>& __atime) + const chrono::time_point<__system_clock_t, _Duration>& __atime) { return __wait_until_impl(__lock, __atime); } template<typename _Clock, typename _Duration> @@ -112,9 +126,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION wait_until(unique_lock<mutex>& __lock, const chrono::time_point<_Clock, _Duration>& __atime) { - // DR 887 - Sync unknown clock to known clock. const typename _Clock::time_point __c_entry = _Clock::now(); - const __clock_t::time_point __s_entry = __clock_t::now(); + const __best_clock_t::time_point __s_entry = __best_clock_t::now(); const auto __delta = __atime - __c_entry; const auto __s_atime = __s_entry + __delta; @@ -171,10 +184,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return &_M_cond; } private: +#if defined(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT) + template<typename _Dur> + cv_status + __wait_until_impl(unique_lock<mutex>& __lock, + const chrono::time_point<__steady_clock_t, _Dur>& __atime) + { + auto __s = chrono::time_point_cast<chrono::seconds>(__atime); + auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s); + + __gthread_time_t __ts = + { + static_cast<std::time_t>(__s.time_since_epoch().count()), + static_cast<long>(__ns.count()) + }; + + pthread_cond_clockwait(&_M_cond, __lock.mutex()->native_handle(), + CLOCK_MONOTONIC, + &__ts); + + return (__steady_clock_t::now() < __atime + ? cv_status::no_timeout : cv_status::timeout); + } +#endif template<typename _Dur> cv_status __wait_until_impl(unique_lock<mutex>& __lock, - const chrono::time_point<__clock_t, _Dur>& __atime) + const chrono::time_point<__system_clock_t, _Dur>& __atime) { auto __s = chrono::time_point_cast<chrono::seconds>(__atime); auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s); @@ -188,7 +224,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __gthread_cond_timedwait(&_M_cond, __lock.mutex()->native_handle(), &__ts); - return (__clock_t::now() < __atime + return (__system_clock_t::now() < __atime ? cv_status::no_timeout : cv_status::timeout); } }; @@ -208,7 +244,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Like above, but mutex is not required to have try_lock. class condition_variable_any { +#if defined(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT) + typedef chrono::steady_clock __clock_t; +#else typedef chrono::system_clock __clock_t; +#endif condition_variable _M_cond; shared_ptr<mutex> _M_mutex; -- git-series 0.9.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] PR libstdc++/41861 Add full steady_clock support to condition_variable 2019-07-15 16:48 ` [PATCH 2/2] " Mike Crowe @ 2019-09-04 13:39 ` Jonathan Wakely 2019-09-04 14:49 ` Mike Crowe 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Wakely @ 2019-09-04 13:39 UTC (permalink / raw) To: Mike Crowe; +Cc: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 3281 bytes --] On 15/07/19 17:47 +0100, Mike Crowe wrote: >The pthread_cond_clockwait function was recently added[1] to glibc, and is >due to be released in glibc 2.30. If this function is available in the C >library it can be used it to fix >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861 by supporting >std::chrono::steady_clock properly with std::condition_variable. > >This means that code using std::condition_variable::wait_for or >std::condition_variable::wait_until with std::chrono::steady_clock is no >longer subject to timing out early or potentially waiting for much longer >if the system clock is warped at an inopportune moment. > >If pthread_cond_clockwait is available then std::chrono::steady_clock is >deemed to be the "best" clock available which means that it is used for the >relative wait_for calls and absolute wait_until calls using user-defined >clocks. Calls explicitly using std::chrono::system_clock continue to use >CLOCK_REALTIME via __gthread_cond_timedwait. > >If pthread_cond_clockwait is not available then std::chrono::system_clock >is deemed to be the "best" clock available which means that the previous >suboptimal behaviour remains. > >[1] https://sourceware.org/git/?p=glibc.git;a=commit;h=afe4de7d283ebd88157126c5494ce1796194c16e > >libstdc++-v3/ > > * include/std/condition_variable: Add include of <bits/c++config.h> > to make _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT available. > (condition_variable): Split __clock_t into __system_clock_t, which > is always system_clock and __best_clock_t, which is the clock that > we wish to convert arbitrary other clocks to. If we know that > pthread_cond_clockwait is available then it is best to convert > clocks to steady_clock, otherwise it's best to use > system_clock. (wait_until): If pthread_cond_clockwait is available, > provide a steady_clock overload. Convert previous __clock_t > overload to use __system_clock_t. Convert generic overload to > convert passed clock to __best_clock_t. (wait_until_impl): Add > steady_clock overload that calls pthread_cond_clockwait. Convert > previous __clock_t overload to use > __system_clock_t. (condition_variable_any): Use steady_clock for > __clock_t if pthread_cond_clockwait is available. > > * acinclude.m4: Detect the availability of POSIX-proposed > pthread_cond_clockwait function. > * configure: Likewise. > * configure.ac: Likewise. > * config.h.in: Add _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT define to > indicate presence of pthread_cond_clockwait function. Thanks, Mike! This patch is much simpler than the previous versions. Thanks for perservering with POSIX and glibc to get the necessary functionality into libc. I've attached a slightly-modified patch, which changes the names of the clock typedefs in std::condition_variable. The names "steady_clock" and "system_clock" are already reserved names, so we can just use those (instead of the uglier __steady_clock_t forms). And we can just keep the original __clock_t name to mean the best clock. Does that look OK to you too? I've confirmed that with glibc 2.30 the new function is detected, and the testsuite passes. I'm running the tests with an older glibc as well. I noticed that the new tests you added in [PATCH 1/2] pass on current trunk, even without [PATCH 2/2], is that expected? [-- Attachment #2: patch.txt --] [-- Type: text/x-patch, Size: 9169 bytes --] commit d3ddd730a7a2ca4a9d207eb3e1229f5cb8c652e4 Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed Sep 4 12:59:04 2019 +0100 PR libstdc++/41861 Add full steady_clock support to condition_variable The pthread_cond_clockwait function is available in glibc since the 2.30 release. If this function is available in the C library it can be used to fix PR libstdc++/41861 by supporting std::chrono::steady_clock properly with std::condition_variable. This means that code using std::condition_variable::wait_for or std::condition_variable::wait_until with std::chrono::steady_clock is no longer subject to timing out early or potentially waiting for much longer if the system clock is warped at an inopportune moment. If pthread_cond_clockwait is available then std::chrono::steady_clock is deemed to be the "best" clock available which means that it is used for the relative wait_for calls and absolute wait_until calls using user-defined clocks. Calls explicitly using std::chrono::system_clock continue to use CLOCK_REALTIME via __gthread_cond_timedwait. If pthread_cond_clockwait is not available then std::chrono::system_clock is deemed to be the "best" clock available which means that the previous suboptimal behaviour remains. 2019-09-04 Mike Crowe <mac@mcrowe.com> PR libstdc++/41861 * acinclude.m4 (GLIBCXX_CHECK_PTHREAD_COND_CLOCKWAIT): Check for new pthread_cond_clockwait function. * configure.ac: Use GLIBCXX_CHECK_PTHREAD_COND_CLOCKWAIT. * configure: Regenerate. * config.h.in: Regenerate. * include/std/condition_variable: (condition_variable): Rename __steady_clock_t typedef and add system_clock. Change __clock_t to be a typedef for the preferred clock to convert arbitrary other clocks to. [_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT] (wait_until): Add a steady_clock overload. (wait_until): Change __clock_t overload to use system_clock. [_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT] (__wait_until_impl): Add steady_clock overload that calls pthread_cond_clockwait. (__wait_until_impl): Change __clock_t overload to use system_clock. (condition_variable_any) [_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT]: Use steady_clock for __clock_t if pthread_cond_clockwait is available. diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4 index 727d6ce4239..bc9095f4799 100644 --- a/libstdc++-v3/acinclude.m4 +++ b/libstdc++-v3/acinclude.m4 @@ -4193,6 +4193,37 @@ AC_DEFUN([GLIBCXX_CHECK_PTHREADS_NUM_PROCESSORS_NP], [ AC_LANG_RESTORE ]) +dnl +dnl Check whether pthread_cond_clockwait is available in <pthread.h> for std::condition_variable to use, +dnl and define _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT. +dnl +AC_DEFUN([GLIBCXX_CHECK_PTHREAD_COND_CLOCKWAIT], [ + + AC_LANG_SAVE + AC_LANG_CPLUSPLUS + ac_save_CXXFLAGS="$CXXFLAGS" + CXXFLAGS="$CXXFLAGS -fno-exceptions" + ac_save_LIBS="$LIBS" + LIBS="$LIBS -lpthread" + + AC_MSG_CHECKING([for pthread_cond_clockwait]) + AC_CACHE_VAL(glibcxx_cv_PTHREAD_COND_CLOCKWAIT, [ + GCC_TRY_COMPILE_OR_LINK( + [#include <pthread.h>], + [pthread_mutex_t mutex; pthread_cond_t cond; struct timespec ts; int n = pthread_cond_clockwait(&cond, &mutex, 0, &ts);], + [glibcxx_cv_PTHREAD_COND_CLOCKWAIT=yes], + [glibcxx_cv_PTHREAD_COND_CLOCKWAIT=no]) + ]) + if test $glibcxx_cv_PTHREAD_COND_CLOCKWAIT = yes; then + AC_DEFINE(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT, 1, [Define if pthread_cond_clockwait is available in <pthread.h>.]) + fi + AC_MSG_RESULT($glibcxx_cv_PTHREAD_COND_CLOCKWAIT) + + CXXFLAGS="$ac_save_CXXFLAGS" + LIBS="$ac_save_LIBS" + AC_LANG_RESTORE +]) + dnl dnl Check whether sysctl is available in <pthread.h>, and define _GLIBCXX_USE_SYSCTL_HW_NCPU. dnl diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac index 80d8202c337..ad4ae0c3b7d 100644 --- a/libstdc++-v3/configure.ac +++ b/libstdc++-v3/configure.ac @@ -220,6 +220,9 @@ GLIBCXX_ENABLE_LIBSTDCXX_TIME # Check for tmpnam which is obsolescent in POSIX.1-2008 GLIBCXX_CHECK_TMPNAM +# For pthread_cond_clockwait +GLIBCXX_CHECK_PTHREAD_COND_CLOCKWAIT + AC_LC_MESSAGES # For hardware_concurrency diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable index a83996aca3d..ff41548aed8 100644 --- a/libstdc++-v3/include/std/condition_variable +++ b/libstdc++-v3/include/std/condition_variable @@ -65,8 +65,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// condition_variable class condition_variable { - typedef chrono::system_clock __clock_t; - typedef chrono::steady_clock __steady_clock_t; + using steady_clock = chrono::steady_clock; + using system_clock = chrono::system_clock; +#if defined(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT) + using __clock_t = steady_clock; +#else + using __clock_t = system_clock; +#endif typedef __gthread_cond_t __native_type; #ifdef __GTHREAD_COND_INIT @@ -101,10 +106,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION wait(__lock); } +#if defined(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT) template<typename _Duration> cv_status wait_until(unique_lock<mutex>& __lock, - const chrono::time_point<__clock_t, _Duration>& __atime) + const chrono::time_point<steady_clock, _Duration>& __atime) + { return __wait_until_impl(__lock, __atime); } +#endif + + template<typename _Duration> + cv_status + wait_until(unique_lock<mutex>& __lock, + const chrono::time_point<system_clock, _Duration>& __atime) { return __wait_until_impl(__lock, __atime); } template<typename _Clock, typename _Duration> @@ -112,7 +125,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION wait_until(unique_lock<mutex>& __lock, const chrono::time_point<_Clock, _Duration>& __atime) { - // DR 887 - Sync unknown clock to known clock. const typename _Clock::time_point __c_entry = _Clock::now(); const __clock_t::time_point __s_entry = __clock_t::now(); const auto __delta = __atime - __c_entry; @@ -145,11 +157,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION wait_for(unique_lock<mutex>& __lock, const chrono::duration<_Rep, _Period>& __rtime) { - using __dur = typename __steady_clock_t::duration; + using __dur = typename steady_clock::duration; auto __reltime = chrono::duration_cast<__dur>(__rtime); if (__reltime < __rtime) ++__reltime; - return wait_until(__lock, __steady_clock_t::now() + __reltime); + return wait_until(__lock, steady_clock::now() + __reltime); } template<typename _Rep, typename _Period, typename _Predicate> @@ -158,11 +170,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION const chrono::duration<_Rep, _Period>& __rtime, _Predicate __p) { - using __dur = typename __steady_clock_t::duration; + using __dur = typename steady_clock::duration; auto __reltime = chrono::duration_cast<__dur>(__rtime); if (__reltime < __rtime) ++__reltime; - return wait_until(__lock, __steady_clock_t::now() + __reltime, + return wait_until(__lock, steady_clock::now() + __reltime, std::move(__p)); } @@ -171,10 +183,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return &_M_cond; } private: +#if defined(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT) template<typename _Dur> cv_status __wait_until_impl(unique_lock<mutex>& __lock, - const chrono::time_point<__clock_t, _Dur>& __atime) + const chrono::time_point<steady_clock, _Dur>& __atime) + { + auto __s = chrono::time_point_cast<chrono::seconds>(__atime); + auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s); + + __gthread_time_t __ts = + { + static_cast<std::time_t>(__s.time_since_epoch().count()), + static_cast<long>(__ns.count()) + }; + + pthread_cond_clockwait(&_M_cond, __lock.mutex()->native_handle(), + CLOCK_MONOTONIC, + &__ts); + + return (steady_clock::now() < __atime + ? cv_status::no_timeout : cv_status::timeout); + } +#endif + + template<typename _Dur> + cv_status + __wait_until_impl(unique_lock<mutex>& __lock, + const chrono::time_point<system_clock, _Dur>& __atime) { auto __s = chrono::time_point_cast<chrono::seconds>(__atime); auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s); @@ -188,7 +224,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __gthread_cond_timedwait(&_M_cond, __lock.mutex()->native_handle(), &__ts); - return (__clock_t::now() < __atime + return (system_clock::now() < __atime ? cv_status::no_timeout : cv_status::timeout); } }; @@ -208,7 +244,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Like above, but mutex is not required to have try_lock. class condition_variable_any { - typedef chrono::system_clock __clock_t; +#if defined(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT) + using __clock_t = chrono::steady_clock; +#else + using __clock_t = chrono::system_clock; +#endif condition_variable _M_cond; shared_ptr<mutex> _M_mutex; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] PR libstdc++/41861 Add full steady_clock support to condition_variable 2019-09-04 13:39 ` Jonathan Wakely @ 2019-09-04 14:49 ` Mike Crowe 2019-09-04 16:14 ` Jonathan Wakely 0 siblings, 1 reply; 10+ messages in thread From: Mike Crowe @ 2019-09-04 14:49 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libstdc++, gcc-patches On Wednesday 04 September 2019 at 14:39:35 +0100, Jonathan Wakely wrote: > On 15/07/19 17:47 +0100, Mike Crowe wrote: > > The pthread_cond_clockwait function was recently added[1] to glibc, and is > > due to be released in glibc 2.30. If this function is available in the C > > library it can be used it to fix > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861 by supporting > > std::chrono::steady_clock properly with std::condition_variable. > > > > This means that code using std::condition_variable::wait_for or > > std::condition_variable::wait_until with std::chrono::steady_clock is no > > longer subject to timing out early or potentially waiting for much longer > > if the system clock is warped at an inopportune moment. > > > > If pthread_cond_clockwait is available then std::chrono::steady_clock is > > deemed to be the "best" clock available which means that it is used for the > > relative wait_for calls and absolute wait_until calls using user-defined > > clocks. Calls explicitly using std::chrono::system_clock continue to use > > CLOCK_REALTIME via __gthread_cond_timedwait. > > > > If pthread_cond_clockwait is not available then std::chrono::system_clock > > is deemed to be the "best" clock available which means that the previous > > suboptimal behaviour remains. > > > > [1] https://sourceware.org/git/?p=glibc.git;a=commit;h=afe4de7d283ebd88157126c5494ce1796194c16e > > > > libstdc++-v3/ > > > > * include/std/condition_variable: Add include of <bits/c++config.h> > > to make _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT available. > > (condition_variable): Split __clock_t into __system_clock_t, which > > is always system_clock and __best_clock_t, which is the clock that > > we wish to convert arbitrary other clocks to. If we know that > > pthread_cond_clockwait is available then it is best to convert > > clocks to steady_clock, otherwise it's best to use > > system_clock. (wait_until): If pthread_cond_clockwait is available, > > provide a steady_clock overload. Convert previous __clock_t > > overload to use __system_clock_t. Convert generic overload to > > convert passed clock to __best_clock_t. (wait_until_impl): Add > > steady_clock overload that calls pthread_cond_clockwait. Convert > > previous __clock_t overload to use > > __system_clock_t. (condition_variable_any): Use steady_clock for > > __clock_t if pthread_cond_clockwait is available. > > > > * acinclude.m4: Detect the availability of POSIX-proposed > > pthread_cond_clockwait function. > > * configure: Likewise. > > * configure.ac: Likewise. > > * config.h.in: Add _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT define to > > indicate presence of pthread_cond_clockwait function. > > Thanks, Mike! > > This patch is much simpler than the previous versions. Thanks for > perservering with POSIX and glibc to get the necessary functionality > into libc. > > I've attached a slightly-modified patch, which changes the names of > the clock typedefs in std::condition_variable. The names > "steady_clock" and "system_clock" are already reserved names, so we > can just use those (instead of the uglier __steady_clock_t forms). And > we can just keep the original __clock_t name to mean the best clock. > Does that look OK to you too? You're far more expert on that stuff than I am, but it looks fine to me. However, you've also removed the <bits/c++config.h> include. Was that intentional? > I've confirmed that with glibc 2.30 the new function is detected, and > the testsuite passes. I'm running the tests with an older glibc as > well. I found that it was necessary to run the test under strace to prove that the correct variant of futex is called too, because... > I noticed that the new tests you added in [PATCH 1/2] pass on current > trunk, even without [PATCH 2/2], is that expected? Unfortunately, yes. CLOCK_MONOTONIC and CLOCK_REALTIME will tick at the same rate, so unless someone warps CLOCK_REALTIME during the test we can't tell the difference between the old implementation of translating steady_clock to system_clock and the new implementation that uses steady_clock directly. :( I added the tests in the hope of finding other mistakes in the new implementation rather than to reproduce the original problem. Maybe I should add comments to the test to make that clear along the lines of those found in testsuite/27_io/objects/char/3_xin.cc ? Thanks for looking at this. Mike. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] PR libstdc++/41861 Add full steady_clock support to condition_variable 2019-09-04 14:49 ` Mike Crowe @ 2019-09-04 16:14 ` Jonathan Wakely 2019-09-04 16:57 ` Mike Crowe 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Wakely @ 2019-09-04 16:14 UTC (permalink / raw) To: Mike Crowe; +Cc: libstdc++, gcc-patches On 04/09/19 15:49 +0100, Mike Crowe wrote: >On Wednesday 04 September 2019 at 14:39:35 +0100, Jonathan Wakely wrote: >> On 15/07/19 17:47 +0100, Mike Crowe wrote: >> > The pthread_cond_clockwait function was recently added[1] to glibc, and is >> > due to be released in glibc 2.30. If this function is available in the C >> > library it can be used it to fix >> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861 by supporting >> > std::chrono::steady_clock properly with std::condition_variable. >> > >> > This means that code using std::condition_variable::wait_for or >> > std::condition_variable::wait_until with std::chrono::steady_clock is no >> > longer subject to timing out early or potentially waiting for much longer >> > if the system clock is warped at an inopportune moment. >> > >> > If pthread_cond_clockwait is available then std::chrono::steady_clock is >> > deemed to be the "best" clock available which means that it is used for the >> > relative wait_for calls and absolute wait_until calls using user-defined >> > clocks. Calls explicitly using std::chrono::system_clock continue to use >> > CLOCK_REALTIME via __gthread_cond_timedwait. >> > >> > If pthread_cond_clockwait is not available then std::chrono::system_clock >> > is deemed to be the "best" clock available which means that the previous >> > suboptimal behaviour remains. >> > >> > [1] https://sourceware.org/git/?p=glibc.git;a=commit;h=afe4de7d283ebd88157126c5494ce1796194c16e >> > >> > libstdc++-v3/ >> > >> > * include/std/condition_variable: Add include of <bits/c++config.h> >> > to make _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT available. >> > (condition_variable): Split __clock_t into __system_clock_t, which >> > is always system_clock and __best_clock_t, which is the clock that >> > we wish to convert arbitrary other clocks to. If we know that >> > pthread_cond_clockwait is available then it is best to convert >> > clocks to steady_clock, otherwise it's best to use >> > system_clock. (wait_until): If pthread_cond_clockwait is available, >> > provide a steady_clock overload. Convert previous __clock_t >> > overload to use __system_clock_t. Convert generic overload to >> > convert passed clock to __best_clock_t. (wait_until_impl): Add >> > steady_clock overload that calls pthread_cond_clockwait. Convert >> > previous __clock_t overload to use >> > __system_clock_t. (condition_variable_any): Use steady_clock for >> > __clock_t if pthread_cond_clockwait is available. >> > >> > * acinclude.m4: Detect the availability of POSIX-proposed >> > pthread_cond_clockwait function. >> > * configure: Likewise. >> > * configure.ac: Likewise. >> > * config.h.in: Add _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT define to >> > indicate presence of pthread_cond_clockwait function. >> >> Thanks, Mike! >> >> This patch is much simpler than the previous versions. Thanks for >> perservering with POSIX and glibc to get the necessary functionality >> into libc. >> >> I've attached a slightly-modified patch, which changes the names of >> the clock typedefs in std::condition_variable. The names >> "steady_clock" and "system_clock" are already reserved names, so we >> can just use those (instead of the uglier __steady_clock_t forms). And >> we can just keep the original __clock_t name to mean the best clock. >> Does that look OK to you too? > >You're far more expert on that stuff than I am, but it looks fine to me. > >However, you've also removed the <bits/c++config.h> include. Was that >intentional? Yes, it's already included via <type_traits> (and probably several other routes). Every header should already be including that. If they didn't, the macros like _GLIBCXX_HAS_GTHREADS, _GLIBCXX_VISIBILITY, and _GLIBCXX_BEGIN_NAMESPACE_VERSION would produce errors. >> I've confirmed that with glibc 2.30 the new function is detected, and >> the testsuite passes. I'm running the tests with an older glibc as >> well. > >I found that it was necessary to run the test under strace to prove that >the correct variant of futex is called too, because... > >> I noticed that the new tests you added in [PATCH 1/2] pass on current >> trunk, even without [PATCH 2/2], is that expected? > >Unfortunately, yes. CLOCK_MONOTONIC and CLOCK_REALTIME will tick at the >same rate, so unless someone warps CLOCK_REALTIME during the test we can't >tell the difference between the old implementation of translating >steady_clock to system_clock and the new implementation that uses >steady_clock directly. :( I added the tests in the hope of finding other >mistakes in the new implementation rather than to reproduce the original >problem. OK, that was what I figured was the case. Thanks for confirming. >Maybe I should add comments to the test to make that clear along the lines >of those found in testsuite/27_io/objects/char/3_xin.cc ? More comments are usually useful, otherwise I'll just ask the same question again and again :-) All my testing passed, so I'll get this committed today. Thanks again for perservering to finally get this fixed! ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] PR libstdc++/41861 Add full steady_clock support to condition_variable 2019-09-04 16:14 ` Jonathan Wakely @ 2019-09-04 16:57 ` Mike Crowe 2019-09-04 17:21 ` Mike Crowe 0 siblings, 1 reply; 10+ messages in thread From: Mike Crowe @ 2019-09-04 16:57 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libstdc++ On Wednesday 04 September 2019 at 17:14:30 +0100, Jonathan Wakely wrote: > On 04/09/19 15:49 +0100, Mike Crowe wrote: > > On Wednesday 04 September 2019 at 14:39:35 +0100, Jonathan Wakely wrote: > > > I noticed that the new tests you added in [PATCH 1/2] pass on current > > > trunk, even without [PATCH 2/2], is that expected? > > > > Unfortunately, yes. CLOCK_MONOTONIC and CLOCK_REALTIME will tick at the > > same rate, so unless someone warps CLOCK_REALTIME during the test we can't > > tell the difference between the old implementation of translating > > steady_clock to system_clock and the new implementation that uses > > steady_clock directly. :( I added the tests in the hope of finding other > > mistakes in the new implementation rather than to reproduce the original > > problem. > > OK, that was what I figured was the case. Thanks for confirming. > > > Maybe I should add comments to the test to make that clear along the lines > > of those found in testsuite/27_io/objects/char/3_xin.cc ? > > More comments are usually useful, otherwise I'll just ask the same > question again and again :-) How about something like: --8<-- It's not possible for this test to automatically ensure that the system_clock test cases result in a wait on CLOCK_REALTIME and steady_clock test cases result in a wait on CLOCK_MONOTONIC. It's recommended to run the test under strace(1) and check whether the expected futex calls are made by glibc. -->8-- Unfortunately I'm unable to determine how I actually managed to run the test under strace. Perhaps I just compiled a similar test myself rather than using dejagnu. :( Mike. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] PR libstdc++/41861 Add full steady_clock support to condition_variable 2019-09-04 16:57 ` Mike Crowe @ 2019-09-04 17:21 ` Mike Crowe 2019-09-04 22:46 ` Jonathan Wakely 0 siblings, 1 reply; 10+ messages in thread From: Mike Crowe @ 2019-09-04 17:21 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libstdc++ On Wednesday 04 September 2019 at 17:57:45 +0100, Mike Crowe wrote: > On Wednesday 04 September 2019 at 17:14:30 +0100, Jonathan Wakely wrote: > > On 04/09/19 15:49 +0100, Mike Crowe wrote: > > > On Wednesday 04 September 2019 at 14:39:35 +0100, Jonathan Wakely wrote: > > > > I noticed that the new tests you added in [PATCH 1/2] pass on current > > > > trunk, even without [PATCH 2/2], is that expected? > > > > > > Unfortunately, yes. CLOCK_MONOTONIC and CLOCK_REALTIME will tick at the > > > same rate, so unless someone warps CLOCK_REALTIME during the test we can't > > > tell the difference between the old implementation of translating > > > steady_clock to system_clock and the new implementation that uses > > > steady_clock directly. :( I added the tests in the hope of finding other > > > mistakes in the new implementation rather than to reproduce the original > > > problem. > > > > OK, that was what I figured was the case. Thanks for confirming. > > > > > Maybe I should add comments to the test to make that clear along the lines > > > of those found in testsuite/27_io/objects/char/3_xin.cc ? > > > > More comments are usually useful, otherwise I'll just ask the same > > question again and again :-) > > How about something like: > > --8<-- > It's not possible for this test to automatically ensure that the > system_clock test cases result in a wait on CLOCK_REALTIME and steady_clock > test cases result in a wait on CLOCK_MONOTONIC. It's recommended to run the > test under strace(1) and check whether the expected futex calls are made by > glibc. > -->8-- > > Unfortunately I'm unable to determine how I actually managed to run the > test under strace. Perhaps I just compiled a similar test myself rather > than using dejagnu. :( Ah, I did it. Here's a new comment: --8<-- It's not possible for this test to automatically ensure that the system_clock test cases result in a wait on CLOCK_REALTIME and steady_clock test cases result in a wait on CLOCK_MONOTONIC. It's recommended to run the test under strace(1) and check whether the expected futex calls are made by glibc. The easiest way to do this is to copy and paste the line used to build the test from the output of: make -j8 check-target-libstdc++-v3 RUNTESTFLAGS="conformance.exp=30_threads/condition_variable/members/* -v -v" to compile the test with only the tests for one clock enabled and then run it as: strace ./2.exe You should see calls to: futex(..., FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME, ...) with large values of tv_sec when using system_clock and calls to: futex(..., FUTEX_WAIT_BITSET_PRIVATE, ...) with values of tv_sec based on the system uptime when using steady_clock. -->8-- HTH. Mike. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] PR libstdc++/41861 Add full steady_clock support to condition_variable 2019-09-04 17:21 ` Mike Crowe @ 2019-09-04 22:46 ` Jonathan Wakely 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Wakely @ 2019-09-04 22:46 UTC (permalink / raw) To: Mike Crowe; +Cc: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 3053 bytes --] On 04/09/19 18:21 +0100, Mike Crowe wrote: >On Wednesday 04 September 2019 at 17:57:45 +0100, Mike Crowe wrote: >> On Wednesday 04 September 2019 at 17:14:30 +0100, Jonathan Wakely wrote: >> > On 04/09/19 15:49 +0100, Mike Crowe wrote: >> > > On Wednesday 04 September 2019 at 14:39:35 +0100, Jonathan Wakely wrote: >> > > > I noticed that the new tests you added in [PATCH 1/2] pass on current >> > > > trunk, even without [PATCH 2/2], is that expected? >> > > >> > > Unfortunately, yes. CLOCK_MONOTONIC and CLOCK_REALTIME will tick at the >> > > same rate, so unless someone warps CLOCK_REALTIME during the test we can't >> > > tell the difference between the old implementation of translating >> > > steady_clock to system_clock and the new implementation that uses >> > > steady_clock directly. :( I added the tests in the hope of finding other >> > > mistakes in the new implementation rather than to reproduce the original >> > > problem. >> > >> > OK, that was what I figured was the case. Thanks for confirming. >> > >> > > Maybe I should add comments to the test to make that clear along the lines >> > > of those found in testsuite/27_io/objects/char/3_xin.cc ? >> > >> > More comments are usually useful, otherwise I'll just ask the same >> > question again and again :-) >> >> How about something like: >> >> --8<-- >> It's not possible for this test to automatically ensure that the >> system_clock test cases result in a wait on CLOCK_REALTIME and steady_clock >> test cases result in a wait on CLOCK_MONOTONIC. It's recommended to run the >> test under strace(1) and check whether the expected futex calls are made by >> glibc. >> -->8-- >> >> Unfortunately I'm unable to determine how I actually managed to run the >> test under strace. Perhaps I just compiled a similar test myself rather >> than using dejagnu. :( > >Ah, I did it. Here's a new comment: > >--8<-- >It's not possible for this test to automatically ensure that the >system_clock test cases result in a wait on CLOCK_REALTIME and steady_clock >test cases result in a wait on CLOCK_MONOTONIC. It's recommended to run the >test under strace(1) and check whether the expected futex calls are made by >glibc. The easiest way to do this is to copy and paste the line used to >build the test from the output of: > > make -j8 check-target-libstdc++-v3 RUNTESTFLAGS="conformance.exp=30_threads/condition_variable/members/* -v -v" > >to compile the test with only the tests for one clock enabled and then run it as: > > strace ./2.exe > >You should see calls to: > > futex(..., FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME, ...) > >with large values of tv_sec when using system_clock and calls to: > > futex(..., FUTEX_WAIT_BITSET_PRIVATE, ...) > >with values of tv_sec based on the system uptime when using steady_clock. >-->8-- Great. I've committed the patch I sent earlier (with the renamed typedefs) and the attached one for the tests (using an abridged form of the comment above, linking to the copy of your mail in the list archives for the full instructions). Thanks! [-- Attachment #2: patch.txt --] [-- Type: text/x-patch, Size: 4733 bytes --] commit 8f84bffbd3c4173d684fc825f0847d857a71f54b Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed Sep 4 12:41:34 2019 +0100 Add user-defined clock to libstdc++ condition_variable tests 2019-09-04 Mike Crowe <mac@mcrowe.com> * testsuite/30_threads/condition_variable/members/2.cc (test01): Parameterise so that test can be run against an arbitrary clock. (main): Test using std::chrono::steady_clock and a user-defined clock in addition to the previous std::chrono::system_clock. * testsuite/30_threads/condition_variable_any/members/2.cc: Likewise. diff --git a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc index 0ecb09d80ed..cbac3fa1932 100644 --- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc +++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc @@ -26,6 +26,7 @@ #include <system_error> #include <testsuite_hooks.h> +template <typename ClockType> void test01() { try @@ -35,10 +36,10 @@ void test01() std::mutex m; std::unique_lock<std::mutex> l(m); - auto then = std::chrono::steady_clock::now(); + auto then = ClockType::now(); std::cv_status result = c1.wait_until(l, then + ms); VERIFY( result == std::cv_status::timeout ); - VERIFY( (std::chrono::steady_clock::now() - then) >= ms ); + VERIFY( (ClockType::now() - then) >= ms ); VERIFY( l.owns_lock() ); } catch (const std::system_error& e) @@ -102,9 +103,39 @@ void test01_alternate_clock() } } +/* User defined clock that ticks in two-thousandths of a second + forty-two minutes ahead of steady_clock. */ +struct user_defined_clock +{ + typedef std::chrono::steady_clock::rep rep; + typedef std::ratio<1, 2000> period; + typedef std::chrono::duration<rep, period> duration; + typedef std::chrono::time_point<user_defined_clock> time_point; + + static constexpr bool is_steady = true; + + static time_point now() noexcept + { + using namespace std::chrono; + const auto steady_since_epoch = steady_clock::now().time_since_epoch(); + const auto user_since_epoch = duration_cast<duration>(steady_since_epoch); + return time_point(user_since_epoch + minutes(42)); + } +}; + +/* +It's not possible for this test to automatically ensure that the +system_clock test cases result in a wait on CLOCK_REALTIME and steady_clock +test cases result in a wait on CLOCK_MONOTONIC. It's recommended to run the +test under strace(1) and check whether the expected futex calls are made by +glibc. See https://gcc.gnu.org/ml/libstdc++/2019-09/msg00022.html for +instructions. +*/ + int main() { - test01(); + test01<std::chrono::steady_clock>(); + test01<std::chrono::system_clock>(); + test01<user_defined_clock>(); test01_alternate_clock(); - return 0; } diff --git a/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc b/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc index e6fbc44f6f9..897fa86f514 100644 --- a/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc +++ b/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc @@ -51,6 +51,7 @@ struct Mutex }; +template <typename ClockType> void test01() { try @@ -60,10 +61,10 @@ void test01() Mutex m; m.lock(); - auto then = std::chrono::steady_clock::now(); + auto then = ClockType::now(); std::cv_status result = c1.wait_until(m, then + ms); VERIFY( result == std::cv_status::timeout ); - VERIFY( (std::chrono::steady_clock::now() - then) >= ms ); + VERIFY( (ClockType::now() - then) >= ms ); VERIFY( m.locked ); } catch (const std::system_error& e) @@ -76,8 +77,29 @@ void test01() } } +/* User defined clock that ticks in two-thousandths of a second + forty-two minutes ahead of steady_clock. */ +struct user_defined_clock +{ + typedef std::chrono::steady_clock::rep rep; + typedef std::ratio<1, 2000> period; + typedef std::chrono::duration<rep, period> duration; + typedef std::chrono::time_point<user_defined_clock> time_point; + + static constexpr bool is_steady = true; + + static time_point now() noexcept + { + using namespace std::chrono; + const auto steady_since_epoch = steady_clock::now().time_since_epoch(); + const auto user_since_epoch = duration_cast<duration>(steady_since_epoch); + return time_point(user_since_epoch + minutes(42)); + } +}; + int main() { - test01(); - return 0; + test01<std::chrono::steady_clock>(); + test01<std::chrono::system_clock>(); + test01<user_defined_clock>(); } ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] Add user-defined clock to libstdc++ condition_variable tests 2019-07-15 16:48 [PATCH 0/2] PR libstdc++/41861 Add full steady_clock support to condition_variable Mike Crowe 2019-07-15 16:48 ` [PATCH 2/2] " Mike Crowe @ 2019-07-15 16:51 ` Mike Crowe 2019-08-19 16:27 ` [PATCH 0/2] PR libstdc++/41861 Add full steady_clock support to condition_variable Mike Crowe 2 siblings, 0 replies; 10+ messages in thread From: Mike Crowe @ 2019-07-15 16:51 UTC (permalink / raw) To: libstdc++, gcc-patches; +Cc: Mike Crowe libstdc++-v3/ * testsuite/30_threads/condition_variable/members/2.cc (test01): Parameterise so that test can be run against an arbitrary clock. (main): Test using std::chrono::steady_clock and a user-defined clock in addition to the previous std::chrono::system_clock. * testsuite/30_threads/condition_variable_any/members/2.cc: Likewise. --- libstdc++-v3/ChangeLog | 9 +++++++++ libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc | 29 ++++++++++++++++++++++++++--- libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc | 29 ++++++++++++++++++++++++++--- 3 files changed, 61 insertions(+), 6 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 8e96a89..a52a704 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,12 @@ +2019-07-15 Mike Crowe <mac@mcrowe.com> + + * testsuite/30_threads/condition_variable/members/2.cc (test01): + Parameterise so that test can be run against an arbitrary clock. + (main): Test using std::chrono::steady_clock and a user-defined + clock in addition to the previous std::chrono::system_clock. + * testsuite/30_threads/condition_variable_any/members/2.cc: + Likewise. + 2019-07-12 Jonathan Wakely <jwakely@redhat.com> * testsuite/29_atomics/atomic_float/1.cc: Fix comment. diff --git a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc index 0ecb09d..02a0d84 100644 --- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc +++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc @@ -26,6 +26,7 @@ #include <system_error> #include <testsuite_hooks.h> +template <typename ClockType> void test01() { try @@ -35,10 +36,10 @@ void test01() std::mutex m; std::unique_lock<std::mutex> l(m); - auto then = std::chrono::steady_clock::now(); + auto then = ClockType::now(); std::cv_status result = c1.wait_until(l, then + ms); VERIFY( result == std::cv_status::timeout ); - VERIFY( (std::chrono::steady_clock::now() - then) >= ms ); + VERIFY( (ClockType::now() - then) >= ms ); VERIFY( l.owns_lock() ); } catch (const std::system_error& e) @@ -102,9 +103,31 @@ void test01_alternate_clock() } } +/* User defined clock that ticks in two-thousandths of a second + forty-two minutes ahead of steady_clock. */ +struct user_defined_clock +{ + typedef std::chrono::steady_clock::rep rep; + typedef std::ratio<1, 2000> period; + typedef std::chrono::duration<rep, period> duration; + typedef std::chrono::time_point<user_defined_clock> time_point; + + static constexpr bool is_steady = true; + + static time_point now() noexcept + { + using namespace std::chrono; + const auto steady_since_epoch = steady_clock::now().time_since_epoch(); + const auto user_since_epoch = duration_cast<duration>(steady_since_epoch); + return time_point(user_since_epoch + minutes(42)); + } +}; + int main() { - test01(); + test01<std::chrono::steady_clock>(); + test01<std::chrono::system_clock>(); + test01<user_defined_clock>(); test01_alternate_clock(); return 0; } diff --git a/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc b/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc index e6fbc44..840c283 100644 --- a/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc +++ b/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc @@ -51,6 +51,7 @@ struct Mutex }; +template <typename ClockType> void test01() { try @@ -60,10 +61,10 @@ void test01() Mutex m; m.lock(); - auto then = std::chrono::steady_clock::now(); + auto then = ClockType::now(); std::cv_status result = c1.wait_until(m, then + ms); VERIFY( result == std::cv_status::timeout ); - VERIFY( (std::chrono::steady_clock::now() - then) >= ms ); + VERIFY( (ClockType::now() - then) >= ms ); VERIFY( m.locked ); } catch (const std::system_error& e) @@ -76,8 +77,30 @@ void test01() } } +/* User defined clock that ticks in two-thousandths of a second + forty-two minutes ahead of steady_clock. */ +struct user_defined_clock +{ + typedef std::chrono::steady_clock::rep rep; + typedef std::ratio<1, 2000> period; + typedef std::chrono::duration<rep, period> duration; + typedef std::chrono::time_point<user_defined_clock> time_point; + + static constexpr bool is_steady = true; + + static time_point now() noexcept + { + using namespace std::chrono; + const auto steady_since_epoch = steady_clock::now().time_since_epoch(); + const auto user_since_epoch = duration_cast<duration>(steady_since_epoch); + return time_point(user_since_epoch + minutes(42)); + } +}; + int main() { - test01(); + test01<std::chrono::steady_clock>(); + test01<std::chrono::system_clock>(); + test01<user_defined_clock>(); return 0; } -- git-series 0.9.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] PR libstdc++/41861 Add full steady_clock support to condition_variable 2019-07-15 16:48 [PATCH 0/2] PR libstdc++/41861 Add full steady_clock support to condition_variable Mike Crowe 2019-07-15 16:48 ` [PATCH 2/2] " Mike Crowe 2019-07-15 16:51 ` [PATCH 1/2] Add user-defined clock to libstdc++ condition_variable tests Mike Crowe @ 2019-08-19 16:27 ` Mike Crowe 2 siblings, 0 replies; 10+ messages in thread From: Mike Crowe @ 2019-08-19 16:27 UTC (permalink / raw) To: libstdc++, gcc-patches On Monday 15 July 2019 at 17:47:47 +0100, Mike Crowe wrote: > The pthread_cond_clockwait function was recently added[1] to glibc, > and is due to be released in glibc 2.30. If this function is available > in the C library it can be used it to fix > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861 by supporting > std::chrono::steady_clock properly with std::condition_variable. > > Mike Crowe (2): > Add user-defined clock to libstdc++ condition_variable tests > PR libstdc++/41861 Add full steady_clock support to condition_variable > > libstdc++-v3/ChangeLog | 34 +++++++++++++++++++++++++++++- > libstdc++-v3/acinclude.m4 | 31 +++++++++++++++++++++++++++- > libstdc++-v3/config.h.in | 3 +++- > libstdc++-v3/configure | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > libstdc++-v3/configure.ac | 3 +++- > libstdc++-v3/include/std/condition_variable | 52 +++++++++++++++++++++++++++++++++++++++------ > libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc | 29 ++++++++++++++++++++++--- > libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc | 29 ++++++++++++++++++++++--- > 8 files changed, 252 insertions(+), 12 deletions(-) > > base-commit: 7b0d7c79224b412ea5073593943ab1525cbbda73 > -- > git-series 0.9.1 glibc 2.30 has now been released and contains pthread_cond_clockwait. Mike. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-09-04 22:46 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-15 16:48 [PATCH 0/2] PR libstdc++/41861 Add full steady_clock support to condition_variable Mike Crowe 2019-07-15 16:48 ` [PATCH 2/2] " Mike Crowe 2019-09-04 13:39 ` Jonathan Wakely 2019-09-04 14:49 ` Mike Crowe 2019-09-04 16:14 ` Jonathan Wakely 2019-09-04 16:57 ` Mike Crowe 2019-09-04 17:21 ` Mike Crowe 2019-09-04 22:46 ` Jonathan Wakely 2019-07-15 16:51 ` [PATCH 1/2] Add user-defined clock to libstdc++ condition_variable tests Mike Crowe 2019-08-19 16:27 ` [PATCH 0/2] PR libstdc++/41861 Add full steady_clock support to condition_variable Mike Crowe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).