| Message ID | 20181011163751.GA8203@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR libstdc++/80538 Only call sleep for non-zero values | expand |
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.
On 11 October 2018 23:36:15 CEST, Jonathan Wakely <jwakely@redhat.com> wrote: >But I'm assuming that systems with no usleep are probably rare, and >can live with rounding up to sleep for a full second. Well conforming implementations usually won't have usleep which was obscolencent in SUSv3 and removed from SUSv4. As you certainly know we have clock_nanosleep / nanosleep nowadays. thanks,
On Sat, 13 Oct 2018 at 08:49, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > > On 11 October 2018 23:36:15 CEST, Jonathan Wakely <jwakely@redhat.com> wrote: > > >But I'm assuming that systems with no usleep are probably rare, and > >can live with rounding up to sleep for a full second. > > Well conforming implementations usually won't have usleep which was obscolencent in SUSv3 and removed from SUSv4. > As you certainly know we have clock_nanosleep / nanosleep nowadays. Yes, of course. We don't even look for usleep unless nanosleep isn't available, so I mean systems with no nanosleep *and* no usleep.
On 13 October 2018 12:56:14 CEST, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: >Yes, of course. We don't even look for usleep unless nanosleep isn't >available, so I mean systems with no nanosleep *and* no usleep. Right. Wasn't obvious to me from just the patch. Sorry for the noise..
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 + } + + if (secs > 0) + { + // Sleep in a loop to handle interruption by signals: + while ((secs = ::sleep(secs))) + { } + } + const auto now = chrono::steady_clock::now(); + if (now >= target) + break; + __s = chrono::duration_cast<chrono::seconds>(target - now); + __ns = chrono::duration_cast<chrono::nanoseconds>(target - (now + __s)); + } #elif defined(_GLIBCXX_HAVE_WIN32_SLEEP) unsigned long ms = __ns.count() / 1000000; if (__ns.count() > 0 && ms == 0) diff --git a/libstdc++-v3/testsuite/30_threads/this_thread/60421.cc b/libstdc++-v3/testsuite/30_threads/this_thread/60421.cc index e7d69a49fb6..cd8d2fdd6f3 100644 --- a/libstdc++-v3/testsuite/30_threads/this_thread/60421.cc +++ b/libstdc++-v3/testsuite/30_threads/this_thread/60421.cc @@ -53,10 +53,19 @@ test02() sleeping = true; std::this_thread::sleep_for(time); result = std::chrono::system_clock::now() >= (start + time); + sleeping = false; }); - while (!sleeping) { } - std::this_thread::sleep_for(std::chrono::milliseconds(500)); - pthread_kill(t.native_handle(), SIGUSR1); + while (!sleeping) + { + // Wait for the thread to start sleeping. + } + while (sleeping) + { + // The sleeping thread should finish eventually, + // even if continually interrupted after less than a second: + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + pthread_kill(t.native_handle(), SIGUSR1); + } t.join(); VERIFY( result ); }
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.