From 086af68c193e485f537360ff198ac2118d1c5d17 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 19 Dec 2017 15:08:18 +0100 Subject: [PATCH] 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 Backport-PR-URL: https://github.com/nodejs/node/pull/18179 PR-URL: https://github.com/nodejs/node/pull/17780 Reviewed-By: James M Snell --- lib/internal/async_hooks.js | 44 ++++++++++++- lib/internal/bootstrap_node.js | 6 +- src/aliased_buffer.h | 29 ++++++++- src/async_wrap.cc | 14 ++--- src/async_wrap.h | 1 - src/env-inl.h | 62 ++++++++++++------- src/env.cc | 17 +++++ src/env.h | 26 ++++---- .../test-async-hooks-recursive-stack.js | 20 ++++++ 9 files changed, 165 insertions(+), 54 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 366a218b7a95c2..8307f67f66d52a 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -19,6 +19,12 @@ 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; // Store the pair executionAsyncId and triggerAsyncId in a std::stack on @@ -26,7 +32,7 @@ const { async_hook_fields, async_id_fields } = async_wrap; // 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,8 @@ 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 } = async_wrap.constants; // Used in AsyncHook and AsyncResource. const init_symbol = Symbol('init'); @@ -332,6 +338,38 @@ function emitDestroyScript(asyncId) { } +// This is the equivalent of the native push_async_ids() call. +function pushAsyncIds(asyncId, triggerAsyncId) { + const offset = async_hook_fields[kStackLength]; + if (offset * 2 >= async_wrap.async_ids_stack.length) + return pushAsyncIds_(asyncId, triggerAsyncId); + 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; +} + + +// 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 (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_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; +} + + module.exports = { // Private API getHookArrays, diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 6a8713c986b076..9af547a923022b 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -357,9 +357,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; @@ -395,7 +395,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..b99b01f5d94ca2 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. @@ -111,11 +126,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_); } @@ -186,8 +207,12 @@ class AliasedBuffer { return GetValue(index); } + size_t Length() const { + return count_; + } + 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 3b0727cdfbcf0a..7e3c0c257ab22d 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -489,13 +489,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(); @@ -534,7 +527,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); @@ -572,6 +564,10 @@ void AsyncWrap::Initialize(Local target, "async_id_fields", env->async_hooks()->async_id_fields().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) \ FORCE_SET_TARGET_FIELD( \ @@ -588,6 +584,7 @@ void AsyncWrap::Initialize(Local target, SET_HOOKS_CONSTANT(kTriggerAsyncId); SET_HOOKS_CONSTANT(kAsyncIdCounter); SET_HOOKS_CONSTANT(kDefaultTriggerAsyncId); + SET_HOOKS_CONSTANT(kStackLength); #undef SET_HOOKS_CONSTANT FORCE_SET_TARGET_FIELD(target, "constants", constants); @@ -617,6 +614,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/async_wrap.h b/src/async_wrap.h index 773a0ad15ce9e9..bc826768d92736 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -123,7 +123,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 effe7523cd41fc..b61634774a64ad 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -81,11 +81,11 @@ inline uint32_t* IsolateData::zero_fill_field() const { return zero_fill_field_; } -inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) - : isolate_(isolate), - 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()); // kDefaultTriggerAsyncId should be -1, this indicates that there is no // specified default value and it should fallback to the executionAsyncId. @@ -102,9 +102,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()); @@ -122,8 +122,13 @@ Environment::AsyncHooks::async_id_fields() { return async_id_fields_; } +inline AliasedBuffer& +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::force_checks() { @@ -131,6 +136,11 @@ inline void Environment::AsyncHooks::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) { // Since async_hooks is experimental, do only perform the check @@ -140,16 +150,21 @@ 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 * 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]; + 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. @@ -161,32 +176,27 @@ 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); 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(); -} + uint32_t offset = fields_[kStackLength] - 1; + async_id_fields_[kExecutionAsyncId] = async_ids_stack_[2 * offset]; + async_id_fields_[kTriggerAsyncId] = async_ids_stack_[2 * offset + 1]; + fields_[kStackLength] = offset; -inline size_t Environment::AsyncHooks::stack_size() { - return async_ids_stack_.size(); + 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; } inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope @@ -210,6 +220,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_++; @@ -298,7 +313,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 3f6abd11284a11..3212dcc46cd4d0 100644 --- a/src/env.cc +++ b/src/env.cc @@ -269,4 +269,21 @@ void Environment::ActivateImmediateCheck() { uv_idle_start(&immediate_idle_handle_, [](uv_idle_t*){ }); } + +void Environment::AsyncHooks::grow_async_ids_stack() { + 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); + + for (uint32_t i = 0; i < old_capacity * 2; ++i) + new_buffer[i] = async_ids_stack_[i]; + async_ids_stack_ = std::move(new_buffer); + + 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 9faffce36f776e..40b8c666b7bdf4 100644 --- a/src/env.h +++ b/src/env.h @@ -41,7 +41,6 @@ #include #include #include -#include #include struct nghttp2_rcbuf; @@ -101,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") \ @@ -306,6 +306,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) \ @@ -339,11 +340,6 @@ class ModuleWrap; class Environment; -struct node_async_ids { - double async_id; - double trigger_async_id; -}; - class IsolateData { public: inline IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop, @@ -394,6 +390,7 @@ class Environment { kPromiseResolve, kTotals, kCheck, + kStackLength, kFieldsCount, }; @@ -405,18 +402,17 @@ class Environment { kUidFieldsCount, }; - AsyncHooks() = delete; - inline AliasedBuffer& fields(); inline AliasedBuffer& async_id_fields(); + inline AliasedBuffer& async_ids_stack(); inline v8::Local provider_string(int idx); inline void 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 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 @@ -438,19 +434,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_; + 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); }; @@ -692,6 +690,8 @@ class Environment { // This needs to be available for the JS-land setImmediate(). void ActivateImmediateCheck(); + static inline Environment* ForAsyncHooks(AsyncHooks* hooks); + private: inline void ThrowError(v8::Local (*fun)(v8::Local), const char* errmsg); 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);