windows: deal with the fact that GetTickCount might lag

We use GetQueuedCompletionStatus(Ex) to sleep the thread until the next timer expires (provided that no other events happen before that). However after waking up from a sleep the GetTickCount() return value may not immediately reflect that some time has passed. This happens because gqcs can sometimes sleep for periods shorter than the GetTickCount clock resulution. This patch changes time tracking so the amount of time waited by gqcs is taken into account. This has the following advantages: * Excessive loop iterations are avoided. * Small timeouts are fired more precisely. * The `loop-stop` test that used to be flaky on Windows now passes consistently.
joyent · Apr 18, 2013 · 579609e · 579609e
1 parent 79f9629
commit 579609e
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 9 deletions.
diff --git a/include/uv-private/uv-win.h b/include/uv-private/uv-win.h
@@ -275,6 +275,8 @@ RB_HEAD(uv_timer_tree_s, uv_timer_s);
   HANDLE iocp;                                                                \
   /* The current time according to the event loop. in msecs. */               \
   uint64_t time;                                                              \
+  /* GetTickCount() result when the event loop time was last updated. */      \
+  DWORD last_tick_count;                                                      \
   /* Tail of a single-linked circular queue of pending reqs. If the queue */  \
   /* is empty, tail_ is NULL. If there is only one item, */                   \
   /* tail_->next_req == tail_ */                                              \

diff --git a/src/win/core.c b/src/win/core.c
@@ -90,6 +90,7 @@ static void uv_loop_init(uv_loop_t* loop) {
   /* To prevent uninitialized memory access, loop->time must be intialized */
   /* to zero before calling uv_update_time for the first time. */
   loop->time = 0;
+  loop->last_tick_count = 0;
   uv_update_time(loop);
 
   QUEUE_INIT(&loop->handle_queue);
@@ -209,6 +210,11 @@ static void uv_poll(uv_loop_t* loop, int block) {
   } else if (GetLastError() != WAIT_TIMEOUT) {
     /* Serious error */
     uv_fatal_error(GetLastError(), "GetQueuedCompletionStatus");
+  } else {
+    /* We're sure that at least `timeout` milliseconds have expired, but */
+    /* this may not be reflected yet in the GetTickCount() return value. */
+    /* Therefore we ensure it's taken into account here. */
+    uv__time_forward(loop, timeout);
   }
 }
 
@@ -243,6 +249,11 @@ static void uv_poll_ex(uv_loop_t* loop, int block) {
   } else if (GetLastError() != WAIT_TIMEOUT) {
     /* Serious error */
     uv_fatal_error(GetLastError(), "GetQueuedCompletionStatusEx");
+  } else if (timeout > 0) {
+    /* We're sure that at least `timeout` milliseconds have expired, but */
+    /* this may not be reflected yet in the GetTickCount() return value. */
+    /* Therefore we ensure it's taken into account here. */
+    uv__time_forward(loop, timeout);
   }
 }
 

diff --git a/src/win/internal.h b/src/win/internal.h
@@ -206,6 +206,7 @@ void uv_poll_endgame(uv_loop_t* loop, uv_poll_t* handle);
 void uv_timer_endgame(uv_loop_t* loop, uv_timer_t* handle);
 
 DWORD uv_get_poll_timeout(uv_loop_t* loop);
+void uv__time_forward(uv_loop_t* loop, uint64_t msecs);
 void uv_process_timers(uv_loop_t* loop);
 
 

diff --git a/src/win/timer.c b/src/win/timer.c
@@ -29,19 +29,38 @@
 
 
 void uv_update_time(uv_loop_t* loop) {
-  DWORD ticks = GetTickCount();
+  DWORD ticks;
+  ULARGE_INTEGER time;
 
-  /* The assumption is made that LARGE_INTEGER.QuadPart has the same type */
-  /* loop->time, which happens to be. Is there any way to assert this? */
-  LARGE_INTEGER* time = (LARGE_INTEGER*) &loop->time;
+  ticks = GetTickCount();
 
-  /* If the timer has wrapped, add 1 to it's high-order dword. */
+  /* GetTickCount() can conceivably wrap around, so when the current tick */
+  /* count is lower than the last tick count, we'll assume it has wrapped. */
   /* uv_poll must make sure that the timer can never overflow more than */
   /* once between two subsequent uv_update_time calls. */
-  if (ticks < time->LowPart) {
-    time->HighPart += 1;
-  }
-  time->LowPart = ticks;
+  time = *(ULARGE_INTEGER*) &loop->time;
+
+  time.LowPart = ticks;
+  if (ticks < loop->last_tick_count)
+    time.HighPart++;
+
+  /* Remember the last tick count. */
+  loop->last_tick_count = ticks;
+
+  /* The GetTickCount() resolution isn't too good. Sometimes it'll happen */
+  /* that GetQueuedCompletionStatus() or GetQueuedCompletionStatusEx() has */
+  /* waited for a couple of ms but this is not reflected in the GetTickCount */
+  /* result yet. Therefore whenever GetQueuedCompletionStatus times out */
+  /* we'll add the number of ms that it has waited to the current loop time. */
+  /* When that happened the loop time might be a little ms farther than what */
+  /* we've just computed, and we shouldn't update the loop time. */
+  if (loop->time < time.QuadPart)
+    loop->time = time.QuadPart;
+}
+
+
+void uv__time_forward(uv_loop_t* loop, uint64_t msecs) {
+  loop->time += msecs;
 }