From 419f18d2e245405b839e7441e5f59bc08e9c8d6c Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Mon, 17 Nov 2014 12:54:03 -0800 Subject: [PATCH] async-wrap: explicitly pass parent When instantiating a new AsyncWrap allow the parent AsyncWrap to be passed. This is useful for cases like TCP incoming connections, so the connection can be tied to the server receiving the connection. Because the current architecture instantiates the *Wrap inside a v8::FunctionCallback, the parent pointer is currently wrapped inside a new v8::External every time and passed as an argument. This adds ~80ns to instantiation time. A future optimization would be to add the v8::External as the data field when creating the v8::FunctionTemplate, change the pointer just before making the call then NULL'ing it out afterwards. This adds enough code complexity that it will not be attempted until the current approach demonstrates it is a bottle neck. PR-URL: https://github.com/joyent/node/pull/8110 Signed-off-by: Trevor Norris Reviewed-by: Fedor Indutny Reviewed-by: Alexis Campailla Reviewed-by: Julien Gilli --- src/async-wrap-inl.h | 3 ++- src/async-wrap.h | 3 ++- src/handle_wrap.cc | 5 +++-- src/handle_wrap.h | 3 ++- src/pipe_wrap.cc | 25 ++++++++++++++++++------- src/pipe_wrap.h | 8 ++++++-- src/stream_wrap.cc | 21 ++++++++++++++------- src/stream_wrap.h | 3 ++- src/tcp_wrap.cc | 25 ++++++++++++++++++------- src/tcp_wrap.h | 5 +++-- src/udp_wrap.cc | 21 ++++++++++++++++----- src/udp_wrap.h | 5 +++-- 12 files changed, 89 insertions(+), 38 deletions(-) diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h index 9bd5744b16..6850a019b6 100644 --- a/src/async-wrap-inl.h +++ b/src/async-wrap-inl.h @@ -36,7 +36,8 @@ namespace node { inline AsyncWrap::AsyncWrap(Environment* env, v8::Handle object, - ProviderType provider) + ProviderType provider, + AsyncWrap* parent) : BaseObject(env, object), provider_type_(provider) { } diff --git a/src/async-wrap.h b/src/async-wrap.h index cfdaf50d52..920d1452ec 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -63,7 +63,8 @@ class AsyncWrap : public BaseObject { inline AsyncWrap(Environment* env, v8::Handle object, - ProviderType provider); + ProviderType provider, + AsyncWrap* parent = NULL); inline ~AsyncWrap(); diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc index f8294723ff..a355e5bbba 100644 --- a/src/handle_wrap.cc +++ b/src/handle_wrap.cc @@ -90,8 +90,9 @@ void HandleWrap::Close(const FunctionCallbackInfo& args) { HandleWrap::HandleWrap(Environment* env, Handle object, uv_handle_t* handle, - AsyncWrap::ProviderType provider) - : AsyncWrap(env, object, provider), + AsyncWrap::ProviderType provider, + AsyncWrap* parent) + : AsyncWrap(env, object, provider, parent), flags_(0), handle__(handle) { handle__->data = this; diff --git a/src/handle_wrap.h b/src/handle_wrap.h index 4f6f6e0323..95551a7563 100644 --- a/src/handle_wrap.h +++ b/src/handle_wrap.h @@ -63,7 +63,8 @@ class HandleWrap : public AsyncWrap { HandleWrap(Environment* env, v8::Handle object, uv_handle_t* handle, - AsyncWrap::ProviderType provider); + AsyncWrap::ProviderType provider, + AsyncWrap* parent = NULL); virtual ~HandleWrap(); private: diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 6eb7f576c6..69cdfcdff3 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -21,6 +21,7 @@ #include "pipe_wrap.h" +#include "async-wrap.h" #include "env.h" #include "env-inl.h" #include "handle_wrap.h" @@ -37,6 +38,7 @@ namespace node { using v8::Boolean; using v8::Context; using v8::EscapableHandleScope; +using v8::External; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -74,12 +76,13 @@ uv_pipe_t* PipeWrap::UVHandle() { } -Local PipeWrap::Instantiate(Environment* env) { +Local PipeWrap::Instantiate(Environment* env, AsyncWrap* parent) { EscapableHandleScope handle_scope(env->isolate()); assert(!env->pipe_constructor_template().IsEmpty()); Local constructor = env->pipe_constructor_template()->GetFunction(); assert(!constructor.IsEmpty()); - Local instance = constructor->NewInstance(); + Local ptr = External::New(env->isolate(), parent); + Local instance = constructor->NewInstance(1, &ptr); assert(!instance.IsEmpty()); return handle_scope.Escape(instance); } @@ -150,17 +153,25 @@ void PipeWrap::New(const FunctionCallbackInfo& args) { // Therefore we assert that we are not trying to call this as a // normal function. assert(args.IsConstructCall()); - HandleScope handle_scope(args.GetIsolate()); Environment* env = Environment::GetCurrent(args.GetIsolate()); - new PipeWrap(env, args.This(), args[0]->IsTrue()); + if (args[0]->IsExternal()) { + void* ptr = args[0].As()->Value(); + new PipeWrap(env, args.This(), false, static_cast(ptr)); + } else { + new PipeWrap(env, args.This(), args[0]->IsTrue(), NULL); + } } -PipeWrap::PipeWrap(Environment* env, Handle object, bool ipc) +PipeWrap::PipeWrap(Environment* env, + Handle object, + bool ipc, + AsyncWrap* parent) : StreamWrap(env, object, reinterpret_cast(&handle_), - AsyncWrap::PROVIDER_PIPEWRAP) { + AsyncWrap::PROVIDER_PIPEWRAP, + parent) { int r = uv_pipe_init(env->event_loop(), &handle_, ipc); assert(r == 0); // How do we proxy this error up to javascript? // Suggestion: uv_pipe_init() returns void. @@ -232,7 +243,7 @@ void PipeWrap::OnConnection(uv_stream_t* handle, int status) { } // Instanciate the client javascript object and handle. - Local client_obj = Instantiate(env); + Local client_obj = Instantiate(env, pipe_wrap); // Unwrap the client javascript object. PipeWrap* wrap = Unwrap(client_obj); diff --git a/src/pipe_wrap.h b/src/pipe_wrap.h index 92492c42b7..959f28f4dc 100644 --- a/src/pipe_wrap.h +++ b/src/pipe_wrap.h @@ -22,6 +22,7 @@ #ifndef SRC_PIPE_WRAP_H_ #define SRC_PIPE_WRAP_H_ +#include "async-wrap.h" #include "env.h" #include "stream_wrap.h" @@ -31,13 +32,16 @@ class PipeWrap : public StreamWrap { public: uv_pipe_t* UVHandle(); - static v8::Local Instantiate(Environment* env); + static v8::Local Instantiate(Environment* env, AsyncWrap* parent); static void Initialize(v8::Handle target, v8::Handle unused, v8::Handle context); private: - PipeWrap(Environment* env, v8::Handle object, bool ipc); + PipeWrap(Environment* env, + v8::Handle object, + bool ipc, + AsyncWrap* parent); static void New(const v8::FunctionCallbackInfo& args); static void Bind(const v8::FunctionCallbackInfo& args); diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index b02c58999d..840b16614a 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -81,8 +81,13 @@ void StreamWrap::Initialize(Handle target, StreamWrap::StreamWrap(Environment* env, Local object, uv_stream_t* stream, - AsyncWrap::ProviderType provider) - : HandleWrap(env, object, reinterpret_cast(stream), provider), + AsyncWrap::ProviderType provider, + AsyncWrap* parent) + : HandleWrap(env, + object, + reinterpret_cast(stream), + provider, + parent), stream_(stream), default_callbacks_(this), callbacks_(&default_callbacks_), @@ -145,12 +150,14 @@ void StreamWrap::OnAlloc(uv_handle_t* handle, template -static Local AcceptHandle(Environment* env, uv_stream_t* pipe) { +static Local AcceptHandle(Environment* env, + uv_stream_t* pipe, + AsyncWrap* parent) { EscapableHandleScope scope(env->isolate()); Local wrap_obj; UVType* handle; - wrap_obj = WrapType::Instantiate(env); + wrap_obj = WrapType::Instantiate(env, parent); if (wrap_obj.IsEmpty()) return Local(); @@ -743,11 +750,11 @@ void StreamWrapCallbacks::DoRead(uv_stream_t* handle, Local pending_obj; if (pending == UV_TCP) { - pending_obj = AcceptHandle(env, handle); + pending_obj = AcceptHandle(env, handle, wrap()); } else if (pending == UV_NAMED_PIPE) { - pending_obj = AcceptHandle(env, handle); + pending_obj = AcceptHandle(env, handle, wrap()); } else if (pending == UV_UDP) { - pending_obj = AcceptHandle(env, handle); + pending_obj = AcceptHandle(env, handle, wrap()); } else { assert(pending == UV_UNKNOWN_HANDLE); } diff --git a/src/stream_wrap.h b/src/stream_wrap.h index 5cb13cd67c..38e5d48422 100644 --- a/src/stream_wrap.h +++ b/src/stream_wrap.h @@ -177,7 +177,8 @@ class StreamWrap : public HandleWrap { StreamWrap(Environment* env, v8::Local object, uv_stream_t* stream, - AsyncWrap::ProviderType provider); + AsyncWrap::ProviderType provider, + AsyncWrap* parent = NULL); ~StreamWrap() { if (!callbacks_gc_ && callbacks_ != &default_callbacks_) { diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 1b54b85c6e..a5b20a6ac5 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -38,6 +38,7 @@ namespace node { using v8::Context; using v8::EscapableHandleScope; +using v8::External; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -70,12 +71,13 @@ static void NewTCPConnectWrap(const FunctionCallbackInfo& args) { } -Local TCPWrap::Instantiate(Environment* env) { +Local TCPWrap::Instantiate(Environment* env, AsyncWrap* parent) { EscapableHandleScope handle_scope(env->isolate()); assert(env->tcp_constructor_template().IsEmpty() == false); Local constructor = env->tcp_constructor_template()->GetFunction(); assert(constructor.IsEmpty() == false); - Local instance = constructor->NewInstance(); + Local ptr = External::New(env->isolate(), parent); + Local instance = constructor->NewInstance(1, &ptr); assert(instance.IsEmpty() == false); return handle_scope.Escape(instance); } @@ -171,18 +173,26 @@ void TCPWrap::New(const FunctionCallbackInfo& args) { // Therefore we assert that we are not trying to call this as a // normal function. assert(args.IsConstructCall()); - HandleScope handle_scope(args.GetIsolate()); Environment* env = Environment::GetCurrent(args.GetIsolate()); - TCPWrap* wrap = new TCPWrap(env, args.This()); + TCPWrap* wrap; + if (args.Length() == 0) { + wrap = new TCPWrap(env, args.This(), NULL); + } else if (args[0]->IsExternal()) { + void* ptr = args[0].As()->Value(); + wrap = new TCPWrap(env, args.This(), static_cast(ptr)); + } else { + UNREACHABLE(); + } assert(wrap); } -TCPWrap::TCPWrap(Environment* env, Handle object) +TCPWrap::TCPWrap(Environment* env, Handle object, AsyncWrap* parent) : StreamWrap(env, object, reinterpret_cast(&handle_), - AsyncWrap::PROVIDER_TCPWRAP) { + AsyncWrap::PROVIDER_TCPWRAP, + parent) { int r = uv_tcp_init(env->event_loop(), &handle_); assert(r == 0); // How do we proxy this error up to javascript? // Suggestion: uv_tcp_init() returns void. @@ -365,7 +375,8 @@ void TCPWrap::OnConnection(uv_stream_t* handle, int status) { if (status == 0) { // Instantiate the client javascript object and handle. - Local client_obj = Instantiate(env); + Local client_obj = + Instantiate(env, static_cast(tcp_wrap)); // Unwrap the client javascript object. TCPWrap* wrap = Unwrap(client_obj); diff --git a/src/tcp_wrap.h b/src/tcp_wrap.h index c9981f572d..c923b387f0 100644 --- a/src/tcp_wrap.h +++ b/src/tcp_wrap.h @@ -22,6 +22,7 @@ #ifndef SRC_TCP_WRAP_H_ #define SRC_TCP_WRAP_H_ +#include "async-wrap.h" #include "env.h" #include "stream_wrap.h" @@ -29,7 +30,7 @@ namespace node { class TCPWrap : public StreamWrap { public: - static v8::Local Instantiate(Environment* env); + static v8::Local Instantiate(Environment* env, AsyncWrap* parent); static void Initialize(v8::Handle target, v8::Handle unused, v8::Handle context); @@ -37,7 +38,7 @@ class TCPWrap : public StreamWrap { uv_tcp_t* UVHandle(); private: - TCPWrap(Environment* env, v8::Handle object); + TCPWrap(Environment* env, v8::Handle object, AsyncWrap* parent); ~TCPWrap(); static void New(const v8::FunctionCallbackInfo& args); diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 8b860689fe..614326fa1f 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -34,6 +34,8 @@ namespace node { using v8::Context; +using v8::EscapableHandleScope; +using v8::External; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -78,7 +80,7 @@ static void NewSendWrap(const FunctionCallbackInfo& args) { } -UDPWrap::UDPWrap(Environment* env, Handle object) +UDPWrap::UDPWrap(Environment* env, Handle object, AsyncWrap* parent) : HandleWrap(env, object, reinterpret_cast(&handle_), @@ -143,9 +145,16 @@ void UDPWrap::Initialize(Handle target, void UDPWrap::New(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); - HandleScope handle_scope(args.GetIsolate()); Environment* env = Environment::GetCurrent(args.GetIsolate()); - new UDPWrap(env, args.This()); + if (args.Length() == 0) { + new UDPWrap(env, args.This(), NULL); + } else if (args[0]->IsExternal()) { + new UDPWrap(env, + args.This(), + static_cast(args[0].As()->Value())); + } else { + UNREACHABLE(); + } } @@ -443,10 +452,12 @@ void UDPWrap::OnRecv(uv_udp_t* handle, } -Local UDPWrap::Instantiate(Environment* env) { +Local UDPWrap::Instantiate(Environment* env, AsyncWrap* parent) { // If this assert fires then Initialize hasn't been called yet. assert(env->udp_constructor_function().IsEmpty() == false); - return env->udp_constructor_function()->NewInstance(); + EscapableHandleScope scope(env->isolate()); + Local ptr = External::New(env->isolate(), parent); + return scope.Escape(env->udp_constructor_function()->NewInstance(1, &ptr)); } diff --git a/src/udp_wrap.h b/src/udp_wrap.h index ab0d6fa8e6..693fa51b71 100644 --- a/src/udp_wrap.h +++ b/src/udp_wrap.h @@ -22,6 +22,7 @@ #ifndef SRC_UDP_WRAP_H_ #define SRC_UDP_WRAP_H_ +#include "async-wrap.h" #include "env.h" #include "handle_wrap.h" #include "req_wrap.h" @@ -53,11 +54,11 @@ class UDPWrap: public HandleWrap { static void SetBroadcast(const v8::FunctionCallbackInfo& args); static void SetTTL(const v8::FunctionCallbackInfo& args); - static v8::Local Instantiate(Environment* env); + static v8::Local Instantiate(Environment* env, AsyncWrap* parent); uv_udp_t* UVHandle(); private: - UDPWrap(Environment* env, v8::Handle object); + UDPWrap(Environment* env, v8::Handle object, AsyncWrap* parent); virtual ~UDPWrap(); static void DoBind(const v8::FunctionCallbackInfo& args,