[RFC 2/2] Add steady_clock support to condition_variable

[RFC 2/2] Add steady_clock support to condition_variable

[Mike Crowe]

If __gthread_cond_timedwaitonclock is available it can be used it to fix
part of 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 changed at an inopportune moment.

If __gthread_cond_timedwaitonclock 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 that aren't choosing to use std::chrono::system_clock.
Calls explicitly using std::chrono::system_clock continue to use
CLOCK_REALTIME.

If __gthread_cond_timedwaitonclock is not available then
std::chrono::system_clock is deemed to be the "best" clock available
which means that the previous suboptimal behaviour remains.

Signed-off-by: Mike Crowe <mac@mcrowe.com>
---
 libstdc++-v3/include/std/condition_variable        | 56 ++++++++++++++++++----
 .../30_threads/condition_variable/members/2.cc     |  8 ++--
 2 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable
index f7da017..625ecfe 100644
--- a/libstdc++-v3/include/std/condition_variable
+++ b/libstdc++-v3/include/std/condition_variable
@@ -63,7 +63,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   /// condition_variable
   class condition_variable
   {
-    typedef chrono::system_clock	__clock_t;
+#ifdef _GTHREAD_USE_COND_TIMEDWAITONCLOCK
+    typedef chrono::steady_clock	__steady_clock_t;
+    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
@@ -98,10 +104,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  wait(__lock);
       }
 
+#ifdef _GTHREAD_USE_COND_TIMEDWAITONCLOCK
     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_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<__system_clock_t, _Duration>& __atime)
       { return __wait_until_impl(__lock, __atime); }
 
     template<typename _Clock, typename _Duration>
@@ -109,9 +123,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;
 
@@ -134,24 +147,47 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       cv_status
       wait_for(unique_lock<mutex>& __lock,
 	       const chrono::duration<_Rep, _Period>& __rtime)
-      { return wait_until(__lock, __clock_t::now() + __rtime); }
+      { return wait_until(__lock, __best_clock_t::now() + __rtime); }
 
     template<typename _Rep, typename _Period, typename _Predicate>
       bool
       wait_for(unique_lock<mutex>& __lock,
 	       const chrono::duration<_Rep, _Period>& __rtime,
 	       _Predicate __p)
-      { return wait_until(__lock, __clock_t::now() + __rtime, std::move(__p)); }
+      { return wait_until(__lock, __best_clock_t::now() + __rtime, std::move(__p)); }
 
     native_handle_type
     native_handle()
     { return &_M_cond; }
 
   private:
+#ifdef _GTHREAD_USE_COND_TIMEDWAITONCLOCK
+    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())
+	  };
+
+	__gthread_cond_timedwaitonclock(&_M_cond, __lock.mutex()->native_handle(),
+					__GTHREAD_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);
@@ -165,7 +201,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);
       }
   };
