Re: [v3] c++0x timed wait's in <condition_variable>

["Chris Fairles" <chris dot fairles at gmail dot com>]

On Tue, Sep 16, 2008 at 9:00 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi Chris,
>
> finally I found the time to study a bit these issues...
>
>> Issue:
>> std::condition_variable can't use std::chrono::system_clock if
>> _GLIBCXX_USE_CLOCK_MONOTONIC is defined because it requires
>> pthread_condattr_setclock et. al. to be implemented somehow in the
>> gthreads backend.
>>
>> Solution A) put in the condattr pthreads functions into the gthreads
>> backend.
>> Issues with A) how well do the pthread attribute classes map to
>> non-pthread threading systems? My guess... not very well.
>
> I understand, but I think that in principle we can delay this issue: if it
> turns out that the other threading systems have trouble implementing these
> POSIX facilities, a macro can be added to the gthr-* files and our
> implementation can dispatch on it.
>
>> Solution B) Don't use a monotonic clock.
>> Issues with B) Currently if _GLIBCXX_USE_CLOCK_MONOTONIC there is no
>> non-monotonic clock since system_clock uses CLOCK_MONOTONIC and
>> monotonic_clock and high_resolution_clock are just typedef's.
>>
>> This patch solves the issue with solution B.
>>
>> If _GLIBCXX_USE_CLOCK_MONOTONIC is defined, monotonic_clock is a
>> separate class instead of a typedef. system_clock only ever uses the
>> clock_gettime(CLOCK_REALTIME,...), gettimeofday or std::time and never
>> clock_gettime(CLOCK_MONOTONIC).
>>
>> Thus, condition_variable can use system_clock as its internal clock
>> and although its never monotonic, there's no need for
>> pthread_condattr_setclock et. al. and it all just works :)
>
> I have some concerns. First, do we have a reference in the POSIX documents
> clarifying that the default for this attribute is definitely REALTIME?

The pthread_condattr_setclock man page says:
"The default value of the clock attribute shall refer to the system clock."

At first I was thinking that the realtime system clock
(CLOCK_REALTIME) is the "system clock" but I'm not convinced since I
can't find any relation between what POSIX calls the "system clock"
like in the statement above and CLOCK_REALTIME.

It may be conincedence that the realtime clock and the "system clock"
have the same epoch and that's why it works. Not robust at all however
the following test passes on my machine:

#include <cassert>
#include <pthread.h>
int main()
{
  pthread_condattr_t a;
  clockid_t c;
  pthread_condattr_init(&a);
  pthread_condattr_getclock(&a, &c);
  assert(c == CLOCK_REALTIME);
  return 0;
}

> Otherwise the whole plan B seems brittle to me... Also, I'd like to
> understand in better detail why exactly we had a monotonic clock as our
> first choice for system_clock.  And also whether we have or nor reasons to
> believe that,  on systems providing clock_gettime indeed, a monotonic clock
> is less likely to be available than a realtime clock.
>
> Reading the pthread_cond_timedwait, pthread_cond_wait page in POSIX, I see
> that the examples use the realtime clock, but given the absolute time
> measure, I think scenarios could be imagined where a monotonic clock would
> have advantages. Comments about that? Probably this is the most important
> technical choice, because, if I understand correctly, a user of the C++0x
> condition_variable, at variance with the C, POSIX one, doesn't have the
> option to choose, we must decide for him which one is better!
>

The c++0x WD states that the thread library should use monotonic
clocks wherever it can (n2723, 30.1.4 p2) so solution B here is only
temporary anyway.

> Therefore, all in all, I think we should wait a bit before going with
> solution B. Depending on your point of view about the above issues we can
> certainly do it, but we can also try to push the gthread changes, which seem
> rather small to me. Maybe we'll have to do that anyway, as soon as possible,
> maybe not for  4.4.0, because we cannot be sure about the default for the
> attribute (see above)...
>
> Paolo.
>

The gthread changes are minimal yes. A new type, __gthread_condattr_t,
and 3 functions __gthread_condattr_init/setclock/destroy. One issue is
that CLOCK_REALTIME/MONOTONIC are suppose to be of type "clockid_t" so
I guess we need a __gthread_clockid_t as well. So 2 types and 3
functions.

I'd rather go for the gthread solution so I can dial up a patch for
that if you agree.

All this being said, I still think monotonic_clock should be separate
from system_clock because in our current setup, its impossible to get
a non-monotonic clock when _GLIBCXX_USE_CLOCK_MONOTONIC is defined.
The standard isn't very clear on this ... it says monotonic_clock's
is_monotonic must be true, yet it can be a synonym for system_clock
and system_clock's is_monotonic is unspecified. This confuses me.

Chris