Skip to content

Commit

Permalink
src: switch from QUEUE to intrusive list
Browse files Browse the repository at this point in the history
This commit also breaks up req_wrap.h into req-wrap.h and req-wrap-inl.h
to work around a circular dependency issue in env.h.

PR-URL: #667
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
  • Loading branch information
bnoordhuis committed Feb 11, 2015
1 parent 58eb00c commit 38dc0cd
Show file tree
Hide file tree
Showing 25 changed files with 171 additions and 181 deletions.
3 changes: 2 additions & 1 deletion node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@
'src/tty_wrap.h',
'src/tcp_wrap.h',
'src/udp_wrap.h',
'src/req_wrap.h',
'src/req-wrap.h',
'src/req-wrap-inl.h',
'src/string_bytes.h',
'src/stream_wrap.h',
'src/tree.h',
Expand Down
3 changes: 2 additions & 1 deletion src/async-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#define SRC_ASYNC_WRAP_H_

#include "base-object.h"
#include "env.h"
#include "v8.h"

namespace node {
Expand Down Expand Up @@ -31,6 +30,8 @@ namespace node {
V(WRITEWRAP) \
V(ZLIB)

class Environment;

class AsyncWrap : public BaseObject {
public:
enum ProviderType {
Expand Down
2 changes: 2 additions & 0 deletions src/base-object-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#define SRC_BASE_OBJECT_INL_H_

#include "base-object.h"
#include "env.h"
#include "env-inl.h"
#include "util.h"
#include "util-inl.h"
#include "v8.h"
Expand Down
3 changes: 2 additions & 1 deletion src/base-object.h
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
#ifndef SRC_BASE_OBJECT_H_
#define SRC_BASE_OBJECT_H_

#include "env.h"
#include "v8.h"

namespace node {

class Environment;

class BaseObject {
public:
BaseObject(Environment* env, v8::Local<v8::Object> handle);
Expand Down
3 changes: 2 additions & 1 deletion src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
#include "env.h"
#include "env-inl.h"
#include "node.h"
#include "req_wrap.h"
#include "req-wrap.h"
#include "req-wrap-inl.h"
#include "tree.h"
#include "util.h"
#include "uv.h"
Expand Down
23 changes: 6 additions & 17 deletions src/debug-agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "v8-debug.h"
#include "util.h"
#include "util-inl.h"
#include "queue.h"

#include <string.h>

Expand Down Expand Up @@ -64,8 +63,6 @@ Agent::Agent(Environment* env) : state_(kNone),

err = uv_mutex_init(&message_mutex_);
CHECK_EQ(err, 0);

QUEUE_INIT(&messages_);
}


Expand All @@ -75,13 +72,8 @@ Agent::~Agent() {
uv_sem_destroy(&start_sem_);
uv_mutex_destroy(&message_mutex_);

// Clean-up messages
while (!QUEUE_EMPTY(&messages_)) {
QUEUE* q = QUEUE_HEAD(&messages_);
QUEUE_REMOVE(q);
AgentMessage* msg = ContainerOf(&AgentMessage::member, q);
while (AgentMessage* msg = messages_.PopFront())
delete msg;
}
}


Expand Down Expand Up @@ -281,13 +273,9 @@ void Agent::ChildSignalCb(uv_async_t* signal) {
Local<Object> api = PersistentToLocal(isolate, a->api_);

uv_mutex_lock(&a->message_mutex_);
while (!QUEUE_EMPTY(&a->messages_)) {
QUEUE* q = QUEUE_HEAD(&a->messages_);
AgentMessage* msg = ContainerOf(&AgentMessage::member, q);

while (AgentMessage* msg = a->messages_.PopFront()) {
// Time to close everything
if (msg->data() == nullptr) {
QUEUE_REMOVE(q);
delete msg;

MakeCallback(isolate, api, "onclose", 0, nullptr);
Expand All @@ -296,10 +284,11 @@ void Agent::ChildSignalCb(uv_async_t* signal) {

// Waiting for client, do not send anything just yet
// TODO(indutny): move this to js-land
if (a->wait_)
if (a->wait_) {
a->messages_.PushFront(msg); // Push message back into the ready queue.
break;
}

QUEUE_REMOVE(q);
Local<Value> argv[] = {
String::NewFromTwoByte(isolate,
msg->data(),
Expand All @@ -321,7 +310,7 @@ void Agent::ChildSignalCb(uv_async_t* signal) {

void Agent::EnqueueMessage(AgentMessage* message) {
uv_mutex_lock(&message_mutex_);
QUEUE_INSERT_TAIL(&messages_, &message->member);
messages_.PushBack(message);
uv_mutex_unlock(&message_mutex_);
uv_async_send(&child_signal_);
}
Expand Down
57 changes: 28 additions & 29 deletions src/debug-agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@
#ifndef SRC_DEBUG_AGENT_H_
#define SRC_DEBUG_AGENT_H_

#include "util.h"
#include "util-inl.h"
#include "uv.h"
#include "v8.h"
#include "v8-debug.h"
#include "queue.h"

#include <string.h>

Expand All @@ -37,7 +38,31 @@ class Environment;
namespace node {
namespace debugger {

class AgentMessage;
class AgentMessage {
public:
AgentMessage(uint16_t* val, int length) : length_(length) {
if (val == nullptr) {
data_ = val;
} else {
data_ = new uint16_t[length];
memcpy(data_, val, length * sizeof(*data_));
}
}

~AgentMessage() {
delete[] data_;
data_ = nullptr;
}

inline const uint16_t* data() const { return data_; }
inline int length() const { return length_; }

ListNode<AgentMessage> member;

private:
uint16_t* data_;
int length_;
};

class Agent {
public:
Expand Down Expand Up @@ -100,37 +125,11 @@ class Agent {
uv_loop_t child_loop_;
v8::Persistent<v8::Object> api_;

QUEUE messages_;
ListHead<AgentMessage, &AgentMessage::member> messages_;

DispatchHandler dispatch_handler_;
};

class AgentMessage {
public:
AgentMessage(uint16_t* val, int length) : length_(length) {
if (val == nullptr) {
data_ = val;
} else {
data_ = new uint16_t[length];
memcpy(data_, val, length * sizeof(*data_));
}
}

~AgentMessage() {
delete[] data_;
data_ = nullptr;
}

inline const uint16_t* data() const { return data_; }
inline int length() const { return length_; }

QUEUE member;

private:
uint16_t* data_;
int length_;
};

} // namespace debugger
} // namespace node

Expand Down
12 changes: 2 additions & 10 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,6 @@ inline Environment::Environment(v8::Local<v8::Context> context,
set_binding_cache_object(v8::Object::New(isolate()));
set_module_load_list_array(v8::Array::New(isolate()));
RB_INIT(&cares_task_list_);
QUEUE_INIT(&req_wrap_queue_);
QUEUE_INIT(&handle_wrap_queue_);
QUEUE_INIT(&handle_cleanup_queue_);
handle_cleanup_waiting_ = 0;
}

Expand All @@ -193,11 +190,7 @@ inline Environment::~Environment() {
}

inline void Environment::CleanupHandles() {
while (!QUEUE_EMPTY(&handle_cleanup_queue_)) {
QUEUE* q = QUEUE_HEAD(&handle_cleanup_queue_);
QUEUE_REMOVE(q);

HandleCleanup* hc = ContainerOf(&HandleCleanup::handle_cleanup_queue_, q);
while (HandleCleanup* hc = handle_cleanup_queue_.PopFront()) {
handle_cleanup_waiting_++;
hc->cb_(this, hc->handle_, hc->arg_);
delete hc;
Expand Down Expand Up @@ -259,8 +252,7 @@ inline uv_check_t* Environment::idle_check_handle() {
inline void Environment::RegisterHandleCleanup(uv_handle_t* handle,
HandleCleanupCb cb,
void *arg) {
HandleCleanup* hc = new HandleCleanup(handle, cb, arg);
QUEUE_INSERT_TAIL(&handle_cleanup_queue_, &hc->handle_cleanup_queue_);
handle_cleanup_queue_.PushBack(new HandleCleanup(handle, cb, arg));
}

inline void Environment::FinishHandleCleanup(uv_handle_t* handle) {
Expand Down
21 changes: 13 additions & 8 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@

#include "ares.h"
#include "debug-agent.h"
#include "handle_wrap.h"
#include "req-wrap.h"
#include "tree.h"
#include "util.h"
#include "uv.h"
#include "v8.h"
#include "queue.h"

#include <stdint.h>

Expand Down Expand Up @@ -333,13 +334,12 @@ class Environment {
: handle_(handle),
cb_(cb),
arg_(arg) {
QUEUE_INIT(&handle_cleanup_queue_);
}

uv_handle_t* handle_;
HandleCleanupCb cb_;
void* arg_;
QUEUE handle_cleanup_queue_;
ListNode<HandleCleanup> handle_cleanup_queue_;
};

static inline Environment* GetCurrent(v8::Isolate* isolate);
Expand Down Expand Up @@ -453,8 +453,12 @@ class Environment {
return &debugger_agent_;
}

inline QUEUE* handle_wrap_queue() { return &handle_wrap_queue_; }
inline QUEUE* req_wrap_queue() { return &req_wrap_queue_; }
typedef ListHead<HandleWrap, &HandleWrap::handle_wrap_queue_> HandleWrapQueue;
typedef ListHead<ReqWrap<uv_req_t>, &ReqWrap<uv_req_t>::req_wrap_queue_>
ReqWrapQueue;

inline HandleWrapQueue* handle_wrap_queue() { return &handle_wrap_queue_; }
inline ReqWrapQueue* req_wrap_queue() { return &req_wrap_queue_; }

private:
static const int kIsolateSlot = NODE_ISOLATE_SLOT;
Expand Down Expand Up @@ -486,9 +490,10 @@ class Environment {
bool printed_error_;
debugger::Agent debugger_agent_;

QUEUE handle_wrap_queue_;
QUEUE req_wrap_queue_;
QUEUE handle_cleanup_queue_;
HandleWrapQueue handle_wrap_queue_;
ReqWrapQueue req_wrap_queue_;
ListHead<HandleCleanup,
&HandleCleanup::handle_cleanup_queue_> handle_cleanup_queue_;
int handle_cleanup_waiting_;

v8::Persistent<v8::External> external_;
Expand Down
4 changes: 1 addition & 3 deletions src/handle_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "util.h"
#include "util-inl.h"
#include "node.h"
#include "queue.h"

namespace node {

Expand Down Expand Up @@ -70,13 +69,12 @@ HandleWrap::HandleWrap(Environment* env,
handle__->data = this;
HandleScope scope(env->isolate());
Wrap(object, this);
QUEUE_INSERT_TAIL(env->handle_wrap_queue(), &handle_wrap_queue_);
env->handle_wrap_queue()->PushBack(this);
}


HandleWrap::~HandleWrap() {
CHECK(persistent().IsEmpty());
QUEUE_REMOVE(&handle_wrap_queue_);
}


Expand Down
9 changes: 5 additions & 4 deletions src/handle_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
#define SRC_HANDLE_WRAP_H_

#include "async-wrap.h"
#include "env.h"
#include "node.h"
#include "queue.h"
#include "util.h"
#include "uv.h"
#include "v8.h"

namespace node {

class Environment;

// Rules:
//
// - Do not throw from handle methods. Set errno.
Expand Down Expand Up @@ -51,9 +51,10 @@ class HandleWrap : public AsyncWrap {
virtual ~HandleWrap() override;

private:
friend class Environment;
friend void GetActiveHandles(const v8::FunctionCallbackInfo<v8::Value>&);
static void OnClose(uv_handle_t* handle);
QUEUE handle_wrap_queue_;
ListNode<HandleWrap> handle_wrap_queue_;
unsigned int flags_;
// Using double underscore due to handle_ member in tcp_wrap. Probably
// tcp_wrap should rename it's member to 'handle'.
Expand Down
Loading

0 comments on commit 38dc0cd

Please sign in to comment.