Re: Adding a clockid_t parameter to functions that accept absolute timespecs (was Re: C++ condition_variable and timed_mutex with steady_clock and pthreads)

2018-10-29 Thread Tom Cherry
On Sat, Oct 27, 2018 at 5:16 PM Mark Harris  wrote:
>
> On Sat, 27 Oct 2018 at 13:14, Mike Crowe  wrote:
>>
>> On Saturday 27 October 2018 at 15:26:07 -0400, Daniel Eischen wrote:
>> >
>> > > On Oct 27, 2018, at 12:14 PM, Mike Crowe  
>> > > wrote:
>> > > Looking through POSIX Base Specifications Issue 7, I
>> > > believe that the following other functions lack a way to use
>> > > CLOCK_MONOTONIC directly (and unlike pthread_cond_timedwait, I can see no
>> > > other way to make them do so):
>> > >
>> > > sem_timedwait
>> > > pthread_mutex_timedlock
>> > > pthread_rwlock_timedrdlock
>> > > pthread_rwlock_timedwrlock
>> > > mq_timedreceive
>> > > mq_timedsend
>> > > posix_trace_timedgetnext_event

The two message queue functions are directly implemented with a system
call on Linux at least, so I'm not sure that we should make new
versions of these unless we get the underlying system call to accept
different clocks.  I believe the posix tracing functions are marked as
obsolescent, so it seems to be best to leave them alone as well.

I agree with the creating these same variants for other four functions.

>> > >
>> > > (Many other functions that take a struct timespec treat it as a relative
>> > > timeout.) Of these, I've also found it necessary to implement a version 
>> > > of
>> > > sem_timedwait that used CLOCK_MONOTONIC.
>> > >
>> > > I originally named my function pthread_cond_timedwaitonclock since it
>> > > seemed that running the words together matched what was happening with
>> > > "timedwait". Others suggested that it should be
>> > > pthread_cond_timedwait_on_clock. I've tried to apply that style in the
>> > > function prototypes below.
>> >
>> > I think at least the "on_clock" should be "onclock" or just "clock" for 
>> > the same reason timedwait is not timed_wait.  Perhaps "_cid" for clock_id?
>> >
>> > Also, "clock_" is prefixed to nanosleep for similar functionality, perhaps 
>> > pthread_clock_cond_timedwait, etc instead of suffixing with "_clock" or 
>> > "_onclock"?
>>
>> I think that the clock prefix has some benefits. This would mean:
>>
>>  sem_clock_timedwait
>>  pthread_clock_mutex_timedlock
>>  pthread_clock_rwlock_timedrdlock
>>  pthread_clock_rwlock_timedwrlock
>>  mq_clock_timedreceive
>>  mq_clock_timedsend
>>  posix_clock_trace_timedgetnext_event
>>  pthread_clock_cond_timedwait
>>
>> Although, if you consider the prefix to contain multiple words then they
>> would be:
>>
>>  sem_clock_timedwait
>>  pthread_mutex_clock_timedlock
>>  pthread_rwlock_clock_timedrdlock
>>  pthread_rwlock_clock_timedwrlock
>>  mq_clock_timedreceive
>>  mq_clock_timedsend
>>  posix_trace_clock_timedgetnext_event
>>  pthread_cond_clock_timedwait
>
>
> How about just replacing "timed" with "clock"?
>
>  sem_clockwait
>  pthread_mutex_clocklock
>  pthread_rwlock_clockrdlock
>  pthread_rwlock_clockwrlock
>  mq_clockreceive
>  mq_clocksend
>  posix_trace_clockgetnext_event
>  pthread_cond_clockwait
>
> To me these names seem more appropriate, given that these calls wait until 
> the specified clock reaches the specified absolute value.  If the clock can 
> be set, or if an implementation were to support using these with, say, 
> CPUTIME clocks, then "timed" does not seem correct since it is not the time 
> taken by the call but rather the behavior of the specified clock that is 
> significant.
>
>  - Mark

pthread_cond_timedwait_onclock() is my top suggestion.  It makes it
clear that this is a modification to the parent
pthread_cond_timedwait() functions and that its semantics should be
identical with the change that it waits on the provided clock as
opposed to the one used during initialization.

I agree with moving this forward to the next required step.  We can
trivially implement them in the Android libc.



Re: Adding a clockid_t parameter to functions that accept absolute timespecs (was Re: C++ condition_variable and timed_mutex with steady_clock and pthreads)

2018-10-27 Thread Mark Harris
On Sat, 27 Oct 2018 at 13:14, Mike Crowe 
wrote:

> On Saturday 27 October 2018 at 15:26:07 -0400, Daniel Eischen wrote:
> >
> > > On Oct 27, 2018, at 12:14 PM, Mike Crowe 
> wrote:
> > > Looking through POSIX Base Specifications Issue 7, I
> > > believe that the following other functions lack a way to use
> > > CLOCK_MONOTONIC directly (and unlike pthread_cond_timedwait, I can see
> no
> > > other way to make them do so):
> > >
> > > sem_timedwait
> > > pthread_mutex_timedlock
> > > pthread_rwlock_timedrdlock
> > > pthread_rwlock_timedwrlock
> > > mq_timedreceive
> > > mq_timedsend
> > > posix_trace_timedgetnext_event
> > >
> > > (Many other functions that take a struct timespec treat it as a
> relative
> > > timeout.) Of these, I've also found it necessary to implement a
> version of
> > > sem_timedwait that used CLOCK_MONOTONIC.
> > >
> > > I originally named my function pthread_cond_timedwaitonclock since it
> > > seemed that running the words together matched what was happening with
> > > "timedwait". Others suggested that it should be
> > > pthread_cond_timedwait_on_clock. I've tried to apply that style in the
> > > function prototypes below.
> >
> > I think at least the "on_clock" should be "onclock" or just "clock" for
> the same reason timedwait is not timed_wait.  Perhaps "_cid" for clock_id?
> >
> > Also, "clock_" is prefixed to nanosleep for similar functionality,
> perhaps pthread_clock_cond_timedwait, etc instead of suffixing with
> "_clock" or "_onclock"?
>
> I think that the clock prefix has some benefits. This would mean:
>
>  sem_clock_timedwait
>  pthread_clock_mutex_timedlock
>  pthread_clock_rwlock_timedrdlock
>  pthread_clock_rwlock_timedwrlock
>  mq_clock_timedreceive
>  mq_clock_timedsend
>  posix_clock_trace_timedgetnext_event
>  pthread_clock_cond_timedwait
>
> Although, if you consider the prefix to contain multiple words then they
> would be:
>
>  sem_clock_timedwait
>  pthread_mutex_clock_timedlock
>  pthread_rwlock_clock_timedrdlock
>  pthread_rwlock_clock_timedwrlock
>  mq_clock_timedreceive
>  mq_clock_timedsend
>  posix_trace_clock_timedgetnext_event
>  pthread_cond_clock_timedwait
>

How about just replacing "timed" with "clock"?

 sem_clockwait
 pthread_mutex_clocklock
 pthread_rwlock_clockrdlock
 pthread_rwlock_clockwrlock
 mq_clockreceive
 mq_clocksend
 posix_trace_clockgetnext_event
 pthread_cond_clockwait

To me these names seem more appropriate, given that these calls wait until
the specified clock reaches the specified absolute value.  If the clock can
be set, or if an implementation were to support using these with, say,
CPUTIME clocks, then "timed" does not seem correct since it is not the time
taken by the call but rather the behavior of the specified clock that is
significant.

 - Mark


Re: Adding a clockid_t parameter to functions that accept absolute timespecs (was Re: C++ condition_variable and timed_mutex with steady_clock and pthreads)

2018-10-27 Thread Mike Crowe
On Saturday 27 October 2018 at 17:07:00 -0400, Daniel Eischen wrote:
> The nice thing about being able to set the clock in an attribute or some
> other way (e.g., pthread_set_preferred_clock) is that you only have to do
> it once, as opposed to potentially modifying lots of other code, even in
> libraries outside your scope. If you're writing a library that waits for
> events for the caller, you don't know what clock to use unless you add it
> to your API. Allowing the clock to be set on a per-thread (or even
> process) basis, might make things easier. Absolute times wouldn't really
> be helped by this though, as you still need to get the time for the
> correct clock.

I've written another blog post[1] that explains why the clock to use is a
property of the wait, although it doesn't address your suggestion directly.
The C++ standard library behaves as if "struct timespec" also contained
clockid_t, although the clock is part of the type so the compiler handles
it with no runtime overhead. This means that C++ libraries can be clock
agnostic.

I think that being able to set these things for a process or a thread makes
writing generic libraries harder. Some libraries may actively want to
choose which clock to use since their use of condition variables is
entirely invisible to the calling application. Others may want to expose
the clock to use. At the moment, neither is really possible for anything
but condition variables, and condition variables need to know the clock to
use in advance.

Thanks.

Mike.

[1] 
http://randombitsofuselessinformation.blogspot.com/2018/10/the-clock-used-for-waiting-on-condition.html



Re: Adding a clockid_t parameter to functions that accept absolute timespecs (was Re: C++ condition_variable and timed_mutex with steady_clock and pthreads)

2018-10-27 Thread Daniel Eischen


> On Oct 27, 2018, at 4:13 PM, Mike Crowe  wrote:
> 
>> On Saturday 27 October 2018 at 15:26:07 -0400, Daniel Eischen wrote:
>> 
>>> On Oct 27, 2018, at 12:14 PM, Mike Crowe  
>>> wrote:
>>> Looking through POSIX Base Specifications Issue 7, I
>>> believe that the following other functions lack a way to use
>>> CLOCK_MONOTONIC directly (and unlike pthread_cond_timedwait, I can see no
>>> other way to make them do so):
>>> 
>>> sem_timedwait
>>> pthread_mutex_timedlock
>>> pthread_rwlock_timedrdlock
>>> pthread_rwlock_timedwrlock
>>> mq_timedreceive
>>> mq_timedsend
>>> posix_trace_timedgetnext_event
>>> 
>>> (Many other functions that take a struct timespec treat it as a relative
>>> timeout.) Of these, I've also found it necessary to implement a version of
>>> sem_timedwait that used CLOCK_MONOTONIC.
>>> 
>>> I originally named my function pthread_cond_timedwaitonclock since it
>>> seemed that running the words together matched what was happening with
>>> "timedwait". Others suggested that it should be
>>> pthread_cond_timedwait_on_clock. I've tried to apply that style in the
>>> function prototypes below.
>> 
>> I think at least the "on_clock" should be "onclock" or just "clock" for the 
>> same reason timedwait is not timed_wait.  Perhaps "_cid" for clock_id?
>> 
>> Also, "clock_" is prefixed to nanosleep for similar functionality, perhaps 
>> pthread_clock_cond_timedwait, etc instead of suffixing with "_clock" or 
>> "_onclock"?
> 
> I think that the clock prefix has some benefits. This would mean:
> 
> sem_clock_timedwait
> pthread_clock_mutex_timedlock
> pthread_clock_rwlock_timedrdlock
> pthread_clock_rwlock_timedwrlock
> mq_clock_timedreceive
> mq_clock_timedsend
> posix_clock_trace_timedgetnext_event
> pthread_clock_cond_timedwait
> 
> Although, if you consider the prefix to contain multiple words then they
> would be:
> 
> sem_clock_timedwait
> pthread_mutex_clock_timedlock
> pthread_rwlock_clock_timedrdlock
> pthread_rwlock_clock_timedwrlock
> mq_clock_timedreceive
> mq_clock_timedsend
> posix_trace_clock_timedgetnext_event
> pthread_cond_clock_timedwait

Either one of those sounds good to me, I might prefer the latter if I had to 
choose.

>> There is still the ability to set the clock in the condition variable
>> attribute.  Should this be deprecated in lieu of the other functionality?
>> If not, which one takes precedence?  Should mutex attributes also grow
>> the ability to set the clock?  For the second question, I'd assume that
>> anything explicitly set in an "onclock" variant would override what was
>> specified in the attribute, or perhaps return an error instead.
> 
> I think it would be best for a supplied clock to override one set in the
> attributes. Returning an error would require extra state to be kept and
> checked, which would unnecessary complicate the implementation.
> 
> I don't think there's any hurry to deprecate the attribute, although that
> may make sense eventually.
> 
> I don't see any particular reason to add a clock attribute for mutexes if
> the above functions are added. Anyone who finds it laborious to pass the
> extra parameter can use a macro or wrapper function.

The nice thing about being able to set the clock in an attribute or some other 
way (e.g., pthread_set_preferred_clock) is that you only have to do it once, as 
opposed to potentially modifying lots of other code, even in libraries outside 
your scope.  If you're writing a library that waits for events for the caller, 
you don't know what clock to use unless you add it to your API.  Allowing the 
clock to be set on a per-thread (or even process) basis, might make things 
easier.  Absolute times wouldn't really be helped by this though, as you still 
need to get the time for the correct clock.

--
DE



Adding a clockid_t parameter to functions that accept absolute timespecs (was Re: C++ condition_variable and timed_mutex with steady_clock and pthreads)

