On Sun, Oct 7, 2018 at 6:09 AM Mike Crowe <theopengr...@mac.mcrowe.com>

> On Saturday 06 October 2018 at 17:43:06 -0400, Daniel Eischen wrote:
> >
> > > On Oct 6, 2018, at 3:00 PM, Mike Crowe <theopengr...@mac.mcrowe.com>
> 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

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.


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

Reply via email to