From d84f16eba76bf256754fd0d42dba2490496e68f2 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 19 Dec 2017 15:08:18 +0100 Subject: [PATCH 1/4] src: use async_context instead of custom struct --- src/env.h | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/env.h b/src/env.h index 13517f75d9fa73..2cc8d1c718597e 100644 --- a/src/env.h +++ b/src/env.h @@ -313,11 +313,6 @@ class ModuleWrap; class Environment; -struct node_async_ids { - double async_id; - double trigger_async_id; -}; - class IsolateData { public: IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop, @@ -424,7 +419,7 @@ class Environment { // Used by provider_string(). v8::Isolate* isolate_; // Stores the ids of the current execution context stack. - std::stack async_ids_stack_; + std::stack async_ids_stack_; // Attached to a Uint32Array that tracks the number of active hooks for // each type. AliasedBuffer fields_; From 6bed72a7b633456523f5313a92ceeacb55fa495e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 20 Dec 2017 06:58:49 +0100 Subject: [PATCH 2/4] async_hooks: use typed array stack as fast path - Communicate the current async stack length through a typed array field rather than a native binding method - Add a new fixed-size `async_ids_fast_stack` typed array that contains the async ID stack up to a fixed limit. This increases performance noticeably, since most of the time the async ID stack will not be more than a handful of levels deep. - Make the JS `pushAsyncIds()` and `popAsyncIds()` functions do the same thing as the native ones if the fast path is applicable. Benchmarks: $ ./node benchmark/compare.js --new ./node --old ./node-master --runs 10 --filter next-tick process | Rscript benchmark/compare.R [00:03:25|% 100| 6/6 files | 20/20 runs | 1/1 configs]: Done improvement confidence p.value process/next-tick-breadth-args.js millions=4 19.72 % *** 3.013913e-06 process/next-tick-breadth.js millions=4 27.33 % *** 5.847983e-11 process/next-tick-depth-args.js millions=12 40.08 % *** 1.237127e-13 process/next-tick-depth.js millions=12 77.27 % *** 1.413290e-11 process/next-tick-exec-args.js millions=5 13.58 % *** 1.245180e-07 process/next-tick-exec.js millions=5 16.80 % *** 2.961386e-07 --- lib/internal/async_hooks.js | 48 +++++++++++++++++-- lib/internal/bootstrap_node.js | 6 +-- src/aliased_buffer.h | 8 +++- src/async_wrap.cc | 15 +++--- src/async_wrap.h | 1 - src/env-inl.h | 45 ++++++++++++----- src/env.h | 10 +++- .../test-async-hooks-recursive-stack.js | 20 ++++++++ 8 files changed, 123 insertions(+), 30 deletions(-) create mode 100644 test/parallel/test-async-hooks-recursive-stack.js diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 4bd5f3191b6e4a..6c48f5aa3c4b26 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -19,14 +19,20 @@ const async_wrap = process.binding('async_wrap'); * retrieving the triggerAsyncId value is passing directly to the * constructor -> value set in kDefaultTriggerAsyncId -> executionAsyncId of * the current resource. + * + * async_ids_fast_stack is a Float64Array that contains part of the async ID + * stack. Each pushAsyncIds() call adds two doubles to it, and each + * popAsyncIds() call removes two doubles from it. + * It has a fixed size, so if that is exceeded, calls to the native + * side are used instead in pushAsyncIds() and popAsyncIds(). */ -const { async_hook_fields, async_id_fields } = async_wrap; +const { async_hook_fields, async_id_fields, async_ids_fast_stack } = async_wrap; // Store the pair executionAsyncId and triggerAsyncId in a std::stack on // Environment::AsyncHooks::ids_stack_ tracks the resource responsible for the // current execution stack. This is unwound as each resource exits. In the case // of a fatal exception this stack is emptied after calling each hook's after() // callback. -const { pushAsyncIds, popAsyncIds } = async_wrap; +const { pushAsyncIds: pushAsyncIds_, popAsyncIds: popAsyncIds_ } = async_wrap; // For performance reasons, only track Proimses when a hook is enabled. const { enablePromiseHook, disablePromiseHook } = async_wrap; // Properties in active_hooks are used to keep track of the set of hooks being @@ -60,8 +66,9 @@ const active_hooks = { // async execution. These are tracked so if the user didn't include callbacks // for a given step, that step can bail out early. const { kInit, kBefore, kAfter, kDestroy, kPromiseResolve, - kCheck, kExecutionAsyncId, kAsyncIdCounter, - kDefaultTriggerAsyncId } = async_wrap.constants; + kCheck, kExecutionAsyncId, kAsyncIdCounter, kTriggerAsyncId, + kDefaultTriggerAsyncId, kStackLength, + kFastStackCapacity } = async_wrap.constants; // Used in AsyncHook and AsyncResource. const init_symbol = Symbol('init'); @@ -332,6 +339,39 @@ function emitDestroyScript(asyncId) { } +// This is the equivalent of the native push_async_ids() call. +function pushAsyncIds(asyncId, triggerAsyncId) { + const stackLength = async_hook_fields[kStackLength]; + if (stackLength >= kFastStackCapacity) + return pushAsyncIds_(asyncId, triggerAsyncId); + const offset = stackLength; + async_ids_fast_stack[offset * 2] = async_id_fields[kExecutionAsyncId]; + async_ids_fast_stack[offset * 2 + 1] = async_id_fields[kTriggerAsyncId]; + async_hook_fields[kStackLength]++; + async_id_fields[kExecutionAsyncId] = asyncId; + async_id_fields[kTriggerAsyncId] = triggerAsyncId; +} + + +// This is the equivalent of the native pop_async_ids() call. +function popAsyncIds(asyncId) { + if (async_hook_fields[kStackLength] === 0) return false; + const stackLength = async_hook_fields[kStackLength]; + + if (stackLength > kFastStackCapacity || + (async_hook_fields[kCheck] > 0 && + async_id_fields[kExecutionAsyncId] !== asyncId)) { + return popAsyncIds_(asyncId); + } + + const offset = stackLength - 1; + async_id_fields[kExecutionAsyncId] = async_ids_fast_stack[2 * offset]; + async_id_fields[kTriggerAsyncId] = async_ids_fast_stack[2 * offset + 1]; + async_hook_fields[kStackLength] = offset; + return offset > 0; +} + + module.exports = { // Private API getHookArrays, diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index f8a1fa39618610..298fc9df3084ba 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -367,9 +367,9 @@ // Arrays containing hook flags and ids for async_hook calls. const { async_hook_fields, async_id_fields } = async_wrap; // Internal functions needed to manipulate the stack. - const { clearAsyncIdStack, asyncIdStackSize } = async_wrap; + const { clearAsyncIdStack } = async_wrap; const { kAfter, kExecutionAsyncId, - kDefaultTriggerAsyncId } = async_wrap.constants; + kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants; process._fatalException = function(er) { var caught; @@ -407,7 +407,7 @@ do { NativeModule.require('internal/async_hooks').emitAfter( async_id_fields[kExecutionAsyncId]); - } while (asyncIdStackSize() > 0); + } while (async_hook_fields[kStackLength] > 0); // Or completely empty the id stack. } else { clearAsyncIdStack(); diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index 21aaeb61141c59..be53671dc1f451 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -111,11 +111,17 @@ class AliasedBuffer { index_(that.index_) { } - inline Reference& operator=(const NativeT &val) { + template + inline Reference& operator=(const T& val) { aliased_buffer_->SetValue(index_, val); return *this; } + // This is not caught by the template operator= above. + inline Reference& operator=(const Reference& val) { + return *this = static_cast(val); + } + operator NativeT() const { return aliased_buffer_->GetValue(index_); } diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 79d54b1000d342..92878f5b7384d1 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -468,13 +468,6 @@ void AsyncWrap::PopAsyncIds(const FunctionCallbackInfo& args) { } -void AsyncWrap::AsyncIdStackSize(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - args.GetReturnValue().Set( - static_cast(env->async_hooks()->stack_size())); -} - - void AsyncWrap::ClearAsyncIdStack(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); env->async_hooks()->clear_async_id_stack(); @@ -513,7 +506,6 @@ void AsyncWrap::Initialize(Local target, env->SetMethod(target, "setupHooks", SetupHooks); env->SetMethod(target, "pushAsyncIds", PushAsyncIds); env->SetMethod(target, "popAsyncIds", PopAsyncIds); - env->SetMethod(target, "asyncIdStackSize", AsyncIdStackSize); env->SetMethod(target, "clearAsyncIdStack", ClearAsyncIdStack); env->SetMethod(target, "queueDestroyAsyncId", QueueDestroyAsyncId); env->SetMethod(target, "enablePromiseHook", EnablePromiseHook); @@ -551,6 +543,11 @@ void AsyncWrap::Initialize(Local target, "async_id_fields", env->async_hooks()->async_id_fields().GetJSArray()); + FORCE_SET_TARGET_FIELD(target, + "async_ids_fast_stack", + env->async_hooks()->async_ids_fast_stack() + .GetJSArray()); + Local constants = Object::New(isolate); #define SET_HOOKS_CONSTANT(name) \ FORCE_SET_TARGET_FIELD( \ @@ -567,6 +564,8 @@ void AsyncWrap::Initialize(Local target, SET_HOOKS_CONSTANT(kTriggerAsyncId); SET_HOOKS_CONSTANT(kAsyncIdCounter); SET_HOOKS_CONSTANT(kDefaultTriggerAsyncId); + SET_HOOKS_CONSTANT(kStackLength); + SET_HOOKS_CONSTANT(kFastStackCapacity); #undef SET_HOOKS_CONSTANT FORCE_SET_TARGET_FIELD(target, "constants", constants); diff --git a/src/async_wrap.h b/src/async_wrap.h index ec9e162ca79bc4..4e69d941da9791 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -122,7 +122,6 @@ class AsyncWrap : public BaseObject { static void GetAsyncId(const v8::FunctionCallbackInfo& args); static void PushAsyncIds(const v8::FunctionCallbackInfo& args); static void PopAsyncIds(const v8::FunctionCallbackInfo& args); - static void AsyncIdStackSize(const v8::FunctionCallbackInfo& args); static void ClearAsyncIdStack( const v8::FunctionCallbackInfo& args); static void AsyncReset(const v8::FunctionCallbackInfo& args); diff --git a/src/env-inl.h b/src/env-inl.h index 976ebae7dd4b9f..488d4a3465df3d 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -55,6 +55,7 @@ inline MultiIsolatePlatform* IsolateData::platform() const { inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) : isolate_(isolate), + async_ids_fast_stack_(isolate, kFastStackCapacity * 2), fields_(isolate, kFieldsCount), async_id_fields_(isolate, kUidFieldsCount) { v8::HandleScope handle_scope(isolate_); @@ -101,6 +102,11 @@ Environment::AsyncHooks::async_id_fields() { return async_id_fields_; } +inline AliasedBuffer& +Environment::AsyncHooks::async_ids_fast_stack() { + return async_ids_fast_stack_; +} + inline v8::Local Environment::AsyncHooks::provider_string(int idx) { return providers_[idx].Get(isolate_); } @@ -110,6 +116,7 @@ inline void Environment::AsyncHooks::no_force_checks() { fields_[kCheck] = fields_[kCheck] - 1; } +// Remember to keep this code aligned with pushAsyncIds() in JS. inline void Environment::AsyncHooks::push_async_ids(double async_id, double trigger_async_id) { // Since async_hooks is experimental, do only perform the check @@ -119,16 +126,26 @@ inline void Environment::AsyncHooks::push_async_ids(double async_id, CHECK_GE(trigger_async_id, -1); } - async_ids_stack_.push({ async_id_fields_[kExecutionAsyncId], - async_id_fields_[kTriggerAsyncId] }); + uint32_t offset = fields_[kStackLength]; + if (offset < kFastStackCapacity) { + async_ids_fast_stack_[2 * offset] = async_id_fields_[kExecutionAsyncId]; + async_ids_fast_stack_[2 * offset + 1] = async_id_fields_[kTriggerAsyncId]; + } else { + async_ids_stack_.push({ + async_id_fields_[kExecutionAsyncId], + async_id_fields_[kTriggerAsyncId] + }); + } + fields_[kStackLength] = fields_[kStackLength] + 1; async_id_fields_[kExecutionAsyncId] = async_id; async_id_fields_[kTriggerAsyncId] = trigger_async_id; } +// Remember to keep this code aligned with popAsyncIds() in JS. inline bool Environment::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 (async_ids_stack_.empty()) return false; + if (fields_[kStackLength] == 0) return false; // Ask for the async_id to be restored as a check that the stack // hasn't been corrupted. @@ -150,15 +167,18 @@ inline bool Environment::AsyncHooks::pop_async_id(double async_id) { ABORT_NO_BACKTRACE(); } - auto async_ids = async_ids_stack_.top(); - async_ids_stack_.pop(); - async_id_fields_[kExecutionAsyncId] = async_ids.async_id; - async_id_fields_[kTriggerAsyncId] = async_ids.trigger_async_id; - return !async_ids_stack_.empty(); -} - -inline size_t Environment::AsyncHooks::stack_size() { - return async_ids_stack_.size(); + uint32_t offset = fields_[kStackLength] - 1; + if (offset >= kFastStackCapacity) { + auto async_ids = async_ids_stack_.top(); + async_ids_stack_.pop(); + async_id_fields_[kExecutionAsyncId] = async_ids.async_id; + async_id_fields_[kTriggerAsyncId] = async_ids.trigger_async_id; + } else { + async_id_fields_[kExecutionAsyncId] = async_ids_fast_stack_[2 * offset]; + async_id_fields_[kTriggerAsyncId] = async_ids_fast_stack_[2 * offset + 1]; + } + fields_[kStackLength] = offset; + return fields_[kStackLength] > 0; } inline void Environment::AsyncHooks::clear_async_id_stack() { @@ -166,6 +186,7 @@ inline void Environment::AsyncHooks::clear_async_id_stack() { async_ids_stack_.pop(); async_id_fields_[kExecutionAsyncId] = 0; async_id_fields_[kTriggerAsyncId] = 0; + fields_[kStackLength] = 0; } inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope diff --git a/src/env.h b/src/env.h index 2cc8d1c718597e..309d71643430d1 100644 --- a/src/env.h +++ b/src/env.h @@ -369,6 +369,7 @@ class Environment { kPromiseResolve, kTotals, kCheck, + kStackLength, kFieldsCount, }; @@ -380,10 +381,15 @@ class Environment { kUidFieldsCount, }; + enum FastStackFields { + kFastStackCapacity = 64 + }; + AsyncHooks() = delete; inline AliasedBuffer& fields(); inline AliasedBuffer& async_id_fields(); + inline AliasedBuffer& async_ids_fast_stack(); inline v8::Local provider_string(int idx); @@ -391,7 +397,6 @@ class Environment { inline void push_async_ids(double async_id, double trigger_async_id); inline bool pop_async_id(double async_id); - inline size_t stack_size(); inline void clear_async_id_stack(); // Used in fatal exceptions. // Used to set the kDefaultTriggerAsyncId in a scope. This is instead of @@ -420,6 +425,9 @@ class Environment { v8::Isolate* isolate_; // Stores the ids of the current execution context stack. std::stack async_ids_stack_; + // Stores the ids of the current execution context stack with limited + // capacity, but with the benefit of much faster access from JS. + AliasedBuffer async_ids_fast_stack_; // Attached to a Uint32Array that tracks the number of active hooks for // each type. AliasedBuffer fields_; diff --git a/test/parallel/test-async-hooks-recursive-stack.js b/test/parallel/test-async-hooks-recursive-stack.js new file mode 100644 index 00000000000000..7ab73dc1bf4538 --- /dev/null +++ b/test/parallel/test-async-hooks-recursive-stack.js @@ -0,0 +1,20 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); + +// This test verifies that the async ID stack can grow indefinitely. + +function recurse(n) { + const a = new async_hooks.AsyncResource('foobar'); + a.emitBefore(); + assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); + assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); + if (n >= 0) + recurse(n - 1); + assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); + assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); + a.emitAfter(); +} + +recurse(1000); From 12c10d1912946aca33d684658446e515889d740c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 21 Dec 2017 20:43:58 +0100 Subject: [PATCH 3/4] [squash] switch to resizable typed array --- lib/internal/async_hooks.js | 20 ++++++------ src/aliased_buffer.h | 17 +++++++++- src/async_wrap.cc | 10 +++--- src/env-inl.h | 65 ++++++++++++++++++------------------- src/env.cc | 18 ++++++++++ src/env.h | 27 ++++++++------- 6 files changed, 93 insertions(+), 64 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 6c48f5aa3c4b26..9c134f0ce873e7 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -26,7 +26,7 @@ const async_wrap = process.binding('async_wrap'); * It has a fixed size, so if that is exceeded, calls to the native * side are used instead in pushAsyncIds() and popAsyncIds(). */ -const { async_hook_fields, async_id_fields, async_ids_fast_stack } = async_wrap; +const { async_hook_fields, async_id_fields } = async_wrap; // Store the pair executionAsyncId and triggerAsyncId in a std::stack on // Environment::AsyncHooks::ids_stack_ tracks the resource responsible for the // current execution stack. This is unwound as each resource exits. In the case @@ -68,7 +68,7 @@ const active_hooks = { const { kInit, kBefore, kAfter, kDestroy, kPromiseResolve, kCheck, kExecutionAsyncId, kAsyncIdCounter, kTriggerAsyncId, kDefaultTriggerAsyncId, kStackLength, - kFastStackCapacity } = async_wrap.constants; + kStackCapacity } = async_wrap.constants; // Used in AsyncHook and AsyncResource. const init_symbol = Symbol('init'); @@ -342,11 +342,11 @@ function emitDestroyScript(asyncId) { // This is the equivalent of the native push_async_ids() call. function pushAsyncIds(asyncId, triggerAsyncId) { const stackLength = async_hook_fields[kStackLength]; - if (stackLength >= kFastStackCapacity) + if (stackLength >= async_hook_fields[kStackCapacity]) return pushAsyncIds_(asyncId, triggerAsyncId); const offset = stackLength; - async_ids_fast_stack[offset * 2] = async_id_fields[kExecutionAsyncId]; - async_ids_fast_stack[offset * 2 + 1] = async_id_fields[kTriggerAsyncId]; + async_wrap.async_ids_stack[offset * 2] = async_id_fields[kExecutionAsyncId]; + async_wrap.async_ids_stack[offset * 2 + 1] = async_id_fields[kTriggerAsyncId]; async_hook_fields[kStackLength]++; async_id_fields[kExecutionAsyncId] = asyncId; async_id_fields[kTriggerAsyncId] = triggerAsyncId; @@ -358,15 +358,15 @@ function popAsyncIds(asyncId) { if (async_hook_fields[kStackLength] === 0) return false; const stackLength = async_hook_fields[kStackLength]; - if (stackLength > kFastStackCapacity || - (async_hook_fields[kCheck] > 0 && - async_id_fields[kExecutionAsyncId] !== asyncId)) { + if (async_hook_fields[kCheck] > 0 && + async_id_fields[kExecutionAsyncId] !== asyncId) { + // Do the same thing as the native code (i.e. crash hard). return popAsyncIds_(asyncId); } const offset = stackLength - 1; - async_id_fields[kExecutionAsyncId] = async_ids_fast_stack[2 * offset]; - async_id_fields[kTriggerAsyncId] = async_ids_fast_stack[2 * offset + 1]; + async_id_fields[kExecutionAsyncId] = async_wrap.async_ids_stack[2 * offset]; + async_id_fields[kTriggerAsyncId] = async_wrap.async_ids_stack[2 * offset + 1]; async_hook_fields[kStackLength] = offset; return offset > 0; } diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index be53671dc1f451..71a75f80faa9ce 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -95,6 +95,21 @@ class AliasedBuffer { js_array_.Reset(); } + AliasedBuffer& operator=(AliasedBuffer&& that) { + this->~AliasedBuffer(); + isolate_ = that.isolate_; + count_ = that.count_; + byte_offset_ = that.byte_offset_; + buffer_ = that.buffer_; + free_buffer_ = that.free_buffer_; + + js_array_.Reset(isolate_, that.js_array_.Get(isolate_)); + + that.buffer_ = nullptr; + that.js_array_.Reset(); + return *this; + } + /** * Helper class that is returned from operator[] to support assignment into * a specified location. @@ -193,7 +208,7 @@ class AliasedBuffer { } private: - v8::Isolate* const isolate_; + v8::Isolate* isolate_; size_t count_; size_t byte_offset_; NativeT* buffer_; diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 92878f5b7384d1..9b268b501e442c 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -543,10 +543,9 @@ void AsyncWrap::Initialize(Local target, "async_id_fields", env->async_hooks()->async_id_fields().GetJSArray()); - FORCE_SET_TARGET_FIELD(target, - "async_ids_fast_stack", - env->async_hooks()->async_ids_fast_stack() - .GetJSArray()); + target->Set(context, + env->async_ids_stack_string(), + env->async_hooks()->async_ids_stack().GetJSArray()).FromJust(); Local constants = Object::New(isolate); #define SET_HOOKS_CONSTANT(name) \ @@ -565,7 +564,7 @@ void AsyncWrap::Initialize(Local target, SET_HOOKS_CONSTANT(kAsyncIdCounter); SET_HOOKS_CONSTANT(kDefaultTriggerAsyncId); SET_HOOKS_CONSTANT(kStackLength); - SET_HOOKS_CONSTANT(kFastStackCapacity); + SET_HOOKS_CONSTANT(kStackCapacity); #undef SET_HOOKS_CONSTANT FORCE_SET_TARGET_FIELD(target, "constants", constants); @@ -595,6 +594,7 @@ void AsyncWrap::Initialize(Local target, env->set_async_hooks_after_function(Local()); env->set_async_hooks_destroy_function(Local()); env->set_async_hooks_promise_resolve_function(Local()); + env->set_async_hooks_binding(target); } diff --git a/src/env-inl.h b/src/env-inl.h index 488d4a3465df3d..ece661802b0ffc 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -53,12 +53,11 @@ inline MultiIsolatePlatform* IsolateData::platform() const { return platform_; } -inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) - : isolate_(isolate), - async_ids_fast_stack_(isolate, kFastStackCapacity * 2), - fields_(isolate, kFieldsCount), - async_id_fields_(isolate, kUidFieldsCount) { - v8::HandleScope handle_scope(isolate_); +inline Environment::AsyncHooks::AsyncHooks() + : async_ids_stack_(env()->isolate(), 16 * 2), + fields_(env()->isolate(), kFieldsCount), + async_id_fields_(env()->isolate(), kUidFieldsCount) { + v8::HandleScope handle_scope(env()->isolate()); // Always perform async_hooks checks, not just when async_hooks is enabled. // TODO(AndreasMadsen): Consider removing this for LTS releases. @@ -67,6 +66,9 @@ inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) // and flag changes won't be included. fields_[kCheck] = 1; + // async_ids_stack_ was initialized to store 16 async_context structs. + fields_[kStackCapacity] = 16; + // kDefaultTriggerAsyncId should be -1, this indicates that there is no // specified default value and it should fallback to the executionAsyncId. // 0 is not used as the magic value, because that indicates a missing context @@ -82,9 +84,9 @@ inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) // strings can be retrieved quickly. #define V(Provider) \ providers_[AsyncWrap::PROVIDER_ ## Provider].Set( \ - isolate_, \ + env()->isolate(), \ v8::String::NewFromOneByte( \ - isolate_, \ + env()->isolate(), \ reinterpret_cast(#Provider), \ v8::NewStringType::kInternalized, \ sizeof(#Provider) - 1).ToLocalChecked()); @@ -103,12 +105,12 @@ Environment::AsyncHooks::async_id_fields() { } inline AliasedBuffer& -Environment::AsyncHooks::async_ids_fast_stack() { - return async_ids_fast_stack_; +Environment::AsyncHooks::async_ids_stack() { + return async_ids_stack_; } inline v8::Local Environment::AsyncHooks::provider_string(int idx) { - return providers_[idx].Get(isolate_); + return providers_[idx].Get(env()->isolate()); } inline void Environment::AsyncHooks::no_force_checks() { @@ -116,6 +118,10 @@ inline void Environment::AsyncHooks::no_force_checks() { fields_[kCheck] = fields_[kCheck] - 1; } +inline Environment* 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) { @@ -127,15 +133,10 @@ inline void Environment::AsyncHooks::push_async_ids(double async_id, } uint32_t offset = fields_[kStackLength]; - if (offset < kFastStackCapacity) { - async_ids_fast_stack_[2 * offset] = async_id_fields_[kExecutionAsyncId]; - async_ids_fast_stack_[2 * offset + 1] = async_id_fields_[kTriggerAsyncId]; - } else { - async_ids_stack_.push({ - async_id_fields_[kExecutionAsyncId], - async_id_fields_[kTriggerAsyncId] - }); - } + if (offset >= fields_[kStackCapacity]) + grow_async_ids_stack(); + async_ids_stack_[2 * offset] = async_id_fields_[kExecutionAsyncId]; + async_ids_stack_[2 * offset + 1] = async_id_fields_[kTriggerAsyncId]; fields_[kStackLength] = fields_[kStackLength] + 1; async_id_fields_[kExecutionAsyncId] = async_id; async_id_fields_[kTriggerAsyncId] = trigger_async_id; @@ -157,10 +158,9 @@ inline bool Environment::AsyncHooks::pop_async_id(double async_id) { "actual: %.f, expected: %.f)\n", async_id_fields_.GetValue(kExecutionAsyncId), async_id); - Environment* env = Environment::GetCurrent(isolate_); DumpBacktrace(stderr); fflush(stderr); - if (!env->abort_on_uncaught_exception()) + if (!env()->abort_on_uncaught_exception()) exit(1); fprintf(stderr, "\n"); fflush(stderr); @@ -168,22 +168,15 @@ inline bool Environment::AsyncHooks::pop_async_id(double async_id) { } uint32_t offset = fields_[kStackLength] - 1; - if (offset >= kFastStackCapacity) { - auto async_ids = async_ids_stack_.top(); - async_ids_stack_.pop(); - async_id_fields_[kExecutionAsyncId] = async_ids.async_id; - async_id_fields_[kTriggerAsyncId] = async_ids.trigger_async_id; - } else { - async_id_fields_[kExecutionAsyncId] = async_ids_fast_stack_[2 * offset]; - async_id_fields_[kTriggerAsyncId] = async_ids_fast_stack_[2 * offset + 1]; - } + CHECK_LT(offset, fields_[kStackCapacity]); + async_id_fields_[kExecutionAsyncId] = async_ids_stack_[2 * offset]; + async_id_fields_[kTriggerAsyncId] = async_ids_stack_[2 * offset + 1]; fields_[kStackLength] = offset; + return fields_[kStackLength] > 0; } inline void Environment::AsyncHooks::clear_async_id_stack() { - while (!async_ids_stack_.empty()) - async_ids_stack_.pop(); async_id_fields_[kExecutionAsyncId] = 0; async_id_fields_[kTriggerAsyncId] = 0; fields_[kStackLength] = 0; @@ -210,6 +203,11 @@ inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope } +Environment* Environment::ForAsyncHooks(AsyncHooks* hooks) { + return ContainerOf(&Environment::async_hooks_, hooks); +} + + inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env) : env_(env) { env_->makecallback_cntr_++; @@ -282,7 +280,6 @@ inline Environment::Environment(IsolateData* isolate_data, v8::Local context) : isolate_(context->GetIsolate()), isolate_data_(isolate_data), - async_hooks_(context->GetIsolate()), timer_base_(uv_now(isolate_data->event_loop())), using_domains_(false), printed_error_(false), diff --git a/src/env.cc b/src/env.cc index 97d253ae1df6ca..9b413447818f15 100644 --- a/src/env.cc +++ b/src/env.cc @@ -403,4 +403,22 @@ void Environment::CollectUVExceptionInfo(v8::Local object, syscall, message, path, dest); } + +void Environment::AsyncHooks::grow_async_ids_stack() { + const uint32_t old_capacity = fields_[kStackCapacity]; + const uint32_t new_capacity = old_capacity * 1.5; + AliasedBuffer new_buffer( + env()->isolate(), new_capacity * 2); + + for (uint32_t i = 0; i < old_capacity * 2; ++i) + new_buffer[i] = async_ids_stack_[i]; + async_ids_stack_ = std::move(new_buffer); + fields_[kStackCapacity] = new_capacity; + + env()->async_hooks_binding()->Set( + env()->context(), + env()->async_ids_stack_string(), + async_ids_stack_.GetJSArray()).FromJust(); +} + } // namespace node diff --git a/src/env.h b/src/env.h index 309d71643430d1..17451103c2201d 100644 --- a/src/env.h +++ b/src/env.h @@ -100,6 +100,7 @@ class ModuleWrap; V(address_string, "address") \ V(args_string, "args") \ V(async, "async") \ + V(async_ids_stack_string, "async_ids_stack") \ V(buffer_string, "buffer") \ V(bytes_string, "bytes") \ V(bytes_parsed_string, "bytesParsed") \ @@ -281,6 +282,7 @@ class ModuleWrap; V(async_hooks_before_function, v8::Function) \ V(async_hooks_after_function, v8::Function) \ V(async_hooks_promise_resolve_function, v8::Function) \ + V(async_hooks_binding, v8::Object) \ V(binding_cache_object, v8::Object) \ V(internal_binding_cache_object, v8::Object) \ V(buffer_prototype_object, v8::Object) \ @@ -370,6 +372,7 @@ class Environment { kTotals, kCheck, kStackLength, + kStackCapacity, kFieldsCount, }; @@ -381,19 +384,14 @@ class Environment { kUidFieldsCount, }; - enum FastStackFields { - kFastStackCapacity = 64 - }; - - AsyncHooks() = delete; - inline AliasedBuffer& fields(); inline AliasedBuffer& async_id_fields(); - inline AliasedBuffer& async_ids_fast_stack(); + 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); @@ -418,22 +416,21 @@ class Environment { private: friend class Environment; // So we can call the constructor. - inline explicit AsyncHooks(v8::Isolate* isolate); + inline AsyncHooks(); // Keep a list of all Persistent strings used for Provider types. v8::Eternal providers_[AsyncWrap::PROVIDERS_LENGTH]; - // Used by provider_string(). - v8::Isolate* isolate_; + // Keep track of the environment copy itself. + Environment* env_; // Stores the ids of the current execution context stack. - std::stack async_ids_stack_; - // Stores the ids of the current execution context stack with limited - // capacity, but with the benefit of much faster access from JS. - AliasedBuffer async_ids_fast_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(); + DISALLOW_COPY_AND_ASSIGN(AsyncHooks); }; @@ -689,6 +686,8 @@ class Environment { inline bool inside_should_not_abort_on_uncaught_scope() const; + static inline Environment* ForAsyncHooks(AsyncHooks* hooks); + private: inline void ThrowError(v8::Local (*fun)(v8::Local), const char* errmsg); From 24c80e7a803fa812e1608ecf1ea274218c2771f3 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 21 Dec 2017 22:00:43 +0100 Subject: [PATCH 4/4] [squash] bnoordhuis comments --- lib/internal/async_hooks.js | 8 +++----- src/aliased_buffer.h | 4 ++++ src/async_wrap.cc | 1 - src/env-inl.h | 6 +----- src/env.cc | 3 +-- src/env.h | 2 -- 6 files changed, 9 insertions(+), 15 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 9c134f0ce873e7..5b0b778038a5bd 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -67,8 +67,7 @@ const active_hooks = { // for a given step, that step can bail out early. const { kInit, kBefore, kAfter, kDestroy, kPromiseResolve, kCheck, kExecutionAsyncId, kAsyncIdCounter, kTriggerAsyncId, - kDefaultTriggerAsyncId, kStackLength, - kStackCapacity } = async_wrap.constants; + kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants; // Used in AsyncHook and AsyncResource. const init_symbol = Symbol('init'); @@ -341,10 +340,9 @@ function emitDestroyScript(asyncId) { // This is the equivalent of the native push_async_ids() call. function pushAsyncIds(asyncId, triggerAsyncId) { - const stackLength = async_hook_fields[kStackLength]; - if (stackLength >= async_hook_fields[kStackCapacity]) + const offset = async_hook_fields[kStackLength]; + if (offset * 2 >= async_wrap.async_ids_stack.length) return pushAsyncIds_(asyncId, triggerAsyncId); - const offset = stackLength; async_wrap.async_ids_stack[offset * 2] = async_id_fields[kExecutionAsyncId]; async_wrap.async_ids_stack[offset * 2 + 1] = async_id_fields[kTriggerAsyncId]; async_hook_fields[kStackLength]++; diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index 71a75f80faa9ce..b99b01f5d94ca2 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -207,6 +207,10 @@ class AliasedBuffer { return GetValue(index); } + size_t Length() const { + return count_; + } + private: v8::Isolate* isolate_; size_t count_; diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 9b268b501e442c..3fbcaa64a0cdf6 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -564,7 +564,6 @@ void AsyncWrap::Initialize(Local target, SET_HOOKS_CONSTANT(kAsyncIdCounter); SET_HOOKS_CONSTANT(kDefaultTriggerAsyncId); SET_HOOKS_CONSTANT(kStackLength); - SET_HOOKS_CONSTANT(kStackCapacity); #undef SET_HOOKS_CONSTANT FORCE_SET_TARGET_FIELD(target, "constants", constants); diff --git a/src/env-inl.h b/src/env-inl.h index ece661802b0ffc..fac26a3d9f5223 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -66,9 +66,6 @@ inline Environment::AsyncHooks::AsyncHooks() // and flag changes won't be included. fields_[kCheck] = 1; - // async_ids_stack_ was initialized to store 16 async_context structs. - fields_[kStackCapacity] = 16; - // kDefaultTriggerAsyncId should be -1, this indicates that there is no // specified default value and it should fallback to the executionAsyncId. // 0 is not used as the magic value, because that indicates a missing context @@ -133,7 +130,7 @@ inline void Environment::AsyncHooks::push_async_ids(double async_id, } uint32_t offset = fields_[kStackLength]; - if (offset >= fields_[kStackCapacity]) + if (offset * 2 >= async_ids_stack_.Length()) grow_async_ids_stack(); async_ids_stack_[2 * offset] = async_id_fields_[kExecutionAsyncId]; async_ids_stack_[2 * offset + 1] = async_id_fields_[kTriggerAsyncId]; @@ -168,7 +165,6 @@ inline bool Environment::AsyncHooks::pop_async_id(double async_id) { } uint32_t offset = fields_[kStackLength] - 1; - CHECK_LT(offset, fields_[kStackCapacity]); async_id_fields_[kExecutionAsyncId] = async_ids_stack_[2 * offset]; async_id_fields_[kTriggerAsyncId] = async_ids_stack_[2 * offset + 1]; fields_[kStackLength] = offset; diff --git a/src/env.cc b/src/env.cc index 9b413447818f15..4ed030d2803a1f 100644 --- a/src/env.cc +++ b/src/env.cc @@ -405,7 +405,7 @@ void Environment::CollectUVExceptionInfo(v8::Local object, void Environment::AsyncHooks::grow_async_ids_stack() { - const uint32_t old_capacity = fields_[kStackCapacity]; + const uint32_t old_capacity = async_ids_stack_.Length() / 2; const uint32_t new_capacity = old_capacity * 1.5; AliasedBuffer new_buffer( env()->isolate(), new_capacity * 2); @@ -413,7 +413,6 @@ void Environment::AsyncHooks::grow_async_ids_stack() { for (uint32_t i = 0; i < old_capacity * 2; ++i) new_buffer[i] = async_ids_stack_[i]; async_ids_stack_ = std::move(new_buffer); - fields_[kStackCapacity] = new_capacity; env()->async_hooks_binding()->Set( env()->context(), diff --git a/src/env.h b/src/env.h index 17451103c2201d..a3af2da2dd67dd 100644 --- a/src/env.h +++ b/src/env.h @@ -41,7 +41,6 @@ #include #include #include -#include #include struct nghttp2_rcbuf; @@ -372,7 +371,6 @@ class Environment { kTotals, kCheck, kStackLength, - kStackCapacity, kFieldsCount, };