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.

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.

Reply via email to