From 4fba66851856a954d6190e109a43a7266a533f44 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 5 Mar 2019 01:28:45 +0100 Subject: [PATCH 01/15] src: make creating per-binding data structures easier Enable the state associated with the individual bindings, e.g. fs or http2, to be moved out of the Environment class, in order for these to be more modular and for Environment to be come less of a collection of random data fields. Do this by using a BaseObject as the data for callbacks, which can hold the per-binding state. By default, no per-binding state is available, although that can be configured when setting up the binding. --- src/base_object-inl.h | 7 +++--- src/base_object.h | 6 ++--- src/env-inl.h | 48 +++++++++++++++++++++++++++++++++----- src/env.cc | 36 ++++++++++++++++++++-------- src/env.h | 17 ++++++++++++++ src/fs_event_wrap.cc | 2 +- src/node.cc | 2 +- src/node_crypto.cc | 4 ++-- src/node_process_object.cc | 4 ++-- src/stream_wrap.cc | 2 +- src/tls_wrap.cc | 2 +- src/udp_wrap.cc | 2 +- 12 files changed, 101 insertions(+), 31 deletions(-) diff --git a/src/base_object-inl.h b/src/base_object-inl.h index fc2611444c1af4..60ccf81cc559c5 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -100,15 +100,16 @@ Environment* BaseObject::env() const { return env_; } -BaseObject* BaseObject::FromJSObject(v8::Local obj) { - CHECK_GT(obj->InternalFieldCount(), 0); +BaseObject* BaseObject::FromJSObject(v8::Local value) { + v8::Local obj = value.As(); + DCHECK_GE(obj->InternalFieldCount(), BaseObject::kSlot); return static_cast( obj->GetAlignedPointerFromInternalField(BaseObject::kSlot)); } template -T* BaseObject::FromJSObject(v8::Local object) { +T* BaseObject::FromJSObject(v8::Local object) { return static_cast(FromJSObject(object)); } diff --git a/src/base_object.h b/src/base_object.h index 2c67445c31fbb6..03c69976ee4a68 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -61,9 +61,9 @@ class BaseObject : public MemoryRetainer { // was also passed to the `BaseObject()` constructor initially. // This may return `nullptr` if the C++ object has not been constructed yet, // e.g. when the JS object used `MakeLazilyInitializedJSTemplate`. - static inline BaseObject* FromJSObject(v8::Local object); + static inline BaseObject* FromJSObject(v8::Local object); template - static inline T* FromJSObject(v8::Local object); + static inline T* FromJSObject(v8::Local object); // Make the `v8::Global` a weak reference and, `delete` this object once // the JS object has been garbage collected and there are no (strong) @@ -152,7 +152,7 @@ class BaseObject : public MemoryRetainer { // Global alias for FromJSObject() to avoid churn. template -inline T* Unwrap(v8::Local obj) { +inline T* Unwrap(v8::Local obj) { return BaseObject::FromJSObject(obj); } diff --git a/src/env-inl.h b/src/env-inl.h index 69a082c95d9cca..dccfa7ce8c6549 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -327,16 +327,48 @@ inline Environment* Environment::GetCurrent( return GetFromCallbackData(info.Data()); } -inline Environment* Environment::GetFromCallbackData(v8::Local val) { +Environment* Environment::GetFromCallbackData(v8::Local val) { DCHECK(val->IsObject()); v8::Local obj = val.As(); - DCHECK_GE(obj->InternalFieldCount(), 1); - Environment* env = - static_cast(obj->GetAlignedPointerFromInternalField(0)); + DCHECK_GE(obj->InternalFieldCount(), + BaseObject::kInternalFieldCount); + Environment* env = Unwrap(obj)->env(); DCHECK(env->as_callback_data_template()->HasInstance(obj)); return env; } +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); +} + +template +Environment::BindingScope::~BindingScope() { + env->set_current_callback_data(env->as_callback_data()); +} + +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); + // This won't compile if T is not a BaseObject subclass. + CHECK_EQ(data, static_cast(data)); + data->MakeWeak(); + return obj; +} + inline Environment* Environment::GetThreadLocalEnv() { return static_cast(uv_key_get(&thread_local_env)); } @@ -1123,7 +1155,7 @@ inline v8::Local v8::Local signature, v8::ConstructorBehavior behavior, v8::SideEffectType side_effect_type) { - v8::Local external = as_callback_data(); + v8::Local external = current_callback_data(); return v8::FunctionTemplate::New(isolate(), callback, external, signature, 0, behavior, side_effect_type); } @@ -1261,7 +1293,7 @@ void Environment::modify_base_object_count(int64_t delta) { } int64_t Environment::base_object_count() const { - return base_object_count_; + return base_object_count_ - initial_base_object_count_; } void Environment::set_main_utf16(std::unique_ptr str) { @@ -1321,6 +1353,10 @@ void Environment::set_process_exit_handler( } } // namespace node +// These two files depend on each other. Including base_object-inl.h after this +// file is the easiest way to avoid issues with that circular dependency. +#include "base_object-inl.h" + #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #endif // SRC_ENV_INL_H_ diff --git a/src/env.cc b/src/env.cc index 0b2103e6e4aef8..e7afdfe31464e7 100644 --- a/src/env.cc +++ b/src/env.cc @@ -258,18 +258,30 @@ 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(); - Local templ = FunctionTemplate::New(isolate()); - templ->InstanceTemplate()->SetInternalFieldCount(1); - Local obj = templ->GetFunction(ctx) - .ToLocalChecked() - ->NewInstance(ctx) - .ToLocalChecked(); - obj->SetAlignedPointerInInternalField(0, this); - set_as_callback_data(obj); - set_as_callback_data_template(templ); + { + 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); + } // Store primordials setup by the per-context script in the environment. Local per_context_bindings = @@ -417,6 +429,10 @@ Environment::Environment(IsolateData* isolate_data, // TODO(joyeecheung): deserialize when the snapshot covers the environment // properties. CreateProperties(); + + // This adjusts the return value of base_object_count() so that tests that + // check the count do not have to account for internally created BaseObjects. + initial_base_object_count_ = base_object_count(); } Environment::~Environment() { @@ -467,7 +483,7 @@ Environment::~Environment() { } } - CHECK_EQ(base_object_count(), 0); + CHECK_EQ(base_object_count_, 0); } void Environment::InitializeLibuv(bool start_profiler_idle_notifier) { diff --git a/src/env.h b/src/env.h index 61e222f02e9f2e..6b26349b370271 100644 --- a/src/env.h +++ b/src/env.h @@ -436,6 +436,7 @@ 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) \ @@ -871,6 +872,21 @@ class Environment : public MemoryRetainer { 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. + template + struct BindingScope { + explicit inline BindingScope(Environment* env); + inline ~BindingScope(); + + T* data = nullptr; + Environment* env; + }; + + template + inline v8::MaybeLocal MakeBindingCallbackData(); + static uv_key_t thread_local_env; static inline Environment* GetThreadLocalEnv(); @@ -1461,6 +1477,7 @@ class Environment : public MemoryRetainer { bool started_cleanup_ = false; int64_t base_object_count_ = 0; + int64_t initial_base_object_count_ = 0; std::atomic_bool is_stopping_ { false }; std::function process_exit_handler_ { diff --git a/src/fs_event_wrap.cc b/src/fs_event_wrap.cc index 858455bff2d42d..b4b110cc2c839c 100644 --- a/src/fs_event_wrap.cc +++ b/src/fs_event_wrap.cc @@ -108,7 +108,7 @@ void FSEventWrap::Initialize(Local target, Local get_initialized_templ = FunctionTemplate::New(env->isolate(), GetInitialized, - env->as_callback_data(), + env->current_callback_data(), Signature::New(env->isolate(), t)); t->PrototypeTemplate()->SetAccessorProperty( diff --git a/src/node.cc b/src/node.cc index 302b277493c178..3f4148465a397c 100644 --- a/src/node.cc +++ b/src/node.cc @@ -331,7 +331,7 @@ MaybeLocal Environment::BootstrapNode() { Local env_string = FIXED_ONE_BYTE_STRING(isolate_, "env"); Local env_var_proxy; - if (!CreateEnvVarProxy(context(), isolate_, as_callback_data()) + if (!CreateEnvVarProxy(context(), isolate_, current_callback_data()) .ToLocal(&env_var_proxy) || process_object()->Set(context(), env_string, env_var_proxy).IsNothing()) { return MaybeLocal(); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index d61890def60bac..f97c29e28e5a5f 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->as_callback_data(), + env->current_callback_data(), Signature::New(env->isolate(), t)); @@ -5101,7 +5101,7 @@ void DiffieHellman::Initialize(Environment* env, Local target) { Local verify_error_getter_templ = FunctionTemplate::New(env->isolate(), DiffieHellman::VerifyErrorGetter, - env->as_callback_data(), + env->current_callback_data(), Signature::New(env->isolate(), t), /* length */ 0, ConstructorBehavior::kThrow, diff --git a/src/node_process_object.cc b/src/node_process_object.cc index ddbb58abe535b0..ee7d771c717c0a 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->as_callback_data(), + env->current_callback_data(), 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->as_callback_data()) + env->current_callback_data()) .FromJust()); } diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index cc0d046ec11f2d..90b2bb93416878 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->as_callback_data(), + env->current_callback_data(), 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 b24c0df0c2bfe8..df6a989dfbe0b3 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->as_callback_data(), + env->current_callback_data(), 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 770782ee9cf38e..1c42481f178a4b 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->as_callback_data(), + env->current_callback_data(), signature); t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(), From 824f84e2a76f6a08ea7c5d952c3e86e09b113706 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 5 Mar 2019 02:16:06 +0100 Subject: [PATCH 02/15] src: move HTTP/2 state out of Environment Moves state that is specific to HTTP/2 into the HTTP/2 implementation as a cleanup. --- src/env-inl.h | 10 ----- src/env.h | 10 +---- src/node_http2.cc | 89 ++++++++++++++++++++------------------ src/node_http2.h | 38 ++++++++++------ src/node_http2_state.h | 77 ++++++++++++++++++--------------- src/node_http_common-inl.h | 2 +- src/node_http_common.h | 2 +- 7 files changed, 119 insertions(+), 109 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index dccfa7ce8c6549..1070c189df3dda 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -630,16 +630,6 @@ inline void Environment::set_http_parser_buffer_in_use(bool in_use) { http_parser_buffer_in_use_ = in_use; } -inline http2::Http2State* Environment::http2_state() const { - return http2_state_.get(); -} - -inline void Environment::set_http2_state( - std::unique_ptr buffer) { - CHECK(!http2_state_); // Should be set only once. - http2_state_ = std::move(buffer); -} - inline AliasedFloat64Array* Environment::fs_stats_field_array() { return &fs_stats_field_array_; } diff --git a/src/env.h b/src/env.h index 6b26349b370271..ff68fe98fe31da 100644 --- a/src/env.h +++ b/src/env.h @@ -33,7 +33,6 @@ #include "handle_wrap.h" #include "node.h" #include "node_binding.h" -#include "node_http2_state.h" #include "node_main_instance.h" #include "node_options.h" #include "req_wrap.h" @@ -50,8 +49,6 @@ #include #include -struct nghttp2_rcbuf; - namespace node { namespace contextify { @@ -515,7 +512,8 @@ class IsolateData : public MemoryRetainer { #undef VP inline v8::Local async_wrap_provider(int index) const; - std::unordered_map> http_static_strs; + std::unordered_map> static_str_map; + inline v8::Isolate* isolate() const; IsolateData(const IsolateData&) = delete; IsolateData& operator=(const IsolateData&) = delete; @@ -1023,9 +1021,6 @@ class Environment : public MemoryRetainer { inline bool http_parser_buffer_in_use() const; inline void set_http_parser_buffer_in_use(bool in_use); - inline http2::Http2State* http2_state() const; - inline void set_http2_state(std::unique_ptr state); - EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; } inline AliasedFloat64Array* fs_stats_field_array(); @@ -1388,7 +1383,6 @@ class Environment : public MemoryRetainer { char* http_parser_buffer_ = nullptr; bool http_parser_buffer_in_use_ = false; - std::unique_ptr http2_state_; EnabledDebugList enabled_debug_list_; AliasedFloat64Array fs_stats_field_array_; diff --git a/src/node_http2.cc b/src/node_http2.cc index cd3b989a57e46e..df6fdfe407f3de 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -4,7 +4,6 @@ #include "node.h" #include "node_buffer.h" #include "node_http2.h" -#include "node_http2_state.h" #include "node_http_common-inl.h" #include "node_mem-inl.h" #include "node_perf.h" @@ -113,7 +112,7 @@ Http2Scope::~Http2Scope() { // instances to configure an appropriate nghttp2_options struct. The class // uses a single TypedArray instance that is shared with the JavaScript side // to more efficiently pass values back and forth. -Http2Options::Http2Options(Environment* env, nghttp2_session_type type) { +Http2Options::Http2Options(Http2State* http2_state, nghttp2_session_type type) { nghttp2_option* option; CHECK_EQ(nghttp2_option_new(&option), 0); CHECK_NOT_NULL(option); @@ -138,7 +137,7 @@ Http2Options::Http2Options(Environment* env, nghttp2_session_type type) { nghttp2_option_set_builtin_recv_extension_type(option, NGHTTP2_ORIGIN); } - AliasedUint32Array& buffer = env->http2_state()->options_buffer; + AliasedUint32Array& buffer = http2_state->options_buffer; uint32_t flags = buffer[IDX_OPTIONS_FLAGS]; if (flags & (1 << IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE)) { @@ -213,8 +212,8 @@ Http2Options::Http2Options(Environment* env, nghttp2_session_type type) { SetMaxSessionMemory(buffer[IDX_OPTIONS_MAX_SESSION_MEMORY] * 1000000); } -void Http2Session::Http2Settings::Init() { - AliasedUint32Array& buffer = env()->http2_state()->settings_buffer; +void Http2Session::Http2Settings::Init(Http2State* http2_state) { + AliasedUint32Array& buffer = http2_state->settings_buffer; uint32_t flags = buffer[IDX_SETTINGS_COUNT]; size_t n = 0; @@ -244,14 +243,14 @@ void Http2Session::Http2Settings::Init() { // The Http2Settings class is used to configure a SETTINGS frame that is // to be sent to the connected peer. The settings are set using a TypedArray // that is shared with the JavaScript side. -Http2Session::Http2Settings::Http2Settings(Environment* env, +Http2Session::Http2Settings::Http2Settings(Http2State* http2_state, Http2Session* session, Local obj, uint64_t start_time) - : AsyncWrap(env, obj, PROVIDER_HTTP2SETTINGS), + : AsyncWrap(http2_state->env(), obj, PROVIDER_HTTP2SETTINGS), session_(session), startTime_(start_time) { - Init(); + Init(http2_state); } // Generates a Buffer that contains the serialized payload of a SETTINGS @@ -272,10 +271,9 @@ Local Http2Session::Http2Settings::Pack() { // Updates the shared TypedArray with the current remote or local settings for // the session. -void Http2Session::Http2Settings::Update(Environment* env, - Http2Session* session, +void Http2Session::Http2Settings::Update(Http2Session* session, get_setting fn) { - AliasedUint32Array& buffer = env->http2_state()->settings_buffer; + AliasedUint32Array& buffer = session->http2_state()->settings_buffer; buffer[IDX_SETTINGS_HEADER_TABLE_SIZE] = fn(**session, NGHTTP2_SETTINGS_HEADER_TABLE_SIZE); buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS] = @@ -293,8 +291,8 @@ void Http2Session::Http2Settings::Update(Environment* env, } // Initializes the shared TypedArray with the default settings values. -void Http2Session::Http2Settings::RefreshDefaults(Environment* env) { - AliasedUint32Array& buffer = env->http2_state()->settings_buffer; +void Http2Session::Http2Settings::RefreshDefaults(Http2State* http2_state) { + AliasedUint32Array& buffer = http2_state->settings_buffer; buffer[IDX_SETTINGS_HEADER_TABLE_SIZE] = DEFAULT_SETTINGS_HEADER_TABLE_SIZE; @@ -469,16 +467,17 @@ void Http2Session::DecreaseAllocatedSize(size_t size) { current_nghttp2_memory_ -= size; } -Http2Session::Http2Session(Environment* env, +Http2Session::Http2Session(Http2State* http2_state, Local wrap, nghttp2_session_type type) - : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_HTTP2SESSION), - session_type_(type) { + : AsyncWrap(http2_state->env(), wrap, AsyncWrap::PROVIDER_HTTP2SESSION), + session_type_(type), + http2_state_(http2_state) { MakeWeak(); statistics_.start_time = uv_hrtime(); // Capture the configuration options for this session - Http2Options opts(env, type); + Http2Options opts(http2_state, type); max_session_memory_ = opts.GetMaxSessionMemory(); @@ -521,13 +520,13 @@ Http2Session::Http2Session(Environment* env, // Make the js_fields_ property accessible to JS land. js_fields_store_ = - ArrayBuffer::NewBackingStore(env->isolate(), sizeof(SessionJSFields)); + ArrayBuffer::NewBackingStore(env()->isolate(), sizeof(SessionJSFields)); js_fields_ = new(js_fields_store_->Data()) SessionJSFields; - Local ab = ArrayBuffer::New(env->isolate(), js_fields_store_); + Local ab = ArrayBuffer::New(env()->isolate(), js_fields_store_); Local uint8_arr = Uint8Array::New(ab, 0, kSessionUint8FieldCount); - USE(wrap->Set(env->context(), env->fields_string(), uint8_arr)); + USE(wrap->Set(env()->context(), env()->fields_string(), uint8_arr)); } Http2Session::~Http2Session() { @@ -551,15 +550,17 @@ inline bool HasHttp2Observer(Environment* env) { } void Http2Stream::EmitStatistics() { + CHECK_NOT_NULL(session()); if (!HasHttp2Observer(env())) return; auto entry = - std::make_unique(env(), id_, statistics_); + std::make_unique( + session()->http2_state(), id_, statistics_); env()->SetImmediate([entry = move(entry)](Environment* env) { if (!HasHttp2Observer(env)) return; HandleScope handle_scope(env->isolate()); - AliasedFloat64Array& buffer = env->http2_state()->stream_stats_buffer; + AliasedFloat64Array& buffer = entry->http2_state()->stream_stats_buffer; buffer[IDX_STREAM_STATS_ID] = entry->id(); if (entry->first_byte() != 0) { buffer[IDX_STREAM_STATS_TIMETOFIRSTBYTE] = @@ -592,12 +593,12 @@ void Http2Session::EmitStatistics() { if (!HasHttp2Observer(env())) return; auto entry = std::make_unique( - env(), statistics_, session_type_); + http2_state(), statistics_, session_type_); env()->SetImmediate([entry = std::move(entry)](Environment* env) { if (!HasHttp2Observer(env)) return; HandleScope handle_scope(env->isolate()); - AliasedFloat64Array& buffer = env->http2_state()->session_stats_buffer; + AliasedFloat64Array& buffer = entry->http2_state()->session_stats_buffer; buffer[IDX_SESSION_STATS_TYPE] = entry->type(); buffer[IDX_SESSION_STATS_PINGRTT] = entry->ping_rtt() / 1e6; buffer[IDX_SESSION_STATS_FRAMESRECEIVED] = entry->frame_count(); @@ -2334,7 +2335,8 @@ 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) { - Environment* env = Environment::GetCurrent(args); + Http2State* state = Unwrap(args.Data()); + Environment* env = state->env(); // TODO(addaleax): We should not be creating a full AsyncWrap for this. Local obj; if (!env->http2settings_constructor_template() @@ -2342,7 +2344,7 @@ void PackSettings(const FunctionCallbackInfo& args) { .ToLocal(&obj)) { return; } - Http2Session::Http2Settings settings(env, nullptr, obj); + Http2Session::Http2Settings settings(state, nullptr, obj); args.GetReturnValue().Set(settings.Pack()); } @@ -2350,8 +2352,8 @@ void PackSettings(const FunctionCallbackInfo& args) { // default SETTINGS. RefreshDefaultSettings updates that TypedArray with the // default values. void RefreshDefaultSettings(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Http2Session::Http2Settings::RefreshDefaults(env); + Http2State* state = Unwrap(args.Data()); + Http2Session::Http2Settings::RefreshDefaults(state); } // Sets the next stream ID the Http2Session. If successful, returns true. @@ -2373,10 +2375,9 @@ void Http2Session::SetNextStreamID(const FunctionCallbackInfo& args) { // values established for each of the settings so those can be read in JS land. template void Http2Session::RefreshSettings(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); Http2Session* session; ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); - Http2Settings::Update(env, session, fn); + Http2Settings::Update(session, fn); Debug(session, "settings refreshed for session"); } @@ -2384,12 +2385,11 @@ void Http2Session::RefreshSettings(const FunctionCallbackInfo& args) { // information of the current Http2Session. This updates the values in the // TypedArray so those can be read in JS land. void Http2Session::RefreshState(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); Http2Session* session; ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); Debug(session, "refreshing state"); - AliasedFloat64Array& buffer = env->http2_state()->session_state_buffer; + AliasedFloat64Array& buffer = session->http2_state()->session_state_buffer; nghttp2_session* s = **session; @@ -2416,11 +2416,12 @@ void Http2Session::RefreshState(const FunctionCallbackInfo& args) { // Constructor for new Http2Session instances. void Http2Session::New(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + Http2State* state = Unwrap(args.Data()); + Environment* env = state->env(); CHECK(args.IsConstructCall()); int32_t val = args[0]->Int32Value(env->context()).ToChecked(); nghttp2_session_type type = static_cast(val); - Http2Session* session = new Http2Session(env, args.This(), type); + Http2Session* session = new Http2Session(state, args.This(), type); session->get_async_id(); // avoid compiler warning Debug(session, "session created"); } @@ -2645,13 +2646,14 @@ void Http2Stream::Priority(const FunctionCallbackInfo& args) { // information about the Http2Stream. This updates the values in that // TypedArray so that the state can be read by JS. void Http2Stream::RefreshState(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); Http2Stream* stream; ASSIGN_OR_RETURN_UNWRAP(&stream, args.Holder()); Debug(stream, "refreshing state"); - AliasedFloat64Array& buffer = env->http2_state()->stream_state_buffer; + CHECK_NOT_NULL(stream->session()); + AliasedFloat64Array& buffer = + stream->session()->http2_state()->stream_state_buffer; nghttp2_stream* str = **stream; nghttp2_session* s = **(stream->session()); @@ -2797,7 +2799,8 @@ void Http2Session::Settings(const FunctionCallbackInfo& args) { return; Http2Settings* settings = session->AddSettings( - MakeDetachedBaseObject(session->env(), session, obj, 0)); + MakeDetachedBaseObject( + session->http2_state(), session, obj, 0)); if (settings == nullptr) return args.GetReturnValue().Set(false); settings->Send(); @@ -2921,6 +2924,10 @@ void SetCallbackFunctions(const FunctionCallbackInfo& args) { #undef SET_FUNCTION } +void Http2State::MemoryInfo(MemoryTracker* tracker) const { + tracker->TrackField("root_buffer", root_buffer); +} + // Set up the process.binding('http2') binding. void Initialize(Local target, Local unused, @@ -2928,9 +2935,11 @@ void Initialize(Local target, void* priv) { Environment* env = Environment::GetCurrent(context); Isolate* isolate = env->isolate(); - HandleScope scope(isolate); + HandleScope handle_scope(isolate); - std::unique_ptr state(new Http2State(isolate)); + Environment::BindingScope binding_scope(env); + Http2State* state = binding_scope.data; + if (state == nullptr) return; #define SET_STATE_TYPEDARRAY(name, field) \ target->Set(context, \ @@ -2953,8 +2962,6 @@ void Initialize(Local target, "sessionStats", state->session_stats_buffer.GetJSArray()); #undef SET_STATE_TYPEDARRAY - env->set_http2_state(std::move(state)); - NODE_DEFINE_CONSTANT(target, kBitfield); NODE_DEFINE_CONSTANT(target, kSessionPriorityListenerCount); NODE_DEFINE_CONSTANT(target, kSessionFrameErrorListenerCount); diff --git a/src/node_http2.h b/src/node_http2.h index b07f9cf4130d7b..f9a0cae85ff2ae 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -189,7 +189,8 @@ class Http2Scope { // configured. class Http2Options { public: - Http2Options(Environment* env, nghttp2_session_type type); + Http2Options(Http2State* http2_state, + nghttp2_session_type type); ~Http2Options() = default; @@ -541,7 +542,7 @@ class Http2Session : public AsyncWrap, public StreamListener, public mem::NgLibMemoryManager { public: - Http2Session(Environment* env, + Http2Session(Http2State* http2_state, v8::Local wrap, nghttp2_session_type type = NGHTTP2_SESSION_SERVER); ~Http2Session() override; @@ -674,6 +675,9 @@ class Http2Session : public AsyncWrap, return env()->event_loop(); } + Http2State* http2_state() { + return http2_state_.get(); + } BaseObjectPtr PopPing(); Http2Ping* AddPing(BaseObjectPtr ping); @@ -871,6 +875,9 @@ class Http2Session : public AsyncWrap, uint32_t invalid_frame_count_ = 0; void PushOutgoingBuffer(NgHttp2StreamWrite&& write); + + BaseObjectPtr http2_state_; + void CopyDataIntoOutgoing(const uint8_t* src, size_t src_length); void ClearOutgoing(int status); @@ -881,11 +888,11 @@ class Http2Session : public AsyncWrap, class Http2SessionPerformanceEntry : public performance::PerformanceEntry { public: Http2SessionPerformanceEntry( - Environment* env, + Http2State* http2_state, const Http2Session::Statistics& stats, nghttp2_session_type type) : performance::PerformanceEntry( - env, "Http2Session", "http2", + http2_state->env(), "Http2Session", "http2", stats.start_time, stats.end_time), ping_rtt_(stats.ping_rtt), @@ -896,7 +903,8 @@ class Http2SessionPerformanceEntry : public performance::PerformanceEntry { stream_count_(stats.stream_count), max_concurrent_streams_(stats.max_concurrent_streams), stream_average_duration_(stats.stream_average_duration), - session_type_(type) { } + session_type_(type), + http2_state_(http2_state) { } uint64_t ping_rtt() const { return ping_rtt_; } uint64_t data_sent() const { return data_sent_; } @@ -907,6 +915,7 @@ class Http2SessionPerformanceEntry : public performance::PerformanceEntry { size_t max_concurrent_streams() const { return max_concurrent_streams_; } double stream_average_duration() const { return stream_average_duration_; } nghttp2_session_type type() const { return session_type_; } + Http2State* http2_state() const { return http2_state_.get(); } void Notify(v8::Local obj) { performance::PerformanceEntry::Notify(env(), kind(), obj); @@ -922,17 +931,18 @@ class Http2SessionPerformanceEntry : public performance::PerformanceEntry { size_t max_concurrent_streams_; double stream_average_duration_; nghttp2_session_type session_type_; + BaseObjectPtr http2_state_; }; class Http2StreamPerformanceEntry : public performance::PerformanceEntry { public: Http2StreamPerformanceEntry( - Environment* env, + Http2State* http2_state, int32_t id, const Http2Stream::Statistics& stats) : performance::PerformanceEntry( - env, "Http2Stream", "http2", + http2_state->env(), "Http2Stream", "http2", stats.start_time, stats.end_time), id_(id), @@ -940,7 +950,8 @@ class Http2StreamPerformanceEntry first_byte_(stats.first_byte), first_byte_sent_(stats.first_byte_sent), sent_bytes_(stats.sent_bytes), - received_bytes_(stats.received_bytes) { } + received_bytes_(stats.received_bytes), + http2_state_(http2_state) { } int32_t id() const { return id_; } uint64_t first_header() const { return first_header_; } @@ -948,6 +959,7 @@ class Http2StreamPerformanceEntry uint64_t first_byte_sent() const { return first_byte_sent_; } uint64_t sent_bytes() const { return sent_bytes_; } uint64_t received_bytes() const { return received_bytes_; } + Http2State* http2_state() const { return http2_state_.get(); } void Notify(v8::Local obj) { performance::PerformanceEntry::Notify(env(), kind(), obj); @@ -960,6 +972,7 @@ class Http2StreamPerformanceEntry uint64_t first_byte_sent_; uint64_t sent_bytes_; uint64_t received_bytes_; + BaseObjectPtr http2_state_; }; class Http2Session::Http2Ping : public AsyncWrap { @@ -987,7 +1000,7 @@ class Http2Session::Http2Ping : public AsyncWrap { // structs. class Http2Session::Http2Settings : public AsyncWrap { public: - Http2Settings(Environment* env, + Http2Settings(Http2State* http2_state, Http2Session* session, v8::Local obj, uint64_t start_time = uv_hrtime()); @@ -1006,15 +1019,14 @@ class Http2Session::Http2Settings : public AsyncWrap { v8::Local Pack(); // Resets the default values in the settings buffer - static void RefreshDefaults(Environment* env); + static void RefreshDefaults(Http2State* http2_state); // Update the local or remote settings for the given session - static void Update(Environment* env, - Http2Session* session, + static void Update(Http2Session* session, get_setting fn); private: - void Init(); + void Init(Http2State* http2_state); Http2Session* session_; uint64_t startTime_; size_t count_ = 0; diff --git a/src/node_http2_state.h b/src/node_http2_state.h index 1ba3677f47a4f6..80efb40cd27589 100644 --- a/src/node_http2_state.h +++ b/src/node_http2_state.h @@ -5,6 +5,8 @@ #include "aliased_buffer.h" +struct nghttp2_rcbuf; + namespace node { namespace http2 { @@ -78,42 +80,43 @@ namespace http2 { IDX_SESSION_STATS_COUNT }; -class Http2State { +class Http2State : public BaseObject { public: - explicit Http2State(v8::Isolate* isolate) : - root_buffer( - isolate, - sizeof(http2_state_internal)), - session_state_buffer( - isolate, - offsetof(http2_state_internal, session_state_buffer), - IDX_SESSION_STATE_COUNT, - root_buffer), - stream_state_buffer( - isolate, - offsetof(http2_state_internal, stream_state_buffer), - IDX_STREAM_STATE_COUNT, - root_buffer), - stream_stats_buffer( - isolate, - offsetof(http2_state_internal, stream_stats_buffer), - IDX_STREAM_STATS_COUNT, - root_buffer), - session_stats_buffer( - isolate, - offsetof(http2_state_internal, session_stats_buffer), - IDX_SESSION_STATS_COUNT, - root_buffer), - options_buffer( - isolate, - offsetof(http2_state_internal, options_buffer), - IDX_OPTIONS_FLAGS + 1, - root_buffer), - settings_buffer( - isolate, - offsetof(http2_state_internal, settings_buffer), - IDX_SETTINGS_COUNT + 1, - root_buffer) { + 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) { } AliasedUint8Array root_buffer; @@ -124,6 +127,10 @@ class Http2State { AliasedUint32Array options_buffer; AliasedUint32Array settings_buffer; + void MemoryInfo(MemoryTracker* tracker) const override; + SET_SELF_SIZE(Http2State) + SET_MEMORY_INFO_NAME(Http2State) + private: struct http2_state_internal { // doubles first so that they are always sizeof(double)-aligned diff --git a/src/node_http_common-inl.h b/src/node_http_common-inl.h index d63cdf79a4b468..2c76dc3a1fd372 100644 --- a/src/node_http_common-inl.h +++ b/src/node_http_common-inl.h @@ -143,7 +143,7 @@ v8::MaybeLocal NgHeader::GetName( // If header_name is not nullptr, then it is a known header with // a statically defined name. We can safely internalize it here. if (header_name != nullptr) { - auto& static_str_map = env_->isolate_data()->http_static_strs; + auto& static_str_map = env_->isolate_data()->static_str_map; v8::Eternal eternal = static_str_map[header_name]; if (eternal.IsEmpty()) { v8::Local str = OneByteString(env_->isolate(), header_name); diff --git a/src/node_http_common.h b/src/node_http_common.h index 41b5f419d94338..d2bdddd93f4649 100644 --- a/src/node_http_common.h +++ b/src/node_http_common.h @@ -409,7 +409,7 @@ class NgRcBufPointer : public MemoryRetainer { NgRcBufPointer ptr) { Environment* env = allocator->env(); if (ptr.IsStatic()) { - auto& static_str_map = env->isolate_data()->http_static_strs; + auto& static_str_map = env->isolate_data()->static_str_map; const char* header_name = reinterpret_cast(ptr.data()); v8::Eternal& eternal = static_str_map[header_name]; if (eternal.IsEmpty()) { From 9f0f5f91cecb4eea8c08fcbce74fc44b10b9f8c7 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 5 Mar 2019 02:32:08 +0100 Subject: [PATCH 03/15] src: move v8 stats buffers out of Environment Moves state that is specific to the `v8` binding into the `v8` binding implementation as a cleanup. --- src/env-inl.h | 33 -------------------- src/env.h | 18 +---------- src/node_v8.cc | 81 +++++++++++++++++++++++++++++++++----------------- 3 files changed, 54 insertions(+), 78 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 1070c189df3dda..d81ea0fb95df44 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -580,39 +580,6 @@ inline double Environment::get_default_trigger_async_id() { return default_trigger_async_id; } -inline double* Environment::heap_statistics_buffer() const { - CHECK_NOT_NULL(heap_statistics_buffer_); - return static_cast(heap_statistics_buffer_->Data()); -} - -inline void Environment::set_heap_statistics_buffer( - std::shared_ptr backing_store) { - CHECK(!heap_statistics_buffer_); // Should be set only once. - heap_statistics_buffer_ = std::move(backing_store); -} - -inline double* Environment::heap_space_statistics_buffer() const { - CHECK(heap_space_statistics_buffer_); - return static_cast(heap_space_statistics_buffer_->Data()); -} - -inline void Environment::set_heap_space_statistics_buffer( - std::shared_ptr backing_store) { - CHECK(!heap_space_statistics_buffer_); // Should be set only once. - heap_space_statistics_buffer_ = std::move(backing_store); -} - -inline double* Environment::heap_code_statistics_buffer() const { - CHECK(heap_code_statistics_buffer_); - return static_cast(heap_code_statistics_buffer_->Data()); -} - -inline void Environment::set_heap_code_statistics_buffer( - std::shared_ptr backing_store) { - CHECK(!heap_code_statistics_buffer_); // Should be set only once. - heap_code_statistics_buffer_ = std::move(backing_store); -} - inline char* Environment::http_parser_buffer() const { return http_parser_buffer_; } diff --git a/src/env.h b/src/env.h index ff68fe98fe31da..9f15b56c6bf213 100644 --- a/src/env.h +++ b/src/env.h @@ -1004,18 +1004,6 @@ class Environment : public MemoryRetainer { inline uint32_t get_next_script_id(); inline uint32_t get_next_function_id(); - inline double* heap_statistics_buffer() const; - inline void set_heap_statistics_buffer( - std::shared_ptr backing_store); - - inline double* heap_space_statistics_buffer() const; - inline void set_heap_space_statistics_buffer( - std::shared_ptr backing_store); - - inline double* heap_code_statistics_buffer() const; - inline void set_heap_code_statistics_buffer( - std::shared_ptr backing_store); - inline char* http_parser_buffer() const; inline void set_http_parser_buffer(char* buffer); inline bool http_parser_buffer_in_use() const; @@ -1377,14 +1365,10 @@ class Environment : public MemoryRetainer { int handle_cleanup_waiting_ = 0; int request_waiting_ = 0; - std::shared_ptr heap_statistics_buffer_; - std::shared_ptr heap_space_statistics_buffer_; - std::shared_ptr heap_code_statistics_buffer_; - char* http_parser_buffer_ = nullptr; bool http_parser_buffer_in_use_ = false; - EnabledDebugList enabled_debug_list_; + AliasedFloat64Array fs_stats_field_array_; AliasedBigUint64Array fs_stats_field_bigint_array_; diff --git a/src/node_v8.cc b/src/node_v8.cc index a59c2170dae068..c09b0a98b70f6c 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -20,7 +20,9 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. #include "node.h" +#include "base_object-inl.h" #include "env-inl.h" +#include "memory_tracker-inl.h" #include "util-inl.h" #include "v8.h" @@ -59,7 +61,7 @@ using v8::Value; V(10, number_of_detached_contexts, kNumberOfDetachedContextsIndex) #define V(a, b, c) +1 -static const size_t kHeapStatisticsPropertiesCount = +static constexpr size_t kHeapStatisticsPropertiesCount = HEAP_STATISTICS_PROPERTIES(V); #undef V @@ -71,10 +73,28 @@ static const size_t kHeapStatisticsPropertiesCount = V(3, physical_space_size, kPhysicalSpaceSizeIndex) #define V(a, b, c) +1 -static const size_t kHeapSpaceStatisticsPropertiesCount = +static constexpr size_t kHeapSpaceStatisticsPropertiesCount = HEAP_SPACE_STATISTICS_PROPERTIES(V); #undef V +class BindingData : public BaseObject { + public: + BindingData(Environment* env, Local obj) : BaseObject(env, obj) {} + + std::shared_ptr heap_statistics_buffer; + std::shared_ptr heap_space_statistics_buffer; + std::shared_ptr heap_code_statistics_buffer; + + void MemoryInfo(MemoryTracker* tracker) const override { + tracker->TrackField("heap_statistics_buffer", heap_statistics_buffer); + tracker->TrackField("heap_space_statistics_buffer", + heap_space_statistics_buffer); + tracker->TrackField("heap_code_statistics_buffer", + heap_code_statistics_buffer); + } + SET_SELF_SIZE(BindingData) + SET_MEMORY_INFO_NAME(BindingData) +}; #define HEAP_CODE_STATISTICS_PROPERTIES(V) \ V(0, code_and_metadata_size, kCodeAndMetadataSizeIndex) \ @@ -96,10 +116,11 @@ void CachedDataVersionTag(const FunctionCallbackInfo& args) { void UpdateHeapStatisticsArrayBuffer(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + BindingData* data = Unwrap(args.Data()); HeapStatistics s; - env->isolate()->GetHeapStatistics(&s); - double* const buffer = env->heap_statistics_buffer(); + args.GetIsolate()->GetHeapStatistics(&s); + double* const buffer = + static_cast(data->heap_statistics_buffer->Data()); #define V(index, name, _) buffer[index] = static_cast(s.name()); HEAP_STATISTICS_PROPERTIES(V) #undef V @@ -107,18 +128,20 @@ void UpdateHeapStatisticsArrayBuffer(const FunctionCallbackInfo& args) { void UpdateHeapSpaceStatisticsBuffer(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + BindingData* data = Unwrap(args.Data()); HeapSpaceStatistics s; - Isolate* const isolate = env->isolate(); - double* buffer = env->heap_space_statistics_buffer(); - size_t number_of_heap_spaces = env->isolate()->NumberOfHeapSpaces(); + Isolate* const isolate = args.GetIsolate(); + size_t number_of_heap_spaces = isolate->NumberOfHeapSpaces(); + + double* const buffer = + static_cast(data->heap_space_statistics_buffer->Data()); for (size_t i = 0; i < number_of_heap_spaces; i++) { isolate->GetHeapSpaceStatistics(&s, i); size_t const property_offset = i * kHeapSpaceStatisticsPropertiesCount; -#define V(index, name, _) buffer[property_offset + index] = \ - static_cast(s.name()); - HEAP_SPACE_STATISTICS_PROPERTIES(V) +#define V(index, name, _) \ + buffer[property_offset + index] = static_cast(s.name()); + HEAP_SPACE_STATISTICS_PROPERTIES(V) #undef V } } @@ -126,10 +149,11 @@ void UpdateHeapSpaceStatisticsBuffer(const FunctionCallbackInfo& args) { void UpdateHeapCodeStatisticsArrayBuffer( const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + BindingData* data = Unwrap(args.Data()); HeapCodeStatistics s; - env->isolate()->GetHeapCodeAndMetadataStatistics(&s); - double* const buffer = env->heap_code_statistics_buffer(); + args.GetIsolate()->GetHeapCodeAndMetadataStatistics(&s); + double* const buffer = + static_cast(data->heap_code_statistics_buffer->Data()); #define V(index, name, _) buffer[index] = static_cast(s.name()); HEAP_CODE_STATISTICS_PROPERTIES(V) #undef V @@ -148,6 +172,9 @@ void Initialize(Local target, Local context, void* priv) { Environment* env = Environment::GetCurrent(context); + Environment::BindingScope binding_scope(env); + BindingData* binding_data = binding_scope.data; + if (binding_data == nullptr) return; env->SetMethodNoSideEffect(target, "cachedDataVersionTag", CachedDataVersionTag); @@ -158,11 +185,11 @@ void Initialize(Local target, UpdateHeapStatisticsArrayBuffer); const size_t heap_statistics_buffer_byte_length = - sizeof(*env->heap_statistics_buffer()) * kHeapStatisticsPropertiesCount; + sizeof(double) * kHeapStatisticsPropertiesCount; Local heap_statistics_ab = ArrayBuffer::New(env->isolate(), heap_statistics_buffer_byte_length); - env->set_heap_statistics_buffer(heap_statistics_ab->GetBackingStore()); + binding_data->heap_statistics_buffer = heap_statistics_ab->GetBackingStore(); target->Set(env->context(), FIXED_ONE_BYTE_STRING(env->isolate(), "heapStatisticsArrayBuffer"), @@ -182,19 +209,17 @@ void Initialize(Local target, UpdateHeapCodeStatisticsArrayBuffer); const size_t heap_code_statistics_buffer_byte_length = - sizeof(*env->heap_code_statistics_buffer()) - * kHeapCodeStatisticsPropertiesCount; + sizeof(double) * kHeapCodeStatisticsPropertiesCount; Local heap_code_statistics_ab = ArrayBuffer::New(env->isolate(), heap_code_statistics_buffer_byte_length); - env->set_heap_code_statistics_buffer( - heap_code_statistics_ab->GetBackingStore()); + binding_data->heap_code_statistics_buffer = + heap_code_statistics_ab->GetBackingStore(); target->Set(env->context(), FIXED_ONE_BYTE_STRING(env->isolate(), "heapCodeStatisticsArrayBuffer"), - heap_code_statistics_ab) - .Check(); + heap_code_statistics_ab).Check(); #define V(i, _, name) \ target->Set(env->context(), \ @@ -236,20 +261,20 @@ void Initialize(Local target, UpdateHeapSpaceStatisticsBuffer); const size_t heap_space_statistics_buffer_byte_length = - sizeof(*env->heap_space_statistics_buffer()) * + sizeof(double) * kHeapSpaceStatisticsPropertiesCount * number_of_heap_spaces; Local heap_space_statistics_ab = ArrayBuffer::New(env->isolate(), heap_space_statistics_buffer_byte_length); - env->set_heap_space_statistics_buffer( - heap_space_statistics_ab->GetBackingStore()); + binding_data->heap_space_statistics_buffer = + heap_space_statistics_ab->GetBackingStore(); + target->Set(env->context(), FIXED_ONE_BYTE_STRING(env->isolate(), "heapSpaceStatisticsArrayBuffer"), - heap_space_statistics_ab) - .Check(); + heap_space_statistics_ab).Check(); #define V(i, _, name) \ target->Set(env->context(), \ From 6da22d81979768ebed09edb9ba6db08479c8e8bc Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 28 Mar 2020 19:48:55 +0100 Subject: [PATCH 04/15] src: move http parser state out of Environment Moves state that is specific to HTTP/1 into the HTTP/1 implementation as a cleanup. --- src/env-inl.h | 17 ---------------- src/env.cc | 2 -- src/env.h | 7 ------- src/node_http_parser.cc | 45 ++++++++++++++++++++++++++++++----------- 4 files changed, 33 insertions(+), 38 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index d81ea0fb95df44..a2c26b182bb203 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -580,23 +580,6 @@ inline double Environment::get_default_trigger_async_id() { return default_trigger_async_id; } -inline char* Environment::http_parser_buffer() const { - return http_parser_buffer_; -} - -inline void Environment::set_http_parser_buffer(char* buffer) { - CHECK_NULL(http_parser_buffer_); // Should be set only once. - http_parser_buffer_ = buffer; -} - -inline bool Environment::http_parser_buffer_in_use() const { - return http_parser_buffer_in_use_; -} - -inline void Environment::set_http_parser_buffer_in_use(bool in_use) { - http_parser_buffer_in_use_ = in_use; -} - inline AliasedFloat64Array* Environment::fs_stats_field_array() { return &fs_stats_field_array_; } diff --git a/src/env.cc b/src/env.cc index e7afdfe31464e7..7f247235708e10 100644 --- a/src/env.cc +++ b/src/env.cc @@ -466,8 +466,6 @@ Environment::~Environment() { tracing_controller->RemoveTraceStateObserver(trace_state_observer_.get()); } - delete[] http_parser_buffer_; - TRACE_EVENT_NESTABLE_ASYNC_END0( TRACING_CATEGORY_NODE1(environment), "Environment", this); diff --git a/src/env.h b/src/env.h index 9f15b56c6bf213..36bd455c1ce3de 100644 --- a/src/env.h +++ b/src/env.h @@ -1004,11 +1004,6 @@ class Environment : public MemoryRetainer { inline uint32_t get_next_script_id(); inline uint32_t get_next_function_id(); - inline char* http_parser_buffer() const; - inline void set_http_parser_buffer(char* buffer); - inline bool http_parser_buffer_in_use() const; - inline void set_http_parser_buffer_in_use(bool in_use); - EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; } inline AliasedFloat64Array* fs_stats_field_array(); @@ -1365,8 +1360,6 @@ class Environment : public MemoryRetainer { int handle_cleanup_waiting_ = 0; int request_waiting_ = 0; - char* http_parser_buffer_ = nullptr; - bool http_parser_buffer_in_use_ = false; EnabledDebugList enabled_debug_list_; AliasedFloat64Array fs_stats_field_array_; diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 2c75e6a532fff1..9f12c1588ecfe9 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -82,6 +82,20 @@ inline bool IsOWS(char c) { return c == ' ' || c == '\t'; } +class BindingData : public BaseObject { + public: + BindingData(Environment* env, Local obj) : BaseObject(env, obj) {} + + std::vector parser_buffer; + bool parser_buffer_in_use = false; + + void MemoryInfo(MemoryTracker* tracker) const override { + tracker->TrackField("parser_buffer", parser_buffer); + } + SET_SELF_SIZE(BindingData) + SET_MEMORY_INFO_NAME(BindingData) +}; + // helper class for the Parser struct StringPtr { StringPtr() { @@ -164,10 +178,11 @@ struct StringPtr { class Parser : public AsyncWrap, public StreamListener { public: - Parser(Environment* env, Local wrap) - : AsyncWrap(env, wrap), + Parser(BindingData* binding_data, Local wrap) + : AsyncWrap(binding_data->env(), wrap), current_buffer_len_(0), - current_buffer_data_(nullptr) { + current_buffer_data_(nullptr), + binding_data_(binding_data) { } @@ -429,8 +444,8 @@ class Parser : public AsyncWrap, public StreamListener { } static void New(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - new Parser(env, args.This()); + BindingData* binding_data = Unwrap(args.Data()); + new Parser(binding_data, args.This()); } @@ -605,14 +620,14 @@ class Parser : public AsyncWrap, public StreamListener { // For most types of streams, OnStreamRead will be immediately after // OnStreamAlloc, and will consume all data, so using a static buffer for // reading is more efficient. For other streams, just use Malloc() directly. - if (env()->http_parser_buffer_in_use()) + if (binding_data_->parser_buffer_in_use) return uv_buf_init(Malloc(suggested_size), suggested_size); - env()->set_http_parser_buffer_in_use(true); + binding_data_->parser_buffer_in_use = true; - if (env()->http_parser_buffer() == nullptr) - env()->set_http_parser_buffer(new char[kAllocBufferSize]); + if (binding_data_->parser_buffer.empty()) + binding_data_->parser_buffer.resize(kAllocBufferSize); - return uv_buf_init(env()->http_parser_buffer(), kAllocBufferSize); + return uv_buf_init(binding_data_->parser_buffer.data(), kAllocBufferSize); } @@ -622,8 +637,8 @@ class Parser : public AsyncWrap, public StreamListener { // is free for re-use, or free() the data if it didn’t come from there // in the first place. auto on_scope_leave = OnScopeLeave([&]() { - if (buf.base == env()->http_parser_buffer()) - env()->set_http_parser_buffer_in_use(false); + if (buf.base == binding_data_->parser_buffer.data()) + binding_data_->parser_buffer_in_use = false; else free(buf.base); }); @@ -863,6 +878,8 @@ class Parser : public AsyncWrap, public StreamListener { uint64_t headers_timeout_; uint64_t header_parsing_start_time_ = 0; + BaseObjectPtr binding_data_; + // These are helper functions for filling `http_parser_settings`, which turn // a member function of Parser into a C-style HTTP parser callback. template struct Proxy; @@ -903,6 +920,10 @@ void InitializeHttpParser(Local target, Local context, void* priv) { Environment* env = Environment::GetCurrent(context); + Environment::BindingScope binding_scope(env); + BindingData* binding_data = binding_scope.data; + if (binding_data == nullptr) return; + Local t = env->NewFunctionTemplate(Parser::New); t->InstanceTemplate()->SetInternalFieldCount(Parser::kInternalFieldCount); t->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "HTTPParser")); From ad653350c1e927c651fd47d75662df429e72b603 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 5 Mar 2019 03:02:57 +0100 Subject: [PATCH 05/15] src: move fs state out of Environment Moves state that is specific to the `fs` binding into the `fs` binding implementation as a cleanup. --- src/env-inl.h | 13 ---- src/env.cc | 14 +--- src/env.h | 16 ----- src/node_dir.cc | 6 +- src/node_file-inl.h | 57 ++++++++++------ src/node_file.cc | 137 ++++++++++++++++++++++----------------- src/node_file.h | 48 +++++++++++--- src/node_stat_watcher.cc | 19 +++--- src/node_stat_watcher.h | 6 +- 9 files changed, 174 insertions(+), 142 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index a2c26b182bb203..f681a0bf1d8dcc 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -580,19 +580,6 @@ inline double Environment::get_default_trigger_async_id() { return default_trigger_async_id; } -inline AliasedFloat64Array* Environment::fs_stats_field_array() { - return &fs_stats_field_array_; -} - -inline AliasedBigUint64Array* Environment::fs_stats_field_bigint_array() { - return &fs_stats_field_bigint_array_; -} - -inline std::vector>& -Environment::file_handle_read_wrap_freelist() { - return file_handle_read_wrap_freelist_; -} - inline std::shared_ptr Environment::options() { return options_; } diff --git a/src/env.cc b/src/env.cc index 7f247235708e10..c6f4175bd678fc 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1,18 +1,19 @@ #include "env.h" #include "async_wrap.h" +#include "base_object-inl.h" #include "debug_utils-inl.h" #include "memory_tracker-inl.h" #include "node_buffer.h" #include "node_context_data.h" #include "node_errors.h" -#include "node_file.h" #include "node_internals.h" #include "node_options-inl.h" #include "node_process.h" #include "node_v8_platform-inl.h" #include "node_worker.h" #include "req_wrap-inl.h" +#include "stream_base.h" #include "tracing/agent.h" #include "tracing/traced_value.h" #include "util-inl.h" @@ -341,8 +342,6 @@ Environment::Environment(IsolateData* isolate_data, flags_(flags), thread_id_(thread_id.id == static_cast(-1) ? AllocateEnvironmentThreadId().id : thread_id.id), - fs_stats_field_array_(isolate_, kFsStatsBufferLength), - fs_stats_field_bigint_array_(isolate_, kFsStatsBufferLength), context_(context->GetIsolate(), context) { // We'll be creating new objects so make sure we've entered the context. HandleScope handle_scope(isolate()); @@ -444,10 +443,6 @@ Environment::~Environment() { isolate()->GetHeapProfiler()->RemoveBuildEmbedderGraphCallback( BuildEmbedderGraph, this); - // Make sure there are no re-used libuv wrapper objects. - // CleanupHandles() should have removed all of them. - CHECK(file_handle_read_wrap_freelist_.empty()); - HandleScope handle_scope(isolate()); #if HAVE_INSPECTOR @@ -614,8 +609,6 @@ void Environment::CleanupHandles() { !handle_wrap_queue_.IsEmpty()) { uv_run(event_loop(), UV_RUN_ONCE); } - - file_handle_read_wrap_freelist_.clear(); } void Environment::StartProfilerIdleNotifier() { @@ -1095,9 +1088,6 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("should_abort_on_uncaught_toggle", should_abort_on_uncaught_toggle_); tracker->TrackField("stream_base_state", stream_base_state_); - tracker->TrackField("fs_stats_field_array", fs_stats_field_array_); - tracker->TrackField("fs_stats_field_bigint_array", - fs_stats_field_bigint_array_); tracker->TrackField("cleanup_hooks", cleanup_hooks_); tracker->TrackField("async_hooks", async_hooks_); tracker->TrackField("immediate_info", immediate_info_); diff --git a/src/env.h b/src/env.h index 36bd455c1ce3de..c1fc037a26758d 100644 --- a/src/env.h +++ b/src/env.h @@ -56,10 +56,6 @@ class ContextifyScript; class CompiledFnEntry; } -namespace fs { -class FileHandleReadWrap; -} - namespace performance { class PerformanceState; } @@ -1006,12 +1002,6 @@ class Environment : public MemoryRetainer { EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; } - inline AliasedFloat64Array* fs_stats_field_array(); - inline AliasedBigUint64Array* fs_stats_field_bigint_array(); - - inline std::vector>& - file_handle_read_wrap_freelist(); - inline performance::PerformanceState* performance_state(); inline std::unordered_map* performance_marks(); @@ -1362,12 +1352,6 @@ class Environment : public MemoryRetainer { EnabledDebugList enabled_debug_list_; - AliasedFloat64Array fs_stats_field_array_; - AliasedBigUint64Array fs_stats_field_bigint_array_; - - std::vector> - file_handle_read_wrap_freelist_; - std::list extra_linked_bindings_; Mutex extra_linked_bindings_mutex_; diff --git a/src/node_dir.cc b/src/node_dir.cc index 9923f042779f2e..c4aaf4bcd3e8ba 100644 --- a/src/node_dir.cc +++ b/src/node_dir.cc @@ -152,7 +152,7 @@ void DirHandle::Close(const FunctionCallbackInfo& args) { dir->closing_ = false; dir->closed_ = true; - FSReqBase* req_wrap_async = GetReqWrap(env, args[0]); + FSReqBase* req_wrap_async = GetReqWrap(args, 0); if (req_wrap_async != nullptr) { // close(req) AsyncCall(env, req_wrap_async, args, "closedir", UTF8, AfterClose, uv_fs_closedir, dir->dir()); @@ -252,7 +252,7 @@ void DirHandle::Read(const FunctionCallbackInfo& args) { dir->dir_->dirents = dir->dirents_.data(); } - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { // dir.read(encoding, bufferSize, req) AsyncCall(env, req_wrap_async, args, "readdir", encoding, AfterDirRead, uv_fs_readdir, dir->dir()); @@ -320,7 +320,7 @@ static void OpenDir(const FunctionCallbackInfo& args) { const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { // openDir(path, encoding, req) AsyncCall(env, req_wrap_async, args, "opendir", encoding, AfterOpenDir, uv_fs_opendir, *path); diff --git a/src/node_file-inl.h b/src/node_file-inl.h index e9ed18a75fa48b..d30d77301005a3 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -39,11 +39,13 @@ void FSContinuationData::Done(int result) { done_cb_(req_); } -FSReqBase::FSReqBase(Environment* env, - v8::Local req, - AsyncWrap::ProviderType type, - bool use_bigint) - : ReqWrap(env, req, type), use_bigint_(use_bigint) { +FSReqBase::FSReqBase(BindingData* binding_data, + v8::Local req, + AsyncWrap::ProviderType type, + bool use_bigint) + : ReqWrap(binding_data->env(), req, type), + use_bigint_(use_bigint), + binding_data_(binding_data) { } void FSReqBase::Init(const char* syscall, @@ -72,9 +74,13 @@ FSReqBase::Init(const char* syscall, size_t len, enum encoding encoding) { return buffer_; } -FSReqCallback::FSReqCallback(Environment* env, - v8::Local req, bool use_bigint) - : FSReqBase(env, req, AsyncWrap::PROVIDER_FSREQCALLBACK, use_bigint) {} +FSReqCallback::FSReqCallback(BindingData* binding_data, + v8::Local req, + bool use_bigint) + : FSReqBase(binding_data, + req, + AsyncWrap::PROVIDER_FSREQCALLBACK, + use_bigint) {} template void FillStatsArray(AliasedBufferBase* fields, @@ -112,18 +118,18 @@ void FillStatsArray(AliasedBufferBase* fields, #undef SET_FIELD_WITH_STAT } -v8::Local FillGlobalStatsArray(Environment* env, +v8::Local FillGlobalStatsArray(BindingData* binding_data, const bool use_bigint, const uv_stat_t* s, const bool second) { const ptrdiff_t offset = second ? static_cast(FsStatsOffset::kFsStatsFieldsNumber) : 0; if (use_bigint) { - auto* const arr = env->fs_stats_field_bigint_array(); + auto* const arr = &binding_data->stats_field_bigint_array; FillStatsArray(arr, s, offset); return arr->GetJSArray(); } else { - auto* const arr = env->fs_stats_field_array(); + auto* const arr = &binding_data->stats_field_array; FillStatsArray(arr, s, offset); return arr->GetJSArray(); } @@ -131,7 +137,9 @@ v8::Local FillGlobalStatsArray(Environment* env, template FSReqPromise* -FSReqPromise::New(Environment* env, bool use_bigint) { +FSReqPromise::New(BindingData* binding_data, + bool use_bigint) { + Environment* env = binding_data->env(); v8::Local obj; if (!env->fsreqpromise_constructor_template() ->NewInstance(env->context()) @@ -143,7 +151,7 @@ FSReqPromise::New(Environment* env, bool use_bigint) { obj->Set(env->context(), env->promise_string(), resolver).IsNothing()) { return nullptr; } - return new FSReqPromise(env, obj, use_bigint); + return new FSReqPromise(binding_data, obj, use_bigint); } template @@ -154,12 +162,15 @@ FSReqPromise::~FSReqPromise() { template FSReqPromise::FSReqPromise( - Environment* env, + BindingData* binding_data, v8::Local obj, bool use_bigint) - : FSReqBase(env, obj, AsyncWrap::PROVIDER_FSREQPROMISE, use_bigint), + : FSReqBase(binding_data, + obj, + AsyncWrap::PROVIDER_FSREQPROMISE, + use_bigint), stats_field_array_( - env->isolate(), + env()->isolate(), static_cast(FsStatsOffset::kFsStatsFieldsNumber)) {} template @@ -208,15 +219,21 @@ void FSReqPromise::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("stats_field_array", stats_field_array_); } -FSReqBase* GetReqWrap(Environment* env, v8::Local value, +FSReqBase* GetReqWrap(const v8::FunctionCallbackInfo& args, + int index, bool use_bigint) { + v8::Local value = args[index]; if (value->IsObject()) { return Unwrap(value.As()); - } else if (value->StrictEquals(env->fs_use_promises_symbol())) { + } + + BindingData* binding_data = Unwrap(args.Data()); + Environment* env = binding_data->env(); + if (value->StrictEquals(env->fs_use_promises_symbol())) { if (use_bigint) { - return FSReqPromise::New(env, use_bigint); + return FSReqPromise::New(binding_data, use_bigint); } else { - return FSReqPromise::New(env, use_bigint); + return FSReqPromise::New(binding_data, use_bigint); } } return nullptr; diff --git a/src/node_file.cc b/src/node_file.cc index d00595d1ca94ab..a428732fa1267b 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -120,30 +120,35 @@ void FSReqBase::MemoryInfo(MemoryTracker* tracker) const { // The FileHandle object wraps a file descriptor and will close it on garbage // collection if necessary. If that happens, a process warning will be // emitted (or a fatal exception will occur if the fd cannot be closed.) -FileHandle::FileHandle(Environment* env, Local obj, int fd) - : AsyncWrap(env, obj, AsyncWrap::PROVIDER_FILEHANDLE), - StreamBase(env), - fd_(fd) { +FileHandle::FileHandle(BindingData* binding_data, + Local obj, int fd) + : AsyncWrap(binding_data->env(), obj, AsyncWrap::PROVIDER_FILEHANDLE), + StreamBase(env()), + fd_(fd), + binding_data_(binding_data) { MakeWeak(); StreamBase::AttachToObject(GetObject()); } -FileHandle* FileHandle::New(Environment* env, int fd, Local obj) { +FileHandle* FileHandle::New(BindingData* binding_data, + int fd, Local obj) { + Environment* env = binding_data->env(); if (obj.IsEmpty() && !env->fd_constructor_template() ->NewInstance(env->context()) .ToLocal(&obj)) { return nullptr; } - return new FileHandle(env, obj, fd); + return new FileHandle(binding_data, obj, fd); } void FileHandle::New(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + BindingData* binding_data = Unwrap(args.Data()); + Environment* env = binding_data->env(); CHECK(args.IsConstructCall()); CHECK(args[0]->IsInt32()); FileHandle* handle = - FileHandle::New(env, args[0].As()->Value(), args.This()); + FileHandle::New(binding_data, args[0].As()->Value(), args.This()); if (handle == nullptr) return; if (args[1]->IsNumber()) handle->read_offset_ = args[1]->IntegerValue(env->context()).FromJust(); @@ -362,7 +367,7 @@ int FileHandle::ReadStart() { if (current_read_) return 0; - std::unique_ptr read_wrap; + BaseObjectPtr read_wrap; if (read_length_ == 0) { EmitRead(UV_EOF); @@ -376,7 +381,7 @@ int FileHandle::ReadStart() { HandleScope handle_scope(env()->isolate()); AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(this); - auto& freelist = env()->file_handle_read_wrap_freelist(); + auto& freelist = binding_data_->file_handle_read_wrap_freelist; if (freelist.size() > 0) { read_wrap = std::move(freelist.back()); freelist.pop_back(); @@ -395,7 +400,7 @@ int FileHandle::ReadStart() { .ToLocal(&wrap_obj)) { return UV_EBUSY; } - read_wrap = std::make_unique(this, wrap_obj); + read_wrap = MakeDetachedBaseObject(this, wrap_obj); } } int64_t recommended_read = 65536; @@ -422,7 +427,7 @@ int FileHandle::ReadStart() { // ReadStart() checks whether current_read_ is set to determine whether // a read is in progress. Moving it into a local variable makes sure that // the ReadStart() call below doesn't think we're still actively reading. - std::unique_ptr read_wrap = + BaseObjectPtr read_wrap = std::move(handle->current_read_); int result = req->result; @@ -433,7 +438,7 @@ int FileHandle::ReadStart() { // Push the read wrap back to the freelist, or let it be destroyed // once we’re exiting the current scope. constexpr size_t wanted_freelist_fill = 100; - auto& freelist = handle->env()->file_handle_read_wrap_freelist(); + auto& freelist = handle->binding_data_->file_handle_read_wrap_freelist; if (freelist.size() < wanted_freelist_fill) { read_wrap->Reset(); freelist.emplace_back(std::move(read_wrap)); @@ -503,7 +508,7 @@ void FSReqCallback::Reject(Local reject) { } void FSReqCallback::ResolveStat(const uv_stat_t* stat) { - Resolve(FillGlobalStatsArray(env(), use_bigint(), stat)); + Resolve(FillGlobalStatsArray(binding_data(), use_bigint(), stat)); } void FSReqCallback::Resolve(Local value) { @@ -522,8 +527,8 @@ void FSReqCallback::SetReturnValue(const FunctionCallbackInfo& args) { void NewFSReqCallback(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); - Environment* env = Environment::GetCurrent(args); - new FSReqCallback(env, args.This(), args[0]->IsTrue()); + BindingData* binding_data = Unwrap(args.Data()); + new FSReqCallback(binding_data, args.This(), args[0]->IsTrue()); } FSReqAfterScope::FSReqAfterScope(FSReqBase* wrap, uv_fs_t* req) @@ -595,7 +600,7 @@ void AfterOpenFileHandle(uv_fs_t* req) { FSReqAfterScope after(req_wrap, req); if (after.Proceed()) { - FileHandle* fd = FileHandle::New(req_wrap->env(), req->result); + FileHandle* fd = FileHandle::New(req_wrap->binding_data(), req->result); if (fd == nullptr) return; req_wrap->Resolve(fd->object()); } @@ -773,7 +778,7 @@ void Access(const FunctionCallbackInfo& args) { BufferValue path(isolate, args[0]); CHECK_NOT_NULL(*path); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { // access(path, mode, req) AsyncCall(env, req_wrap_async, args, "access", UTF8, AfterNoArgs, uv_fs_access, *path, mode); @@ -796,7 +801,7 @@ void Close(const FunctionCallbackInfo& args) { CHECK(args[0]->IsInt32()); int fd = args[0].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[1]); + FSReqBase* req_wrap_async = GetReqWrap(args, 1); if (req_wrap_async != nullptr) { // close(fd, req) AsyncCall(env, req_wrap_async, args, "close", UTF8, AfterNoArgs, uv_fs_close, fd); @@ -929,7 +934,8 @@ static void InternalModuleStat(const FunctionCallbackInfo& args) { } static void Stat(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + BindingData* binding_data = Unwrap(args.Data()); + Environment* env = binding_data->env(); const int argc = args.Length(); CHECK_GE(argc, 2); @@ -938,7 +944,7 @@ static void Stat(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(*path); bool use_bigint = args[1]->IsTrue(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2], use_bigint); + FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint); if (req_wrap_async != nullptr) { // stat(path, use_bigint, req) AsyncCall(env, req_wrap_async, args, "stat", UTF8, AfterStat, uv_fs_stat, *path); @@ -952,14 +958,15 @@ static void Stat(const FunctionCallbackInfo& args) { return; // error info is in ctx } - Local arr = FillGlobalStatsArray(env, use_bigint, + Local arr = FillGlobalStatsArray(binding_data, use_bigint, static_cast(req_wrap_sync.req.ptr)); args.GetReturnValue().Set(arr); } } static void LStat(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + BindingData* binding_data = Unwrap(args.Data()); + Environment* env = binding_data->env(); const int argc = args.Length(); CHECK_GE(argc, 3); @@ -968,7 +975,7 @@ static void LStat(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(*path); bool use_bigint = args[1]->IsTrue(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2], use_bigint); + FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint); if (req_wrap_async != nullptr) { // lstat(path, use_bigint, req) AsyncCall(env, req_wrap_async, args, "lstat", UTF8, AfterStat, uv_fs_lstat, *path); @@ -983,14 +990,15 @@ static void LStat(const FunctionCallbackInfo& args) { return; // error info is in ctx } - Local arr = FillGlobalStatsArray(env, use_bigint, + Local arr = FillGlobalStatsArray(binding_data, use_bigint, static_cast(req_wrap_sync.req.ptr)); args.GetReturnValue().Set(arr); } } static void FStat(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + BindingData* binding_data = Unwrap(args.Data()); + Environment* env = binding_data->env(); const int argc = args.Length(); CHECK_GE(argc, 2); @@ -999,7 +1007,7 @@ static void FStat(const FunctionCallbackInfo& args) { int fd = args[0].As()->Value(); bool use_bigint = args[1]->IsTrue(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2], use_bigint); + FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint); if (req_wrap_async != nullptr) { // fstat(fd, use_bigint, req) AsyncCall(env, req_wrap_async, args, "fstat", UTF8, AfterStat, uv_fs_fstat, fd); @@ -1013,7 +1021,7 @@ static void FStat(const FunctionCallbackInfo& args) { return; // error info is in ctx } - Local arr = FillGlobalStatsArray(env, use_bigint, + Local arr = FillGlobalStatsArray(binding_data, use_bigint, static_cast(req_wrap_sync.req.ptr)); args.GetReturnValue().Set(arr); } @@ -1034,7 +1042,7 @@ static void Symlink(const FunctionCallbackInfo& args) { CHECK(args[2]->IsInt32()); int flags = args[2].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // symlink(target, path, flags, req) AsyncDestCall(env, req_wrap_async, args, "symlink", *path, path.length(), UTF8, AfterNoArgs, uv_fs_symlink, *target, *path, flags); @@ -1061,7 +1069,7 @@ static void Link(const FunctionCallbackInfo& args) { BufferValue dest(isolate, args[1]); CHECK_NOT_NULL(*dest); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { // link(src, dest, req) AsyncDestCall(env, req_wrap_async, args, "link", *dest, dest.length(), UTF8, AfterNoArgs, uv_fs_link, *src, *dest); @@ -1087,7 +1095,7 @@ static void ReadLink(const FunctionCallbackInfo& args) { const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { // readlink(path, encoding, req) AsyncCall(env, req_wrap_async, args, "readlink", encoding, AfterStringPtr, uv_fs_readlink, *path); @@ -1130,7 +1138,7 @@ static void Rename(const FunctionCallbackInfo& args) { BufferValue new_path(isolate, args[1]); CHECK_NOT_NULL(*new_path); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { AsyncDestCall(env, req_wrap_async, args, "rename", *new_path, new_path.length(), UTF8, AfterNoArgs, uv_fs_rename, @@ -1157,7 +1165,7 @@ static void FTruncate(const FunctionCallbackInfo& args) { CHECK(IsSafeJsInt(args[1])); const int64_t len = args[1].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { AsyncCall(env, req_wrap_async, args, "ftruncate", UTF8, AfterNoArgs, uv_fs_ftruncate, fd, len); @@ -1180,7 +1188,7 @@ static void Fdatasync(const FunctionCallbackInfo& args) { CHECK(args[0]->IsInt32()); const int fd = args[0].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[1]); + FSReqBase* req_wrap_async = GetReqWrap(args, 1); if (req_wrap_async != nullptr) { AsyncCall(env, req_wrap_async, args, "fdatasync", UTF8, AfterNoArgs, uv_fs_fdatasync, fd); @@ -1202,7 +1210,7 @@ static void Fsync(const FunctionCallbackInfo& args) { CHECK(args[0]->IsInt32()); const int fd = args[0].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[1]); + FSReqBase* req_wrap_async = GetReqWrap(args, 1); if (req_wrap_async != nullptr) { AsyncCall(env, req_wrap_async, args, "fsync", UTF8, AfterNoArgs, uv_fs_fsync, fd); @@ -1224,7 +1232,7 @@ static void Unlink(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[0]); CHECK_NOT_NULL(*path); - FSReqBase* req_wrap_async = GetReqWrap(env, args[1]); + FSReqBase* req_wrap_async = GetReqWrap(args, 1); if (req_wrap_async != nullptr) { AsyncCall(env, req_wrap_async, args, "unlink", UTF8, AfterNoArgs, uv_fs_unlink, *path); @@ -1246,7 +1254,7 @@ static void RMDir(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[0]); CHECK_NOT_NULL(*path); - FSReqBase* req_wrap_async = GetReqWrap(env, args[1]); // rmdir(path, req) + FSReqBase* req_wrap_async = GetReqWrap(args, 1); // rmdir(path, req) if (req_wrap_async != nullptr) { AsyncCall(env, req_wrap_async, args, "rmdir", UTF8, AfterNoArgs, uv_fs_rmdir, *path); @@ -1456,7 +1464,7 @@ static void MKDir(const FunctionCallbackInfo& args) { CHECK(args[2]->IsBoolean()); bool mkdirp = args[2]->IsTrue(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // mkdir(path, mode, req) AsyncCall(env, req_wrap_async, args, "mkdir", UTF8, mkdirp ? AfterMkdirp : AfterNoArgs, @@ -1502,7 +1510,7 @@ static void RealPath(const FunctionCallbackInfo& args) { const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { // realpath(path, encoding, req) AsyncCall(env, req_wrap_async, args, "realpath", encoding, AfterStringPtr, uv_fs_realpath, *path); @@ -1548,7 +1556,7 @@ static void ReadDir(const FunctionCallbackInfo& args) { bool with_types = args[2]->IsTrue(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // readdir(path, encoding, withTypes, req) if (with_types) { AsyncCall(env, req_wrap_async, args, "scandir", encoding, @@ -1636,7 +1644,7 @@ static void Open(const FunctionCallbackInfo& args) { CHECK(args[2]->IsInt32()); const int mode = args[2].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // open(path, flags, mode, req) AsyncCall(env, req_wrap_async, args, "open", UTF8, AfterInteger, uv_fs_open, *path, flags, mode); @@ -1652,7 +1660,8 @@ static void Open(const FunctionCallbackInfo& args) { } static void OpenFileHandle(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + BindingData* binding_data = Unwrap(args.Data()); + Environment* env = binding_data->env(); Isolate* isolate = env->isolate(); const int argc = args.Length(); @@ -1667,7 +1676,7 @@ static void OpenFileHandle(const FunctionCallbackInfo& args) { CHECK(args[2]->IsInt32()); const int mode = args[2].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // openFileHandle(path, flags, mode, req) AsyncCall(env, req_wrap_async, args, "open", UTF8, AfterOpenFileHandle, uv_fs_open, *path, flags, mode); @@ -1681,7 +1690,7 @@ static void OpenFileHandle(const FunctionCallbackInfo& args) { if (result < 0) { return; // syscall failed, no need to continue, error info is in ctx } - FileHandle* fd = FileHandle::New(env, result); + FileHandle* fd = FileHandle::New(binding_data, result); if (fd == nullptr) return; args.GetReturnValue().Set(fd->object()); } @@ -1703,7 +1712,7 @@ static void CopyFile(const FunctionCallbackInfo& args) { CHECK(args[2]->IsInt32()); const int flags = args[2].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // copyFile(src, dest, flags, req) AsyncDestCall(env, req_wrap_async, args, "copyfile", *dest, dest.length(), UTF8, AfterNoArgs, @@ -1759,7 +1768,7 @@ static void WriteBuffer(const FunctionCallbackInfo& args) { char* buf = buffer_data + off; uv_buf_t uvbuf = uv_buf_init(buf, len); - FSReqBase* req_wrap_async = GetReqWrap(env, args[5]); + FSReqBase* req_wrap_async = GetReqWrap(args, 5); if (req_wrap_async != nullptr) { // write(fd, buffer, off, len, pos, req) AsyncCall(env, req_wrap_async, args, "write", UTF8, AfterInteger, uv_fs_write, fd, &uvbuf, 1, pos); @@ -1804,7 +1813,7 @@ static void WriteBuffers(const FunctionCallbackInfo& args) { iovs[i] = uv_buf_init(Buffer::Data(chunk), Buffer::Length(chunk)); } - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // writeBuffers(fd, chunks, pos, req) AsyncCall(env, req_wrap_async, args, "write", UTF8, AfterInteger, uv_fs_write, fd, *iovs, iovs.length(), pos); @@ -1846,7 +1855,7 @@ static void WriteString(const FunctionCallbackInfo& args) { char* buf = nullptr; size_t len; - FSReqBase* req_wrap_async = GetReqWrap(env, args[4]); + FSReqBase* req_wrap_async = GetReqWrap(args, 4); const bool is_async = req_wrap_async != nullptr; // Avoid copying the string when it is externalized but only when: @@ -1960,7 +1969,7 @@ static void Read(const FunctionCallbackInfo& args) { char* buf = buffer_data + off; uv_buf_t uvbuf = uv_buf_init(buf, len); - FSReqBase* req_wrap_async = GetReqWrap(env, args[5]); + FSReqBase* req_wrap_async = GetReqWrap(args, 5); if (req_wrap_async != nullptr) { // read(fd, buffer, offset, len, pos, req) AsyncCall(env, req_wrap_async, args, "read", UTF8, AfterInteger, uv_fs_read, fd, &uvbuf, 1, pos); @@ -2037,7 +2046,7 @@ static void Chmod(const FunctionCallbackInfo& args) { CHECK(args[1]->IsInt32()); int mode = args[1].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { // chmod(path, mode, req) AsyncCall(env, req_wrap_async, args, "chmod", UTF8, AfterNoArgs, uv_fs_chmod, *path, mode); @@ -2067,7 +2076,7 @@ static void FChmod(const FunctionCallbackInfo& args) { CHECK(args[1]->IsInt32()); const int mode = args[1].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { // fchmod(fd, mode, req) AsyncCall(env, req_wrap_async, args, "fchmod", UTF8, AfterNoArgs, uv_fs_fchmod, fd, mode); @@ -2100,7 +2109,7 @@ static void Chown(const FunctionCallbackInfo& args) { CHECK(args[2]->IsUint32()); const uv_gid_t gid = static_cast(args[2].As()->Value()); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // chown(path, uid, gid, req) AsyncCall(env, req_wrap_async, args, "chown", UTF8, AfterNoArgs, uv_fs_chown, *path, uid, gid); @@ -2133,7 +2142,7 @@ static void FChown(const FunctionCallbackInfo& args) { CHECK(args[2]->IsUint32()); const uv_gid_t gid = static_cast(args[2].As()->Value()); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // fchown(fd, uid, gid, req) AsyncCall(env, req_wrap_async, args, "fchown", UTF8, AfterNoArgs, uv_fs_fchown, fd, uid, gid); @@ -2163,7 +2172,7 @@ static void LChown(const FunctionCallbackInfo& args) { CHECK(args[2]->IsUint32()); const uv_gid_t gid = static_cast(args[2].As()->Value()); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // lchown(path, uid, gid, req) AsyncCall(env, req_wrap_async, args, "lchown", UTF8, AfterNoArgs, uv_fs_lchown, *path, uid, gid); @@ -2193,7 +2202,7 @@ static void UTimes(const FunctionCallbackInfo& args) { CHECK(args[2]->IsNumber()); const double mtime = args[2].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // utimes(path, atime, mtime, req) AsyncCall(env, req_wrap_async, args, "utime", UTF8, AfterNoArgs, uv_fs_utime, *path, atime, mtime); @@ -2222,7 +2231,7 @@ static void FUTimes(const FunctionCallbackInfo& args) { CHECK(args[2]->IsNumber()); const double mtime = args[2].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // futimes(fd, atime, mtime, req) AsyncCall(env, req_wrap_async, args, "futime", UTF8, AfterNoArgs, uv_fs_futime, fd, atime, mtime); @@ -2248,7 +2257,7 @@ static void Mkdtemp(const FunctionCallbackInfo& args) { const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); - FSReqBase* req_wrap_async = GetReqWrap(env, args[2]); + FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { // mkdtemp(tmpl, encoding, req) AsyncCall(env, req_wrap_async, args, "mkdtemp", encoding, AfterStringPath, uv_fs_mkdtemp, *tmpl); @@ -2273,12 +2282,22 @@ static void Mkdtemp(const FunctionCallbackInfo& args) { } } +void BindingData::MemoryInfo(MemoryTracker* tracker) const { + tracker->TrackField("stats_field_array", stats_field_array); + tracker->TrackField("stats_field_bigint_array", stats_field_bigint_array); + tracker->TrackField("file_handle_read_wrap_freelist", + file_handle_read_wrap_freelist); +} + void Initialize(Local target, Local unused, Local context, void* priv) { Environment* env = Environment::GetCurrent(context); Isolate* isolate = env->isolate(); + Environment::BindingScope binding_scope(env); + BindingData* binding_data = binding_scope.data; + if (binding_data == nullptr) return; env->SetMethod(target, "access", Access); env->SetMethod(target, "close", Close); @@ -2331,11 +2350,11 @@ void Initialize(Local target, target->Set(context, FIXED_ONE_BYTE_STRING(isolate, "statValues"), - env->fs_stats_field_array()->GetJSArray()).Check(); + binding_data->stats_field_array.GetJSArray()).Check(); target->Set(context, FIXED_ONE_BYTE_STRING(isolate, "bigintStatValues"), - env->fs_stats_field_bigint_array()->GetJSArray()).Check(); + binding_data->stats_field_bigint_array.GetJSArray()).Check(); StatWatcher::Initialize(env, target); diff --git a/src/node_file.h b/src/node_file.h index 1ebfe06c1fbdbf..1fda81361fef79 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -11,6 +11,26 @@ namespace node { namespace fs { +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) {} + + AliasedFloat64Array stats_field_array; + AliasedBigUint64Array stats_field_bigint_array; + + std::vector> + file_handle_read_wrap_freelist; + + void MemoryInfo(MemoryTracker* tracker) const override; + SET_SELF_SIZE(BindingData) + SET_MEMORY_INFO_NAME(BindingData) +}; + // structure used to store state during a complex operation, e.g., mkdirp. class FSContinuationData : public MemoryRetainer { public: @@ -43,7 +63,7 @@ class FSReqBase : public ReqWrap { public: typedef MaybeStackBuffer FSReqBuffer; - inline FSReqBase(Environment* env, + inline FSReqBase(BindingData* binding_data, v8::Local req, AsyncWrap::ProviderType type, bool use_bigint); @@ -83,12 +103,16 @@ class FSReqBase : public ReqWrap { void MemoryInfo(MemoryTracker* tracker) const override; + BindingData* binding_data() { return binding_data_.get(); } + private: std::unique_ptr continuation_data_; enum encoding encoding_ = UTF8; bool has_data_ = false; - const char* syscall_ = nullptr; bool use_bigint_ = false; + const char* syscall_ = nullptr; + + BaseObjectPtr binding_data_; // Typically, the content of buffer_ is something like a file name, so // something around 64 bytes should be enough. @@ -97,7 +121,7 @@ class FSReqBase : public ReqWrap { class FSReqCallback final : public FSReqBase { public: - inline FSReqCallback(Environment* env, + inline FSReqCallback(BindingData* binding_data, v8::Local req, bool use_bigint); @@ -118,7 +142,7 @@ void FillStatsArray(AliasedBufferBase* fields, const uv_stat_t* s, const size_t offset = 0); -inline v8::Local FillGlobalStatsArray(Environment* env, +inline v8::Local FillGlobalStatsArray(BindingData* binding_data, const bool use_bigint, const uv_stat_t* s, const bool second = false); @@ -126,7 +150,8 @@ inline v8::Local FillGlobalStatsArray(Environment* env, template class FSReqPromise final : public FSReqBase { public: - static inline FSReqPromise* New(Environment* env, bool use_bigint); + static inline FSReqPromise* New(BindingData* binding_data, + bool use_bigint); inline ~FSReqPromise() override; inline void Reject(v8::Local reject) override; @@ -145,7 +170,7 @@ class FSReqPromise final : public FSReqBase { FSReqPromise& operator=(const FSReqPromise&&) = delete; private: - inline FSReqPromise(Environment* env, + inline FSReqPromise(BindingData* binding_data, v8::Local obj, bool use_bigint); @@ -202,7 +227,7 @@ class FileHandleReadWrap final : public ReqWrap { // the object is garbage collected class FileHandle final : public AsyncWrap, public StreamBase { public: - static FileHandle* New(Environment* env, + static FileHandle* New(BindingData* binding_data, int fd, v8::Local obj = v8::Local()); ~FileHandle() override; @@ -246,7 +271,7 @@ class FileHandle final : public AsyncWrap, public StreamBase { FileHandle& operator=(const FileHandle&&) = delete; private: - FileHandle(Environment* env, v8::Local obj, int fd); + FileHandle(BindingData* binding_data, v8::Local obj, int fd); // Synchronous close that emits a warning void Close(); @@ -295,7 +320,9 @@ class FileHandle final : public AsyncWrap, public StreamBase { int64_t read_length_ = -1; bool reading_ = false; - std::unique_ptr current_read_ = nullptr; + BaseObjectPtr current_read_; + + BaseObjectPtr binding_data_; }; int MKDirpSync(uv_loop_t* loop, @@ -327,7 +354,8 @@ class FSReqWrapSync { // TODO(addaleax): Currently, callers check the return value and assume // that nullptr indicates a synchronous call, rather than a failure. // Failure conditions should be disambiguated and handled appropriately. -inline FSReqBase* GetReqWrap(Environment* env, v8::Local value, +inline FSReqBase* GetReqWrap(const v8::FunctionCallbackInfo& args, + int index, bool use_bigint = false); // Returns nullptr if the operation fails from the start. diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc index 05c540bbff17cc..af23affc412935 100644 --- a/src/node_stat_watcher.cc +++ b/src/node_stat_watcher.cc @@ -61,15 +61,16 @@ void StatWatcher::Initialize(Environment* env, Local target) { } -StatWatcher::StatWatcher(Environment* env, +StatWatcher::StatWatcher(fs::BindingData* binding_data, Local wrap, bool use_bigint) - : HandleWrap(env, + : HandleWrap(binding_data->env(), wrap, reinterpret_cast(&watcher_), AsyncWrap::PROVIDER_STATWATCHER), - use_bigint_(use_bigint) { - CHECK_EQ(0, uv_fs_poll_init(env->event_loop(), &watcher_)); + use_bigint_(use_bigint), + binding_data_(binding_data) { + CHECK_EQ(0, uv_fs_poll_init(env()->event_loop(), &watcher_)); } @@ -82,8 +83,10 @@ void StatWatcher::Callback(uv_fs_poll_t* handle, HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); - Local arr = fs::FillGlobalStatsArray(env, wrap->use_bigint_, curr); - USE(fs::FillGlobalStatsArray(env, wrap->use_bigint_, prev, true)); + Local arr = fs::FillGlobalStatsArray( + wrap->binding_data_.get(), wrap->use_bigint_, curr); + USE(fs::FillGlobalStatsArray( + wrap->binding_data_.get(), wrap->use_bigint_, prev, true)); Local argv[2] = { Integer::New(env->isolate(), status), arr }; wrap->MakeCallback(env->onchange_string(), arraysize(argv), argv); @@ -92,8 +95,8 @@ void StatWatcher::Callback(uv_fs_poll_t* handle, void StatWatcher::New(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); - Environment* env = Environment::GetCurrent(args); - new StatWatcher(env, args.This(), args[0]->IsTrue()); + fs::BindingData* binding_data = Unwrap(args.Data()); + new StatWatcher(binding_data, args.This(), args[0]->IsTrue()); } // wrap.start(filename, interval) diff --git a/src/node_stat_watcher.h b/src/node_stat_watcher.h index 9669806ce796c9..81a8903018624b 100644 --- a/src/node_stat_watcher.h +++ b/src/node_stat_watcher.h @@ -30,6 +30,9 @@ #include "v8.h" namespace node { +namespace fs { +struct BindingData; +} class Environment; @@ -38,7 +41,7 @@ class StatWatcher : public HandleWrap { static void Initialize(Environment* env, v8::Local target); protected: - StatWatcher(Environment* env, + StatWatcher(fs::BindingData* binding_data, v8::Local wrap, bool use_bigint); @@ -57,6 +60,7 @@ class StatWatcher : public HandleWrap { uv_fs_poll_t watcher_; const bool use_bigint_; + BaseObjectPtr binding_data_; }; } // namespace node From 610dfe5d9d133f9fc9b5b0df41ecb0c7621ad9ae Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 28 Mar 2020 23:59:50 +0100 Subject: [PATCH 06/15] src,doc: add documentation for per-binding state pattern --- src/README.md | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/README.md b/src/README.md index 2e59c51c3c37e9..8a681fa0068751 100644 --- a/src/README.md +++ b/src/README.md @@ -395,6 +395,50 @@ void Initialize(Local target, NODE_MODULE_CONTEXT_AWARE_INTERNAL(cares_wrap, Initialize) ``` + +#### Per-binding state + +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 object is always a [`BaseObject`][]. + +```c++ +// In the HTTP parser source code file: +class BindingData : public BaseObject { + public: + BindingData(Environment* env, Local obj) : BaseObject(env, obj) {} + + std::vector parser_buffer; + bool parser_buffer_in_use = false; + + // ... +}; + +// Available for binding functions, e.g. the HTTP Parser constructor: +static void New(const FunctionCallbackInfo& args) { + BindingData* binding_data = Unwrap(args.Data()); + new Parser(binding_data, args.This()); +} + +// ... because the initialization function told the Environment to use this +// BindingData class for all functions created by it: +void InitializeHttpParser(Local target, + Local unused, + Local context, + void* priv) { + Environment* env = Environment::GetCurrent(context); + Environment::BindingScope binding_scope(env); + BindingData* binding_data = binding_scope.data; + if (binding_data == nullptr) return; + + // Created within the Environment::BindingScope + Local t = env->NewFunctionTemplate(Parser::New); + ... +} +``` + ### Exception handling From 9222827c1f4a46c1736305a96abba2e49762b004 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 29 Mar 2020 17:17:48 +0200 Subject: [PATCH 07/15] fixup! src,doc: add documentation for per-binding state pattern --- src/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/README.md b/src/README.md index 8a681fa0068751..e2b256fba20de8 100644 --- a/src/README.md +++ b/src/README.md @@ -430,8 +430,8 @@ void InitializeHttpParser(Local target, void* priv) { Environment* env = Environment::GetCurrent(context); Environment::BindingScope binding_scope(env); + if (!binding_scope) return; BindingData* binding_data = binding_scope.data; - if (binding_data == nullptr) return; // Created within the Environment::BindingScope Local t = env->NewFunctionTemplate(Parser::New); From a68f057b2fd9774aa190977e46c88176a62b870a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 29 Mar 2020 17:18:01 +0200 Subject: [PATCH 08/15] fixup! src: make creating per-binding data structures easier --- src/env.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/env.h b/src/env.h index c1fc037a26758d..0d77e65bdf1872 100644 --- a/src/env.h +++ b/src/env.h @@ -876,6 +876,9 @@ class Environment : public MemoryRetainer { T* data = nullptr; Environment* env; + + inline operator bool() const { return data != nullptr; } + inline bool operator !() const { return data == nullptr; } }; template From d9e7d3854b3d0a44e65786d59ce39e947943f0f6 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 29 Mar 2020 17:18:10 +0200 Subject: [PATCH 09/15] fixup! src: move fs state out of Environment --- src/node_file.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_file.cc b/src/node_file.cc index a428732fa1267b..fb719c0729e313 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2296,8 +2296,8 @@ void Initialize(Local target, Environment* env = Environment::GetCurrent(context); Isolate* isolate = env->isolate(); Environment::BindingScope binding_scope(env); + if (!binding_scope) return; BindingData* binding_data = binding_scope.data; - if (binding_data == nullptr) return; env->SetMethod(target, "access", Access); env->SetMethod(target, "close", Close); From 5a2317d8413c877cc12a113292540a8d81825b2b Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 29 Mar 2020 17:18:17 +0200 Subject: [PATCH 10/15] fixup! src: move v8 stats buffers out of Environment --- src/node_v8.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_v8.cc b/src/node_v8.cc index c09b0a98b70f6c..cf62dd8dda9b4d 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -173,8 +173,8 @@ void Initialize(Local target, void* priv) { Environment* env = Environment::GetCurrent(context); Environment::BindingScope binding_scope(env); + if (!binding_scope) return; BindingData* binding_data = binding_scope.data; - if (binding_data == nullptr) return; env->SetMethodNoSideEffect(target, "cachedDataVersionTag", CachedDataVersionTag); From 38e471c9f652ff3fa5e7f44fcd7f137576bdea8f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 29 Mar 2020 17:18:26 +0200 Subject: [PATCH 11/15] fixup! src: move HTTP/2 state out of Environment --- src/node_http2.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index df6fdfe407f3de..d884be3e7553fc 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2938,8 +2938,8 @@ void Initialize(Local target, HandleScope handle_scope(isolate); Environment::BindingScope binding_scope(env); + if (!binding_scope) return; Http2State* state = binding_scope.data; - if (state == nullptr) return; #define SET_STATE_TYPEDARRAY(name, field) \ target->Set(context, \ From 0273dbf53837ac4a8bfa5e167e26d44de1973a64 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 29 Mar 2020 17:18:33 +0200 Subject: [PATCH 12/15] fixup! src: move http parser state out of Environment --- src/node_http_parser.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 9f12c1588ecfe9..3c839c3c4c0579 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -921,8 +921,8 @@ void InitializeHttpParser(Local target, void* priv) { Environment* env = Environment::GetCurrent(context); Environment::BindingScope binding_scope(env); + if (!binding_scope) return; BindingData* binding_data = binding_scope.data; - if (binding_data == nullptr) return; Local t = env->NewFunctionTemplate(Parser::New); t->InstanceTemplate()->SetInternalFieldCount(Parser::kInternalFieldCount); From 72d24ee97a520b232632ff48c32307cc8101b1df Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 30 Mar 2020 20:09:03 +0200 Subject: [PATCH 13/15] fixup! src: move v8 stats buffers out of Environment --- src/node_v8.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node_v8.cc b/src/node_v8.cc index cf62dd8dda9b4d..a0f477430419f9 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -30,6 +30,7 @@ namespace node { using v8::Array; using v8::ArrayBuffer; +using v8::BackingStore; using v8::Context; using v8::FunctionCallbackInfo; using v8::HeapCodeStatistics; From 54324b699b5bb7a76e57e4ecc1ab209731cf6740 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 30 Mar 2020 20:09:19 +0200 Subject: [PATCH 14/15] fixup! fixup! src: move http parser state out of Environment --- src/node_http_parser.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 3c839c3c4c0579..f98e9f5c7b6dab 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -922,7 +922,6 @@ void InitializeHttpParser(Local target, Environment* env = Environment::GetCurrent(context); Environment::BindingScope binding_scope(env); if (!binding_scope) return; - BindingData* binding_data = binding_scope.data; Local t = env->NewFunctionTemplate(Parser::New); t->InstanceTemplate()->SetInternalFieldCount(Parser::kInternalFieldCount); From 330e24bbb5f57094299e11293265df7aef3cff7e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 30 Mar 2020 20:09:29 +0200 Subject: [PATCH 15/15] fixup! src: move fs state out of Environment --- src/node_file.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_file.cc b/src/node_file.cc index fb719c0729e313..ea0f465fe8de54 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2015,7 +2015,7 @@ static void ReadBuffers(const FunctionCallbackInfo& args) { iovs[i] = uv_buf_init(Buffer::Data(buffer), Buffer::Length(buffer)); } - FSReqBase* req_wrap_async = GetReqWrap(env, args[3]); + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // readBuffers(fd, buffers, pos, req) AsyncCall(env, req_wrap_async, args, "read", UTF8, AfterInteger, uv_fs_read, fd, *iovs, iovs.length(), pos);