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.
