[PATCH] PR libstdc++/80538 Only call sleep for non-zero values

[Jonathan Wakely]

On 11/10/18 17:37 +0100, Jonathan Wakely wrote:
>Avoid a system call when no sleep is required. Sleep in a loop (actually
>two loops) to handle interruption by signals.
>
>	PR libstdc++/80538
>	* src/c++11/thread.cc (this_thread::__sleep_for)
>	[_GLIBCXX_HAVE_SLEEP]: Only call sleep for non-zero values.
>	Loop while sleep call is interrupted and until steady_clock
>	shows requested duration has elapsed.
>	(!_GLIBCXX_HAVE_USLEEP]: Use the _GLIBCXX_HAVE_SLEEP code path, but
>	avoiding the usleep call.
>	* testsuite/30_threads/this_thread/60421.cc: Test repeated
>	signal interruptions.
>
>Tested x86_64-linux (by manually fudging the configure macros to test
>the !_GLIBCXX_USE_NANOSLEEP paths).
>
>Committed to trunk.
>
>

>commit 7cad0b3cbb85a78dce40535f897ba27886469da9
>Author: Jonathan Wakely <jwakely@redhat.com>
>Date:   Fri Apr 28 17:43:25 2017 +0100
>
>    PR libstdc++/80538 Only call sleep for non-zero values
>
>    Avoid a system call when no sleep is required. Sleep in a loop (actually
>    two loops) to handle interruption by signals.
>
>            PR libstdc++/80538
>            * src/c++11/thread.cc (this_thread::__sleep_for)
>            [_GLIBCXX_HAVE_SLEEP]: Only call sleep for non-zero values.
>            Loop while sleep call is interrupted and until steady_clock
>            shows requested duration has elapsed.
>            (!_GLIBCXX_HAVE_USLEEP]: Use the _GLIBCXX_HAVE_SLEEP code path, but
>            avoiding the usleep call.
>            * testsuite/30_threads/this_thread/60421.cc: Test repeated
>            signal interruptions.
>
>diff --git a/libstdc++-v3/src/c++11/thread.cc b/libstdc++-v3/src/c++11/thread.cc
>index c62cb71bf99..564eae6f166 100644
>--- a/libstdc++-v3/src/c++11/thread.cc
>+++ b/libstdc++-v3/src/c++11/thread.cc
>@@ -194,18 +194,35 @@ namespace this_thread
>     while (::nanosleep(&__ts, &__ts) == -1 && errno == EINTR)
>       { }
> #elif defined(_GLIBCXX_HAVE_SLEEP)
>-# ifdef _GLIBCXX_HAVE_USLEEP
>-    ::sleep(__s.count());
>-    if (__ns.count() > 0)
>+    const auto target = chrono::steady_clock::now() + __s + __ns;
>+    while (true)
>       {
>-        long __us = __ns.count() / 1000;
>-        if (__us == 0)
>-          __us = 1;
>-        ::usleep(__us);
>-      }
>+	unsigned secs = __s.count();
>+	if (__ns.count() > 0)
>+	  {
>+# ifdef _GLIBCXX_HAVE_USLEEP
>+	    long us = __ns.count() / 1000;
>+	    if (us == 0)
>+	      us = 1;
>+	    ::usleep(us);
> # else
>-    ::sleep(__s.count() + (__ns.count() >= 1000000));
>+	    if (__ns.count() > 1000000 || secs == 0)
>+	      ++secs; // No sub-second sleep function, so round up.
> # endif

I wonder if this would be worthwhile:

--- a/libstdc++-v3/src/c++11/thread.cc
+++ b/libstdc++-v3/src/c++11/thread.cc
@@ -208,6 +208,8 @@ namespace this_thread
 # else
            if (__ns.count() > 1000000 || secs == 0)
              ++secs; // No sub-second sleep function, so round up.
+           else
+             __gthread_yield();
 # endif
          }

That way for durations smaller than 1ms we give other threads a chance
to run, instead of sleeping for the __s seconds, looping back to this
test, and then sleeping an entire extra second.

But I'm assuming that systems with no usleep are probably rare, and
can live with rounding up to sleep for a full second.