diff mbox

Default to --enable-libstdcxx-time=auto

Message ID 20130524202632.078f82ea@oakwood
State New
Headers show

Commit Message

Benjamin Kosnik May 25, 2013, 3:26 a.m. UTC 
> So, there is a minor issue that what is std::chrono::steady_clock has
> changed, if you say use it as a function parameter, it will mangle
> differently before/after.  Guess not that big a deal, after all, C++11
> support is still experimental, right?

Right, ditto, yes.
 
> But the more important issue is that std::chrono::system_clock broke,
> code compiled against the old headers will assume
> std::chrono::system_clock::duration is microseconds resolution on
> Linux, while code compiled against the new headers will assume it is
> in nanoseconds resolution.  That is because of:
> #ifdef _GLIBCXX_USE_CLOCK_REALTIME
>       typedef chrono::nanoseconds
> duration; #elif defined(_GLIBCXX_USE_GETTIMEOFDAY)
>       typedef chrono::microseconds
> duration; #else
>       typedef chrono::seconds
> duration; #endif
> 
> Thus, I'm afraid we can't ABI compatibly change either of these
> macros, unless we e.g. keep old std::chrono::system_clock as is and
> introduce std::chrono::__whatever::system_clock or whatever, that
> will be typedefed as std::chrono::system_clock.

If we want to care about C++11 ABI compat on release branches
(?), here's another solution.

It scraps the renaming/aliasing approach, and just creates a
compatibility-chrono.cc that mimics the default configuration in 4.8.0.
Users who specially-configured a build with --enable-libstdcxx-time
configure options in 4.8.0 are breaking an experimental
C++ ABI anyway, so I'm not going to solve for anything but the default
case.

For chrono.cc, there is an inline namespace that mangles system_clock
and steady_clock in a new way to prevent ambiguous uses. In addition, I
make a controversial decision and just forward-declare clocks that
libstdc++ cannot do correctly, instead of using typedefs that clearly
are only going to get us into trouble as we change things down the
road. That's probably only appropriate for trunk.

Flame on, cowboys, flame on.

This still doesn't solve the solaris 9/10 librt issue.

tested x86_64/linux

-benjamin

Comments

Jakub Jelinek May 25, 2013, 6:21 a.m. UTC | #1 
On Fri, May 24, 2013 at 08:26:32PM -0700, Benjamin De Kosnik wrote:
> It scraps the renaming/aliasing approach, and just creates a
> compatibility-chrono.cc that mimics the default configuration in 4.8.0.

Yeah, I think that is reasonable, with one nit, see below.

> Users who specially-configured a build with --enable-libstdcxx-time
> configure options in 4.8.0 are breaking an experimental
> C++ ABI anyway, so I'm not going to solve for anything but the default
> case.
> 
> For chrono.cc, there is an inline namespace that mangles system_clock
> and steady_clock in a new way to prevent ambiguous uses. In addition, I
> make a controversial decision and just forward-declare clocks that
> libstdc++ cannot do correctly, instead of using typedefs that clearly
> are only going to get us into trouble as we change things down the
> road. That's probably only appropriate for trunk.

