On Tue, Oct 9, 2018 at 6:19 AM Mike Crowe <theopengr...@mac.mcrowe.com> wrote: > > 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[1]. 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[2] 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[3] and one that > > >>>>> implements std::condition_variable from scratch, borrowing the > > >>>>> implementation from our libc[4], 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.
That is literally what we intended when we added these functions. I'm biased here, but Linux and therefore Android only have two clocks that can be waited on: CLOCK_REALTIME and CLOCK_MONOTONIC. In our experience, developers virtually never intend on waiting on CLOCK_REALTIME despite that being the default option. > > 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. Despite my above intentions, I completely understand why POSIX widely would want a function that takes a clockid_t and fully support that idea. That would solve our issues too and if we can get libc++ to properly wait on CLOCK_MONOTONIC, then that's a huge step forward. > > Mike.