2018-10-27 Thread Mike Crowe
On Saturday 27 October 2018 at 15:26:07 -0400, Daniel Eischen wrote:
> 
> > On Oct 27, 2018, at 12:14 PM, Mike Crowe  
> > wrote:
> > Looking through POSIX Base Specifications Issue 7, I
> > believe that the following other functions lack a way to use
> > CLOCK_MONOTONIC directly (and unlike pthread_cond_timedwait, I can see no
> > other way to make them do so):
> > 
> > sem_timedwait
> > pthread_mutex_timedlock
> > pthread_rwlock_timedrdlock
> > pthread_rwlock_timedwrlock
> > mq_timedreceive
> > mq_timedsend
> > posix_trace_timedgetnext_event
> > 
> > (Many other functions that take a struct timespec treat it as a relative
> > timeout.) Of these, I've also found it necessary to implement a version of
> > sem_timedwait that used CLOCK_MONOTONIC.
> > 
> > I originally named my function pthread_cond_timedwaitonclock since it
> > seemed that running the words together matched what was happening with
> > "timedwait". Others suggested that it should be
> > pthread_cond_timedwait_on_clock. I've tried to apply that style in the
> > function prototypes below.
> 
> I think at least the "on_clock" should be "onclock" or just "clock" for the 
> same reason timedwait is not timed_wait.  Perhaps "_cid" for clock_id?
> 
> Also, "clock_" is prefixed to nanosleep for similar functionality, perhaps 
> pthread_clock_cond_timedwait, etc instead of suffixing with "_clock" or 
> "_onclock"?

I think that the clock prefix has some benefits. This would mean:

 sem_clock_timedwait
 pthread_clock_mutex_timedlock
 pthread_clock_rwlock_timedrdlock
 pthread_clock_rwlock_timedwrlock
 mq_clock_timedreceive
 mq_clock_timedsend
 posix_clock_trace_timedgetnext_event
 pthread_clock_cond_timedwait

Although, if you consider the prefix to contain multiple words then they
would be:

 sem_clock_timedwait
 pthread_mutex_clock_timedlock
 pthread_rwlock_clock_timedrdlock
 pthread_rwlock_clock_timedwrlock
 mq_clock_timedreceive
 mq_clock_timedsend
 posix_trace_clock_timedgetnext_event
 pthread_cond_clock_timedwait

> There is still the ability to set the clock in the condition variable
> attribute.  Should this be deprecated in lieu of the other functionality?
> If not, which one takes precedence?  Should mutex attributes also grow
> the ability to set the clock?  For the second question, I'd assume that
> anything explicitly set in an "onclock" variant would override what was
> specified in the attribute, or perhaps return an error instead.

I think it would be best for a supplied clock to override one set in the
attributes. Returning an error would require extra state to be kept and
checked, which would unnecessary complicate the implementation.

I don't think there's any hurry to deprecate the attribute, although that
may make sense eventually.

I don't see any particular reason to add a clock attribute for mutexes if
the above functions are added. Anyone who finds it laborious to pass the
extra parameter can use a macro or wrapper function.

Thanks.

Mike.



Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-10-27 Thread Daniel Eischen


> On Oct 27, 2018, at 12:14 PM, Mike Crowe  wrote:
> 
> On Tue, Oct 9, 2018 at 6:19 AM Mike Crowe  wrote:
>>> 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.
> 
>> On Tuesday 09 October 2018 at 10:16:38 -0700, Tom Cherry wrote:
>> 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.
> 
> So, where do we go from here? I believe that I've responded to everyone
> that raised objections.
> 
> I've read through the documents I could find on the Open Group web site,
> but it's not clear to me what we need to do next.
> 
> Would it be better to propose the complete set of functions that are
> missing the ability to wait on CLOCK_MONOTONIC in one go, along with
> standard wording?

I would tend to agree.

> Looking through POSIX Base Specifications Issue 7, I
> believe that the following other functions lack a way to use
> CLOCK_MONOTONIC directly (and unlike pthread_cond_timedwait, I can see no
> other way to make them do so):
> 
> sem_timedwait
> pthread_mutex_timedlock
> pthread_rwlock_timedrdlock
> pthread_rwlock_timedwrlock
> mq_timedreceive
> mq_timedsend
> posix_trace_timedgetnext_event
> 
> (Many other functions that take a struct timespec treat it as a relative
> timeout.) Of these, I've also found it necessary to implement a version of
> sem_timedwait that used CLOCK_MONOTONIC.
> 
> I originally named my function pthread_cond_timedwaitonclock since it
> seemed that running the words together matched what was happening with
> "timedwait". Others suggested that it should be
> pthread_cond_timedwait_on_clock. I've tried to apply that style in the
> function prototypes below.

I think at least the "on_clock" should be "onclock" or just "clock" for the 
same reason timedwait is not timed_wait.  Perhaps "_cid" for clock_id?

Also, "clock_" is prefixed to nanosleep for similar functionality, perhaps 
pthread_clock_cond_timedwait, etc instead of suffixing with "_clock" or 
"_onclock"?

There is still the ability to set the clock in the condition variable 
attribute.  Should this be deprecated in lieu of the other functionality? If 
not, which one takes precedence?  Should mutex attributes also grow the ability 
to set the clock?  For the second question, I'd assume that anything explicitly 
set in an "onclock" variant would override what was specified in the attribute, 
or perhaps return an error instead.

--
DE



Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-10-27 Thread Mike Crowe
On Tue, Oct 9, 2018 at 6:19 AM Mike Crowe  wrote:
>> 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.

On Tuesday 09 October 2018 at 10:16:38 -0700, Tom Cherry wrote:
> 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.

So, where do we go from here? I believe that I've responded to everyone
that raised objections.

I've read through the documents I could find on the Open Group web site,
but it's not clear to me what we need to do next.

Would it be better to propose the complete set of functions that are
missing the ability to wait on CLOCK_MONOTONIC in one go, along with
standard wording? Looking through POSIX Base Specifications Issue 7, I
believe that the following other functions lack a way to use
CLOCK_MONOTONIC directly (and unlike pthread_cond_timedwait, I can see no
other way to make them do so):

 sem_timedwait
 pthread_mutex_timedlock
 pthread_rwlock_timedrdlock
 pthread_rwlock_timedwrlock
 mq_timedreceive
 mq_timedsend
 posix_trace_timedgetnext_event

(Many other functions that take a struct timespec treat it as a relative
timeout.) Of these, I've also found it necessary to implement a version of
sem_timedwait that used CLOCK_MONOTONIC.