> +    // To support the (forward) evolving definition of the library's
> +    // defined clocks, wrap inside inline namespace so that these
> +    // types are uniquely mangled. This way, new code can use the
> +    // current clocks, while the library can contain old definitions.
> +    // At some point, when these clocks settle down, the inlined
> +    // namespaces can be removed.
> +    // XXX GLIBCXX_ABI Deprecated
> +    inline namespace _V2 {
> +
>      /// system_clock
>      struct system_clock
>      {

In this _V2 inline namespace, I think we should change that:
    struct system_clock
    {
#ifdef _GLIBCXX_USE_CLOCK_REALTIME
      typedef chrono::nanoseconds                               duration;
#elif defined(_GLIBCXX_USE_GETTIMEOFDAY)
      typedef chrono::microseconds                              duration;
#else
      typedef chrono::seconds                                   duration;
#endif
into:
    struct system_clock
    {
      typedef chrono::nanoseconds                               duration;

Won't the templates then DTRT in:
  return time_point(duration(chrono::seconds(tv.tv_sec)
		    + chrono::microseconds(tv.tv_usec)));
(multiply microseconds by 1000 and seconds by 1000000000)
      return system_clock::from_time_t(__sec);
(multiply seconds by 1000000000)?

While this change isn't really necessary for Linux, where usually
_GLIBCXX_USE_CLOCK_REALTIME will be in 4.8.1+ defined and thus the
nanoseconds resolution will be used anyway, on other OSes it might
be already a problem, say on Solaris or FreeBSD GCC 4.8.1 will have
by default likely microseconds resolution, while on the trunk nanoseconds.
By making it always count in nanoseconds, even if on some OSes the low
3 or 9 decimal digits will be always zero, we'd prepared to change the
system_clock::now() implementation any time to provide better resolution, as
a libstdc++ internal implementation detail.

Or is this undesirable for users for some reason?

> @@ -742,10 +751,18 @@ _GLIBCXX_END_NAMESPACE_VERSION
>        now() noexcept;
>      };
>  #else
> -    typedef system_clock steady_clock;
> +    // Forward declare only.
> +    struct steady_clock;
>  #endif
>  
> +#ifdef _GLIBCXX_USE_CLOCK_REALTIME
>      typedef system_clock high_resolution_clock;
> +#else
> +    // Forward declare only.
> +    struct high_resolution_clock;
> +#endif
> +
> +  } // end inline namespace _V2

Yeah, I bet this hunk should be left out from 4.8.1 version of the patch.

Perhaps we also want some testcase that will roughly test it, like
grab time(NULL), std::chrono::system_clock::now() and std::chrono::steady_clock::now(),
then sleep(3), grab both clocks again and time(NULL) last, then if the
difference between time() is at least 2 seconds and less than say 10, verify
the difference between the clocks is say in a 1 to 20 seconds interval
(yeah, it could fail badly if one changes system time in between once or
several times)?  But maybe it would be too flakey.

	Jakub
Jakub Jelinek May 25, 2013, 7:56 a.m. UTC | #2 
On Sat, May 25, 2013 at 08:21:46AM +0200, Jakub Jelinek wrote:
> In this _V2 inline namespace, I think we should change that:
>     struct system_clock
>     {
> #ifdef _GLIBCXX_USE_CLOCK_REALTIME
>       typedef chrono::nanoseconds                               duration;
> #elif defined(_GLIBCXX_USE_GETTIMEOFDAY)
>       typedef chrono::microseconds                              duration;
> #else
>       typedef chrono::seconds                                   duration;
> #endif
> into:
>     struct system_clock
>     {
>       typedef chrono::nanoseconds                               duration;
> 
> Won't the templates then DTRT in:
>   return time_point(duration(chrono::seconds(tv.tv_sec)
> 		    + chrono::microseconds(tv.tv_usec)));
> (multiply microseconds by 1000 and seconds by 1000000000)
>       return system_clock::from_time_t(__sec);
> (multiply seconds by 1000000000)?
> 
> While this change isn't really necessary for Linux, where usually
> _GLIBCXX_USE_CLOCK_REALTIME will be in 4.8.1+ defined and thus the
> nanoseconds resolution will be used anyway, on other OSes it might
> be already a problem, say on Solaris or FreeBSD GCC 4.8.1 will have
> by default likely microseconds resolution, while on the trunk nanoseconds.
> By making it always count in nanoseconds, even if on some OSes the low
> 3 or 9 decimal digits will be always zero, we'd prepared to change the
> system_clock::now() implementation any time to provide better resolution, as
> a libstdc++ internal implementation detail.
> 
> Or is this undesirable for users for some reason?

Additional comment to that, the fact that the OS has clock_gettime with
CLOCK_REALTIME resp. gettimeofday still doesn't say anything about the
resolution/precision of the clock, you can have clock_gettime
(CLOCK_REALTIME, &tp) that only provides second or millisecond resolution,
or hundreds if microseconds, tens of nanoseconds etc.  There is clock_getres
in POSIX, but <chrono> doesn't query this and if the resolution decision
has to be static (nanoseconds vs. microseconds vs. seconds), I'd hope it
should be ok to always use nanoseconds duration, especially when most of the
important targets will either in 4.8.1+ or at least in 4.9.0+ use
clock_gettime.

	Jakub
diff mbox

Patch

2013-05-24  Benjamin Kosnik  <bkoz@redhat.com>

	* include/std/chrono: Wrap clocks in inline namespace _V2.
	* src/c++11/chrono.cc: Same.
	* src/c++11/compatibility-chrono.cc: Revert to previous chrono.cc
	file, with default configure macros selected.

	* config/abi/pre/gnu.ver (GLIBCXX_3.4.19): Use symbols from inline
	namespace.
	* config/abi/post/x86_64-linux-gnu/baseline_symbols.txt: Fix up.

diff --git a/libstdc++-v3/config/abi/post/x86_64-linux-gnu/baseline_symbols.txt b/libstdc++-v3/config/abi/post/x86_64-linux-gnu/baseline_symbols.txt
index 165638b..0217375 100644
--- a/libstdc++-v3/config/abi/post/x86_64-linux-gnu/baseline_symbols.txt
+++ b/libstdc++-v3/config/abi/post/x86_64-linux-gnu/baseline_symbols.txt
@@ -1937,9 +1937,9 @@  FUNC:_ZNSt6__norm15_List_node_base7_M_hookEPS0_@@GLIBCXX_3.4.14
 FUNC:_ZNSt6__norm15_List_node_base7reverseEv@@GLIBCXX_3.4.9
 FUNC:_ZNSt6__norm15_List_node_base8transferEPS0_S1_@@GLIBCXX_3.4.9
 FUNC:_ZNSt6__norm15_List_node_base9_M_unhookEv@@GLIBCXX_3.4.14
-FUNC:_ZNSt6chrono12steady_clock3nowEv@@GLIBCXX_3.4.19
-FUNC:_ZNSt6chrono12steady_clock3nowEv@GLIBCXX_3.4.17
 FUNC:_ZNSt6chrono12system_clock3nowEv@@GLIBCXX_3.4.11
+FUNC:_ZNSt6chrono3_V212steady_clock3nowEv@@GLIBCXX_3.4.19
+FUNC:_ZNSt6chrono3_V212system_clock3nowEv@@GLIBCXX_3.4.19
 FUNC:_ZNSt6gslice8_IndexerC1EmRKSt8valarrayImES4_@@GLIBCXX_3.4
 FUNC:_ZNSt6gslice8_IndexerC2EmRKSt8valarrayImES4_@@GLIBCXX_3.4
 FUNC:_ZNSt6locale11_M_coalesceERKS_S1_i@@GLIBCXX_3.4
@@ -2953,6 +2953,8 @@  OBJECT:1:_ZNSt21__numeric_limits_base9is_iec559E@@GLIBCXX_3.4
 OBJECT:1:_ZNSt21__numeric_limits_base9is_moduloE@@GLIBCXX_3.4
 OBJECT:1:_ZNSt21__numeric_limits_base9is_signedE@@GLIBCXX_3.4
 OBJECT:1:_ZNSt6chrono12system_clock12is_monotonicE@@GLIBCXX_3.4.11
+OBJECT:1:_ZNSt6chrono3_V212steady_clock9is_steadyE@@GLIBCXX_3.4.19
+OBJECT:1:_ZNSt6chrono3_V212system_clock9is_steadyE@@GLIBCXX_3.4.19
 OBJECT:1:_ZSt10adopt_lock@@GLIBCXX_3.4.11
 OBJECT:1:_ZSt10defer_lock@@GLIBCXX_3.4.11
 OBJECT:1:_ZSt11try_to_lock@@GLIBCXX_3.4.11
diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index 9c64a0d..9e1f4da 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1320,11 +1320,8 @@  GLIBCXX_3.4.17 {
     _ZNSt13__future_base19_Async_state_commonD1Ev;
     _ZNSt13__future_base19_Async_state_commonD2Ev;
 
-#ifdef HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT
-   # GLIBCXX_ABI compatibility only.
     # std::chrono::steady_clock::now()
     _ZNSt6chrono12steady_clock3nowEv;
-#endif
 
 } GLIBCXX_3.4.16;
 
@@ -1349,8 +1346,11 @@  GLIBCXX_3.4.18 {
 
 GLIBCXX_3.4.19 {
 
-    # std::chrono::steady_clock::now()
-    _ZNSt6chrono12steady_clock3nowEv;
+    # chrono second generation
+    _ZNSt6chrono3_V212steady_clock3nowEv;
+    _ZNSt6chrono3_V212steady_clock9is_steadyE;
+    _ZNSt6chrono3_V212system_clock3nowEv;
+    _ZNSt6chrono3_V212system_clock9is_steadyE;
 
 } GLIBCXX_3.4.18;
 
diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 7111319..19ea8c1 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -686,6 +686,15 @@  _GLIBCXX_END_NAMESPACE_VERSION
 		 const time_point<_Clock, _Dur2>& __rhs)
       { return !(__lhs < __rhs); }
 
+    // To support the (forward) evolving definition of the library's
+    // defined clocks, wrap inside inline namespace so that these
+    // types are uniquely mangled. This way, new code can use the
+    // current clocks, while the library can contain old definitions.
+    // At some point, when these clocks settle down, the inlined
+    // namespaces can be removed.
+    // XXX GLIBCXX_ABI Deprecated
+    inline namespace _V2 {
+
     /// system_clock
     struct system_clock
     {
@@ -742,10 +751,18 @@  _GLIBCXX_END_NAMESPACE_VERSION
       now() noexcept;
     };
 #else
-    typedef system_clock steady_clock;
+    // Forward declare only.
+    struct steady_clock;
 #endif
 
+#ifdef _GLIBCXX_USE_CLOCK_REALTIME
     typedef system_clock high_resolution_clock;
+#else
+    // Forward declare only.
+    struct high_resolution_clock;
+#endif
+
+  } // end inline namespace _V2
 
   _GLIBCXX_END_NAMESPACE_VERSION
   } // namespace chrono
diff --git a/libstdc++-v3/src/c++11/chrono.cc b/libstdc++-v3/src/c++11/chrono.cc
index 15de4eb..219d674 100644
--- a/libstdc++-v3/src/c++11/chrono.cc
+++ b/libstdc++-v3/src/c++11/chrono.cc
@@ -23,28 +23,17 @@ 
 // <http://www.gnu.org/licenses/>.
 
 #include <bits/c++config.h>
-
-#ifndef _GLIBCXX_USE_CLOCK_MONOTONIC
-// If !_GLIBCXX_USE_CLOCK_MONOTONIC, std::chrono::steady_clock
-// is just a typedef to std::chrono::system_clock, for ABI compatibility
-// force it not to be a typedef now and export it anyway.  Programs
-// using the headers where it is a typedef will actually just use
-// std::chrono::system_clock instead, and this remains here just as a fallback.
-#define _GLIBCXX_USE_CLOCK_MONOTONIC
-#include <chrono>
-#undef _GLIBCXX_USE_CLOCK_MONOTONIC
-#else
 #include <chrono>
-#endif
 
 #ifdef _GLIBCXX_USE_C99_STDINT_TR1
 
-// conditional inclusion of sys/time.h for gettimeofday
+// Conditional inclusion of sys/time.h for gettimeofday
 #if !defined(_GLIBCXX_USE_CLOCK_MONOTONIC) && \
     !defined(_GLIBCXX_USE_CLOCK_REALTIME) && \
      defined(_GLIBCXX_USE_GETTIMEOFDAY)
 #include <sys/time.h>
 #endif
+
 #ifdef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL
 #include <unistd.h>
 #include <sys/syscall.h>
@@ -56,7 +45,9 @@  namespace std _GLIBCXX_VISIBILITY(default)
   {
   _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
-#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
+    // XXX GLIBCXX_ABI Deprecated
+    inline namespace _V2 {
+
     constexpr bool system_clock::is_steady;
 
     system_clock::time_point
@@ -83,11 +74,9 @@  namespace std _GLIBCXX_VISIBILITY(default)
       return system_clock::from_time_t(__sec);
 #endif
     }
-#endif
+
     
-#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
     constexpr bool steady_clock::is_steady;
-#endif
 
 #if defined(_GLIBCXX_SYMVER_GNU) && defined(_GLIBCXX_SHARED) \
     && defined(_GLIBCXX_HAVE_AS_SYMVER_DIRECTIVE) \
@@ -113,6 +102,8 @@  namespace std _GLIBCXX_VISIBILITY(default)
 #endif
     }
 
+  } // end inline namespace _V2
+
   _GLIBCXX_END_NAMESPACE_VERSION
   } // namespace chrono
 } // namespace std
