Skip to content

Commit

Permalink
src: use unordered_set instead of custom rb tree
Browse files Browse the repository at this point in the history
Use a standard hash-based container instead of the custom included
red/black tree implementation. There is likely no noticeable
performance difference, and if there is one, it is very likely
to be an improvement.

PR-URL: nodejs#14826
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
addaleax committed Aug 17, 2017
1 parent 60f2fa9 commit 2e01445
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 802 deletions.
2 changes: 0 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -891,8 +891,6 @@ jslint-ci:

CPPLINT_EXCLUDE ?=
CPPLINT_EXCLUDE += src/node_root_certs.h
CPPLINT_EXCLUDE += src/queue.h
CPPLINT_EXCLUDE += src/tree.h
CPPLINT_EXCLUDE += $(wildcard test/addons/??_*/*.cc test/addons/??_*/*.h)
CPPLINT_EXCLUDE += $(wildcard test/addons-napi/??_*/*.cc test/addons-napi/??_*/*.h)
# These files were copied more or less verbatim from V8.
Expand Down
1 change: 0 additions & 1 deletion node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@
'src/tracing/node_trace_buffer.h',
'src/tracing/node_trace_writer.h',
'src/tracing/trace_event.h'
'src/tree.h',
'src/util.h',
'src/util-inl.h',
'deps/http_parser/http_parser.h',
Expand Down
47 changes: 23 additions & 24 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "node.h"
#include "req-wrap.h"
#include "req-wrap-inl.h"
#include "tree.h"
#include "util.h"
#include "util-inl.h"
#include "uv.h"
Expand All @@ -37,6 +36,7 @@
#include <stdlib.h>
#include <string.h>
#include <vector>
#include <unordered_set>

#if defined(__ANDROID__) || \
defined(__MINGW32__) || \
Expand Down Expand Up @@ -122,10 +122,22 @@ struct node_ares_task {
ChannelWrap* channel;
ares_socket_t sock;
uv_poll_t poll_watcher;
RB_ENTRY(node_ares_task) node;
};

RB_HEAD(node_ares_task_list, node_ares_task);
struct TaskHash {
size_t operator()(node_ares_task* a) const {
return std::hash<ares_socket_t>()(a->sock);
}
};

struct TaskEqual {
inline bool operator()(node_ares_task* a, node_ares_task* b) const {
return a->sock == b->sock;
}
};

using node_ares_task_list =
std::unordered_set<node_ares_task*, TaskHash, TaskEqual>;

class ChannelWrap : public AsyncWrap {
public:
Expand Down Expand Up @@ -169,8 +181,6 @@ ChannelWrap::ChannelWrap(Environment* env,
query_last_ok_(true),
is_servers_default_(true),
library_inited_(false) {
RB_INIT(&task_list_);

MakeWeak<ChannelWrap>(this);

Setup();
Expand Down Expand Up @@ -222,25 +232,12 @@ GetNameInfoReqWrap::~GetNameInfoReqWrap() {
}


int cmp_ares_tasks(const node_ares_task* a, const node_ares_task* b) {
if (a->sock < b->sock)
return -1;
if (a->sock > b->sock)
return 1;
return 0;
}


RB_GENERATE_STATIC(node_ares_task_list, node_ares_task, node, cmp_ares_tasks)



/* This is called once per second by loop->timer. It is used to constantly */
/* call back into c-ares for possibly processing timeouts. */
void ChannelWrap::AresTimeout(uv_timer_t* handle) {
ChannelWrap* channel = static_cast<ChannelWrap*>(handle->data);
CHECK_EQ(channel->timer_handle(), handle);
CHECK_EQ(false, RB_EMPTY(channel->task_list()));
CHECK_EQ(false, channel->task_list()->empty());
ares_process_fd(channel->cares_channel(), ARES_SOCKET_BAD, ARES_SOCKET_BAD);
}

Expand Down Expand Up @@ -306,7 +303,9 @@ void ares_sockstate_cb(void* data,

node_ares_task lookup_task;
lookup_task.sock = sock;
task = RB_FIND(node_ares_task_list, channel->task_list(), &lookup_task);
auto it = channel->task_list()->find(&lookup_task);

task = (it == channel->task_list()->end()) ? nullptr : *it;

if (read || write) {
if (!task) {
Expand All @@ -315,7 +314,7 @@ void ares_sockstate_cb(void* data,
/* If this is the first socket then start the timer. */
uv_timer_t* timer_handle = channel->timer_handle();
if (!uv_is_active(reinterpret_cast<uv_handle_t*>(timer_handle))) {
CHECK(RB_EMPTY(channel->task_list()));
CHECK(channel->task_list()->empty());
uv_timer_start(timer_handle, ChannelWrap::AresTimeout, 1000, 1000);
}

Expand All @@ -327,7 +326,7 @@ void ares_sockstate_cb(void* data,
return;
}

RB_INSERT(node_ares_task_list, channel->task_list(), task);
channel->task_list()->insert(task);
}

/* This should never fail. If it fails anyway, the query will eventually */
Expand All @@ -343,11 +342,11 @@ void ares_sockstate_cb(void* data,
CHECK(task &&
"When an ares socket is closed we should have a handle for it");

RB_REMOVE(node_ares_task_list, channel->task_list(), task);
channel->task_list()->erase(it);
uv_close(reinterpret_cast<uv_handle_t*>(&task->poll_watcher),
ares_poll_close_cb);

if (RB_EMPTY(channel->task_list())) {
if (channel->task_list()->empty()) {
uv_timer_stop(channel->timer_handle());
}
}
Expand Down
1 change: 0 additions & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#endif
#include "handle_wrap.h"
#include "req-wrap.h"
#include "tree.h"
#include "util.h"
#include "uv.h"
#include "v8.h"
Expand Down
Loading

0 comments on commit 2e01445

Please sign in to comment.