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)
On Monday 29 October 2018 at 13:42:59 -0700, Tom Cherry wrote: > 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. The libc implementation can accept the clock parameter, but fail if it is passed anything other then CLOCK_REALTIME. Maybe one day the system call can gain CLOCK_MONOTONIC support, and then support can be implemented in libc. If we're going to add these clockid_t parameters, then we might as well add them everywhere. > I believe the posix tracing functions are marked as > obsolescent, so it seems to be best to leave them alone as well. That makes sense to me. Let me refine that "everywhere" to not include stuff that's deprecated. :) > I agree with the creating these same variants for other four functions. So, it looks like we just need to agree on the names then, and I can try to write some words. I shall endeavour to write a summary of the naming options over the weekend. Thanks. Mike.
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)
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)
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)
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)
> 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)
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
> 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
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
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 > required
Re: C++ condition_variable and timed_mutex with steady_clock and pthreads
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
> 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
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 underly
Re: C++ condition_variable and timed_mutex with steady_clock and pthreads
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
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
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
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 impre
Re: C++ condition_variable and timed_mutex with steady_clock and pthreads
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 c
Re: C++ condition_variable and timed_mutex with steady_clock and pthreads
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
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 d
Re: C++ condition_variable and timed_mutex with steady_clock and pthreads
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 boot
C++ condition_variable and timed_mutex with steady_clock and pthreads
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