I originally named my function pthread_cond_timedwaitonclock since it
seemed that running the words together matched what was happening with
"timedwait". Others suggested that it should be
pthread_cond_timedwait_on_clock. I've tried to apply that style in the
function prototypes below.

int sem_timedwait_on_clock(sem_t *restrict sem,
   clockid_t clock,
   const struct timespec *restrict abstime)
int pthread_mutex_timedlock_on_clock(pthread_mutex_t *restrict mutex,
 clockid_t clock,
 const struct timespec *restrict abstime)
int pthread_rwlock_timedrdlock_on_clock(pthread_rwlock_t *restrict rwlock,
clockid_t clock,
const struct timespec *restrict abstime)
int pthread_rwlock_timedwrlock_on_clock(pthread_rwlock_t *restrict rwlock,
clockid_t clock,
const struct timespec *restrict abstime)
int pthread_cond_timedwait_on_clock(pthread_cond_t *restrict cond,
pthread_mutex_t *restrict mutex,
clockid_t clock,
const struct timespec *restrict abstime)
ssize_t mq_timedreceive_on_clock(mqd_t mqdes, char *restrict msg_ptr,
 size_t msg_len,
 unsigned int *restrict msg_prio,
 clockid_t clock,
 const struct timespec *restrict abs_timeout)
int mq_timedsend_on_clock(mqd_t mqdes, const char *restrict msg_ptr,
  size_t msg_len, unsigned int msg_prio,
  clockid_t clock,
  const struct timespec *restrict abs_timeout)
int posix_trace_timedgetnext_event_on_clock(trace_id_t trid,
struct posix_trace_event_info 
*restrict event,
void *restrict data, size_t 
num_bytes,
size_t *restrict data_len,
int *restrict unavailable,
clockid_t clock,
const struct timespec *restrict 
abstime);

If implementors are worried about the extra work required to implement
these new functions then perhaps only CLOCK_REALTIME should be mandatory so
they can just call their original implemenation in that case.

Any guidance as to where we go next gratefully received.

Thanks.

Mike.



Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-10-09 Thread Tom Cherry
On Tue, Oct 9, 2018 at 6:19 AM Mike Crowe  wrote:
>
> On Monday 08 October 2018 at 22:06:10 -0400, Daniel Eischen wrote:
> >
> > > On Oct 8, 2018, at 1:43 PM, enh  wrote:
> > >
> > >> On Sat, Oct 6, 2018 at 12:09 PM Mike Crowe  
> > >> 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 
> >   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 
>  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
> 

Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-10-09 Thread Mike Crowe
On Monday 08 October 2018 at 22:06:10 -0400, Daniel Eischen wrote:
> 
> > On Oct 8, 2018, at 1:43 PM, enh  wrote:
> > 
> >> On Sat, Oct 6, 2018 at 12:09 PM Mike Crowe  
> >> 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 
>   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 
 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.



Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-10-08 Thread Daniel Eischen


> On Oct 8, 2018, at 1:43 PM, enh  wrote:
> 
>> On Sat, Oct 6, 2018 at 12:09 PM Mike Crowe  
>> 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  
 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.  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.

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.

