From 6307c03e80a349bb8a144654434cfc443dfde489 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 19 Mar 2019 18:18:22 +0100 Subject: [PATCH 1/3] 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% --- 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 f1f1498e6c6128..a682e33a954217 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(kField)); +} + +template +void BaseObject::InternalFieldSet(v8::Local property, + v8::Local value, + const v8::PropertyCallbackInfo& info) { + // This could be e.g. value->IsFunction(). + CHECK_IMPLIES(typecheck != nullptr, ((*value)->*typecheck)()); + info.This()->SetInternalField(kField, value); +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/base_object.h b/src/base_object.h index 090bb70aebeaf8..d2dc0c77c6d481 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 70c566dce9a08f..ca95d3f5e6319c 100644 --- a/src/env.h +++ b/src/env.h @@ -236,7 +236,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 6690abc78d6c67..6ce8c1dbdbb945 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 e5f4273476423a..ddde2a5948f355 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 95a239ee52bf96..6e957b1eca84ed 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 cbf35194881e43..b2666f1c8b6a4e 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 0103129a66c5ab..a6f6f7bb375419 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 3874d2d9af0456..310e490f052e7c 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 c7145a2ab2b236..b61c742971013a 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 0cf9f1a114c64d..a9f942150b2332 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 af9806e4889b4c..4d16fa41413753 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 a0eb51e6eea13d..cf579af85ec124 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -959,7 +959,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 423f53c014d784..405b70343f761e 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); From 6c6817ff0ff3a65a0fca6af376b4a6fce7b01ada Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 21 Mar 2019 20:19:52 +0100 Subject: [PATCH 2/3] fixup! src: store onread callback in internal field --- src/base_object-inl.h | 8 ++++---- src/base_object.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/base_object-inl.h b/src/base_object-inl.h index a682e33a954217..20efdb4264128e 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -123,20 +123,20 @@ BaseObject::MakeLazilyInitializedJSTemplate(Environment* env) { return t; } -template +template void BaseObject::InternalFieldGet( v8::Local property, const v8::PropertyCallbackInfo& info) { - info.GetReturnValue().Set(info.This()->GetInternalField(kField)); + info.GetReturnValue().Set(info.This()->GetInternalField(Field)); } -template +template void BaseObject::InternalFieldSet(v8::Local property, v8::Local value, const v8::PropertyCallbackInfo& info) { // This could be e.g. value->IsFunction(). CHECK_IMPLIES(typecheck != nullptr, ((*value)->*typecheck)()); - info.This()->SetInternalField(kField, value); + info.This()->SetInternalField(Field, value); } } // namespace node diff --git a/src/base_object.h b/src/base_object.h index d2dc0c77c6d481..b8aeeb7200c891 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -74,10 +74,10 @@ class BaseObject : public MemoryRetainer { Environment* env); // Setter/Getter pair for internal fields that can be passed to SetAccessor. - template + template static void InternalFieldGet(v8::Local property, const v8::PropertyCallbackInfo& info); - template + template static void InternalFieldSet(v8::Local property, v8::Local value, const v8::PropertyCallbackInfo& info); From c3ef805d9a463baa3441e9b197a1405741f317e3 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 26 Mar 2019 12:08:10 +0100 Subject: [PATCH 3/3] fixup! fixup! src: store onread callback in internal field --- src/base_object-inl.h | 2 +- src/base_object.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/base_object-inl.h b/src/base_object-inl.h index 20efdb4264128e..e3d645056a5dc2 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -135,7 +135,7 @@ void BaseObject::InternalFieldSet(v8::Local property, v8::Local value, const v8::PropertyCallbackInfo& info) { // This could be e.g. value->IsFunction(). - CHECK_IMPLIES(typecheck != nullptr, ((*value)->*typecheck)()); + CHECK(((*value)->*typecheck)()); info.This()->SetInternalField(Field, value); } diff --git a/src/base_object.h b/src/base_object.h index b8aeeb7200c891..4796a052c0b5c1 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -77,7 +77,7 @@ class BaseObject : public MemoryRetainer { template static void InternalFieldGet(v8::Local property, const v8::PropertyCallbackInfo& info); - template + template static void InternalFieldSet(v8::Local property, v8::Local value, const v8::PropertyCallbackInfo& info);