src: store onread callback in internal field

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: nodejs#26837 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos · Mar 27, 2019 · c82ee2b · c82ee2b
1 parent bab0241
commit c82ee2b
Show file tree

Hide file tree

Showing 14 changed files with 46 additions and 12 deletions.
diff --git a/src/base_object-inl.h b/src/base_object-inl.h
@@ -123,6 +123,22 @@ BaseObject::MakeLazilyInitializedJSTemplate(Environment* env) {
   return t;
 }
 
+template <int Field>
+void BaseObject::InternalFieldGet(
+    v8::Local<v8::String> property,
+    const v8::PropertyCallbackInfo<v8::Value>& info) {
+  info.GetReturnValue().Set(info.This()->GetInternalField(Field));
+}
+
+template <int Field, bool (v8::Value::* typecheck)() const>
+void BaseObject::InternalFieldSet(v8::Local<v8::String> property,
+                                  v8::Local<v8::Value> value,
+                                  const v8::PropertyCallbackInfo<void>& 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
@@ -73,6 +73,15 @@ class BaseObject : public MemoryRetainer {
   static inline v8::Local<v8::FunctionTemplate> MakeLazilyInitializedJSTemplate(
       Environment* env);
 
+  // Setter/Getter pair for internal fields that can be passed to SetAccessor.
+  template <int Field>
+  static void InternalFieldGet(v8::Local<v8::String> property,
+                               const v8::PropertyCallbackInfo<v8::Value>& info);
+  template <int Field, bool (v8::Value::* typecheck)() const>
+  static void InternalFieldSet(v8::Local<v8::String> property,
+                               v8::Local<v8::Value> value,
+                               const v8::PropertyCallbackInfo<void>& info);
+
  private:
   BaseObject();
 

diff --git a/src/env.h b/src/env.h
@@ -235,7 +235,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
@@ -396,7 +396,7 @@ void Initialize(Local<Object> target,
   Local<FunctionTemplate> os = FunctionTemplate::New(env->isolate());
   os->Inherit(AsyncWrap::GetConstructorTemplate(env));
   Local<ObjectTemplate> 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
@@ -205,7 +205,7 @@ void JSStream::Initialize(Local<Object> 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<WriteWrap>);

diff --git a/src/node_file.cc b/src/node_file.cc
@@ -2231,7 +2231,7 @@ void Initialize(Local<Object> target,
   env->SetProtoMethod(fd, "close", FileHandle::Close);
   env->SetProtoMethod(fd, "releaseFD", FileHandle::ReleaseFD);
   Local<ObjectTemplate> fdt = fd->InstanceTemplate();
-  fdt->SetInternalFieldCount(StreamBase::kStreamBaseField + 1);
+  fdt->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
   Local<String> handleString =
        FIXED_ONE_BYTE_STRING(isolate, "FileHandle");
   fd->SetClassName(handleString);

diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -3012,7 +3012,7 @@ void Initialize(Local<Object> target,
   stream->Inherit(AsyncWrap::GetConstructorTemplate(env));
   StreamBase::AddMethods(env, stream);
   Local<ObjectTemplate> 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
@@ -75,7 +75,7 @@ void PipeWrap::Initialize(Local<Object> target,
   Local<String> 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
@@ -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<Value> onread = wrap->object()->GetInternalField(kOnReadFunctionField);
+  CHECK(onread->IsFunction());
+  wrap->MakeCallback(onread.As<Function>(), arraysize(argv), argv);
 }
 
 
@@ -376,6 +379,10 @@ void StreamBase::AddMethods(Environment* env, Local<FunctionTemplate> 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<kOnReadFunctionField>,
+      BaseObject::InternalFieldSet<kOnReadFunctionField, &Value::IsFunction>);
 }
 
 void StreamBase::GetFD(const FunctionCallbackInfo<Value>& args) {

diff --git 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<v8::FunctionTemplate> target);
 

diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc
@@ -136,7 +136,7 @@ Local<FunctionTemplate> LibuvStreamWrap::GetConstructorTemplate(
         FIXED_ONE_BYTE_STRING(env->isolate(), "LibuvStreamWrap"));
     tmpl->Inherit(HandleWrap::GetConstructorTemplate(env));
     tmpl->InstanceTemplate()->SetInternalFieldCount(
-        StreamBase::kStreamBaseField + 1);
+        StreamBase::kStreamBaseFieldCount);
     Local<FunctionTemplate> get_write_queue_size =
         FunctionTemplate::New(env->isolate(),
                               GetWriteQueueSize,

diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc
@@ -80,13 +80,12 @@ void TCPWrap::Initialize(Local<Object> target,
   Local<String> 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
@@ -924,7 +924,7 @@ void TLSWrap::Initialize(Local<Object> target,
       FIXED_ONE_BYTE_STRING(env->isolate(), "TLSWrap");
   t->SetClassName(tlsWrapString);
   t->InstanceTemplate()
-    ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1);
+    ->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
 
   Local<FunctionTemplate> get_write_queue_size =
       FunctionTemplate::New(env->isolate(),

diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc
@@ -52,7 +52,7 @@ void TTYWrap::Initialize(Local<Object> target,
   Local<FunctionTemplate> 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);