@@ -185,7 +221,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // Like above, but mutex is not required to have try_lock.
   class condition_variable_any
   {
+#ifdef _GTHREAD_USE_COND_TIMEDWAITONCLOCK
+    typedef chrono::steady_clock	__clock_t;
+#else
     typedef chrono::system_clock	__clock_t;
+#endif
     condition_variable			_M_cond;
     shared_ptr<mutex>			_M_mutex;
 
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 04945f1..79d511e 100644
--- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc
+++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc
@@ -27,6 +27,7 @@
 #include <system_error>
 #include <testsuite_hooks.h>
 
+template <typename CLOCK>
 void test01()
 {
   bool test __attribute__((unused)) = true;
@@ -38,10 +39,10 @@ void test01()
       std::mutex m;
       std::unique_lock<std::mutex> l(m);
 
-      auto then = std::chrono::steady_clock::now();
+      auto then = CLOCK::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( (CLOCK::now() - then) >= ms );
       VERIFY( l.owns_lock() );
     }
   catch (const std::system_error& e)
@@ -56,6 +57,7 @@ void test01()
 
 int main()
 {
-  test01();
+  test01<std::chrono::steady_clock>();
+  test01<std::chrono::system_clock>();
   return 0;
 }
-- 
2.1.4

Re: [RFC 2/2] Add steady_clock support to condition_variable

[Jonathan Wakely]

On 06/07/15 13:55 +0100, Mike Crowe wrote:
>If __gthread_cond_timedwaitonclock is available it can be used it to fix
>part of 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 changed at an inopportune moment.
>
>If __gthread_cond_timedwaitonclock 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 that aren't choosing to use std::chrono::system_clock.
>Calls explicitly using std::chrono::system_clock continue to use
>CLOCK_REALTIME.
>
>If __gthread_cond_timedwaitonclock is not available then
>std::chrono::system_clock is deemed to be the "best" clock available
>which means that the previous suboptimal behaviour remains.

From a quick glance this looks like a good change. As I said in my
other mail about the gthr-posix.h changes, it might be better to keep
this change to libstdc++ rather than splitting it across libstdc++ and
libgcc's gthr-posix.h

Do you have a copyright assignment in place for GCC contributions?

Re: [RFC 2/2] Add steady_clock support to condition_variable

[Mike Crowe]

On Monday 06 July 2015 at 14:54:06 +0100, Jonathan Wakely wrote:
> Do you have a copyright assignment in place for GCC contributions?

Not yet. From my reading of https://gcc.gnu.org/contribute.html I should
probably ask you for the forms. :-)

Mike.

Re: [RFC 2/2] Add steady_clock support to condition_variable

[Jonathan Wakely]

On 06/07/15 15:18 +0100, Mike Crowe wrote:
>On Monday 06 July 2015 at 14:54:06 +0100, Jonathan Wakely wrote:
>> Do you have a copyright assignment in place for GCC contributions?
>
>Not yet. From my reading of https://gcc.gnu.org/contribute.html I should
>probably ask you for the forms. :-)

Yep :-) I'll send them offlist.

[RFCv2] Add steady_clock support to condition_variable

[Mike Crowe]

If pthread_cond_timedwaitonclock_np is available in the C library it can
be used it to fix part of
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 changed at an inopportune moment.

If pthread_cond_timedwaitonclock_np 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.

If pthread_cond_timedwaitonclock_np is not available then
std::chrono::system_clock is deemed to be the "best" clock available
which means that the previous suboptimal behaviour remains.

