From 95c5bf8db1535c5e709dda95e9f4656730ae703b Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 14 Jun 2018 19:22:57 +0200 Subject: [PATCH] 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 Reviewed-By: Colin Ihrig --- CMakeLists.txt | 3 +- Makefile.am | 3 +- include/uv/win.h | 11 ++- src/{unix => }/timer.c | 19 ++-- src/unix/internal.h | 5 -- src/uv-common.h | 4 + src/win/core.c | 26 +++++- src/win/handle-inl.h | 3 +- src/win/internal.h | 9 -- src/win/timer.c | 195 ----------------------------------------- uv.gyp | 3 +- 11 files changed, 51 insertions(+), 230 deletions(-) rename src/{unix => }/timer.c (92%) delete mode 100644 src/win/timer.c diff --git a/CMakeLists.txt b/CMakeLists.txt index f13e5295..a133edb8 100644 --- a/CMakeLists.txt +++ b/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) diff --git a/Makefile.am b/Makefile.am index 5f07dba2..04aecab5 100644 --- a/Makefile.am +++ b/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 diff --git a/include/uv/win.h b/include/uv/win.h index f0133529..d6b8b3a7 100644 --- a/include/uv/win.h +++ b/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 */ \ - struct uv_timer_tree_s timers; \ + /* TODO(bnoordhuis) Stop heap-allocating |timer_heap| in libuv v2.x. */ \ + 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; \ - uint64_t due; \ + void* heap_node[3]; \ + int unused; \ + uint64_t timeout; \ uint64_t repeat; \ uint64_t start_id; \ uv_timer_cb timer_cb; diff --git a/src/unix/timer.c b/src/timer.c similarity index 92% rename from src/unix/timer.c rename to src/timer.c index 54dabfe7..2bf449a7 100644 --- a/src/unix/timer.c +++ b/src/timer.c @@ -19,13 +19,22 @@ */ #include "uv.h" -#include "internal.h" +#include "uv-common.h" #include "heap-inl.h" #include #include +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; diff --git a/src/unix/internal.h b/src/unix/internal.h index b0c5dcad..4d7f7fb2 100644 --- a/src/unix/internal.h +++ b/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); diff --git a/src/uv-common.h b/src/uv-common.h index 85bcbe6c..dcb5f865 100644 --- a/src/uv-common.h +++ b/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) diff --git a/src/win/core.c b/src/win/core.c index d6af282a..ab2503ce 100644 --- a/src/win/core.c +++ b/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); diff --git a/src/win/handle-inl.h b/src/win/handle-inl.h index ed843072..4d045920 100644 --- a/src/win/handle-inl.h +++ b/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: diff --git a/src/win/internal.h b/src/win/internal.h index fa926d9a..69ef3045 100644 --- a/src/win/internal.h +++ b/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 */ diff --git a/src/win/timer.c b/src/win/timer.c deleted file mode 100644 index eda5c24f..00000000 --- a/src/win/timer.c +++ /dev/null @@ -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 -#include - -#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); - } -} diff --git a/uv.gyp b/uv.gyp index 8aaf541b..37dcb360 100644 --- a/uv.gyp +++ b/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', ],