unix,win: merge timers implementation

Merge src/unix/timer.c and src/win/timer.c into src/timer.c.  This
changes the Windows implementation from a binary tree to a binary
heap for generally better performance.

PR-URL: https://github.com/libuv/libuv/pull/1882
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

CMakeLists.txt

@@ -15,6 +15,7 @@ set(uv_sources
     src/fs-poll.c
     src/inet.c
     src/threadpool.c
+    src/timer.c
     src/uv-common.c
     src/uv-data-getter-setters.c
     src/version.c)
@@ -197,7 +198,6 @@ if(WIN32)
        src/win/stream.c
        src/win/tcp.c
        src/win/tty.c
-       src/win/timer.c
        src/win/udp.c
        src/win/util.c
        src/win/winapi.c
@@ -223,7 +223,6 @@ else()
        src/unix/stream.c
        src/unix/tcp.c
        src/unix/thread.c
-       src/unix/timer.c
        src/unix/tty.c
        src/unix/udp.c)
   list(APPEND uv_test_sources test/runner-unix.c)

Makefile.am

@@ -32,6 +32,7 @@ libuv_la_SOURCES = src/fs-poll.c \
                    src/inet.c \
                    src/queue.h \
                    src/threadpool.c \
+                   src/timer.c \
                    src/uv-data-getter-setters.c \
                    src/uv-common.c \
                    src/uv-common.h \
@@ -74,7 +75,6 @@ libuv_la_SOURCES += src/win/async.c \
                     src/win/stream-inl.h \
                     src/win/tcp.c \
                     src/win/thread.c \
-                    src/win/timer.c \
                     src/win/tty.c \
                     src/win/udp.c \
                     src/win/util.c \
@@ -105,7 +105,6 @@ libuv_la_SOURCES += src/unix/async.c \
                    src/unix/stream.c \
                    src/unix/tcp.c \
                    src/unix/thread.c \
-                   src/unix/timer.c \
                    src/unix/tty.c \
                    src/unix/udp.c
 

include/uv/win.h

@@ -308,8 +308,6 @@ typedef struct {
   char* errmsg;
 } uv_lib_t;
 
-RB_HEAD(uv_timer_tree_s, uv_timer_s);
-
 #define UV_LOOP_PRIVATE_FIELDS                                                \
     /* The loop's I/O completion port */                                      \
   HANDLE iocp;                                                                \
@@ -321,8 +319,8 @@ RB_HEAD(uv_timer_tree_s, uv_timer_s);
   uv_req_t* pending_reqs_tail;                                                \
   /* Head of a single-linked list of closed handles */                        \
   uv_handle_t* endgame_handles;                                               \
-  /* The head of the timers tree */                                           \
+  /* TODO(bnoordhuis) Stop heap-allocating |timer_heap| in libuv v2.x. */     \
-  struct uv_timer_tree_s timers;                                              \
+  void* timer_heap;                                                           \
     /* Lists of active loop (prepare / check / idle) watchers */              \
   uv_prepare_t* prepare_handles;                                              \
   uv_check_t* check_handles;                                                  \
@@ -529,8 +527,9 @@ RB_HEAD(uv_timer_tree_s, uv_timer_s);
   unsigned char events;
 
 #define UV_TIMER_PRIVATE_FIELDS                                               \
-  RB_ENTRY(uv_timer_s) tree_entry;                                            \
+  void* heap_node[3];                                                         \
-  uint64_t due;                                                               \
+  int unused;                                                                 \
+  uint64_t timeout;                                                           \
   uint64_t repeat;                                                            \
   uint64_t start_id;                                                          \
   uv_timer_cb timer_cb;

src/timer.c

@@ -19,13 +19,22 @@
  */
 
 #include "uv.h"
-#include "internal.h"
+#include "uv-common.h"
 #include "heap-inl.h"
 
 #include <assert.h>
 #include <limits.h>
 
 
