Re: [AD] review timed-wait changes

["David Capello" <davidcapello@xxxxxxxxxx>]

Hi Peter

On Sun, Apr 6, 2008 at 11:42 AM, Peter Wang <novalazy@xxxxxxxxxx> wrote:
>  2. change the interface of _al_cond_timedwait() to take an absolute
>  timeout rather than a relative one.  There's no really compelling reason
>  for this but the old interface made the pthread implementation a bit
>  convoluted, converting structured time representations to floats then
>  back again.  The new interface makes the Win32 implementation a little
>  convoluted, but hopefully less so.  Could somebody apply the patch and
>  check that exnew_timedwait works in Windows? Also check if the code to
>  handle timeGetTime() overflows makes sense.

I applied the patch. It gives some warnings in the compilation:

src/win/wxthread.c: In function `_al_cond_timeout_init':
src/win/wxthread.c:307: warning: comparison of unsigned expression < 0
is always false
src/win/wxthread.c:308: warning: implicit declaration of function `timeGetTime'
src/win/wxthread.c: In function `_al_cond_timedwait':
src/win/wxthread.c:332: warning: comparison between signed and unsigned


Also the exnew_timedwait.c needs a little change
-      if (al_wait_for_event(queue, &event, 100)) {
+      if (al_wait_for_event_timed(queue, &event, 0.100)) {


About the overflows, I think that we could simplified the
implementation in wxthread.c using this:

/* it isn't included, but timeGetTime is defined there */
#include <mmsystem.h>
...
void _al_cond_timeout_init(_AL_COND_TIMEOUT *timeout, unsigned int rel_msecs)
{
   timeout->abstime = timeGetTime() + rel_msecs;
}
...
int _al_cond_timedwait(_AL_COND *cond, _AL_MUTEX *mtxExternal,
   const _AL_COND_TIMEOUT *timeout)
{
   DWORD now;
   DWORD rel_msecs;

   ASSERT(cond);
   ASSERT(mtxExternal);

   now = timeGetTime();
   rel_msecs = timeout->abstime - now;
   if (rel_msecs == INFINITE) {
      rel_msecs--;
   }

   return cond_wait(cond, mtxExternal, rel_msecs);
}


I think that we should call one time at the beginning the
timeBeginPeriod and timeEndPeriod routines to get more
precision with timeGetTime in Win2K.

>  Question: should al_wait_for_event_timed() take an absolute timeout
>  specification, or should we provide both?  Relative timeouts are
>  convenient for one off calls, but absolute timeouts would be much better
>  when waiting for multiple events in a loop, e.g.
>
>     float abst = al_current_time() + X;
>     while (al_wait_for_event_timed(queue, &ev, abst)) {
>         handle ev
>     }
>     /* time's up */

Could be convenient, anyway the relative-timeout is the more
common option in all APIs. I think that we should provide both
options. The absolute-timeout routine could be called
al_wait_for_event_until() or something.