--
DE



Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-10-08 Thread enh
On Sat, Oct 6, 2018 at 12:09 PM Mike Crowe  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  
> > wrote:
> > >
> > > On Wednesday 12 September 2018 at 11:29:26 -0700, Tom Cherry wrote:
> > > > On Sun, Sep 9, 2018 at 8:28 AM Mike Crowe  
> > > > wrote:
> > > > >
> > > > > On Thursday 30 August 2018 at 21:10:48 +0100, Mike Crowe wrote:
> > > > > > C++11's std::condition_variable, std::timed_mutex and
> > > > > > std::recursive_timed_mutex support waiting with a timeout specified 
> > > > > > using
> > > > > > an arbitrary clock. It's common to use std::chrono::steady_clock 
> > > > > > (which
> > > > > > corresponds to CLOCK_MONOTONIC) or std::chrono::system_clock (which
> > > > > > corresponds to CLOCK_REALTIME) for these waits, and other clocks 
> > > > > > end up
> > > > > > being converted to one of those when performing the wait.
> > > > > >
> > > > > > Unfortunately, I don't believe that it's possible to implement 
> > > > > > these on top
> > > > > > of the pthread equivalents correctly for std::chrono::steady_clock 
> > > > > > (i.e.
> > > > > > CLOCK_MONOTONIC) since:
> > > > > >
> > > > > > 1. pthread_mutex_timedlock only supports a time measured against
> > > > > >CLOCK_REALTIME.
> > > > > >
> > > > > > 2. The clock used for pthread_cond_timedwait must be specified when 
> > > > > > the
> > > > > >condition variable is created by pthread_cond_init. The clock to 
> > > > > > be used
> > > > > >for future waits is not known at the point that the
> > > > > >std::condition_variable constructor is called.
> > > > > >
> > > > > > I'm most interested in the std::condition_variable case. since I 
> > > > > > have code
> > > > > > that wants to wait using std::chrono::steady_clock.
> > > > > >
> > > > > > There are a number of possible workarounds for these. I've 
> > > > > > described some
> > > > > > in a blog post[1] and in defect 0001164[2]. But, my favourite 
> > > > > > solution is
> > > > > > to introduce a variant of pthread_cond_timedwait that takes a an 
> > > > > > additional
> > > > > > clockid_t parameter:
> > > > > >
> > > > > >  int pthread_cond_timedwaitonclock(pthread_cond_t *cond,
> > > > > >pthread_mutex_t *mutex,
> > > > > >clockid_t clockid,
> > > > > >const struct timespec 
> > > > > > *abstimeout);
> > > > > >
> > > > > > I've proposed[3] an implementation of this function for glibc (as
> > > > > > pthread_cond_timedwaitonclock_np) and it was suggested that I ought 
> > > > > > to
> > > > > > raise it here. (This led me to enter defect 0001164, but since that 
> > > > > > yielded
> > > > > > no response I've finally got round to writing this email too.)
> > > > > >
> > > > > > The equivalent for mutex would probably be:
> > > > > >
> > > > > >  int pthread_mutex_timedlockonclock(pthread_mutex_t *mutex,
> > > > > > clockid_t clockid,
> > > > > > const struct timespec 
> > > > > > *abstimeout);
> > > > > >
> > > > > > but I've not yet made any attempt to implement it in glibc.
> > > > > >
> > > > > > Would making the C++ standard library implementable on top of POSIX 
> > > > > > be
> > > > > > considered a good enough reason to add such functions?
> > > > > >
> > > > > > Are these functions generic enough or perhaps too generic? Android 
> > > > > > had a
> > > > > > pthread_cond_timedwait_monotonic function for a while, but 
> > > > > > deprecated it
> > > > > > when they added support for pthread_condattr_setclock.
> > > >
> > > > 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 

Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-10-08 Thread Tom Cherry
On Sun, Oct 7, 2018 at 6:09 AM Mike Crowe 
wrote:

> On Saturday 06 October 2018 at 17:43:06 -0400, Daniel Eischen wrote:
> >
> > > On Oct 6, 2018, at 3:00 PM, Mike Crowe 
> wrote:
> > >
> > > Thank you for your detailed responses and patch links. I'm sorry it's
> taken
> > > me so long to respond. I was waiting to see if there were any other
> responses.
> >
> > Aside from what may be needed by the implementation for the proposed C++
> > standard functionality, I don't understand why initializing a mutex or
> > condition variable (via the attribute) with the desired clock is not
> > sufficient.  I can see adding an analogue to pthread_condattr_setclock()
> > for a mutex attribute, but what is the real need to provide both
> > real-time and monotonic sleeps on the same mutex or condition variable?
> > Initialize them once with the clock you want, and be done with it.
>
> For application code written to talk directly to the pthread functions, I
> agree. But for a library, like the C++ standard library, that wishes to
> make the clock to use part of the timeout then this is not possible.
>
> If the choice of clock is distant from the wait (perhaps the condition
> variable is passed into or out of library code) then there is a higher risk
> of the wrong clock being accidentally used, and there is no way to detect
> that automatically at compile time or run time[1]. Libraries, such as the
> C++ library, completely avoid that class of bugs at compile time using
> their type system. A C library could too, if it so wished, but it would
> require separate functions to accept the different clock types.
>

+1 to this.  As I wrote in my commit for Android, I frankly think it's a
better interface to have the clock provided at the same point that the
timespec is provided.  It makes it harder to erroneously provide a timespec
for a different clock than the one used to initialize the condition
variable.

One other reason why the pthread_condattr_setclock() function is not enough
to solve the C++ use case, is that the constructor for
std::condition_variable is defined to be constexpr, so it cannot call any
API functions such as pthread_condattr_setclock().  This could be solved
via a condition variable initializer #define that defaults to using the
monotonic clock.  We also created this in Android in our quest to solve
this issue across API levels [1].  I fully understand if that is too C++
specific for this list to be interested in though.


>
> > I'd almost rather see the timespec changed (controversial much?!) to add
> > the clock id rather than keep adding new interfaces to support
> > waits/sleeps on different clocks.
>
> I think that if we were starting from scratch then that wouldn't be a bad
> plan, and it would support the C++ library use case. I can't see it
> happening now though.
>

Agreed here too that this would be ideal but likely impossible at this
current point.


>
> > Or maybe to provide a hint to the implementation as to what clock to use
> > on a per-thread basis via something like pthread_attr_setpreferredclock()
> > and pthread_setpreferredclock().  If you had these, I suppose you could
> > implement the C++ functionality by saving the current thread's preferred
> > clock, setting it to the desired clock, calling the mutex lock or CV wait
> > function, then restoring the thread's clock before returning to the
> > caller.
>
> That all seems like a lot of overhead of messing about with TLS just to
> influence a single flag passed to the syscall, that may not even end up
> needing to be called. I imagine that the overhead of passing a single extra
> parameter to a function is tiny compared to that, especially when the
> thread's preferred clock is not already in the data cache.
>

Agreed.  That is a novel solution but seems overall more complex without a
benefit than simply passing an additional clock argument or having this
separate set of monotonic waiter functions.


>
> Thanks.
>
> Mike.
>
> [1] Well, I suppose on platforms that have 64-bit time_t, a high bit could
> always be set for CLOCK_MONOTONIC which would allow detection at run time.
>

I'm also new to this list, so I can't help with the verbiage, but it looks
good to me.

Thanks
Tom

[1] https://android-review.googlesource.com/c/platform/bionic/+/349297/


Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-10-07 Thread Mike Crowe
On Saturday 06 October 2018 at 17:43:06 -0400, Daniel Eischen wrote:
> 
> > On Oct 6, 2018, at 3:00 PM, Mike Crowe  wrote:
> > 
> > Thank you for your detailed responses and patch links. I'm sorry it's taken
> > me so long to respond. I was waiting to see if there were any other 
> > responses.
> 
> Aside from what may be needed by the implementation for the proposed C++
> standard functionality, I don't understand why initializing a mutex or
> condition variable (via the attribute) with the desired clock is not
> sufficient.  I can see adding an analogue to pthread_condattr_setclock()
> for a mutex attribute, but what is the real need to provide both
> real-time and monotonic sleeps on the same mutex or condition variable?
> Initialize them once with the clock you want, and be done with it.

For application code written to talk directly to the pthread functions, I
agree. But for a library, like the C++ standard library, that wishes to
make the clock to use part of the timeout then this is not possible.

If the choice of clock is distant from the wait (perhaps the condition
variable is passed into or out of library code) then there is a higher risk
of the wrong clock being accidentally used, and there is no way to detect
that automatically at compile time or run time[1]. Libraries, such as the
C++ library, completely avoid that class of bugs at compile time using
their type system. A C library could too, if it so wished, but it would
require separate functions to accept the different clock types.

> I'd almost rather see the timespec changed (controversial much?!) to add
> the clock id rather than keep adding new interfaces to support
> waits/sleeps on different clocks.

I think that if we were starting from scratch then that wouldn't be a bad
plan, and it would support the C++ library use case. I can't see it
happening now though.

> Or maybe to provide a hint to the implementation as to what clock to use
> on a per-thread basis via something like pthread_attr_setpreferredclock()
> and pthread_setpreferredclock().  If you had these, I suppose you could
> implement the C++ functionality by saving the current thread's preferred
> clock, setting it to the desired clock, calling the mutex lock or CV wait
> function, then restoring the thread's clock before returning to the
> caller.

That all seems like a lot of overhead of messing about with TLS just to
influence a single flag passed to the syscall, that may not even end up
needing to be called. I imagine that the overhead of passing a single extra
parameter to a function is tiny compared to that, especially when the
thread's preferred clock is not already in the data cache.

Thanks.

Mike.

[1] Well, I suppose on platforms that have 64-bit time_t, a high bit could
always be set for CLOCK_MONOTONIC which would allow detection at run time.



Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-10-06 Thread Daniel Eischen



Sent from my iPhone

> On Oct 6, 2018, at 5:43 PM, Daniel Eischen  wrote:
> 
> 
>> On Oct 6, 2018, at 3:00 PM, Mike Crowe  wrote:
>> 
>> Thank you for your detailed responses and patch links. I'm sorry it's taken
>> me so long to respond. I was waiting to see if there were any other 
>> responses.
> 
> Aside from what may be needed by the implementation for the proposed C++ 
> standard functionality, I don't understand why initializing a mutex or 
> condition variable (via the attribute) with the desired clock is not 
> sufficient.  I can see adding an analogue to pthread_condattr_setclock() for 
> a mutex attribute, but what is the real need to provide both real-time and 
> monotonic sleeps on the same mutex or condition variable?  Initialize them 
> once with the clock you want, and be done with it.
> 
> I'd almost rather see the timespec changed (controversial much?!) to add the 
> clock id rather than keep adding new interfaces to support waits/sleeps on 
> different clocks.  Or maybe to provide a hint to the implementation as to 
> what clock to use on a per-thread basis via something like 
> pthread_attr_setpreferredclock() and pthread_setpreferredclock().  If you had 
> these, I suppose you could implement the C++ functionality by saving the 
> current thread's preferred clock, setting it to the desired clock, calling 
> the mutex lock or CV wait function, then restoring the thread's clock before 
> returning to the caller.
> 
> --
> DE




Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-10-06 Thread Mike Crowe
On Wednesday 12 September 2018 at 13:47:59 -0700, Tom Cherry wrote:
> On Wed, Sep 12, 2018 at 11:56 AM Mike Crowe  
> wrote:
> >
> > On Wednesday 12 September 2018 at 11:29:26 -0700, Tom Cherry wrote:
> > > On Sun, Sep 9, 2018 at 8:28 AM Mike Crowe  
> > > wrote:
> > > >
> > > > On Thursday 30 August 2018 at 21:10:48 +0100, Mike Crowe wrote:
> > > > > C++11's std::condition_variable, std::timed_mutex and
> > > > > std::recursive_timed_mutex support waiting with a timeout specified 
> > > > > using
> > > > > an arbitrary clock. It's common to use std::chrono::steady_clock 
> > > > > (which
> > > > > corresponds to CLOCK_MONOTONIC) or std::chrono::system_clock (which
> > > > > corresponds to CLOCK_REALTIME) for these waits, and other clocks end 
> > > > > up
> > > > > being converted to one of those when performing the wait.
> > > > >
> > > > > Unfortunately, I don't believe that it's possible to implement these 
> > > > > on top
> > > > > of the pthread equivalents correctly for std::chrono::steady_clock 
> > > > > (i.e.
> > > > > CLOCK_MONOTONIC) since:
> > > > >
> > > > > 1. pthread_mutex_timedlock only supports a time measured against
> > > > >CLOCK_REALTIME.
> > > > >
> > > > > 2. The clock used for pthread_cond_timedwait must be specified when 
> > > > > the
> > > > >condition variable is created by pthread_cond_init. The clock to 
> > > > > be used
> > > > >for future waits is not known at the point that the
> > > > >std::condition_variable constructor is called.
> > > > >
> > > > > I'm most interested in the std::condition_variable case. since I have 
> > > > > code
> > > > > that wants to wait using std::chrono::steady_clock.
> > > > >
> > > > > There are a number of possible workarounds for these. I've described 
> > > > > some
> > > > > in a blog post[1] and in defect 0001164[2]. But, my favourite 
> > > > > solution is
> > > > > to introduce a variant of pthread_cond_timedwait that takes a an 
> > > > > additional
> > > > > clockid_t parameter:
> > > > >
> > > > >  int pthread_cond_timedwaitonclock(pthread_cond_t *cond,
> > > > >pthread_mutex_t *mutex,
> > > > >clockid_t clockid,
> > > > >const struct timespec *abstimeout);
> > > > >
> > > > > I've proposed[3] an implementation of this function for glibc (as
> > > > > pthread_cond_timedwaitonclock_np) and it was suggested that I ought to
> > > > > raise it here. (This led me to enter defect 0001164, but since that 
> > > > > yielded
> > > > > no response I've finally got round to writing this email too.)
> > > > >
> > > > > The equivalent for mutex would probably be:
> > > > >
> > > > >  int pthread_mutex_timedlockonclock(pthread_mutex_t *mutex,
> > > > > clockid_t clockid,
> > > > > const struct timespec 
> > > > > *abstimeout);
> > > > >
> > > > > but I've not yet made any attempt to implement it in glibc.
> > > > >
> > > > > Would making the C++ standard library implementable on top of POSIX be
> > > > > considered a good enough reason to add such functions?
> > > > >
> > > > > Are these functions generic enough or perhaps too generic? Android 
> > > > > had a
> > > > > pthread_cond_timedwait_monotonic function for a while, but deprecated 
> > > > > it
> > > > > when they added support for pthread_condattr_setclock.
> > >
> > > 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 had got the 

Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-09-12 Thread Tom Cherry
On Wed, Sep 12, 2018 at 11:56 AM Mike Crowe  wrote:
>
> On Wednesday 12 September 2018 at 11:29:26 -0700, Tom Cherry wrote:
> > On Sun, Sep 9, 2018 at 8:28 AM Mike Crowe  
> > wrote:
> > >
> > > On Thursday 30 August 2018 at 21:10:48 +0100, Mike Crowe wrote:
> > > > C++11's std::condition_variable, std::timed_mutex and
> > > > std::recursive_timed_mutex support waiting with a timeout specified 
> > > > using
> > > > an arbitrary clock. It's common to use std::chrono::steady_clock (which
> > > > corresponds to CLOCK_MONOTONIC) or std::chrono::system_clock (which
> > > > corresponds to CLOCK_REALTIME) for these waits, and other clocks end up
> > > > being converted to one of those when performing the wait.
> > > >
> > > > Unfortunately, I don't believe that it's possible to implement these on 
> > > > top
> > > > of the pthread equivalents correctly for std::chrono::steady_clock (i.e.
> > > > CLOCK_MONOTONIC) since:
> > > >
> > > > 1. pthread_mutex_timedlock only supports a time measured against
> > > >CLOCK_REALTIME.
> > > >
> > > > 2. The clock used for pthread_cond_timedwait must be specified when the
> > > >condition variable is created by pthread_cond_init. The clock to be 
> > > > used
> > > >for future waits is not known at the point that the
> > > >std::condition_variable constructor is called.
> > > >
> > > > I'm most interested in the std::condition_variable case. since I have 
> > > > code
> > > > that wants to wait using std::chrono::steady_clock.
> > > >
> > > > There are a number of possible workarounds for these. I've described 
> > > > some
> > > > in a blog post[1] and in defect 0001164[2]. But, my favourite solution 
> > > > is
> > > > to introduce a variant of pthread_cond_timedwait that takes a an 
> > > > additional
> > > > clockid_t parameter:
> > > >
> > > >  int pthread_cond_timedwaitonclock(pthread_cond_t *cond,
> > > >pthread_mutex_t *mutex,
> > > >clockid_t clockid,
> > > >const struct timespec *abstimeout);
> > > >
> > > > I've proposed[3] an implementation of this function for glibc (as
> > > > pthread_cond_timedwaitonclock_np) and it was suggested that I ought to
> > > > raise it here. (This led me to enter defect 0001164, but since that 
> > > > yielded
> > > > no response I've finally got round to writing this email too.)
> > > >
> > > > The equivalent for mutex would probably be:
> > > >
> > > >  int pthread_mutex_timedlockonclock(pthread_mutex_t *mutex,
> > > > clockid_t clockid,
> > > > const struct timespec *abstimeout);
> > > >
> > > > but I've not yet made any attempt to implement it in glibc.
> > > >
> > > > Would making the C++ standard library implementable on top of POSIX be
> > > > considered a good enough reason to add such functions?
> > > >
> > > > Are these functions generic enough or perhaps too generic? Android had a
> > > > pthread_cond_timedwait_monotonic function for a while, but deprecated it
> > > > when they added support for pthread_condattr_setclock.
> >
> > 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 had got the impression that people in the "enterprise" world wanted to
> continue to be able to wait on CLOCK_REALTIME. Maybe that's not as true as
> I thought it was.

The existing pthread_cond_timedwait() would still use CLOCK_REALTIME
by default, so users would get to chose between those two clocks
depending on the function that they used.  (Of 

Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-09-12 Thread Mike Crowe
On Wednesday 12 September 2018 at 11:29:26 -0700, Tom Cherry wrote:
> On Sun, Sep 9, 2018 at 8:28 AM Mike Crowe  wrote:
> >
> > On Thursday 30 August 2018 at 21:10:48 +0100, Mike Crowe wrote:
> > > C++11's std::condition_variable, std::timed_mutex and
> > > std::recursive_timed_mutex support waiting with a timeout specified using
> > > an arbitrary clock. It's common to use std::chrono::steady_clock (which
> > > corresponds to CLOCK_MONOTONIC) or std::chrono::system_clock (which
> > > corresponds to CLOCK_REALTIME) for these waits, and other clocks end up
> > > being converted to one of those when performing the wait.
> > >
> > > Unfortunately, I don't believe that it's possible to implement these on 
> > > top
> > > of the pthread equivalents correctly for std::chrono::steady_clock (i.e.
> > > CLOCK_MONOTONIC) since:
> > >
> > > 1. pthread_mutex_timedlock only supports a time measured against
> > >CLOCK_REALTIME.
> > >
> > > 2. The clock used for pthread_cond_timedwait must be specified when the
> > >condition variable is created by pthread_cond_init. The clock to be 
> > > used
> > >for future waits is not known at the point that the
> > >std::condition_variable constructor is called.
> > >
> > > I'm most interested in the std::condition_variable case. since I have code
> > > that wants to wait using std::chrono::steady_clock.
> > >
> > > There are a number of possible workarounds for these. I've described some
> > > in a blog post[1] and in defect 0001164[2]. But, my favourite solution is
> > > to introduce a variant of pthread_cond_timedwait that takes a an 
> > > additional
> > > clockid_t parameter:
> > >
> > >  int pthread_cond_timedwaitonclock(pthread_cond_t *cond,
> > >pthread_mutex_t *mutex,
> > >clockid_t clockid,
> > >const struct timespec *abstimeout);
> > >
> > > I've proposed[3] an implementation of this function for glibc (as
> > > pthread_cond_timedwaitonclock_np) and it was suggested that I ought to
> > > raise it here. (This led me to enter defect 0001164, but since that 
> > > yielded
> > > no response I've finally got round to writing this email too.)
> > >
> > > The equivalent for mutex would probably be:
> > >
> > >  int pthread_mutex_timedlockonclock(pthread_mutex_t *mutex,
> > > clockid_t clockid,
> > > const struct timespec *abstimeout);
> > >
> > > but I've not yet made any attempt to implement it in glibc.
> > >
> > > Would making the C++ standard library implementable on top of POSIX be
> > > considered a good enough reason to add such functions?
> > >
> > > Are these functions generic enough or perhaps too generic? Android had a
> > > pthread_cond_timedwait_monotonic function for a while, but deprecated it
> > > when they added support for pthread_condattr_setclock.
> 
> 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 had got the impression that people in the "enterprise" world wanted to
continue to be able to wait on CLOCK_REALTIME. Maybe that's not as true as
I thought it was.

[snip]

> > > An application as a whole is interested in real time or in clock
> > > monotonic time.
> >
> > I don't believe that's true, especially when code is hidden away in
> > libraries. A calendar application may wish to use CLOCK_REALTIME for events
> > but CLOCK_MONOTONIC for notification timeouts.
> 
> Realistically, there are better waiting mechanisms than condition
> variables that a calender application should use when waiting for
> 

Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-09-12 Thread Tom Cherry
On Sun, Sep 9, 2018 at 8:28 AM Mike Crowe  wrote:
>
> On Thursday 30 August 2018 at 21:10:48 +0100, Mike Crowe wrote:
> > C++11's std::condition_variable, std::timed_mutex and
> > std::recursive_timed_mutex support waiting with a timeout specified using
> > an arbitrary clock. It's common to use std::chrono::steady_clock (which
> > corresponds to CLOCK_MONOTONIC) or std::chrono::system_clock (which
> > corresponds to CLOCK_REALTIME) for these waits, and other clocks end up
> > being converted to one of those when performing the wait.
> >
> > Unfortunately, I don't believe that it's possible to implement these on top
> > of the pthread equivalents correctly for std::chrono::steady_clock (i.e.
> > CLOCK_MONOTONIC) since:
> >
> > 1. pthread_mutex_timedlock only supports a time measured against
> >CLOCK_REALTIME.
> >
> > 2. The clock used for pthread_cond_timedwait must be specified when the
> >condition variable is created by pthread_cond_init. The clock to be used
> >for future waits is not known at the point that the
> >std::condition_variable constructor is called.
> >
> > I'm most interested in the std::condition_variable case. since I have code
> > that wants to wait using std::chrono::steady_clock.
> >
> > There are a number of possible workarounds for these. I've described some
> > in a blog post[1] and in defect 0001164[2]. But, my favourite solution is
> > to introduce a variant of pthread_cond_timedwait that takes a an additional
> > clockid_t parameter:
> >
> >  int pthread_cond_timedwaitonclock(pthread_cond_t *cond,
> >pthread_mutex_t *mutex,
> >clockid_t clockid,
> >const struct timespec *abstimeout);
> >
> > I've proposed[3] an implementation of this function for glibc (as
> > pthread_cond_timedwaitonclock_np) and it was suggested that I ought to
> > raise it here. (This led me to enter defect 0001164, but since that yielded
> > no response I've finally got round to writing this email too.)
> >
> > The equivalent for mutex would probably be:
> >
> >  int pthread_mutex_timedlockonclock(pthread_mutex_t *mutex,
> > clockid_t clockid,
> > const struct timespec *abstimeout);
> >
> > but I've not yet made any attempt to implement it in glibc.
> >
> > Would making the C++ standard library implementable on top of POSIX be
> > considered a good enough reason to add such functions?
> >
> > Are these functions generic enough or perhaps too generic? Android had a
> > pthread_cond_timedwait_monotonic function for a while, but deprecated it
> > when they added support for pthread_condattr_setclock.

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.

> >
> > Thanks.
> >
> > Mike.
> >
> > [1] 
> > https://randombitsofuselessinformation.blogspot.com/2018/06/its-about-time-monotonic-time.html
> > [2] http://austingroupbugs.net/view.php?id=1164
> > [3] 
> > http://libc-alpha.sourceware.narkive.com/3ZPZzEOy/rfcv4-add-pthread-cond-timedwaitonclock-np#post1
>
> I received one direct reply to this email from someone who wasn't sure if
> their response was worth of the list. I thought it was. I understood their
> main points to be:
>
> > The library code now has to track the clock for each invocation of
> > pthread_cond_timedwaitonclock and not (once) per condition variable.
>
> Only if the underlying implementation expects that.

Note: our implementation in Android does not need this, nor should any
other implementations.  It's simply a separate flag passed to the
underlying futex call.  It is possible to have 

Re: C++ condition_variable and timed_mutex with steady_clock and pthreads

2018-09-09 Thread Mike Crowe
On Thursday 30 August 2018 at 21:10:48 +0100, Mike Crowe wrote:
> C++11's std::condition_variable, std::timed_mutex and
> std::recursive_timed_mutex support waiting with a timeout specified using
> an arbitrary clock. It's common to use std::chrono::steady_clock (which
> corresponds to CLOCK_MONOTONIC) or std::chrono::system_clock (which
> corresponds to CLOCK_REALTIME) for these waits, and other clocks end up
> being converted to one of those when performing the wait.
> 
> Unfortunately, I don't believe that it's possible to implement these on top
> of the pthread equivalents correctly for std::chrono::steady_clock (i.e.
> CLOCK_MONOTONIC) since:
> 
> 1. pthread_mutex_timedlock only supports a time measured against
>CLOCK_REALTIME.
> 
> 2. The clock used for pthread_cond_timedwait must be specified when the
>condition variable is created by pthread_cond_init. The clock to be used
>for future waits is not known at the point that the
>std::condition_variable constructor is called.
> 
> I'm most interested in the std::condition_variable case. since I have code
> that wants to wait using std::chrono::steady_clock.
> 
> There are a number of possible workarounds for these. I've described some
> in a blog post[1] and in defect 0001164[2]. But, my favourite solution is
> to introduce a variant of pthread_cond_timedwait that takes a an additional
> clockid_t parameter:
> 
>  int pthread_cond_timedwaitonclock(pthread_cond_t *cond,
>pthread_mutex_t *mutex,
>clockid_t clockid,
>const struct timespec *abstimeout);
> 
> I've proposed[3] an implementation of this function for glibc (as
> pthread_cond_timedwaitonclock_np) and it was suggested that I ought to
> raise it here. (This led me to enter defect 0001164, but since that yielded
> no response I've finally got round to writing this email too.)
> 
> The equivalent for mutex would probably be:
> 
>  int pthread_mutex_timedlockonclock(pthread_mutex_t *mutex,
> clockid_t clockid,
> const struct timespec *abstimeout);
> 
> but I've not yet made any attempt to implement it in glibc.
> 
> Would making the C++ standard library implementable on top of POSIX be
> considered a good enough reason to add such functions?
> 
> Are these functions generic enough or perhaps too generic? Android had a
> pthread_cond_timedwait_monotonic function for a while, but deprecated it
> when they added support for pthread_condattr_setclock.
> 
> Thanks.
> 
> Mike.
> 
> [1] 
> https://randombitsofuselessinformation.blogspot.com/2018/06/its-about-time-monotonic-time.html
> [2] http://austingroupbugs.net/view.php?id=1164
> [3] 
> http://libc-alpha.sourceware.narkive.com/3ZPZzEOy/rfcv4-add-pthread-cond-timedwaitonclock-np#post1

I received one direct reply to this email from someone who wasn't sure if
their response was worth of the list. I thought it was. I understood their
main points to be:

> The library code now has to track the clock for each invocation of
> pthread_cond_timedwaitonclock and not (once) per condition variable.

Only if the underlying implementation expects that.

At least in the glibc implementation I've been working with, the clock to
use is a parameter to the underlying operating system futex wait operation.
Supplying it at construction time means that the clock must be stored
within the condition variable and retrieved when the wait operation is
called. I have implemented pthread_cond_timedwait in terms of
pthread_cond_timedwaitonclock.

> The situations where a single condition variable (or mutex) has waiting
> threads that use different clocks would seem to be very rare;

I agree, but the clock to use is a property of the wait and not a property
of the condition variable so it makes more sense (and it is easier to
reason about the code locally) if the clock is supplied in the call rather
than at construction. In C++ the clock is part of the type of the parameter
passed to the wait operation so it can be automatically inferred.

> An application as a whole is interested in real time or in clock
> monotonic time.

I don't believe that's true, especially when code is hidden away in
libraries. A calendar application may wish to use CLOCK_REALTIME for events
but CLOCK_MONOTONIC for notification timeouts.

> pthread_mutex_timedlockonclock can be implemented using a condition
> variable configured to use an appropriate clock.

It can be, if pthread_cond_timedwaitonclock is available, otherwise you're
back to not knowing the clock at construction time. I'm sceptical that
adding the overhead of a condition variable for every std::timed_mutex
instance will be acceptable to C++ standard library authors.

> Non-monotonic changes to CLOCK_REALTIME should be rare since methods
> exist to correct a slightly-wrong clock without it going backwards.

Systems, particularly embedded ones, may