From 86fdaa745572a3bec4370ac7bd05d18e6bf64c01 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 20 Apr 2020 03:51:05 +0800 Subject: [PATCH] src: retrieve binding data from the context Instead of passing them through the data bound to function templates, store references to them in a list embedded inside the context. This makes the function templates more context-independent, and makes it possible to embed binding data in non-main contexts. Co-authored-by: Anna Henningsen PR-URL: https://github.com/nodejs/node/pull/33139 Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- src/README.md | 24 +++++++---- src/env-inl.h | 78 ++++++++++++++++++----------------- src/env.cc | 18 ++------ src/env.h | 35 ++++++++-------- src/fs_event_wrap.cc | 2 +- src/node.cc | 3 +- src/node_binding.cc | 6 ++- src/node_context_data.h | 5 +++ src/node_crypto.cc | 4 +- src/node_env_var.cc | 6 +-- src/node_file-inl.h | 2 +- src/node_file.cc | 21 ++++++---- src/node_file.h | 8 ++-- src/node_http2.cc | 14 ++++--- src/node_http2_state.h | 67 ++++++++++++++---------------- src/node_http_parser.cc | 15 +++++-- src/node_native_module_env.cc | 2 +- src/node_process.h | 3 +- src/node_process_object.cc | 4 +- src/node_stat_watcher.cc | 3 +- src/node_v8.cc | 16 ++++--- src/stream_wrap.cc | 2 +- src/tls_wrap.cc | 2 +- src/udp_wrap.cc | 2 +- src/util-inl.h | 31 ++++++++++++++ src/util.h | 21 ++++++++++ 26 files changed, 233 insertions(+), 161 deletions(-) diff --git a/src/README.md b/src/README.md index e2b256fba20de8..f940da3fe2475d 100644 --- a/src/README.md +++ b/src/README.md @@ -400,16 +400,23 @@ NODE_MODULE_CONTEXT_AWARE_INTERNAL(cares_wrap, Initialize) Some internal bindings, such as the HTTP parser, maintain internal state that only affects that particular binding. In that case, one common way to store -that state is through the use of `Environment::BindingScope`, which gives all -new functions created within it access to an object for storing such state. +that state is through the use of `Environment::AddBindingData`, which gives +binding functions access to an object for storing such state. That object is always a [`BaseObject`][]. +Its class needs to have a static `binding_data_name` field that based on a +constant string, in order to disambiguate it from other classes of this type, +and which could e.g. match the binding’s name (in the example above, that would +be `cares_wrap`). + ```c++ // In the HTTP parser source code file: class BindingData : public BaseObject { public: BindingData(Environment* env, Local obj) : BaseObject(env, obj) {} + static constexpr FastStringKey binding_data_name { "http_parser" }; + std::vector parser_buffer; bool parser_buffer_in_use = false; @@ -418,22 +425,21 @@ class BindingData : public BaseObject { // Available for binding functions, e.g. the HTTP Parser constructor: static void New(const FunctionCallbackInfo& args) { - BindingData* binding_data = Unwrap(args.Data()); + BindingData* binding_data = Environment::GetBindingData(args); new Parser(binding_data, args.This()); } -// ... because the initialization function told the Environment to use this -// BindingData class for all functions created by it: +// ... because the initialization function told the Environment to store the +// BindingData object: void InitializeHttpParser(Local target, Local unused, Local context, void* priv) { Environment* env = Environment::GetCurrent(context); - Environment::BindingScope binding_scope(env); - if (!binding_scope) return; - BindingData* binding_data = binding_scope.data; + BindingData* const binding_data = + env->AddBindingData(context, target); + if (binding_data == nullptr) return; - // Created within the Environment::BindingScope Local t = env->NewFunctionTemplate(Parser::New); ... } diff --git a/src/env-inl.h b/src/env-inl.h index cab967f10db530..853ba78de6d548 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -287,6 +287,10 @@ inline void Environment::AssignToContext(v8::Local context, // Used by Environment::GetCurrent to know that we are on a node context. context->SetAlignedPointerInEmbedderData( ContextEmbedderIndex::kContextTag, Environment::kNodeContextTagPtr); + // Used to retrieve bindings + context->SetAlignedPointerInEmbedderData( + ContextEmbedderIndex::kBindingListIndex, &(this->bindings_)); + #if HAVE_INSPECTOR inspector_agent()->ContextCreated(context, info); #endif // HAVE_INSPECTOR @@ -318,55 +322,55 @@ inline Environment* Environment::GetCurrent(v8::Local context) { inline Environment* Environment::GetCurrent( const v8::FunctionCallbackInfo& info) { - return GetFromCallbackData(info.Data()); + return GetCurrent(info.GetIsolate()->GetCurrentContext()); } template inline Environment* Environment::GetCurrent( const v8::PropertyCallbackInfo& info) { - return GetFromCallbackData(info.Data()); + return GetCurrent(info.GetIsolate()->GetCurrentContext()); } -Environment* Environment::GetFromCallbackData(v8::Local val) { - DCHECK(val->IsObject()); - v8::Local obj = val.As(); - DCHECK_GE(obj->InternalFieldCount(), - BaseObject::kInternalFieldCount); - Environment* env = Unwrap(obj)->env(); - DCHECK(env->as_callback_data_template()->HasInstance(obj)); - return env; +template +inline T* Environment::GetBindingData(const v8::PropertyCallbackInfo& info) { + return GetBindingData(info.GetIsolate()->GetCurrentContext()); } template -Environment::BindingScope::BindingScope(Environment* env) : env(env) { - v8::Local callback_data; - if (!env->MakeBindingCallbackData().ToLocal(&callback_data)) - return; - data = Unwrap(callback_data); - - // No nesting allowed currently. - CHECK_EQ(env->current_callback_data(), env->as_callback_data()); - env->set_current_callback_data(callback_data); +inline T* Environment::GetBindingData( + const v8::FunctionCallbackInfo& info) { + return GetBindingData(info.GetIsolate()->GetCurrentContext()); } template -Environment::BindingScope::~BindingScope() { - env->set_current_callback_data(env->as_callback_data()); +inline T* Environment::GetBindingData(v8::Local context) { + BindingDataStore* map = static_cast( + context->GetAlignedPointerFromEmbedderData( + ContextEmbedderIndex::kBindingListIndex)); + DCHECK_NOT_NULL(map); + auto it = map->find(T::binding_data_name); + if (UNLIKELY(it == map->end())) return nullptr; + T* result = static_cast(it->second.get()); + DCHECK_NOT_NULL(result); + DCHECK_EQ(result->env(), GetCurrent(context)); + return result; } template -v8::MaybeLocal Environment::MakeBindingCallbackData() { - v8::Local ctor; - v8::Local obj; - if (!as_callback_data_template()->GetFunction(context()).ToLocal(&ctor) || - !ctor->NewInstance(context()).ToLocal(&obj)) { - return v8::MaybeLocal(); - } - T* data = new T(this, obj); +inline T* Environment::AddBindingData( + v8::Local context, + v8::Local target) { + DCHECK_EQ(GetCurrent(context), this); // This won't compile if T is not a BaseObject subclass. - CHECK_EQ(data, static_cast(data)); - data->MakeWeak(); - return obj; + BaseObjectPtr item = MakeDetachedBaseObject(this, target); + BindingDataStore* map = static_cast( + context->GetAlignedPointerFromEmbedderData( + ContextEmbedderIndex::kBindingListIndex)); + DCHECK_NOT_NULL(map); + auto result = map->emplace(T::binding_data_name, item); + CHECK(result.second); + DCHECK_EQ(GetBindingData(context), item.get()); + return item.get(); } inline Environment* Environment::GetThreadLocalEnv() { @@ -1085,8 +1089,7 @@ inline v8::Local v8::Local signature, v8::ConstructorBehavior behavior, v8::SideEffectType side_effect_type) { - v8::Local external = current_callback_data(); - return v8::FunctionTemplate::New(isolate(), callback, external, + return v8::FunctionTemplate::New(isolate(), callback, v8::Local(), signature, 0, behavior, side_effect_type); } @@ -1278,9 +1281,10 @@ void Environment::set_process_exit_handler( ENVIRONMENT_STRONG_PERSISTENT_VALUES(V) #undef V - inline v8::Local Environment::context() const { - return PersistentToLocal::Strong(context_); - } +v8::Local Environment::context() const { + return PersistentToLocal::Strong(context_); +} + } // namespace node // These two files depend on each other. Including base_object-inl.h after this diff --git a/src/env.cc b/src/env.cc index f966bfba1ee7e6..3efa5c3b9c98ab 100644 --- a/src/env.cc +++ b/src/env.cc @@ -261,29 +261,17 @@ void TrackingTraceStateObserver::UpdateTraceCategoryState() { USE(cb->Call(env_->context(), Undefined(isolate), arraysize(args), args)); } -class NoBindingData : public BaseObject { - public: - NoBindingData(Environment* env, Local obj) : BaseObject(env, obj) {} - - SET_NO_MEMORY_INFO() - SET_MEMORY_INFO_NAME(NoBindingData) - SET_SELF_SIZE(NoBindingData) -}; - void Environment::CreateProperties() { HandleScope handle_scope(isolate_); Local ctx = context(); + { Context::Scope context_scope(ctx); Local templ = FunctionTemplate::New(isolate()); templ->InstanceTemplate()->SetInternalFieldCount( BaseObject::kInternalFieldCount); - set_as_callback_data_template(templ); - Local obj = MakeBindingCallbackData() - .ToLocalChecked(); - set_as_callback_data(obj); - set_current_callback_data(obj); + set_binding_data_ctor_template(templ); } // Store primordials setup by the per-context script in the environment. @@ -674,6 +662,8 @@ void Environment::RunCleanup() { started_cleanup_ = true; TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment), "RunCleanup", this); + bindings_.clear(); + initial_base_object_count_ = 0; CleanupHandles(); while (!cleanup_hooks_.empty()) { diff --git a/src/env.h b/src/env.h index e2dc5e866f451c..146754d03ab728 100644 --- a/src/env.h +++ b/src/env.h @@ -390,9 +390,9 @@ constexpr size_t kFsStatsBufferLength = V(zero_return_string, "ZERO_RETURN") #define ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V) \ - V(as_callback_data_template, v8::FunctionTemplate) \ V(async_wrap_ctor_template, v8::FunctionTemplate) \ V(async_wrap_object_ctor_template, v8::FunctionTemplate) \ + V(binding_data_ctor_template, v8::FunctionTemplate) \ V(compiled_fn_entry_template, v8::ObjectTemplate) \ V(dir_instance_template, v8::ObjectTemplate) \ V(fd_constructor_template, v8::ObjectTemplate) \ @@ -420,7 +420,6 @@ constexpr size_t kFsStatsBufferLength = V(worker_heap_snapshot_taker_template, v8::ObjectTemplate) #define ENVIRONMENT_STRONG_PERSISTENT_VALUES(V) \ - V(as_callback_data, v8::Object) \ V(async_hooks_after_function, v8::Function) \ V(async_hooks_before_function, v8::Function) \ V(async_hooks_binding, v8::Object) \ @@ -429,7 +428,6 @@ constexpr size_t kFsStatsBufferLength = V(async_hooks_promise_resolve_function, v8::Function) \ V(buffer_prototype_object, v8::Object) \ V(crypto_key_object_constructor, v8::Function) \ - V(current_callback_data, v8::Object) \ V(domain_callback, v8::Function) \ V(domexception_function, v8::Function) \ V(enhance_fatal_stack_after_inspector, v8::Function) \ @@ -864,25 +862,24 @@ class Environment : public MemoryRetainer { static inline Environment* GetCurrent( const v8::PropertyCallbackInfo& info); - static inline Environment* GetFromCallbackData(v8::Local val); - // Methods created using SetMethod(), SetPrototypeMethod(), etc. inside // this scope can access the created T* object using - // Unwrap(args.Data()) later. + // GetBindingData(args) later. template - struct BindingScope { - explicit inline BindingScope(Environment* env); - inline ~BindingScope(); - - T* data = nullptr; - Environment* env; - - inline operator bool() const { return data != nullptr; } - inline bool operator !() const { return data == nullptr; } - }; - + T* AddBindingData(v8::Local context, + v8::Local target); + template + static inline T* GetBindingData(const v8::PropertyCallbackInfo& info); template - inline v8::MaybeLocal MakeBindingCallbackData(); + static inline T* GetBindingData( + const v8::FunctionCallbackInfo& info); + template + static inline T* GetBindingData(v8::Local context); + + typedef std::unordered_map< + FastStringKey, + BaseObjectPtr, + FastStringKey::Hash> BindingDataStore; static uv_key_t thread_local_env; static inline Environment* GetThreadLocalEnv(); @@ -1428,6 +1425,8 @@ class Environment : public MemoryRetainer { void RequestInterruptFromV8(); static void CheckImmediate(uv_check_t* handle); + BindingDataStore bindings_; + // Use an unordered_set, so that we have efficient insertion and removal. std::unordered_set target, Local get_initialized_templ = FunctionTemplate::New(env->isolate(), GetInitialized, - env->current_callback_data(), + Local(), Signature::New(env->isolate(), t)); t->PrototypeTemplate()->SetAccessorProperty( diff --git a/src/node.cc b/src/node.cc index 649ac43fbe7f80..240f1f2e3b00f3 100644 --- a/src/node.cc +++ b/src/node.cc @@ -331,8 +331,7 @@ MaybeLocal Environment::BootstrapNode() { Local env_string = FIXED_ONE_BYTE_STRING(isolate_, "env"); Local env_var_proxy; - if (!CreateEnvVarProxy(context(), isolate_, current_callback_data()) - .ToLocal(&env_var_proxy) || + if (!CreateEnvVarProxy(context(), isolate_).ToLocal(&env_var_proxy) || process_object()->Set(context(), env_string, env_var_proxy).IsNothing()) { return MaybeLocal(); } diff --git a/src/node_binding.cc b/src/node_binding.cc index 592d0ca2a397e2..fdd84c39a20b01 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -230,6 +230,7 @@ namespace node { using v8::Context; using v8::Exception; +using v8::Function; using v8::FunctionCallbackInfo; using v8::Local; using v8::NewStringType; @@ -556,8 +557,11 @@ inline struct node_module* FindModule(struct node_module* list, static Local InitModule(Environment* env, node_module* mod, Local module) { - Local exports = Object::New(env->isolate()); // Internal bindings don't have a "module" object, only exports. + Local ctor = env->binding_data_ctor_template() + ->GetFunction(env->context()) + .ToLocalChecked(); + Local exports = ctor->NewInstance(env->context()).ToLocalChecked(); CHECK_NULL(mod->nm_register_func); CHECK_NOT_NULL(mod->nm_context_register_func); Local unused = Undefined(env->isolate()); diff --git a/src/node_context_data.h b/src/node_context_data.h index 807ba56c7c69fb..912e65b42707fa 100644 --- a/src/node_context_data.h +++ b/src/node_context_data.h @@ -25,11 +25,16 @@ namespace node { #define NODE_CONTEXT_TAG 35 #endif +#ifndef NODE_BINDING_LIST +#define NODE_BINDING_LIST_INDEX 36 +#endif + enum ContextEmbedderIndex { kEnvironment = NODE_CONTEXT_EMBEDDER_DATA_INDEX, kSandboxObject = NODE_CONTEXT_SANDBOX_OBJECT_INDEX, kAllowWasmCodeGeneration = NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX, kContextTag = NODE_CONTEXT_TAG, + kBindingListIndex = NODE_BINDING_LIST_INDEX }; } // namespace node diff --git a/src/node_crypto.cc b/src/node_crypto.cc index afdb2e3c270348..d53a6d2f2325fe 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -505,7 +505,7 @@ void SecureContext::Initialize(Environment* env, Local target) { Local ctx_getter_templ = FunctionTemplate::New(env->isolate(), CtxGetter, - env->current_callback_data(), + Local(), Signature::New(env->isolate(), t)); @@ -5103,7 +5103,7 @@ void DiffieHellman::Initialize(Environment* env, Local target) { Local verify_error_getter_templ = FunctionTemplate::New(env->isolate(), DiffieHellman::VerifyErrorGetter, - env->current_callback_data(), + Local(), Signature::New(env->isolate(), t), /* length */ 0, ConstructorBehavior::kThrow, diff --git a/src/node_env_var.cc b/src/node_env_var.cc index 8eaf79538a26e7..23eaad48586130 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -377,13 +377,11 @@ static void EnvEnumerator(const PropertyCallbackInfo& info) { env->env_vars()->Enumerate(env->isolate())); } -MaybeLocal CreateEnvVarProxy(Local context, - Isolate* isolate, - Local data) { +MaybeLocal CreateEnvVarProxy(Local context, Isolate* isolate) { EscapableHandleScope scope(isolate); Local env_proxy_template = ObjectTemplate::New(isolate); env_proxy_template->SetHandler(NamedPropertyHandlerConfiguration( - EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, data, + EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, Local(), PropertyHandlerFlags::kHasNoSideEffect)); return scope.EscapeMaybe(env_proxy_template->NewInstance(context)); } diff --git a/src/node_file-inl.h b/src/node_file-inl.h index d30d77301005a3..2ad8c73f05e490 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -227,7 +227,7 @@ FSReqBase* GetReqWrap(const v8::FunctionCallbackInfo& args, return Unwrap(value.As()); } - BindingData* binding_data = Unwrap(args.Data()); + BindingData* binding_data = Environment::GetBindingData(args); Environment* env = binding_data->env(); if (value->StrictEquals(env->fs_use_promises_symbol())) { if (use_bigint) { diff --git a/src/node_file.cc b/src/node_file.cc index 203ec5c8ca57b1..be18b18fddd413 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -158,7 +158,7 @@ FileHandle* FileHandle::New(BindingData* binding_data, } void FileHandle::New(const FunctionCallbackInfo& args) { - BindingData* binding_data = Unwrap(args.Data()); + BindingData* binding_data = Environment::GetBindingData(args); Environment* env = binding_data->env(); CHECK(args.IsConstructCall()); CHECK(args[0]->IsInt32()); @@ -543,7 +543,7 @@ void FSReqCallback::SetReturnValue(const FunctionCallbackInfo& args) { void NewFSReqCallback(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); - BindingData* binding_data = Unwrap(args.Data()); + BindingData* binding_data = Environment::GetBindingData(args); new FSReqCallback(binding_data, args.This(), args[0]->IsTrue()); } @@ -948,7 +948,7 @@ static void InternalModuleStat(const FunctionCallbackInfo& args) { } static void Stat(const FunctionCallbackInfo& args) { - BindingData* binding_data = Unwrap(args.Data()); + BindingData* binding_data = Environment::GetBindingData(args); Environment* env = binding_data->env(); const int argc = args.Length(); @@ -979,7 +979,7 @@ static void Stat(const FunctionCallbackInfo& args) { } static void LStat(const FunctionCallbackInfo& args) { - BindingData* binding_data = Unwrap(args.Data()); + BindingData* binding_data = Environment::GetBindingData(args); Environment* env = binding_data->env(); const int argc = args.Length(); @@ -1011,7 +1011,7 @@ static void LStat(const FunctionCallbackInfo& args) { } static void FStat(const FunctionCallbackInfo& args) { - BindingData* binding_data = Unwrap(args.Data()); + BindingData* binding_data = Environment::GetBindingData(args); Environment* env = binding_data->env(); const int argc = args.Length(); @@ -1674,7 +1674,7 @@ static void Open(const FunctionCallbackInfo& args) { } static void OpenFileHandle(const FunctionCallbackInfo& args) { - BindingData* binding_data = Unwrap(args.Data()); + BindingData* binding_data = Environment::GetBindingData(args); Environment* env = binding_data->env(); Isolate* isolate = env->isolate(); @@ -2303,15 +2303,18 @@ void BindingData::MemoryInfo(MemoryTracker* tracker) const { file_handle_read_wrap_freelist); } +// TODO(addaleax): Remove once we're on C++17. +constexpr FastStringKey BindingData::binding_data_name; + void Initialize(Local target, Local unused, Local context, void* priv) { Environment* env = Environment::GetCurrent(context); Isolate* isolate = env->isolate(); - Environment::BindingScope binding_scope(env); - if (!binding_scope) return; - BindingData* binding_data = binding_scope.data; + BindingData* const binding_data = + env->AddBindingData(context, target); + if (binding_data == nullptr) return; env->SetMethod(target, "access", Access); env->SetMethod(target, "close", Close); diff --git a/src/node_file.h b/src/node_file.h index 1fda81361fef79..7690f272848425 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -16,9 +16,9 @@ class FileHandleReadWrap; class BindingData : public BaseObject { public: explicit BindingData(Environment* env, v8::Local wrap) - : BaseObject(env, wrap), - stats_field_array(env->isolate(), kFsStatsBufferLength), - stats_field_bigint_array(env->isolate(), kFsStatsBufferLength) {} + : BaseObject(env, wrap), + stats_field_array(env->isolate(), kFsStatsBufferLength), + stats_field_bigint_array(env->isolate(), kFsStatsBufferLength) {} AliasedFloat64Array stats_field_array; AliasedBigUint64Array stats_field_bigint_array; @@ -26,6 +26,8 @@ class BindingData : public BaseObject { std::vector> file_handle_read_wrap_freelist; + static constexpr FastStringKey binding_data_name { "fs" }; + void MemoryInfo(MemoryTracker* tracker) const override; SET_SELF_SIZE(BindingData) SET_MEMORY_INFO_NAME(BindingData) diff --git a/src/node_http2.cc b/src/node_http2.cc index 6637166e954267..25f353bb1477e0 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2351,7 +2351,7 @@ void HttpErrorString(const FunctionCallbackInfo& args) { // would be suitable, for instance, for creating the Base64 // output for an HTTP2-Settings header field. void PackSettings(const FunctionCallbackInfo& args) { - Http2State* state = Unwrap(args.Data()); + Http2State* state = Environment::GetBindingData(args); args.GetReturnValue().Set(Http2Settings::Pack(state)); } @@ -2359,7 +2359,7 @@ void PackSettings(const FunctionCallbackInfo& args) { // default SETTINGS. RefreshDefaultSettings updates that TypedArray with the // default values. void RefreshDefaultSettings(const FunctionCallbackInfo& args) { - Http2State* state = Unwrap(args.Data()); + Http2State* state = Environment::GetBindingData(args); Http2Settings::RefreshDefaults(state); } @@ -2423,7 +2423,7 @@ void Http2Session::RefreshState(const FunctionCallbackInfo& args) { // Constructor for new Http2Session instances. void Http2Session::New(const FunctionCallbackInfo& args) { - Http2State* state = Unwrap(args.Data()); + Http2State* state = Environment::GetBindingData(args); Environment* env = state->env(); CHECK(args.IsConstructCall()); SessionType type = @@ -2941,6 +2941,9 @@ void Http2State::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("root_buffer", root_buffer); } +// TODO(addaleax): Remove once we're on C++17. +constexpr FastStringKey Http2State::binding_data_name; + // Set up the process.binding('http2') binding. void Initialize(Local target, Local unused, @@ -2950,9 +2953,8 @@ void Initialize(Local target, Isolate* isolate = env->isolate(); HandleScope handle_scope(isolate); - Environment::BindingScope binding_scope(env); - if (!binding_scope) return; - Http2State* state = binding_scope.data; + Http2State* const state = env->AddBindingData(context, target); + if (state == nullptr) return; #define SET_STATE_TYPEDARRAY(name, field) \ target->Set(context, \ diff --git a/src/node_http2_state.h b/src/node_http2_state.h index 80efb40cd27589..2e9d3e5d6bceb5 100644 --- a/src/node_http2_state.h +++ b/src/node_http2_state.h @@ -83,41 +83,36 @@ namespace http2 { class Http2State : public BaseObject { public: Http2State(Environment* env, v8::Local obj) - : BaseObject(env, obj), - root_buffer( - env->isolate(), - sizeof(http2_state_internal)), - session_state_buffer( - env->isolate(), - offsetof(http2_state_internal, session_state_buffer), - IDX_SESSION_STATE_COUNT, - root_buffer), - stream_state_buffer( - env->isolate(), - offsetof(http2_state_internal, stream_state_buffer), - IDX_STREAM_STATE_COUNT, - root_buffer), - stream_stats_buffer( - env->isolate(), - offsetof(http2_state_internal, stream_stats_buffer), - IDX_STREAM_STATS_COUNT, - root_buffer), - session_stats_buffer( - env->isolate(), - offsetof(http2_state_internal, session_stats_buffer), - IDX_SESSION_STATS_COUNT, - root_buffer), - options_buffer( - env->isolate(), - offsetof(http2_state_internal, options_buffer), - IDX_OPTIONS_FLAGS + 1, - root_buffer), - settings_buffer( - env->isolate(), - offsetof(http2_state_internal, settings_buffer), - IDX_SETTINGS_COUNT + 1, - root_buffer) { - } + : BaseObject(env, obj), + root_buffer(env->isolate(), sizeof(http2_state_internal)), + session_state_buffer( + env->isolate(), + offsetof(http2_state_internal, session_state_buffer), + IDX_SESSION_STATE_COUNT, + root_buffer), + stream_state_buffer( + env->isolate(), + offsetof(http2_state_internal, stream_state_buffer), + IDX_STREAM_STATE_COUNT, + root_buffer), + stream_stats_buffer( + env->isolate(), + offsetof(http2_state_internal, stream_stats_buffer), + IDX_STREAM_STATS_COUNT, + root_buffer), + session_stats_buffer( + env->isolate(), + offsetof(http2_state_internal, session_stats_buffer), + IDX_SESSION_STATS_COUNT, + root_buffer), + options_buffer(env->isolate(), + offsetof(http2_state_internal, options_buffer), + IDX_OPTIONS_FLAGS + 1, + root_buffer), + settings_buffer(env->isolate(), + offsetof(http2_state_internal, settings_buffer), + IDX_SETTINGS_COUNT + 1, + root_buffer) {} AliasedUint8Array root_buffer; AliasedFloat64Array session_state_buffer; @@ -131,6 +126,8 @@ class Http2State : public BaseObject { SET_SELF_SIZE(Http2State) SET_MEMORY_INFO_NAME(Http2State) + static constexpr FastStringKey binding_data_name { "http2" }; + private: struct http2_state_internal { // doubles first so that they are always sizeof(double)-aligned diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index f98e9f5c7b6dab..c7a3df8d067af4 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -84,7 +84,10 @@ inline bool IsOWS(char c) { class BindingData : public BaseObject { public: - BindingData(Environment* env, Local obj) : BaseObject(env, obj) {} + BindingData(Environment* env, Local obj) + : BaseObject(env, obj) {} + + static constexpr FastStringKey binding_data_name { "http_parser" }; std::vector parser_buffer; bool parser_buffer_in_use = false; @@ -96,6 +99,9 @@ class BindingData : public BaseObject { SET_MEMORY_INFO_NAME(BindingData) }; +// TODO(addaleax): Remove once we're on C++17. +constexpr FastStringKey BindingData::binding_data_name; + // helper class for the Parser struct StringPtr { StringPtr() { @@ -444,7 +450,7 @@ class Parser : public AsyncWrap, public StreamListener { } static void New(const FunctionCallbackInfo& args) { - BindingData* binding_data = Unwrap(args.Data()); + BindingData* binding_data = Environment::GetBindingData(args); new Parser(binding_data, args.This()); } @@ -920,8 +926,9 @@ void InitializeHttpParser(Local target, Local context, void* priv) { Environment* env = Environment::GetCurrent(context); - Environment::BindingScope binding_scope(env); - if (!binding_scope) return; + BindingData* const binding_data = + env->AddBindingData(context, target); + if (binding_data == nullptr) return; Local t = env->NewFunctionTemplate(Parser::New); t->InstanceTemplate()->SetInternalFieldCount(Parser::kInternalFieldCount); diff --git a/src/node_native_module_env.cc b/src/node_native_module_env.cc index b48ae962dd639f..be647b01c640b0 100644 --- a/src/node_native_module_env.cc +++ b/src/node_native_module_env.cc @@ -190,7 +190,7 @@ void NativeModuleEnv::Initialize(Local target, FIXED_ONE_BYTE_STRING(env->isolate(), "moduleCategories"), GetModuleCategories, nullptr, - env->as_callback_data(), + Local(), DEFAULT, None, SideEffectType::kHasNoSideEffect) diff --git a/src/node_process.h b/src/node_process.h index ad86b449f95290..2e87385348d936 100644 --- a/src/node_process.h +++ b/src/node_process.h @@ -8,8 +8,7 @@ namespace node { v8::MaybeLocal CreateEnvVarProxy(v8::Local context, - v8::Isolate* isolate, - v8::Local data); + v8::Isolate* isolate); // Most of the time, it's best to use `console.error` to write // to the process.stderr stream. However, in some cases, such as diff --git a/src/node_process_object.cc b/src/node_process_object.cc index ee7d771c717c0a..ca17da9583efc8 100644 --- a/src/node_process_object.cc +++ b/src/node_process_object.cc @@ -147,7 +147,7 @@ void PatchProcessObject(const FunctionCallbackInfo& args) { FIXED_ONE_BYTE_STRING(isolate, "title"), ProcessTitleGetter, env->owns_process_state() ? ProcessTitleSetter : nullptr, - env->current_callback_data(), + Local(), DEFAULT, None, SideEffectType::kHasNoSideEffect) @@ -198,7 +198,7 @@ void PatchProcessObject(const FunctionCallbackInfo& args) { FIXED_ONE_BYTE_STRING(isolate, "debugPort"), DebugPortGetter, env->owns_process_state() ? DebugPortSetter : nullptr, - env->current_callback_data()) + Local()) .FromJust()); } diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc index af23affc412935..70903525baa735 100644 --- a/src/node_stat_watcher.cc +++ b/src/node_stat_watcher.cc @@ -95,7 +95,8 @@ void StatWatcher::Callback(uv_fs_poll_t* handle, void StatWatcher::New(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); - fs::BindingData* binding_data = Unwrap(args.Data()); + fs::BindingData* binding_data = + Environment::GetBindingData(args); new StatWatcher(binding_data, args.This(), args[0]->IsTrue()); } diff --git a/src/node_v8.cc b/src/node_v8.cc index 7174d9ae7f830b..047ca594095d16 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -96,6 +96,8 @@ class BindingData : public BaseObject { heap_code_statistics_buffer(env->isolate(), kHeapCodeStatisticsPropertiesCount) {} + static constexpr FastStringKey binding_data_name { "v8" }; + AliasedFloat64Array heap_statistics_buffer; AliasedFloat64Array heap_space_statistics_buffer; AliasedFloat64Array heap_code_statistics_buffer; @@ -111,6 +113,8 @@ class BindingData : public BaseObject { SET_MEMORY_INFO_NAME(BindingData) }; +// TODO(addaleax): Remove once we're on C++17. +constexpr FastStringKey BindingData::binding_data_name; void CachedDataVersionTag(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -121,7 +125,7 @@ void CachedDataVersionTag(const FunctionCallbackInfo& args) { } void UpdateHeapStatisticsBuffer(const FunctionCallbackInfo& args) { - BindingData* data = Unwrap(args.Data()); + BindingData* data = Environment::GetBindingData(args); HeapStatistics s; args.GetIsolate()->GetHeapStatistics(&s); AliasedFloat64Array& buffer = data->heap_statistics_buffer; @@ -132,7 +136,7 @@ void UpdateHeapStatisticsBuffer(const FunctionCallbackInfo& args) { void UpdateHeapSpaceStatisticsBuffer(const FunctionCallbackInfo& args) { - BindingData* data = Unwrap(args.Data()); + BindingData* data = Environment::GetBindingData(args); HeapSpaceStatistics s; Isolate* const isolate = args.GetIsolate(); CHECK(args[0]->IsUint32()); @@ -147,7 +151,7 @@ void UpdateHeapSpaceStatisticsBuffer(const FunctionCallbackInfo& args) { } void UpdateHeapCodeStatisticsBuffer(const FunctionCallbackInfo& args) { - BindingData* data = Unwrap(args.Data()); + BindingData* data = Environment::GetBindingData(args); HeapCodeStatistics s; args.GetIsolate()->GetHeapCodeAndMetadataStatistics(&s); AliasedFloat64Array& buffer = data->heap_code_statistics_buffer; @@ -170,9 +174,9 @@ void Initialize(Local target, Local context, void* priv) { Environment* env = Environment::GetCurrent(context); - Environment::BindingScope binding_scope(env); - if (!binding_scope) return; - BindingData* binding_data = binding_scope.data; + BindingData* const binding_data = + env->AddBindingData(context, target); + if (binding_data == nullptr) return; env->SetMethodNoSideEffect(target, "cachedDataVersionTag", CachedDataVersionTag); diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 90b2bb93416878..bd396110fccade 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -142,7 +142,7 @@ Local LibuvStreamWrap::GetConstructorTemplate( Local get_write_queue_size = FunctionTemplate::New(env->isolate(), GetWriteQueueSize, - env->current_callback_data(), + Local(), Signature::New(env->isolate(), tmpl)); tmpl->PrototypeTemplate()->SetAccessorProperty( env->write_queue_size_string(), diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index df6a989dfbe0b3..0c2c2bbc014cc3 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -1276,7 +1276,7 @@ void TLSWrap::Initialize(Local target, Local get_write_queue_size = FunctionTemplate::New(env->isolate(), GetWriteQueueSize, - env->current_callback_data(), + Local(), Signature::New(env->isolate(), t)); t->PrototypeTemplate()->SetAccessorProperty( env->write_queue_size_string(), diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 1c42481f178a4b..9013bc9fe335c3 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -145,7 +145,7 @@ void UDPWrap::Initialize(Local target, Local get_fd_templ = FunctionTemplate::New(env->isolate(), UDPWrap::GetFD, - env->current_callback_data(), + Local(), signature); t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(), diff --git a/src/util-inl.h b/src/util-inl.h index d44ee09fefbb3a..6b4bdfe034d64f 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -533,6 +533,37 @@ inline bool IsSafeJsInt(v8::Local v) { return false; } +constexpr size_t FastStringKey::HashImpl(const char* str) { + // Low-quality hash (djb2), but just fine for current use cases. + size_t h = 5381; + do { + h = h * 33 + *str; // NOLINT(readability/pointer_notation) + } while (*(str++) != '\0'); + return h; +} + +constexpr size_t FastStringKey::Hash::operator()( + const FastStringKey& key) const { + return key.cached_hash_; +} + +constexpr bool FastStringKey::operator==(const FastStringKey& other) const { + const char* p1 = name_; + const char* p2 = other.name_; + if (p1 == p2) return true; + do { + if (*(p1++) != *(p2++)) return false; + } while (*p1 != '\0'); + return true; +} + +constexpr FastStringKey::FastStringKey(const char* name) + : name_(name), cached_hash_(HashImpl(name)) {} + +constexpr const char* FastStringKey::c_str() const { + return name_; +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/util.h b/src/util.h index 5eaa20b760168b..979091e6d6e002 100644 --- a/src/util.h +++ b/src/util.h @@ -764,6 +764,27 @@ class PersistentToLocal { } }; +// Can be used as a key for std::unordered_map when lookup performance is more +// important than size and the keys are statically used to avoid redundant hash +// computations. +class FastStringKey { + public: + constexpr explicit FastStringKey(const char* name); + + struct Hash { + constexpr size_t operator()(const FastStringKey& key) const; + }; + constexpr bool operator==(const FastStringKey& other) const; + + constexpr const char* c_str() const; + + private: + static constexpr size_t HashImpl(const char* str); + + const char* name_; + size_t cached_hash_; +}; + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS