From 3018441304697fe801fc81385229b5a78a62cc54 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 19 Mar 2019 18:18:22 +0100 Subject: [PATCH] src: store onread callback in internal field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This gives a slight performance improvement. At 2000 runs: confidence improvement accuracy (*) (**) (***) net/net-c2s.js dur=5 type='buf' len=64 *** 0.54 % ±0.16% ±0.21% ±0.27% PR-URL: https://github.com/nodejs/node/pull/26837 Reviewed-By: Joyee Cheung Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis --- src/base_object-inl.h | 16 ++++++++++++++++ src/base_object.h | 9 +++++++++ src/env.h | 1 - src/heap_utils.cc | 2 +- src/js_stream.cc | 2 +- src/node_file.cc | 2 +- src/node_http2.cc | 2 +- src/pipe_wrap.cc | 2 +- src/stream_base.cc | 9 ++++++++- src/stream_base.h | 4 ++++ src/stream_wrap.cc | 2 +- src/tcp_wrap.cc | 3 +-- src/tls_wrap.cc | 2 +- src/tty_wrap.cc | 2 +- 14 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/base_object-inl.h b/src/base_object-inl.h index f1f1498e6c..e3d645056a 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -123,6 +123,22 @@ BaseObject::MakeLazilyInitializedJSTemplate(Environment* env) { return t; } +template +void BaseObject::InternalFieldGet( + v8::Local property, + const v8::PropertyCallbackInfo& info) { + info.GetReturnValue().Set(info.This()->GetInternalField(Field)); +} + +template +void BaseObject::InternalFieldSet(v8::Local property, + v8::Local value, + const v8::PropertyCallbackInfo& info) { + // This could be e.g. value->IsFunction(). + CHECK(((*value)->*typecheck)()); + info.This()->SetInternalField(Field, value); +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/base_object.h b/src/base_object.h index 090bb70aeb..4796a052c0 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -73,6 +73,15 @@ class BaseObject : public MemoryRetainer { static inline v8::Local MakeLazilyInitializedJSTemplate( Environment* env); + // Setter/Getter pair for internal fields that can be passed to SetAccessor. + template + static void InternalFieldGet(v8::Local property, + const v8::PropertyCallbackInfo& info); + template + static void InternalFieldSet(v8::Local property, + v8::Local value, + const v8::PropertyCallbackInfo& info); + private: BaseObject(); diff --git a/src/env.h b/src/env.h index dfe9af01ae..fcd36f5a92 100644 --- a/src/env.h +++ b/src/env.h @@ -241,7 +241,6 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2; V(onmessage_string, "onmessage") \ V(onnewsession_string, "onnewsession") \ V(onocspresponse_string, "onocspresponse") \ - V(onread_string, "onread") \ V(onreadstart_string, "onreadstart") \ V(onreadstop_string, "onreadstop") \ V(onshutdown_string, "onshutdown") \ diff --git a/src/heap_utils.cc b/src/heap_utils.cc index 6690abc78d..6ce8c1dbdb 100644 --- a/src/heap_utils.cc +++ b/src/heap_utils.cc @@ -396,7 +396,7 @@ void Initialize(Local target, Local os = FunctionTemplate::New(env->isolate()); os->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local ost = os->InstanceTemplate(); - ost->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); + ost->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount); os->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "HeapSnapshotStream")); StreamBase::AddMethods(env, os); env->set_streambaseoutputstream_constructor_template(ost); diff --git a/src/js_stream.cc b/src/js_stream.cc index e5f4273476..ddde2a5948 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -205,7 +205,7 @@ void JSStream::Initialize(Local target, FIXED_ONE_BYTE_STRING(env->isolate(), "JSStream"); t->SetClassName(jsStreamString); t->InstanceTemplate() - ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); + ->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount); t->Inherit(AsyncWrap::GetConstructorTemplate(env)); env->SetProtoMethod(t, "finishWrite", Finish); diff --git a/src/node_file.cc b/src/node_file.cc index 063293e788..ec0a00e956 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2231,7 +2231,7 @@ void Initialize(Local target, env->SetProtoMethod(fd, "close", FileHandle::Close); env->SetProtoMethod(fd, "releaseFD", FileHandle::ReleaseFD); Local fdt = fd->InstanceTemplate(); - fdt->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); + fdt->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount); Local handleString = FIXED_ONE_BYTE_STRING(isolate, "FileHandle"); fd->SetClassName(handleString); diff --git a/src/node_http2.cc b/src/node_http2.cc index cbf3519488..b2666f1c8b 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -3011,7 +3011,7 @@ void Initialize(Local target, stream->Inherit(AsyncWrap::GetConstructorTemplate(env)); StreamBase::AddMethods(env, stream); Local streamt = stream->InstanceTemplate(); - streamt->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); + streamt->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount); env->set_http2stream_constructor_template(streamt); target->Set(context, FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Stream"), diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 0103129a66..a6f6f7bb37 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -75,7 +75,7 @@ void PipeWrap::Initialize(Local target, Local pipeString = FIXED_ONE_BYTE_STRING(env->isolate(), "Pipe"); t->SetClassName(pipeString); t->InstanceTemplate() - ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); + ->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount); t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env)); diff --git a/src/stream_base.cc b/src/stream_base.cc index 3874d2d9af..310e490f05 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -21,6 +21,7 @@ using v8::Context; using v8::DontDelete; using v8::DontEnum; using v8::External; +using v8::Function; using v8::FunctionCallbackInfo; using v8::HandleScope; using v8::Integer; @@ -314,7 +315,9 @@ void StreamBase::CallJSOnreadMethod(ssize_t nread, AsyncWrap* wrap = GetAsyncWrap(); CHECK_NOT_NULL(wrap); - wrap->MakeCallback(env->onread_string(), arraysize(argv), argv); + Local onread = wrap->object()->GetInternalField(kOnReadFunctionField); + CHECK(onread->IsFunction()); + wrap->MakeCallback(onread.As(), arraysize(argv), argv); } @@ -376,6 +379,10 @@ void StreamBase::AddMethods(Environment* env, Local t) { t->PrototypeTemplate()->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "isStreamBase"), True(env->isolate())); + t->PrototypeTemplate()->SetAccessor( + FIXED_ONE_BYTE_STRING(env->isolate(), "onread"), + BaseObject::InternalFieldGet, + BaseObject::InternalFieldSet); } void StreamBase::GetFD(const FunctionCallbackInfo& args) { diff --git a/src/stream_base.h b/src/stream_base.h index c7145a2ab2..b61c742971 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -260,7 +260,11 @@ class StreamResource { class StreamBase : public StreamResource { public: + // 0 is reserved for the BaseObject pointer. static constexpr int kStreamBaseField = 1; + static constexpr int kOnReadFunctionField = 2; + static constexpr int kStreamBaseFieldCount = 3; + static void AddMethods(Environment* env, v8::Local target); diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 0cf9f1a114..a9f942150b 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -136,7 +136,7 @@ Local LibuvStreamWrap::GetConstructorTemplate( FIXED_ONE_BYTE_STRING(env->isolate(), "LibuvStreamWrap")); tmpl->Inherit(HandleWrap::GetConstructorTemplate(env)); tmpl->InstanceTemplate()->SetInternalFieldCount( - StreamBase::kStreamBaseField + 1); + StreamBase::kStreamBaseFieldCount); Local get_write_queue_size = FunctionTemplate::New(env->isolate(), GetWriteQueueSize, diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index af9806e488..4d16fa4141 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -80,13 +80,12 @@ void TCPWrap::Initialize(Local target, Local tcpString = FIXED_ONE_BYTE_STRING(env->isolate(), "TCP"); t->SetClassName(tcpString); t->InstanceTemplate() - ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); + ->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount); // Init properties t->InstanceTemplate()->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "reading"), Boolean::New(env->isolate(), false)); t->InstanceTemplate()->Set(env->owner_symbol(), Null(env->isolate())); - t->InstanceTemplate()->Set(env->onread_string(), Null(env->isolate())); t->InstanceTemplate()->Set(env->onconnection_string(), Null(env->isolate())); t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env)); diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 0ce90e02bb..59400d8f3a 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -1062,7 +1062,7 @@ void TLSWrap::Initialize(Local target, FIXED_ONE_BYTE_STRING(env->isolate(), "TLSWrap"); t->SetClassName(tlsWrapString); t->InstanceTemplate() - ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); + ->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount); Local get_write_queue_size = FunctionTemplate::New(env->isolate(), diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc index 423f53c014..405b70343f 100644 --- a/src/tty_wrap.cc +++ b/src/tty_wrap.cc @@ -52,7 +52,7 @@ void TTYWrap::Initialize(Local target, Local t = env->NewFunctionTemplate(New); t->SetClassName(ttyString); t->InstanceTemplate() - ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); + ->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount); t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env)); env->SetProtoMethodNoSideEffect(t, "getWindowSize", TTYWrap::GetWindowSize);