diff --git a/libstdc++-v3/src/c++11/compatibility-chrono.cc b/libstdc++-v3/src/c++11/compatibility-chrono.cc
index ada0946..60d2108 100644
--- a/libstdc++-v3/src/c++11/compatibility-chrono.cc
+++ b/libstdc++-v3/src/c++11/compatibility-chrono.cc
@@ -22,37 +22,84 @@ 
 // see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 // <http://www.gnu.org/licenses/>.
 
-#define _GLIBCXX_COMPATIBILITY_CXX0X
 #include <bits/c++config.h>
 
-#if defined(_GLIBCXX_SYMVER_GNU) && defined(_GLIBCXX_SHARED) \
-    && defined(_GLIBCXX_HAVE_AS_SYMVER_DIRECTIVE)\
-    && defined(_GLIBCXX_HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT)
+#ifdef _GLIBCXX_USE_C99_STDINT_TR1
 
+#ifdef _GLIBCXX_USE_GETTIMEOFDAY
+#include <sys/time.h>
+#endif
+
+#define system_clock system_clockXX
 #define steady_clock steady_clockXX
-#include "chrono.cc"
-
-// The rename syntax for default exported names is
-//   asm (".symver name1,exportedname@GLIBCXX_3.4.17")
-//   asm (".symver name2,exportedname@@GLIBCXX_3.4.19")
-// In the future, GLIBCXX_ABI > 6 should remove all uses of
-// _GLIBCXX_*_SYMVER macros in this file.
-
-#define _GLIBCXX_3_4_17_SYMVER(XXname, name) \
-   extern "C" void \
-   _X##name() \
-   __attribute__ ((alias(#XXname))); \
-   asm (".symver " "_X" #name "," #name "@GLIBCXX_3.4.17");
-
-#define _GLIBCXX_3_4_19_SYMVER(XXname, name) \
-   extern "C" void \
-   _Y##name() \
-   __attribute__ ((alias(#XXname))); \
-   asm (".symver " "_Y" #name  "," #name "@@GLIBCXX_3.4.19");
-
-_GLIBCXX_3_4_17_SYMVER(_ZNSt6chrono14steady_clockXX3nowEv,
-		       _ZNSt6chrono12steady_clock3nowEv)
-_GLIBCXX_3_4_19_SYMVER(_ZNSt6chrono14steady_clockXX3nowEv,
-		       _ZNSt6chrono12steady_clock3nowEv)
+#include <chrono>
+#undef system_clock
+#undef steady_clock
 
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+  namespace chrono
+  {
+  _GLIBCXX_BEGIN_NAMESPACE_VERSION
+ 
+    // NB: Default configuration was no realtime.
+    struct system_clock
+    {
+#ifdef _GLIBCXX_USE_GETTIMEOFDAY
+      typedef chrono::microseconds    				duration;
+#else
+      typedef chrono::seconds	      				duration;
 #endif
+
+      typedef duration::rep    					rep;
+      typedef duration::period 					period;
+      typedef chrono::time_point<system_clock, duration> 	time_point;
+
+      static_assert(system_clock::duration::min()
+		    < system_clock::duration::zero(),
+		    "a clock's minimum duration cannot be less than its epoch");
+
+      static constexpr bool is_steady = false;
+
+      static time_point
+      now() noexcept;
+
+      // Map to C API
+      static std::time_t
+      to_time_t(const time_point& __t) noexcept
+      {
+	return std::time_t(duration_cast<chrono::seconds>
+			   (__t.time_since_epoch()).count());
+      }
+
+      static time_point
+      from_time_t(std::time_t __t) noexcept
+      {
+	typedef chrono::time_point<system_clock, seconds>	__from;
+	return time_point_cast<system_clock::duration>
+	       (__from(chrono::seconds(__t)));
+      }
+    };
+
+    constexpr bool system_clock::is_steady;
+
+    system_clock::time_point
+    system_clock::now() noexcept
+    {
+#ifdef _GLIBCXX_USE_GETTIMEOFDAY
+      timeval tv;
+      // EINVAL, EFAULT
+      gettimeofday(&tv, 0);
+      return time_point(duration(chrono::seconds(tv.tv_sec)
+				 + chrono::microseconds(tv.tv_usec)));
+#else
+      std::time_t __sec = std::time(0);
+      return system_clock::from_time_t(__sec);
+#endif
+    }
+
+  _GLIBCXX_END_NAMESPACE_VERSION
+  } // namespace chrono
+} // namespace std
+
+#endif // _GLIBCXX_USE_C99_STDINT_TR1