On Monday 08 October 2018 at 22:06:10 -0400, Daniel Eischen wrote: > > > On Oct 8, 2018, at 1:43 PM, enh <e...@google.com> wrote: > > > >> On Sat, Oct 6, 2018 at 12:09 PM Mike Crowe <theopengr...@mac.mcrowe.com> > >> wrote: > >> > >>> On Wednesday 12 September 2018 at 13:47:59 -0700, Tom Cherry wrote: > >>>> On Wed, Sep 12, 2018 at 11:56 AM Mike Crowe > >>>> <theopengr...@mac.mcrowe.com> wrote: > >>>>> > >>>>> We (libc team for Android) have observed this problem too and have > >>>>> seen it cause issues on multiple occasions, enough times that we > >>>>> actually submitted a workaround that always converts the input > >>>>> timespec to CLOCK_MONOTONIC and waits with that clock. Of course > >>>>> this only ameliorates the problem, but does not completely solve it, > >>>>> since there is still a period in time before the conversion to > >>>>> CLOCK_MONOTONIC happens, that if CLOCK_REALTIME changes abruptly, > >>>>> we're back to the same bad behavior as before. > >>>>> > >>>>> For this reason, we added pthread_cond_timedwait_monotonic_np() back > >>>>> in the P release of Android along with equivalent functions for the > >>>>> rest of the timed waits. I've also pushed various patches to libc++, > >>>>> one that builds upon these changes to our libc and one that > >>>>> implements std::condition_variable from scratch, borrowing the > >>>>> implementation from our libc, however neither made progress. > >>>>> > >>>>> As for having a separate clock id parameter, we thought about that > >>>>> during the review of the above changes, but we both thought that 1) > >>>>> there's no strong reason to wait on any clock other than > >>>>> CLOCK_MONOTONIC for these functions and 2) the underlying futex > >>>>> implementation only allows CLOCK_REALTIME and CLOCK_MONOTONIC, so we'd > >>>>> have to do conversions in userspace and inevitably wait on > >>>>> CLOCK_MONOTONIC anyway, so it's best to be clear and only give > >>>>> CLOCK_MONOTONIC as an option. > >>>> > > I'd rather see the clock id passed in as it's more generally useful. By > using _monotonic, it implies that there are only 2 clocks that matter, > the default clock is widely chosen wrong, and that you can't change it on > a per-thread basis, not that you want other clock options.
The Linux futex system call currently has a single bit to switch between CLOCK_REALTIME and CLOCK_MONOTONIC. There's nothing so stop it adding support for other clocks, but doing so would require even more messing about than futex already has. > I can't change the c++ standard, but it doesn't seem like an elegant > solution to add a lot of duplicate _monotonic functions that serve only > the monotonic clock. I admit, I am not familiar with the history behind > this. The C++11 standard provides three clocks: system_clock, high_resolution_clock and steady_clock. In libstdc++ the second is simply an alias for the first. system_clock uses CLOCK_REALTIME and steady_clock uses CLOCK_MONOTONIC. C++20 adds more clocks (but I suspect that libstdc++ will end up treating most of them as aliases for system_clock too.) However, there's nothing to stop implementations or even client code creating new clocks. Time points measured against these clocks can be passed to std::condition_variable::wait_until etc. The current implementations convert these time points to system_clock (slightly inaccurately since it's not possible to get the current time on two different clocks at exactly the same time.) With proper support for CLOCK_MONOTONIC, I think that conversion to steady_clock would be more appropriate. > It is conceivable that you could wait on more than just the real-time and > monotonic clocks. FreeBSD, at least via pthread_condattr_setclock(), > allows you to wait on _REALTIME, _VIRTUAL, _PROF, and _MONOTONIC clocks. > In addition to those, it defines several more. Solaris defines 6 clocks, > though I'm not sure how many are waitable. All of these clocks could be supported in a way that's compatible with the C++ standard. If pthread_cond_timedwaitonclock or similar existed, then it would even be possible for an implementation to support a way to convert a clock type to a clockid_t at compile time so that direct support for new clocks could be added without modifying the implementations standard library code. For example: template <typename Clock> constexpr clockid_t __clockid_for_clock(); Such an implementation could fall back at both compile time and, if necessary, at run time to converting the timeout to a different clock if required. So, my favourite solution is to invent an equivalent of pthread_cond_timedwait that accepts a clockid_t since it feels more future-proof, but adding the Android pthread_cond_timedwait_monotonic instead would solve my current problems. Mike.