From 7b0bbaf00e4945e65aa1259a82fa30a7225e8720 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 20 Mar 2019 19:05:02 +0800 Subject: [PATCH 1/8] src: move AsyncHooks out of Environment --- src/api/callback.cc | 2 - src/async_wrap-inl.h | 10 ++- src/async_wrap.cc | 1 - src/env-inl.h | 35 +++++------ src/env.cc | 3 +- src/env.h | 143 +++++++++++++++++++++--------------------- src/pipe_wrap.cc | 2 - src/stream_base-inl.h | 2 - src/tcp_wrap.cc | 2 - src/udp_wrap.cc | 3 - 10 files changed, 91 insertions(+), 112 deletions(-) diff --git a/src/api/callback.cc b/src/api/callback.cc index b90f5ca92daf09..97bdb48f681a24 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -17,8 +17,6 @@ using v8::Object; using v8::String; using v8::Value; -using AsyncHooks = Environment::AsyncHooks; - CallbackScope::CallbackScope(Isolate* isolate, Local object, async_context asyncContext) diff --git a/src/async_wrap-inl.h b/src/async_wrap-inl.h index 4405bb3a9baa52..6ef968933b8c1f 100644 --- a/src/async_wrap-inl.h +++ b/src/async_wrap-inl.h @@ -48,15 +48,13 @@ inline double AsyncWrap::get_trigger_async_id() const { inline AsyncWrap::AsyncScope::AsyncScope(AsyncWrap* wrap) : wrap_(wrap) { Environment* env = wrap->env(); - if (env->async_hooks()->fields()[Environment::AsyncHooks::kBefore] == 0) - return; + if (env->async_hooks()->fields()[AsyncHooks::kBefore] == 0) return; EmitBefore(env, wrap->get_async_id()); } inline AsyncWrap::AsyncScope::~AsyncScope() { Environment* env = wrap_->env(); - if (env->async_hooks()->fields()[Environment::AsyncHooks::kAfter] == 0) - return; + if (env->async_hooks()->fields()[AsyncHooks::kAfter] == 0) return; EmitAfter(env, wrap_->get_async_id()); } @@ -94,8 +92,8 @@ inline v8::MaybeLocal AsyncWrap::MakeCallback( // Defined here to avoid a circular dependency with env-inl.h. -inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope - ::DefaultTriggerAsyncIdScope(AsyncWrap* async_wrap) +inline AsyncHooks::DefaultTriggerAsyncIdScope ::DefaultTriggerAsyncIdScope( + AsyncWrap* async_wrap) : DefaultTriggerAsyncIdScope(async_wrap->env(), async_wrap->get_async_id()) {} diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 2087db2667c467..0326527fb7f2ab 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -56,7 +56,6 @@ using v8::Value; using v8::WeakCallbackInfo; using v8::WeakCallbackType; -using AsyncHooks = node::Environment::AsyncHooks; using TryCatchScope = node::errors::TryCatchScope; namespace node { diff --git a/src/env-inl.h b/src/env-inl.h index b32f3ee63d9b21..384701d1176d29 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -64,7 +64,7 @@ inline MultiIsolatePlatform* IsolateData::platform() const { return platform_; } -inline Environment::AsyncHooks::AsyncHooks() +inline AsyncHooks::AsyncHooks() : async_ids_stack_(env()->isolate(), 16 * 2), fields_(env()->isolate(), kFieldsCount), async_id_fields_(env()->isolate(), kUidFieldsCount) { @@ -102,36 +102,33 @@ inline Environment::AsyncHooks::AsyncHooks() #undef V } -inline AliasedBuffer& -Environment::AsyncHooks::fields() { +inline AliasedBuffer& AsyncHooks::fields() { return fields_; } -inline AliasedBuffer& -Environment::AsyncHooks::async_id_fields() { +inline AliasedBuffer& AsyncHooks::async_id_fields() { return async_id_fields_; } -inline AliasedBuffer& -Environment::AsyncHooks::async_ids_stack() { +inline AliasedBuffer& AsyncHooks::async_ids_stack() { return async_ids_stack_; } -inline v8::Local Environment::AsyncHooks::provider_string(int idx) { +inline v8::Local AsyncHooks::provider_string(int idx) { return providers_[idx].Get(env()->isolate()); } -inline void Environment::AsyncHooks::no_force_checks() { +inline void AsyncHooks::no_force_checks() { fields_[kCheck] -= 1; } -inline Environment* Environment::AsyncHooks::env() { +inline Environment* AsyncHooks::env() { return Environment::ForAsyncHooks(this); } // Remember to keep this code aligned with pushAsyncIds() in JS. -inline void Environment::AsyncHooks::push_async_ids(double async_id, - double trigger_async_id) { +inline void AsyncHooks::push_async_ids(double async_id, + double trigger_async_id) { // Since async_hooks is experimental, do only perform the check // when async_hooks is enabled. if (fields_[kCheck] > 0) { @@ -150,7 +147,7 @@ inline void Environment::AsyncHooks::push_async_ids(double async_id, } // Remember to keep this code aligned with popAsyncIds() in JS. -inline bool Environment::AsyncHooks::pop_async_id(double async_id) { +inline bool AsyncHooks::pop_async_id(double async_id) { // In case of an exception then this may have already been reset, if the // stack was multiple MakeCallback()'s deep. if (fields_[kStackLength] == 0) return false; @@ -183,7 +180,7 @@ inline bool Environment::AsyncHooks::pop_async_id(double async_id) { } // Keep in sync with clearAsyncIdStack in lib/internal/async_hooks.js. -inline void Environment::AsyncHooks::clear_async_id_stack() { +inline void AsyncHooks::clear_async_id_stack() { async_id_fields_[kExecutionAsyncId] = 0; async_id_fields_[kTriggerAsyncId] = 0; fields_[kStackLength] = 0; @@ -192,9 +189,8 @@ inline void Environment::AsyncHooks::clear_async_id_stack() { // The DefaultTriggerAsyncIdScope(AsyncWrap*) constructor is defined in // async_wrap-inl.h to avoid a circular dependency. -inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope - ::DefaultTriggerAsyncIdScope(Environment* env, - double default_trigger_async_id) +inline AsyncHooks::DefaultTriggerAsyncIdScope ::DefaultTriggerAsyncIdScope( + Environment* env, double default_trigger_async_id) : async_hooks_(env->async_hooks()) { if (env->async_hooks()->fields()[AsyncHooks::kCheck] > 0) { CHECK_GE(default_trigger_async_id, 0); @@ -206,8 +202,7 @@ inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope default_trigger_async_id; } -inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope - ::~DefaultTriggerAsyncIdScope() { +inline AsyncHooks::DefaultTriggerAsyncIdScope ::~DefaultTriggerAsyncIdScope() { async_hooks_->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = old_default_trigger_async_id_; } @@ -430,7 +425,7 @@ inline void Environment::set_is_in_inspector_console_call(bool value) { } #endif -inline Environment::AsyncHooks* Environment::async_hooks() { +inline AsyncHooks* Environment::async_hooks() { return &async_hooks_; } diff --git a/src/env.cc b/src/env.cc index 768e14d796d5bf..f1e5affaf855f6 100644 --- a/src/env.cc +++ b/src/env.cc @@ -889,8 +889,7 @@ void Environment::CollectUVExceptionInfo(Local object, syscall, message, path, dest); } - -void Environment::AsyncHooks::grow_async_ids_stack() { +void AsyncHooks::grow_async_ids_stack() { async_ids_stack_.reserve(async_ids_stack_.Length() * 3); env()->async_hooks_binding()->Set( diff --git a/src/env.h b/src/env.h index 70c566dce9a08f..1b7a0695be8ce8 100644 --- a/src/env.h +++ b/src/env.h @@ -532,87 +532,86 @@ class AsyncRequest : public MemoryRetainer { bool stop_ = true; }; -class Environment { +class AsyncHooks { public: - Environment(const Environment&) = delete; - Environment& operator=(const Environment&) = delete; + // Reason for both UidFields and Fields are that one is stored as a double* + // and the other as a uint32_t*. + enum Fields { + kInit, + kBefore, + kAfter, + kDestroy, + kPromiseResolve, + kTotals, + kCheck, + kStackLength, + kFieldsCount, + }; - class AsyncHooks { - public: - // Reason for both UidFields and Fields are that one is stored as a double* - // and the other as a uint32_t*. - enum Fields { - kInit, - kBefore, - kAfter, - kDestroy, - kPromiseResolve, - kTotals, - kCheck, - kStackLength, - kFieldsCount, - }; + enum UidFields { + kExecutionAsyncId, + kTriggerAsyncId, + kAsyncIdCounter, + kDefaultTriggerAsyncId, + kUidFieldsCount, + }; - enum UidFields { - kExecutionAsyncId, - kTriggerAsyncId, - kAsyncIdCounter, - kDefaultTriggerAsyncId, - kUidFieldsCount, - }; + inline AliasedBuffer& fields(); + inline AliasedBuffer& async_id_fields(); + inline AliasedBuffer& async_ids_stack(); - inline AliasedBuffer& fields(); - inline AliasedBuffer& async_id_fields(); - inline AliasedBuffer& async_ids_stack(); - - inline v8::Local provider_string(int idx); - - inline void no_force_checks(); - inline Environment* env(); - - inline void push_async_ids(double async_id, double trigger_async_id); - inline bool pop_async_id(double async_id); - inline void clear_async_id_stack(); // Used in fatal exceptions. - - AsyncHooks(const AsyncHooks&) = delete; - AsyncHooks& operator=(const AsyncHooks&) = delete; - - // Used to set the kDefaultTriggerAsyncId in a scope. This is instead of - // passing the trigger_async_id along with other constructor arguments. - class DefaultTriggerAsyncIdScope { - public: - DefaultTriggerAsyncIdScope() = delete; - explicit DefaultTriggerAsyncIdScope(Environment* env, - double init_trigger_async_id); - explicit DefaultTriggerAsyncIdScope(AsyncWrap* async_wrap); - ~DefaultTriggerAsyncIdScope(); - - DefaultTriggerAsyncIdScope(const DefaultTriggerAsyncIdScope&) = delete; - DefaultTriggerAsyncIdScope& operator=(const DefaultTriggerAsyncIdScope&) = - delete; - - private: - AsyncHooks* async_hooks_; - double old_default_trigger_async_id_; - }; + inline v8::Local provider_string(int idx); + inline void no_force_checks(); + inline Environment* env(); - private: - friend class Environment; // So we can call the constructor. - inline AsyncHooks(); - // Keep a list of all Persistent strings used for Provider types. - v8::Eternal providers_[AsyncWrap::PROVIDERS_LENGTH]; - // Stores the ids of the current execution context stack. - AliasedBuffer async_ids_stack_; - // Attached to a Uint32Array that tracks the number of active hooks for - // each type. - AliasedBuffer fields_; - // Attached to a Float64Array that tracks the state of async resources. - AliasedBuffer async_id_fields_; + inline void push_async_ids(double async_id, double trigger_async_id); + inline bool pop_async_id(double async_id); + inline void clear_async_id_stack(); // Used in fatal exceptions. - void grow_async_ids_stack(); + AsyncHooks(const AsyncHooks&) = delete; + AsyncHooks& operator=(const AsyncHooks&) = delete; + + // Used to set the kDefaultTriggerAsyncId in a scope. This is instead of + // passing the trigger_async_id along with other constructor arguments. + class DefaultTriggerAsyncIdScope { + public: + DefaultTriggerAsyncIdScope() = delete; + explicit DefaultTriggerAsyncIdScope(Environment* env, + double init_trigger_async_id); + explicit DefaultTriggerAsyncIdScope(AsyncWrap* async_wrap); + ~DefaultTriggerAsyncIdScope(); + + DefaultTriggerAsyncIdScope(const DefaultTriggerAsyncIdScope&) = delete; + DefaultTriggerAsyncIdScope& operator=(const DefaultTriggerAsyncIdScope&) = + delete; + + private: + AsyncHooks* async_hooks_; + double old_default_trigger_async_id_; }; + private: + friend class Environment; // So we can call the constructor. + inline AsyncHooks(); + // Keep a list of all Persistent strings used for Provider types. + v8::Eternal providers_[AsyncWrap::PROVIDERS_LENGTH]; + // Stores the ids of the current execution context stack. + AliasedBuffer async_ids_stack_; + // Attached to a Uint32Array that tracks the number of active hooks for + // each type. + AliasedBuffer fields_; + // Attached to a Float64Array that tracks the state of async resources. + AliasedBuffer async_id_fields_; + + void grow_async_ids_stack(); +}; + +class Environment { + public: + Environment(const Environment&) = delete; + Environment& operator=(const Environment&) = delete; + class AsyncCallbackScope { public: AsyncCallbackScope() = delete; diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 0103129a66c5ab..5b96c5c63384bf 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -47,8 +47,6 @@ using v8::Object; using v8::String; using v8::Value; -using AsyncHooks = Environment::AsyncHooks; - MaybeLocal PipeWrap::Instantiate(Environment* env, AsyncWrap* parent, PipeWrap::SocketType type) { diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index dbf3c07b920cf8..08a02e186cfe73 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -22,8 +22,6 @@ using v8::PropertyCallbackInfo; using v8::String; using v8::Value; -using AsyncHooks = Environment::AsyncHooks; - inline void StreamReq::AttachToObject(v8::Local req_wrap_obj) { CHECK_EQ(req_wrap_obj->GetAlignedPointerFromInternalField(kStreamReqField), nullptr); diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index af9806e4889b4c..9f6fd3beb6a70c 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -52,8 +52,6 @@ using v8::String; using v8::Uint32; using v8::Value; -using AsyncHooks = Environment::AsyncHooks; - MaybeLocal TCPWrap::Instantiate(Environment* env, AsyncWrap* parent, TCPWrap::SocketType type) { diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index f1693aa39b9735..e568bb66a6682d 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -46,9 +46,6 @@ using v8::Uint32; using v8::Undefined; using v8::Value; -using AsyncHooks = Environment::AsyncHooks; - - class SendWrap : public ReqWrap { public: SendWrap(Environment* env, Local req_wrap_obj, bool have_callback); From 1a311a55f6eccb0378e00dab7350ebc1c1c04617 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 20 Mar 2019 19:19:02 +0800 Subject: [PATCH 2/8] src: move AsyncCallbackScope out of Environment --- src/api/callback.cc | 4 ++-- src/env-inl.h | 20 +++++++++++++------- src/env.h | 30 ++++++++++++++++-------------- src/node.cc | 2 +- src/node_http_parser_impl.h | 4 ++-- src/node_internals.h | 2 +- src/node_worker.cc | 2 +- 7 files changed, 36 insertions(+), 28 deletions(-) diff --git a/src/api/callback.cc b/src/api/callback.cc index 97bdb48f681a24..a94d252d94625d 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -93,7 +93,7 @@ void InternalCallbackScope::Close() { AsyncWrap::EmitAfter(env_, async_context_.async_id); } - if (env_->makecallback_depth() > 1) { + if (env_->async_callback_scope_depth() > 1) { return; } @@ -217,7 +217,7 @@ MaybeLocal MakeCallback(Isolate* isolate, Context::Scope context_scope(env->context()); MaybeLocal ret = InternalMakeCallback(env, recv, callback, argc, argv, asyncContext); - if (ret.IsEmpty() && env->makecallback_depth() == 0) { + if (ret.IsEmpty() && env->async_callback_scope_depth() == 0) { // This is only for legacy compatibility and we may want to look into // removing/adjusting it. return Undefined(env->isolate()); diff --git a/src/env-inl.h b/src/env-inl.h index 384701d1176d29..c1145427a33312 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -212,18 +212,24 @@ Environment* Environment::ForAsyncHooks(AsyncHooks* hooks) { return ContainerOf(&Environment::async_hooks_, hooks); } +inline AsyncCallbackScope::AsyncCallbackScope(Environment* env) : env_(env) { + env_->PushAsyncCallbackScope(); +} -inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env) - : env_(env) { - env_->makecallback_cntr_++; +inline AsyncCallbackScope::~AsyncCallbackScope() { + env_->PopAsyncCallbackScope(); +} + +inline size_t Environment::async_callback_scope_depth() const { + return async_callback_scope_depth_; } -inline Environment::AsyncCallbackScope::~AsyncCallbackScope() { - env_->makecallback_cntr_--; +inline void Environment::PushAsyncCallbackScope() { + async_callback_scope_depth_++; } -inline size_t Environment::makecallback_depth() const { - return makecallback_cntr_; +inline void Environment::PopAsyncCallbackScope() { + async_callback_scope_depth_--; } inline Environment::ImmediateInfo::ImmediateInfo(v8::Isolate* isolate) diff --git a/src/env.h b/src/env.h index 1b7a0695be8ce8..5f55ecfac28097 100644 --- a/src/env.h +++ b/src/env.h @@ -607,24 +607,26 @@ class AsyncHooks { void grow_async_ids_stack(); }; +class AsyncCallbackScope { + public: + AsyncCallbackScope() = delete; + explicit AsyncCallbackScope(Environment* env); + ~AsyncCallbackScope(); + AsyncCallbackScope(const AsyncCallbackScope&) = delete; + AsyncCallbackScope& operator=(const AsyncCallbackScope&) = delete; + + private: + Environment* env_; +}; + class Environment { public: Environment(const Environment&) = delete; Environment& operator=(const Environment&) = delete; - class AsyncCallbackScope { - public: - AsyncCallbackScope() = delete; - explicit AsyncCallbackScope(Environment* env); - ~AsyncCallbackScope(); - AsyncCallbackScope(const AsyncCallbackScope&) = delete; - AsyncCallbackScope& operator=(const AsyncCallbackScope&) = delete; - - private: - Environment* env_; - }; - - inline size_t makecallback_depth() const; + inline size_t async_callback_scope_depth() const; + inline void PushAsyncCallbackScope(); + inline void PopAsyncCallbackScope(); class ImmediateInfo { public: @@ -1079,7 +1081,7 @@ class Environment { bool printed_error_ = false; bool emit_env_nonstring_warning_ = true; bool emit_err_name_warning_ = true; - size_t makecallback_cntr_ = 0; + size_t async_callback_scope_depth_ = 0; std::vector destroy_async_id_list_; std::shared_ptr options_; diff --git a/src/node.cc b/src/node.cc index b42f0169de5e3a..50ee2391f4a5c6 100644 --- a/src/node.cc +++ b/src/node.cc @@ -821,7 +821,7 @@ inline int StartNodeWithIsolate(Isolate* isolate, #endif // HAVE_INSPECTOR && NODE_USE_V8_PLATFORM { - Environment::AsyncCallbackScope callback_scope(&env); + AsyncCallbackScope callback_scope(&env); env.async_hooks()->push_async_ids(1, 0); LoadEnvironment(&env); env.async_hooks()->pop_async_id(1); diff --git a/src/node_http_parser_impl.h b/src/node_http_parser_impl.h index a154af5cfbaf4e..a2eea3bb3d7ce7 100644 --- a/src/node_http_parser_impl.h +++ b/src/node_http_parser_impl.h @@ -323,7 +323,7 @@ class Parser : public AsyncWrap, public StreamListener { argv[A_UPGRADE] = Boolean::New(env()->isolate(), parser_.upgrade); - Environment::AsyncCallbackScope callback_scope(env()); + AsyncCallbackScope callback_scope(env()); MaybeLocal head_response = MakeCallback(cb.As(), arraysize(argv), argv); @@ -394,7 +394,7 @@ class Parser : public AsyncWrap, public StreamListener { if (!cb->IsFunction()) return 0; - Environment::AsyncCallbackScope callback_scope(env()); + AsyncCallbackScope callback_scope(env()); MaybeLocal r = MakeCallback(cb.As(), 0, nullptr); diff --git a/src/node_internals.h b/src/node_internals.h index bc6a36d9db4a21..cf796178a4dd82 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -213,7 +213,7 @@ class InternalCallbackScope { Environment* env_; async_context async_context_; v8::Local object_; - Environment::AsyncCallbackScope callback_scope_; + AsyncCallbackScope callback_scope_; bool failed_ = false; bool pushed_ids_ = false; bool closed_ = false; diff --git a/src/node_worker.cc b/src/node_worker.cc index b7ccbaffa7686f..b402b726e85aa0 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -277,7 +277,7 @@ void Worker::Run() { inspector_started = true; HandleScope handle_scope(isolate_); - Environment::AsyncCallbackScope callback_scope(env_.get()); + AsyncCallbackScope callback_scope(env_.get()); env_->async_hooks()->push_async_ids(1, 0); if (!RunBootstrapping(env_.get()).IsEmpty()) { CreateEnvMessagePort(env_.get()); From c827efde6cdace7f28dc09c44a9f4d9e2088a720 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 20 Mar 2019 19:31:19 +0800 Subject: [PATCH 3/8] src: move ImmediateInfo out of Environment --- src/env-inl.h | 20 +++++++++---------- src/env.h | 53 ++++++++++++++++++++++----------------------------- 2 files changed, 33 insertions(+), 40 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index c1145427a33312..9bdd95db38e6f1 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -232,39 +232,39 @@ inline void Environment::PopAsyncCallbackScope() { async_callback_scope_depth_--; } -inline Environment::ImmediateInfo::ImmediateInfo(v8::Isolate* isolate) +inline ImmediateInfo::ImmediateInfo(v8::Isolate* isolate) : fields_(isolate, kFieldsCount) {} inline AliasedBuffer& - Environment::ImmediateInfo::fields() { + ImmediateInfo::fields() { return fields_; } -inline uint32_t Environment::ImmediateInfo::count() const { +inline uint32_t ImmediateInfo::count() const { return fields_[kCount]; } -inline uint32_t Environment::ImmediateInfo::ref_count() const { +inline uint32_t ImmediateInfo::ref_count() const { return fields_[kRefCount]; } -inline bool Environment::ImmediateInfo::has_outstanding() const { +inline bool ImmediateInfo::has_outstanding() const { return fields_[kHasOutstanding] == 1; } -inline void Environment::ImmediateInfo::count_inc(uint32_t increment) { +inline void ImmediateInfo::count_inc(uint32_t increment) { fields_[kCount] += increment; } -inline void Environment::ImmediateInfo::count_dec(uint32_t decrement) { +inline void ImmediateInfo::count_dec(uint32_t decrement) { fields_[kCount] -= decrement; } -inline void Environment::ImmediateInfo::ref_count_inc(uint32_t increment) { +inline void ImmediateInfo::ref_count_inc(uint32_t increment) { fields_[kRefCount] += increment; } -inline void Environment::ImmediateInfo::ref_count_dec(uint32_t decrement) { +inline void ImmediateInfo::ref_count_dec(uint32_t decrement) { fields_[kRefCount] -= decrement; } @@ -435,7 +435,7 @@ inline AsyncHooks* Environment::async_hooks() { return &async_hooks_; } -inline Environment::ImmediateInfo* Environment::immediate_info() { +inline ImmediateInfo* Environment::immediate_info() { return &immediate_info_; } diff --git a/src/env.h b/src/env.h index 5f55ecfac28097..43390a8322a572 100644 --- a/src/env.h +++ b/src/env.h @@ -619,6 +619,29 @@ class AsyncCallbackScope { Environment* env_; }; +class ImmediateInfo { + public: + inline AliasedBuffer& fields(); + inline uint32_t count() const; + inline uint32_t ref_count() const; + inline bool has_outstanding() const; + inline void count_inc(uint32_t increment); + inline void count_dec(uint32_t decrement); + inline void ref_count_inc(uint32_t increment); + inline void ref_count_dec(uint32_t decrement); + + ImmediateInfo(const ImmediateInfo&) = delete; + ImmediateInfo& operator=(const ImmediateInfo&) = delete; + + private: + friend class Environment; // So we can call the constructor. + inline explicit ImmediateInfo(v8::Isolate* isolate); + + enum Fields { kCount, kRefCount, kHasOutstanding, kFieldsCount }; + + AliasedBuffer fields_; +}; + class Environment { public: Environment(const Environment&) = delete; @@ -628,36 +651,6 @@ class Environment { inline void PushAsyncCallbackScope(); inline void PopAsyncCallbackScope(); - class ImmediateInfo { - public: - inline AliasedBuffer& fields(); - inline uint32_t count() const; - inline uint32_t ref_count() const; - inline bool has_outstanding() const; - - inline void count_inc(uint32_t increment); - inline void count_dec(uint32_t decrement); - - inline void ref_count_inc(uint32_t increment); - inline void ref_count_dec(uint32_t decrement); - - ImmediateInfo(const ImmediateInfo&) = delete; - ImmediateInfo& operator=(const ImmediateInfo&) = delete; - - private: - friend class Environment; // So we can call the constructor. - inline explicit ImmediateInfo(v8::Isolate* isolate); - - enum Fields { - kCount, - kRefCount, - kHasOutstanding, - kFieldsCount - }; - - AliasedBuffer fields_; - }; - class TickInfo { public: inline AliasedBuffer& fields(); From d8ad2f07c76558d96ae1f83979da3d7f4aeff15d Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 20 Mar 2019 19:42:02 +0800 Subject: [PATCH 4/8] src: move TickInfo out of Environment --- src/api/callback.cc | 2 +- src/env-inl.h | 10 +++++----- src/env.h | 40 ++++++++++++++++++---------------------- 3 files changed, 24 insertions(+), 28 deletions(-) diff --git a/src/api/callback.cc b/src/api/callback.cc index a94d252d94625d..0c5879fb3e0184 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -97,7 +97,7 @@ void InternalCallbackScope::Close() { return; } - Environment::TickInfo* tick_info = env_->tick_info(); + TickInfo* tick_info = env_->tick_info(); if (!env_->can_call_into_js()) return; if (!tick_info->has_tick_scheduled()) { diff --git a/src/env-inl.h b/src/env-inl.h index 9bdd95db38e6f1..01df21b0f98054 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -268,18 +268,18 @@ inline void ImmediateInfo::ref_count_dec(uint32_t decrement) { fields_[kRefCount] -= decrement; } -inline Environment::TickInfo::TickInfo(v8::Isolate* isolate) +inline TickInfo::TickInfo(v8::Isolate* isolate) : fields_(isolate, kFieldsCount) {} -inline AliasedBuffer& Environment::TickInfo::fields() { +inline AliasedBuffer& TickInfo::fields() { return fields_; } -inline bool Environment::TickInfo::has_tick_scheduled() const { +inline bool TickInfo::has_tick_scheduled() const { return fields_[kHasTickScheduled] == 1; } -inline bool Environment::TickInfo::has_rejection_to_warn() const { +inline bool TickInfo::has_rejection_to_warn() const { return fields_[kHasRejectionToWarn] == 1; } @@ -439,7 +439,7 @@ inline ImmediateInfo* Environment::immediate_info() { return &immediate_info_; } -inline Environment::TickInfo* Environment::tick_info() { +inline TickInfo* Environment::tick_info() { return &tick_info_; } diff --git a/src/env.h b/src/env.h index 43390a8322a572..2bca109d12707c 100644 --- a/src/env.h +++ b/src/env.h @@ -642,6 +642,24 @@ class ImmediateInfo { AliasedBuffer fields_; }; +class TickInfo { + public: + inline AliasedBuffer& fields(); + inline bool has_tick_scheduled() const; + inline bool has_rejection_to_warn() const; + + TickInfo(const TickInfo&) = delete; + TickInfo& operator=(const TickInfo&) = delete; + + private: + friend class Environment; // So we can call the constructor. + inline explicit TickInfo(v8::Isolate* isolate); + + enum Fields { kHasTickScheduled = 0, kHasRejectionToWarn, kFieldsCount }; + + AliasedBuffer fields_; +}; + class Environment { public: Environment(const Environment&) = delete; @@ -651,28 +669,6 @@ class Environment { inline void PushAsyncCallbackScope(); inline void PopAsyncCallbackScope(); - class TickInfo { - public: - inline AliasedBuffer& fields(); - inline bool has_tick_scheduled() const; - inline bool has_rejection_to_warn() const; - - TickInfo(const TickInfo&) = delete; - TickInfo& operator=(const TickInfo&) = delete; - - private: - friend class Environment; // So we can call the constructor. - inline explicit TickInfo(v8::Isolate* isolate); - - enum Fields { - kHasTickScheduled = 0, - kHasRejectionToWarn, - kFieldsCount - }; - - AliasedBuffer fields_; - }; - enum Flags { kNoFlags = 0, kIsMainThread = 1 << 0, From 3a28707ea15f7d047d3d7498aad54a775f8e51e0 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 20 Mar 2019 19:45:28 +0800 Subject: [PATCH 5/8] src: move TrackingTraceStateObserver out of Environment --- src/env.cc | 2 +- src/env.h | 38 +++++++++++++++++++------------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/env.cc b/src/env.cc index f1e5affaf855f6..a2a62dd0b56ebd 100644 --- a/src/env.cc +++ b/src/env.cc @@ -140,7 +140,7 @@ void InitThreadLocalOnce() { CHECK_EQ(0, uv_key_create(&Environment::thread_local_env)); } -void Environment::TrackingTraceStateObserver::UpdateTraceCategoryState() { +void TrackingTraceStateObserver::UpdateTraceCategoryState() { if (!env_->owns_process_state()) { // Ideally, we’d have a consistent story that treats all threads/Environment // instances equally here. However, tracing is essentially global, and this diff --git a/src/env.h b/src/env.h index 2bca109d12707c..83cc6baf6b2bda 100644 --- a/src/env.h +++ b/src/env.h @@ -660,6 +660,25 @@ class TickInfo { AliasedBuffer fields_; }; +class TrackingTraceStateObserver : + public v8::TracingController::TraceStateObserver { + public: + explicit TrackingTraceStateObserver(Environment* env) : env_(env) {} + + void OnTraceEnabled() override { + UpdateTraceCategoryState(); + } + + void OnTraceDisabled() override { + UpdateTraceCategoryState(); + } + + private: + void UpdateTraceCategoryState(); + + Environment* env_; +}; + class Environment { public: Environment(const Environment&) = delete; @@ -976,25 +995,6 @@ class Environment { // This needs to be available for the JS-land setImmediate(). void ToggleImmediateRef(bool ref); - class TrackingTraceStateObserver : - public v8::TracingController::TraceStateObserver { - public: - explicit TrackingTraceStateObserver(Environment* env) : env_(env) {} - - void OnTraceEnabled() override { - UpdateTraceCategoryState(); - } - - void OnTraceDisabled() override { - UpdateTraceCategoryState(); - } - - private: - void UpdateTraceCategoryState(); - - Environment* env_; - }; - class ShouldNotAbortOnUncaughtScope { public: explicit inline ShouldNotAbortOnUncaughtScope(Environment* env); From be197ca41410f44397ed29452139b9abc4440bf3 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 20 Mar 2019 22:52:28 +0800 Subject: [PATCH 6/8] src: move ShouldNotAbortOnUncaughtScope out of Environment --- src/env-inl.h | 20 ++++++++++++++------ src/env.h | 22 ++++++++++++---------- src/module_wrap.cc | 4 ++-- src/node_contextify.cc | 2 +- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 01df21b0f98054..6afed4581e7cdb 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -487,24 +487,32 @@ inline uint32_t Environment::get_next_function_id() { return function_id_counter_++; } -Environment::ShouldNotAbortOnUncaughtScope::ShouldNotAbortOnUncaughtScope( +ShouldNotAbortOnUncaughtScope::ShouldNotAbortOnUncaughtScope( Environment* env) : env_(env) { - env_->should_not_abort_scope_counter_++; + env_->PushShouldNotAbortOnUncaughtScope(); } -Environment::ShouldNotAbortOnUncaughtScope::~ShouldNotAbortOnUncaughtScope() { +ShouldNotAbortOnUncaughtScope::~ShouldNotAbortOnUncaughtScope() { Close(); } -void Environment::ShouldNotAbortOnUncaughtScope::Close() { +void ShouldNotAbortOnUncaughtScope::Close() { if (env_ != nullptr) { - env_->should_not_abort_scope_counter_--; + env_->PopShouldNotAbortOnUncaughtScope(); env_ = nullptr; } } -bool Environment::inside_should_not_abort_on_uncaught_scope() const { +inline void Environment::PushShouldNotAbortOnUncaughtScope() { + should_not_abort_scope_counter_++; +} + +inline void Environment::PopShouldNotAbortOnUncaughtScope() { + should_not_abort_scope_counter_--; +} + +inline bool Environment::inside_should_not_abort_on_uncaught_scope() const { return should_not_abort_scope_counter_ > 0; } diff --git a/src/env.h b/src/env.h index 83cc6baf6b2bda..0bd40c36b6c574 100644 --- a/src/env.h +++ b/src/env.h @@ -679,6 +679,16 @@ class TrackingTraceStateObserver : Environment* env_; }; +class ShouldNotAbortOnUncaughtScope { + public: + explicit inline ShouldNotAbortOnUncaughtScope(Environment* env); + inline void Close(); + inline ~ShouldNotAbortOnUncaughtScope(); + + private: + Environment* env_; +}; + class Environment { public: Environment(const Environment&) = delete; @@ -995,16 +1005,8 @@ class Environment { // This needs to be available for the JS-land setImmediate(). void ToggleImmediateRef(bool ref); - class ShouldNotAbortOnUncaughtScope { - public: - explicit inline ShouldNotAbortOnUncaughtScope(Environment* env); - inline void Close(); - inline ~ShouldNotAbortOnUncaughtScope(); - - private: - Environment* env_; - }; - + inline void PushShouldNotAbortOnUncaughtScope(); + inline void PopShouldNotAbortOnUncaughtScope(); inline bool inside_should_not_abort_on_uncaught_scope() const; static inline Environment* ForAsyncHooks(AsyncHooks* hooks); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index ac5d28fb2352ba..56149d0cc759c9 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -136,7 +136,7 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { column_offset = Integer::New(isolate, 0); } - Environment::ShouldNotAbortOnUncaughtScope no_abort_scope(env); + ShouldNotAbortOnUncaughtScope no_abort_scope(env); TryCatchScope try_catch(env); Local module; @@ -280,7 +280,7 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo& args) { CHECK(args[1]->IsBoolean()); bool break_on_sigint = args[1]->IsTrue(); - Environment::ShouldNotAbortOnUncaughtScope no_abort_scope(env); + ShouldNotAbortOnUncaughtScope no_abort_scope(env); TryCatchScope try_catch(env); bool timed_out = false; diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 621fa7eb16fe79..087727d93a9837 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -719,7 +719,7 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { compile_options = ScriptCompiler::kConsumeCodeCache; TryCatchScope try_catch(env); - Environment::ShouldNotAbortOnUncaughtScope no_abort_scope(env); + ShouldNotAbortOnUncaughtScope no_abort_scope(env); Context::Scope scope(parsing_context); MaybeLocal v8_script = ScriptCompiler::CompileUnboundScript( From b3ba0852c80160b556cabc368c4d2a6b0bdfbd26 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 22 Mar 2019 20:44:22 +0800 Subject: [PATCH 7/8] fixup! src: move ShouldNotAbortOnUncaughtScope out of Environment --- src/env.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/env.h b/src/env.h index 0bd40c36b6c574..9974120083f6ed 100644 --- a/src/env.h +++ b/src/env.h @@ -680,7 +680,7 @@ class TrackingTraceStateObserver : }; class ShouldNotAbortOnUncaughtScope { - public: + public: explicit inline ShouldNotAbortOnUncaughtScope(Environment* env); inline void Close(); inline ~ShouldNotAbortOnUncaughtScope(); From 8b9d63a7a50c49f464eea49a8e9f70f7fdf54553 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 26 Mar 2019 13:51:48 -0400 Subject: [PATCH 8/8] fixup! fixup! src: move ShouldNotAbortOnUncaughtScope out of Environment --- src/env.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/env.h b/src/env.h index 9974120083f6ed..45577523b5f6ad 100644 --- a/src/env.h +++ b/src/env.h @@ -685,7 +685,7 @@ class ShouldNotAbortOnUncaughtScope { inline void Close(); inline ~ShouldNotAbortOnUncaughtScope(); - private: + private: Environment* env_; };