(I have a patch that adds pthread_cond_timedwaitonclock_np to glibc
ready to submit but I wanted to get feedback on this part of the change
before submitting the glibc patch. See
https://gitlab.com/mikecrowe/glibc-monotonic .)

Signed-off-by: Mike Crowe <mac@mcrowe.com>
---
 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        | 57 ++++++++++++---
 .../30_threads/condition_variable/members/2.cc     | 27 ++++++-
 .../30_threads/condition_variable_any/members/2.cc | 27 ++++++-
 7 files changed, 217 insertions(+), 14 deletions(-)

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 11f48f9..c4d369d 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -3652,6 +3652,37 @@ AC_DEFUN([GLIBCXX_CHECK_PTHREADS_NUM_PROCESSORS_NP], [
 ])
 
 dnl
+dnl Check whether pthread_cond_timedwaitonclock_np is available in <pthread.h> for std::condition_variable to use,
+dnl and define _GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP.
+dnl
+AC_DEFUN([GLIBCXX_CHECK_PTHREAD_COND_TIMEDWAITONCLOCK_NP], [
+
+  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_timedwaitonclock_np])
+  AC_CACHE_VAL(glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP, [
+    GCC_TRY_COMPILE_OR_LINK(
+      [#include <pthread.h>],
+      [pthread_mutex_t mutex; pthread_cond_t cond; struct timespec ts; int n = pthread_cond_timedwaitonclock_np(&cond, &mutex, 0, &ts);],
+      [glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=yes],
+      [glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=no])
+  ])
+  if test $glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP = yes; then
+    AC_DEFINE(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP, 1, [Define if pthread_cond_timedwaitonclock_np is available in <pthread.h>.])
+  fi
+  AC_MSG_RESULT($glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP)
+
+  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 337f614..a46efd8 100644
--- a/libstdc++-v3/config.h.in
+++ b/libstdc++-v3/config.h.in
@@ -869,6 +869,9 @@
 /* Define if pthreads_num_processors_np is available in <pthread.h>. */
 #undef _GLIBCXX_USE_PTHREADS_NUM_PROCESSORS_NP
 
+/* Define if pthread_cond_timedwaitonclock_np is available in <pthread.h>. */
+#undef _GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP
+
 /* 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 e9521d6..19434c4 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -20220,6 +20220,89 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 
 
 
+# For pthread_cond_timedwaitonclock_np
+
+
+
+  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_timedwaitonclock_np" >&5
+$as_echo_n "checking for pthread_cond_timedwaitonclock_np... " >&6; }
+  if test "${glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP+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_timedwaitonclock_np(&cond, &mutex, 0, &ts);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=yes
+else
+  glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=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_timedwaitonclock_np(&cond, &mutex, 0, &ts);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_link "$LINENO"; then :
+  glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=yes
+else
+  glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+
+fi
+
+  if test $glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP = yes; then
+
+$as_echo "#define _GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP 1" >>confdefs.h
+
+  fi
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP" >&5
+$as_echo "$glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP" >&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" = x""yes; then :
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 96ff16f..f59981c 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -218,6 +218,9 @@ GLIBCXX_ENABLE_LIBSTDCXX_TIME
 # Check for tmpnam which is obsolescent in POSIX.1-2008
 GLIBCXX_CHECK_TMPNAM
 
+# For pthread_cond_timedwaitonclock_np
+GLIBCXX_CHECK_PTHREAD_COND_TIMEDWAITONCLOCK_NP
+
 AC_LC_MESSAGES
 
 # For hardware_concurrency
diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable
index f7da017..883af12 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 <mutex>
 #include <ext/concurrence.h>
@@ -63,7 +64,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   /// condition_variable
   class condition_variable
   {
-    typedef chrono::system_clock	__clock_t;
+#if defined(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP)
+    typedef chrono::steady_clock	__steady_clock_t;
+    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
@@ -98,10 +105,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  wait(__lock);
       }
 
+#if defined(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP)
     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_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<__system_clock_t, _Duration>& __atime)
       { return __wait_until_impl(__lock, __atime); }
 
     template<typename _Clock, typename _Duration>
@@ -109,9 +124,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;
 
@@ -134,24 +148,47 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       cv_status
       wait_for(unique_lock<mutex>& __lock,
 	       const chrono::duration<_Rep, _Period>& __rtime)
-      { return wait_until(__lock, __clock_t::now() + __rtime); }
+      { return wait_until(__lock, __best_clock_t::now() + __rtime); }
 
     template<typename _Rep, typename _Period, typename _Predicate>
       bool
       wait_for(unique_lock<mutex>& __lock,
 	       const chrono::duration<_Rep, _Period>& __rtime,
 	       _Predicate __p)
-      { return wait_until(__lock, __clock_t::now() + __rtime, std::move(__p)); }
+      { return wait_until(__lock, __best_clock_t::now() + __rtime, std::move(__p)); }
 
     native_handle_type
     native_handle()
     { return &_M_cond; }
 
   private:
+#if defined(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP)
+    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_timedwaitonclock_np(&_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);
@@ -165,7 +202,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);
       }
   };
@@ -185,7 +222,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_TIMEDWAITONCLOCK_NP)
+    typedef chrono::steady_clock	__clock_t;
+#else
     typedef chrono::system_clock	__clock_t;
+#endif
     condition_variable			_M_cond;
     shared_ptr<mutex>			_M_mutex;
 
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 04945f1..6687e9c 100644
--- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc
+++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc
@@ -27,6 +27,7 @@
 #include <system_error>
 #include <testsuite_hooks.h>
 
+template <typename ClockType>
 void test01()
 {
   bool test __attribute__((unused)) = true;
@@ -38,10 +39,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)
@@ -54,8 +55,28 @@ 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;
+
+  const bool is_steady = true;
+
+  static time_point now() noexcept
+  {
+    using namespace std::chrono;
+    return time_point(duration_cast<duration>(steady_clock::now().time_since_epoch()) + minutes(42));
+  }
+};
+
 int main()
 {
-  test01();
+  test01<std::chrono::steady_clock>();
+  test01<std::chrono::system_clock>();
+  test01<user_defined_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 75d8ac3..2b42dca 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
@@ -52,6 +52,7 @@ struct Mutex
 };
 
 
+template <typename ClockType>
 void test01()
 {
   bool test __attribute__((unused)) = true;
@@ -63,10 +64,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)
@@ -79,8 +80,28 @@ 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;
+
+  const bool is_steady = true;
+
+  static time_point now() noexcept
+  {
+    using namespace std::chrono;
+    return time_point(duration_cast<duration>(steady_clock::now().time_since_epoch()) + minutes(42));
+  }
+};
+
 int main()
 {
-  test01();
+  test01<std::chrono::steady_clock>();
+  test01<std::chrono::system_clock>();
+  test01<user_defined_clock>();
   return 0;
 }
-- 
2.1.4

Re: [RFC 1/2] gthread: Add __gthread_cond_timedwaitonclock

[Jonathan Wakely]

On 06/07/15 13:55 +0100, Mike Crowe wrote:
>diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h
>index fb59816..0e01866 100644
>--- a/libgcc/gthr-posix.h
>+++ b/libgcc/gthr-posix.h
>@@ -33,6 +33,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> #define __GTHREADS_CXX0X 1
>
> #include <pthread.h>
>+#include <time.h>
>
> #if ((defined(_LIBOBJC) || defined(_LIBOBJC_WEAK)) \
>      || !defined(_GTHREAD_USE_MUTEX_TIMEDLOCK))
>@@ -44,6 +45,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> # endif
> #endif
>
>+
>+#if defined(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP)
>+# define _GTHREAD_USE_COND_TIMEDWAITONCLOCK 1
>+#endif

This isn't correct, because it's possible to include <gthr.h> before
including any C++ Standard Library header, so the _GLIBCXX_ macro
defined in libstdc++'s c++config.h will not have been defined.

It might make sense to just do this internally in libstdc++ and not
involve gthr-posix.h at all, this is what we do for pthread_rwlock_t
usage in <shared_mutex> so you might want to follow that model.

How portable is pthread_cond_timedwaitonclock_np? Is it unique to
glibc or do any other posix systems provide it?

Re: [RFC 1/2] gthread: Add __gthread_cond_timedwaitonclock

[Mike Crowe]

On Monday 06 July 2015 at 14:51:42 +0100, Jonathan Wakely wrote:
> On 06/07/15 13:55 +0100, Mike Crowe wrote:
> >diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h
> >index fb59816..0e01866 100644
> >--- a/libgcc/gthr-posix.h
> >+++ b/libgcc/gthr-posix.h
> >@@ -33,6 +33,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> >#define __GTHREADS_CXX0X 1
> >
> >#include <pthread.h>
> >+#include <time.h>
> >
> >#if ((defined(_LIBOBJC) || defined(_LIBOBJC_WEAK)) \
> >     || !defined(_GTHREAD_USE_MUTEX_TIMEDLOCK))
> >@@ -44,6 +45,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> ># endif
> >#endif
> >
> >+
> >+#if defined(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP)
> >+# define _GTHREAD_USE_COND_TIMEDWAITONCLOCK 1
> >+#endif
> 
> This isn't correct, because it's possible to include <gthr.h> before
> including any C++ Standard Library header, so the _GLIBCXX_ macro
> defined in libstdc++'s c++config.h will not have been defined.

Presumably gthr-posix.h could just include c++config.h to avoid that, but
perhaps it's not that simple which is why gthr-posix.h swings through hoops
to define _GTHREAD_USE_MUTEX_TIMEDLOCK itself?

> It might make sense to just do this internally in libstdc++ and not
> involve gthr-posix.h at all, this is what we do for pthread_rwlock_t
> usage in <shared_mutex> so you might want to follow that model.

I implemented the new function in gthreads because that seemed to be what
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861 was suggesting and I'd
incorrectly assumed that there were non-pthreads backends for gthreads too.

I'm can try and re-implement condition_variable using pthreads directly but
the change will be somewhat larger and it would break non-pthreads
platforms.

> How portable is pthread_cond_timedwaitonclock_np? Is it unique to
> glibc or do any other posix systems provide it?

It's not at all portable; I invented it because it seemed to be the most
straightforward way to correctly support std::condition_variable's wait
operations using std::chrono::steady_clock on Linux. I haven't even
suggested adding the function on the glibc mailing list yet.

I looked briefly to see if I could find anyone else solving the problem on
Posix platforms but they all seem to be converting to system_clock too. :(

Thanks.

Mike.

Re: [RFC 1/2] gthread: Add __gthread_cond_timedwaitonclock

[Jonathan Wakely]

On 06/07/15 15:10 +0100, Mike Crowe wrote:
>On Monday 06 July 2015 at 14:51:42 +0100, Jonathan Wakely wrote:
>> This isn't correct, because it's possible to include <gthr.h> before
>> including any C++ Standard Library header, so the _GLIBCXX_ macro
>> defined in libstdc++'s c++config.h will not have been defined.
>
>Presumably gthr-posix.h could just include c++config.h to avoid that, but
>perhaps it's not that simple which is why gthr-posix.h swings through hoops
>to define _GTHREAD_USE_MUTEX_TIMEDLOCK itself?

In theory you could have a GCC installation without libstdc++ so no
c++config.h. These days we could use __has_include(bits/c++config.h)
to detect that and so could actually solve this problem, but I still
don't think we want this in gthr-posix.h anyway.

>> It might make sense to just do this internally in libstdc++ and not
>> involve gthr-posix.h at all, this is what we do for pthread_rwlock_t
>> usage in <shared_mutex> so you might want to follow that model.
>
>I implemented the new function in gthreads because that seemed to be what
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861 was suggesting and I'd
>incorrectly assumed that there were non-pthreads backends for gthreads too.

There are non-pthreads backends for gthreads, that's the point of
gthreads. But:

1) If you add a new function to gthreads then it becomes part of the
ghtreads API and so in theory it should be implementable by the other
backends. This probably never would be.

2) This isn't even a function that's provided by most pthreads
implementations, so can't even be provided by most users of
gthr-posix.h

3) It is only needed for one place in lisbtdc++ so doesn't need to be
in the generic gthreads API.

>I'm can try and re-implement condition_variable using pthreads directly but
>the change will be somewhat larger and it would break non-pthreads
>platforms.

That's not what I mean. I just mean that we don't need to add a
wrapper in ghtreads and then use that wrapper in <condition_variable>.
You can simply use pthread_cond_timedwaitonclock_np function directly
in <condition_variable> when it is available (as determined by the
_GLIBCXX_ macro, which we do still want).

The comparison to <shared_mutex> was meant to point out that we just
use pthreads directly there, instead of adding rwlock support to
gthreads and then using that. We could do the same for uses of this
function, while stil using gthreads for the rest of
<condition_variable>.