+static struct heap *timer_heap(const uv_loop_t* loop) {
+#ifdef _WIN32
+  return (struct heap*) loop->timer_heap;
+#else
+  return (struct heap*) &loop->timer_heap;
+#endif
+}
+
+
 static int timer_less_than(const struct heap_node* ha,
                            const struct heap_node* hb) {
   const uv_timer_t* a;
@@ -81,7 +90,7 @@ int uv_timer_start(uv_timer_t* handle,
   /* start_id is the second index to be compared in uv__timer_cmp() */
   handle->start_id = handle->loop->timer_counter++;
 
-  heap_insert((struct heap*) &handle->loop->timer_heap,
+  heap_insert(timer_heap(handle->loop),
               (struct heap_node*) &handle->heap_node,
               timer_less_than);
   uv__handle_start(handle);
@@ -94,7 +103,7 @@ int uv_timer_stop(uv_timer_t* handle) {
   if (!uv__is_active(handle))
     return 0;
 
-  heap_remove((struct heap*) &handle->loop->timer_heap,
+  heap_remove(timer_heap(handle->loop),
               (struct heap_node*) &handle->heap_node,
               timer_less_than);
   uv__handle_stop(handle);
@@ -131,7 +140,7 @@ int uv__next_timeout(const uv_loop_t* loop) {
   const uv_timer_t* handle;
   uint64_t diff;
 
-  heap_node = heap_min((const struct heap*) &loop->timer_heap);
+  heap_node = heap_min(timer_heap(loop));
   if (heap_node == NULL)
     return -1; /* block indefinitely */
 
@@ -152,7 +161,7 @@ void uv__run_timers(uv_loop_t* loop) {
   uv_timer_t* handle;
 
   for (;;) {
-    heap_node = heap_min((struct heap*) &loop->timer_heap);
+    heap_node = heap_min(timer_heap(loop));
     if (heap_node == NULL)
       break;
 

src/unix/internal.h

@@ -252,10 +252,6 @@ int uv__tcp_keepalive(int fd, int on, unsigned int delay);
 /* pipe */
 int uv_pipe_listen(uv_pipe_t* handle, int backlog, uv_connection_cb cb);
 
-/* timer */
-void uv__run_timers(uv_loop_t* loop);
-int uv__next_timeout(const uv_loop_t* loop);
-
 /* signal */
 void uv__signal_close(uv_signal_t* handle);
 void uv__signal_global_once_init(void);
@@ -280,7 +276,6 @@ void uv__prepare_close(uv_prepare_t* handle);
 void uv__process_close(uv_process_t* handle);
 void uv__stream_close(uv_stream_t* handle);
 void uv__tcp_close(uv_tcp_t* handle);
-void uv__timer_close(uv_timer_t* handle);
 void uv__udp_close(uv_udp_t* handle);
 void uv__udp_finish_close(uv_udp_t* handle);
 uv_handle_type uv__handle_type(int fd);

src/uv-common.h

@@ -132,6 +132,10 @@ int uv__socket_sockopt(uv_handle_t* handle, int optname, int* value);
 
 void uv__fs_scandir_cleanup(uv_fs_t* req);
 
+int uv__next_timeout(const uv_loop_t* loop);
+void uv__run_timers(uv_loop_t* loop);
+void uv__timer_close(uv_timer_t* handle);
+
 #define uv__has_active_reqs(loop)                                             \
   ((loop)->active_reqs.count > 0)
 

src/win/core.c

@@ -33,6 +33,7 @@
 #include "internal.h"
 #include "queue.h"
 #include "handle-inl.h"
+#include "heap-inl.h"
 #include "req-inl.h"
 
 /* uv_once initialization guards */
@@ -221,6 +222,7 @@ static void uv_init(void) {
 
 
 int uv_loop_init(uv_loop_t* loop) {
+  struct heap* timer_heap;
   int err;
 
   /* Initialize libuv itself first */
@@ -246,7 +248,11 @@ int uv_loop_init(uv_loop_t* loop) {
 
   loop->endgame_handles = NULL;
 
-  RB_INIT(&loop->timers);
+  loop->timer_heap = timer_heap = uv__malloc(sizeof(*timer_heap));
+  if (timer_heap == NULL)
+    goto fail_timers_alloc;
+
+  heap_init(timer_heap);
 
   loop->check_handles = NULL;
   loop->prepare_handles = NULL;
@@ -285,6 +291,10 @@ fail_async_init:
   uv_mutex_destroy(&loop->wq_mutex);
 
 fail_mutex_init:
+  uv__free(timer_heap);
+  loop->timer_heap = NULL;
+
+fail_timers_alloc:
   CloseHandle(loop->iocp);
   loop->iocp = INVALID_HANDLE_VALUE;
 
@@ -292,6 +302,13 @@ fail_mutex_init:
 }
 
 
+void uv_update_time(uv_loop_t* loop) {
+  uint64_t new_time = uv__hrtime(1000);
+  assert(new_time >= loop->time);
+  loop->time = new_time;
+}
+
+
 void uv__once_init(void) {
   uv_once(&uv_init_guard_, uv_init);
 }
@@ -320,6 +337,9 @@ void uv__loop_close(uv_loop_t* loop) {
   uv_mutex_unlock(&loop->wq_mutex);
   uv_mutex_destroy(&loop->wq_mutex);
 
+  uv__free(loop->timer_heap);
+  loop->timer_heap = NULL;
+
   CloseHandle(loop->iocp);
 }
 
@@ -441,7 +461,7 @@ int uv_run(uv_loop_t *loop, uv_run_mode mode) {
 
   while (r != 0 && loop->stop_flag == 0) {
     uv_update_time(loop);
-    uv_process_timers(loop);
+    uv__run_timers(loop);
 
     ran_pending = uv_process_reqs(loop);
     uv_idle_invoke(loop);
@@ -465,7 +485,7 @@ int uv_run(uv_loop_t *loop, uv_run_mode mode) {
        * UV_RUN_NOWAIT makes no guarantees about progress so it's omitted from
        * the check.
        */
-      uv_process_timers(loop);
+      uv__run_timers(loop);
     }
 
     r = uv__loop_alive(loop);

src/win/handle-inl.h

@@ -126,7 +126,8 @@ INLINE static void uv_process_endgames(uv_loop_t* loop) {
         break;
 
       case UV_TIMER:
-        uv_timer_endgame(loop, (uv_timer_t*) handle);
+        uv__timer_close((uv_timer_t*) handle);
+        uv__handle_close(handle);
         break;
 
       case UV_PREPARE:

src/win/internal.h

@@ -246,15 +246,6 @@ int uv_poll_close(uv_loop_t* loop, uv_poll_t* handle);
 void uv_poll_endgame(uv_loop_t* loop, uv_poll_t* handle);
 
 
-/*
- * Timers
- */
-void uv_timer_endgame(uv_loop_t* loop, uv_timer_t* handle);
-
-DWORD uv__next_timeout(const uv_loop_t* loop);
-void uv_process_timers(uv_loop_t* loop);
-
-
 /*
  * Loop watchers
  */

src/win/timer.c

@@ -1,195 +0,0 @@
-/* Copyright Joyent, Inc. and other Node contributors. All rights reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to
- * deal in the Software without restriction, including without limitation the
- * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
- * sell copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- */
-
-#include <assert.h>
-#include <limits.h>
-
-#include "uv.h"
-#include "internal.h"
-#include "uv/tree.h"
-#include "handle-inl.h"
-
-
-/* The number of milliseconds in one second. */
-#define UV__MILLISEC 1000
-
-
-void uv_update_time(uv_loop_t* loop) {
-  uint64_t new_time = uv__hrtime(UV__MILLISEC);
-  assert(new_time >= loop->time);
-  loop->time = new_time;
-}
-
-
-static int uv_timer_compare(uv_timer_t* a, uv_timer_t* b) {
-  if (a->due < b->due)
-    return -1;
-  if (a->due > b->due)
-    return 1;
-  /*
-   *  compare start_id when both has the same due. start_id is
-   *  allocated with loop->timer_counter in uv_timer_start().
-   */
-  if (a->start_id < b->start_id)
-    return -1;
-  if (a->start_id > b->start_id)
-    return 1;
-  return 0;
-}
-
-
-RB_GENERATE_STATIC(uv_timer_tree_s, uv_timer_s, tree_entry, uv_timer_compare)
-
-
-int uv_timer_init(uv_loop_t* loop, uv_timer_t* handle) {
-  uv__handle_init(loop, (uv_handle_t*) handle, UV_TIMER);
-  handle->timer_cb = NULL;
-  handle->repeat = 0;
-
-  return 0;
-}
-
-
-void uv_timer_endgame(uv_loop_t* loop, uv_timer_t* handle) {
-  if (handle->flags & UV__HANDLE_CLOSING) {
-    assert(!(handle->flags & UV_HANDLE_CLOSED));
-    uv__handle_close(handle);
-  }
-}
-
-
-static uint64_t get_clamped_due_time(uint64_t loop_time, uint64_t timeout) {
-  uint64_t clamped_timeout;
-
-  clamped_timeout = loop_time + timeout;
-  if (clamped_timeout < timeout)
-    clamped_timeout = (uint64_t) -1;
-
-  return clamped_timeout;
-}
-
-
-int uv_timer_start(uv_timer_t* handle, uv_timer_cb timer_cb, uint64_t timeout,
-    uint64_t repeat) {
-  uv_loop_t* loop = handle->loop;
-  uv_timer_t* old;
-
-  if (timer_cb == NULL)
-    return UV_EINVAL;
-
-  if (uv__is_active(handle))
-    uv_timer_stop(handle);
-
-  handle->timer_cb = timer_cb;
-  handle->due = get_clamped_due_time(loop->time, timeout);
-  handle->repeat = repeat;
-  uv__handle_start(handle);
-
-  /* start_id is the second index to be compared in uv__timer_cmp() */
-  handle->start_id = handle->loop->timer_counter++;
-
-  old = RB_INSERT(uv_timer_tree_s, &loop->timers, handle);
-  assert(old == NULL);
-
-  return 0;
-}
-
-
-int uv_timer_stop(uv_timer_t* handle) {
-  uv_loop_t* loop = handle->loop;
-
-  if (!uv__is_active(handle))
-    return 0;
-
-  RB_REMOVE(uv_timer_tree_s, &loop->timers, handle);
-  uv__handle_stop(handle);
-
-  return 0;
-}
-
-
-int uv_timer_again(uv_timer_t* handle) {
-  /* If timer_cb is NULL that means that the timer was never started. */
-  if (!handle->timer_cb) {
-    return UV_EINVAL;
-  }
-
-  if (handle->repeat) {
-    uv_timer_stop(handle);
-    uv_timer_start(handle, handle->timer_cb, handle->repeat, handle->repeat);
-  }
-
-  return 0;
-}
-
-
-void uv_timer_set_repeat(uv_timer_t* handle, uint64_t repeat) {
-  assert(handle->type == UV_TIMER);
-  handle->repeat = repeat;
-}
-
-
-uint64_t uv_timer_get_repeat(const uv_timer_t* handle) {
-  assert(handle->type == UV_TIMER);
-  return handle->repeat;
-}
-
-
-DWORD uv__next_timeout(const uv_loop_t* loop) {
-  uv_timer_t* timer;
-  int64_t delta;
-
-  /* Check if there are any running timers
-   * Need to cast away const first, since RB_MIN doesn't know what we are
-   * going to do with this return value, it can't be marked const
-   */
-  timer = RB_MIN(uv_timer_tree_s, &((uv_loop_t*)loop)->timers);
-  if (timer) {
-    delta = timer->due - loop->time;
-    if (delta >= UINT_MAX - 1) {
-      /* A timeout value of UINT_MAX means infinite, so that's no good. */
-      return UINT_MAX - 1;
-    } else if (delta < 0) {
-      /* Negative timeout values are not allowed */
-      return 0;
-    } else {
-      return (DWORD)delta;
-    }
-  } else {
-    /* No timers */
-    return INFINITE;
-  }
-}
-
-
-void uv_process_timers(uv_loop_t* loop) {
-  uv_timer_t* timer;
-
-  /* Call timer callbacks */
-  for (timer = RB_MIN(uv_timer_tree_s, &loop->timers);
-       timer != NULL && timer->due <= loop->time;
-       timer = RB_MIN(uv_timer_tree_s, &loop->timers)) {
-
-    uv_timer_stop(timer);
-    uv_timer_again(timer);
-    timer->timer_cb((uv_timer_t*) timer);
-  }
-}

uv.gyp

@@ -73,6 +73,7 @@
         'src/inet.c',
         'src/queue.h',
         'src/threadpool.c',
+        'src/timer.c',
         'src/uv-data-getter-setters.c',
         'src/uv-common.c',
         'src/uv-common.h',
@@ -123,7 +124,6 @@
             'src/win/stream-inl.h',
             'src/win/tcp.c',
             'src/win/tty.c',
-            'src/win/timer.c',
             'src/win/udp.c',
             'src/win/util.c',
             'src/win/winapi.c',
@@ -168,7 +168,6 @@
             'src/unix/stream.c',
             'src/unix/tcp.c',
             'src/unix/thread.c',
-            'src/unix/timer.c',
             'src/unix/tty.c',
             'src/unix/udp.c',
           ],