From 91658268e160fe6b459592b3405fca2527c6563b Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 12 Jul 2020 03:10:25 +0200 Subject: [PATCH 1/4] async_hooks: improve resource stack performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes some of the performance overhead that came with `executionAsyncResource()` by using the JS resource array only as a cache for the values provided by C++. The fact that we now use an entry trampoline is used to pass the resource without requiring extra C++/JS boundary crossings, and the direct accesses to the JS resource array from C++ are removed in all fast paths. This particularly improves performance when async hooks are not being used. This is a continuation of https://github.com/nodejs/node/pull/33575 and shares some of its code with it. ./node benchmark/compare.js --new ./node --old ./node-master --runs 30 --filter messageport worker | Rscript benchmark/compare.R [00:06:14|% 100| 1/1 files | 60/60 runs | 2/2 configs]: Done confidence improvement accuracy (*) (**) (***) worker/messageport.js n=1000000 payload='object' ** 12.64 % ±7.30% ±9.72% ±12.65% worker/messageport.js n=1000000 payload='string' * 11.08 % ±9.00% ±11.98% ±15.59% ./node benchmark/compare.js --new ./node --old ./node-master --runs 20 --filter async-resource-vs-destroy async_hooks | Rscript benchmark/compare.R [00:22:35|% 100| 1/1 files | 40/40 runs | 6/6 configs]: Done confidence improvement accuracy (*) async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='async-local-storage' benchmarker='autocannon' 1.60 % ±7.35% async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='async-resource' benchmarker='autocannon' 6.05 % ±6.57% async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='destroy' benchmarker='autocannon' * 8.27 % ±7.50% async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='async-local-storage' benchmarker='autocannon' 7.42 % ±8.22% async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='async-resource' benchmarker='autocannon' 4.33 % ±7.84% async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='destroy' benchmarker='autocannon' 5.96 % ±7.15% (**) (***) async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='async-local-storage' benchmarker='autocannon' ±9.84% ±12.94% async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='async-resource' benchmarker='autocannon' ±8.81% ±11.60% async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='destroy' benchmarker='autocannon' ±10.07% ±13.28% async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='async-local-storage' benchmarker='autocannon' ±11.01% ±14.48% async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='async-resource' benchmarker='autocannon' ±10.50% ±13.81% async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='destroy' benchmarker='autocannon' ±9.58% ±12.62% Refs: https://github.com/nodejs/node/pull/33575 --- lib/internal/async_hooks.js | 31 ++++++++++--------- src/api/callback.cc | 12 ++++--- src/async_wrap.cc | 24 ++++++++++++-- src/async_wrap.h | 4 +++ src/env-inl.h | 62 ++++++++++++++++++++++++++++++------- src/env.h | 10 ++++-- 6 files changed, 108 insertions(+), 35 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 591c1c8e79d6c6..8ed220a1a47271 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -50,7 +50,9 @@ const { // each hook's after() callback. const { pushAsyncContext: pushAsyncContext_, - popAsyncContext: popAsyncContext_ + popAsyncContext: popAsyncContext_, + executionAsyncResource: executionAsyncResource_, + clearAsyncIdStack, } = async_wrap; // For performance reasons, only track Promises when a hook is enabled. const { enablePromiseHook, disablePromiseHook } = async_wrap; @@ -89,7 +91,8 @@ const { resource_symbol, owner_symbol } = internalBinding('symbols'); // for a given step, that step can bail out early. const { kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve, kCheck, kExecutionAsyncId, kAsyncIdCounter, kTriggerAsyncId, - kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants; + kDefaultTriggerAsyncId, kStackLength, kUsesExecutionAsyncResource +} = async_wrap.constants; const { async_id_symbol, trigger_async_id_symbol } = internalBinding('symbols'); @@ -111,7 +114,10 @@ function useDomainTrampoline(fn) { domain_cb = fn; } -function callbackTrampoline(asyncId, cb, ...args) { +function callbackTrampoline(asyncId, resource, cb, ...args) { + const index = async_hook_fields[kStackLength] - 1; + execution_async_resources[index] = resource; + if (asyncId !== 0 && hasHooks(kBefore)) emitBeforeNative(asyncId); @@ -126,6 +132,7 @@ function callbackTrampoline(asyncId, cb, ...args) { if (asyncId !== 0 && hasHooks(kAfter)) emitAfterNative(asyncId); + execution_async_resources.pop(); return result; } @@ -134,9 +141,15 @@ setCallbackTrampoline(callbackTrampoline); const topLevelResource = {}; function executionAsyncResource() { + // Indicate to the native layer that this function is likely to be used, + // in which case it will inform JS about the current async resource via + // the trampoline above. + async_hook_fields[kUsesExecutionAsyncResource] = 1; + const index = async_hook_fields[kStackLength] - 1; if (index === -1) return topLevelResource; - const resource = execution_async_resources[index]; + const resource = execution_async_resources[index] || + executionAsyncResource_(index); return lookupPublicResource(resource); } @@ -478,16 +491,6 @@ function emitDestroyScript(asyncId) { } -// Keep in sync with Environment::AsyncHooks::clear_async_id_stack -// in src/env-inl.h. -function clearAsyncIdStack() { - async_id_fields[kExecutionAsyncId] = 0; - async_id_fields[kTriggerAsyncId] = 0; - async_hook_fields[kStackLength] = 0; - execution_async_resources.splice(0, execution_async_resources.length); -} - - function hasAsyncIdStack() { return hasHooks(kStackLength); } diff --git a/src/api/callback.cc b/src/api/callback.cc index 9f52c25cf0d900..99c53c55a2a544 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -161,11 +161,12 @@ MaybeLocal InternalMakeCallback(Environment* env, Local hook_cb = env->async_hooks_callback_trampoline(); int flags = InternalCallbackScope::kNoFlags; int hook_count = 0; + AsyncHooks* async_hooks = env->async_hooks(); if (!hook_cb.IsEmpty()) { flags = InternalCallbackScope::kSkipAsyncHooks; - AsyncHooks* async_hooks = env->async_hooks(); hook_count = async_hooks->fields()[AsyncHooks::kBefore] + - async_hooks->fields()[AsyncHooks::kAfter]; + async_hooks->fields()[AsyncHooks::kAfter] + + async_hooks->fields()[AsyncHooks::kUsesExecutionAsyncResource]; } InternalCallbackScope scope(env, resource, asyncContext, flags); @@ -176,11 +177,12 @@ MaybeLocal InternalMakeCallback(Environment* env, MaybeLocal ret; if (hook_count != 0) { - MaybeStackBuffer, 16> args(2 + argc); + MaybeStackBuffer, 16> args(3 + argc); args[0] = v8::Number::New(env->isolate(), asyncContext.async_id); - args[1] = callback; + args[1] = resource; + args[2] = callback; for (int i = 0; i < argc; i++) { - args[i + 2] = argv[i]; + args[i + 3] = argv[i]; } ret = hook_cb->Call(env->context(), recv, args.length(), &args[0]); } else { diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 123f85f38a715f..3147e4da884e1f 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -502,7 +502,8 @@ void AsyncWrap::PushAsyncContext(const FunctionCallbackInfo& args) { // then the checks in push_async_ids() and pop_async_id() will. double async_id = args[0]->NumberValue(env->context()).FromJust(); double trigger_async_id = args[1]->NumberValue(env->context()).FromJust(); - env->async_hooks()->push_async_context(async_id, trigger_async_id, args[2]); + env->async_hooks()->push_async_context( + async_id, trigger_async_id, args[2].As()); } @@ -513,6 +514,22 @@ void AsyncWrap::PopAsyncContext(const FunctionCallbackInfo& args) { } +void AsyncWrap::ExecutionAsyncResource( + const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + uint32_t index; + if (!args[0]->Uint32Value(env->context()).To(&index)) return; + args.GetReturnValue().Set( + env->async_hooks()->native_execution_async_resource(index)); +} + + +void AsyncWrap::ClearAsyncIdStack(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + env->async_hooks()->clear_async_id_stack(); +} + + void AsyncWrap::AsyncReset(const FunctionCallbackInfo& args) { CHECK(args[0]->IsObject()); @@ -586,6 +603,8 @@ void AsyncWrap::Initialize(Local target, env->SetMethod(target, "setCallbackTrampoline", SetCallbackTrampoline); env->SetMethod(target, "pushAsyncContext", PushAsyncContext); env->SetMethod(target, "popAsyncContext", PopAsyncContext); + env->SetMethod(target, "executionAsyncResource", ExecutionAsyncResource); + env->SetMethod(target, "clearAsyncIdStack", ClearAsyncIdStack); env->SetMethod(target, "queueDestroyAsyncId", QueueDestroyAsyncId); env->SetMethod(target, "enablePromiseHook", EnablePromiseHook); env->SetMethod(target, "disablePromiseHook", DisablePromiseHook); @@ -624,7 +643,7 @@ void AsyncWrap::Initialize(Local target, FORCE_SET_TARGET_FIELD(target, "execution_async_resources", - env->async_hooks()->execution_async_resources()); + env->async_hooks()->js_execution_async_resources()); target->Set(context, env->async_ids_stack_string(), @@ -646,6 +665,7 @@ void AsyncWrap::Initialize(Local target, SET_HOOKS_CONSTANT(kTriggerAsyncId); SET_HOOKS_CONSTANT(kAsyncIdCounter); SET_HOOKS_CONSTANT(kDefaultTriggerAsyncId); + SET_HOOKS_CONSTANT(kUsesExecutionAsyncResource); SET_HOOKS_CONSTANT(kStackLength); #undef SET_HOOKS_CONSTANT FORCE_SET_TARGET_FIELD(target, "constants", constants); diff --git a/src/async_wrap.h b/src/async_wrap.h index dfaad1de79888f..e202314331f33e 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -143,6 +143,10 @@ class AsyncWrap : public BaseObject { static void GetAsyncId(const v8::FunctionCallbackInfo& args); static void PushAsyncContext(const v8::FunctionCallbackInfo& args); static void PopAsyncContext(const v8::FunctionCallbackInfo& args); + static void ExecutionAsyncResource( + const v8::FunctionCallbackInfo& args); + static void ClearAsyncIdStack( + const v8::FunctionCallbackInfo& args); static void AsyncReset(const v8::FunctionCallbackInfo& args); static void GetProviderType(const v8::FunctionCallbackInfo& args); static void QueueDestroyAsyncId( diff --git a/src/env-inl.h b/src/env-inl.h index 7016e63d326dbd..3db3542f102814 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -105,8 +105,17 @@ inline AliasedFloat64Array& AsyncHooks::async_ids_stack() { return async_ids_stack_; } -inline v8::Local AsyncHooks::execution_async_resources() { - return PersistentToLocal::Strong(execution_async_resources_); +v8::Local AsyncHooks::js_execution_async_resources() { + if (UNLIKELY(js_execution_async_resources_.IsEmpty())) { + js_execution_async_resources_.Reset( + env()->isolate(), v8::Array::New(env()->isolate())); + } + return PersistentToLocal::Strong(js_execution_async_resources_); +} + +v8::Local AsyncHooks::native_execution_async_resource(size_t i) { + if (i >= native_execution_async_resources_.size()) return {}; + return PersistentToLocal::Strong(native_execution_async_resources_[i]); } inline v8::Local AsyncHooks::provider_string(int idx) { @@ -124,7 +133,7 @@ inline Environment* AsyncHooks::env() { // Remember to keep this code aligned with pushAsyncContext() in JS. inline void AsyncHooks::push_async_context(double async_id, double trigger_async_id, - v8::Local resource) { + v8::Local resource) { v8::HandleScope handle_scope(env()->isolate()); // Since async_hooks is experimental, do only perform the check @@ -143,8 +152,12 @@ inline void AsyncHooks::push_async_context(double async_id, async_id_fields_[kExecutionAsyncId] = async_id; async_id_fields_[kTriggerAsyncId] = trigger_async_id; - auto resources = execution_async_resources(); - USE(resources->Set(env()->context(), offset, resource)); +#ifdef DEBUG + for (uint32_t i = offset; i < native_execution_async_resources_.size(); i++) + CHECK(native_execution_async_resources_[i].IsEmpty()); +#endif + native_execution_async_resources_.resize(offset + 1); + native_execution_async_resources_[offset].Reset(env()->isolate(), resource); } // Remember to keep this code aligned with popAsyncContext() in JS. @@ -177,17 +190,44 @@ inline bool AsyncHooks::pop_async_context(double async_id) { async_id_fields_[kTriggerAsyncId] = async_ids_stack_[2 * offset + 1]; fields_[kStackLength] = offset; - auto resources = execution_async_resources(); - USE(resources->Delete(env()->context(), offset)); + if (LIKELY(offset < native_execution_async_resources_.size() && + !native_execution_async_resources_[offset].IsEmpty())) { +#ifdef DEBUG + for (uint32_t i = offset + 1; + i < native_execution_async_resources_.size(); + i++) { + CHECK(native_execution_async_resources_[i].IsEmpty()); + } +#endif + native_execution_async_resources_.resize(offset); + if (native_execution_async_resources_.size() < + native_execution_async_resources_.capacity() / 2 && + native_execution_async_resources_.size() > 16) { + native_execution_async_resources_.shrink_to_fit(); + } + } + + if (UNLIKELY(js_execution_async_resources()->Length() > offset)) { + USE(js_execution_async_resources()->Set( + env()->context(), + env()->length_string(), + v8::Integer::NewFromUnsigned(env()->isolate(), offset))); + } return fields_[kStackLength] > 0; } -// Keep in sync with clearAsyncIdStack in lib/internal/async_hooks.js. -inline void AsyncHooks::clear_async_id_stack() { - auto isolate = env()->isolate(); +void AsyncHooks::clear_async_id_stack() { + v8::Isolate* isolate = env()->isolate(); v8::HandleScope handle_scope(isolate); - execution_async_resources_.Reset(isolate, v8::Array::New(isolate)); + if (!js_execution_async_resources_.IsEmpty()) { + USE(PersistentToLocal::Strong(js_execution_async_resources_)->Set( + env()->context(), + env()->length_string(), + v8::Integer::NewFromUnsigned(isolate, 0))); + } + native_execution_async_resources_.clear(); + native_execution_async_resources_.shrink_to_fit(); async_id_fields_[kExecutionAsyncId] = 0; async_id_fields_[kTriggerAsyncId] = 0; diff --git a/src/env.h b/src/env.h index 0c1d785cc2743c..ee2a66a6c70ec2 100644 --- a/src/env.h +++ b/src/env.h @@ -275,6 +275,7 @@ constexpr size_t kFsStatsBufferLength = V(issuercert_string, "issuerCertificate") \ V(kill_signal_string, "killSignal") \ V(kind_string, "kind") \ + V(length_string, "length") \ V(library_string, "library") \ V(mac_string, "mac") \ V(max_buffer_string, "maxBuffer") \ @@ -661,6 +662,7 @@ class AsyncHooks : public MemoryRetainer { kTotals, kCheck, kStackLength, + kUsesExecutionAsyncResource, kFieldsCount, }; @@ -675,7 +677,8 @@ class AsyncHooks : public MemoryRetainer { inline AliasedUint32Array& fields(); inline AliasedFloat64Array& async_id_fields(); inline AliasedFloat64Array& async_ids_stack(); - inline v8::Local execution_async_resources(); + inline v8::Local js_execution_async_resources(); + inline v8::Local native_execution_async_resource(size_t index); inline v8::Local provider_string(int idx); @@ -683,7 +686,7 @@ class AsyncHooks : public MemoryRetainer { inline Environment* env(); inline void push_async_context(double async_id, double trigger_async_id, - v8::Local execution_async_resource_); + v8::Local execution_async_resource_); inline bool pop_async_context(double async_id); inline void clear_async_id_stack(); // Used in fatal exceptions. @@ -728,7 +731,8 @@ class AsyncHooks : public MemoryRetainer { void grow_async_ids_stack(); - v8::Global execution_async_resources_; + v8::Global js_execution_async_resources_; + std::vector> native_execution_async_resources_; }; class ImmediateInfo : public MemoryRetainer { From 64ac36f53d8239d93f30fe9575800cc0fa661d7d Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 12 Jul 2020 04:34:25 +0200 Subject: [PATCH 2/4] fixup! async_hooks: improve resource stack performance --- src/env-inl.h | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 3db3542f102814..ee01fb1c83b8d9 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -134,8 +134,6 @@ inline Environment* AsyncHooks::env() { inline void AsyncHooks::push_async_context(double async_id, double trigger_async_id, v8::Local resource) { - v8::HandleScope handle_scope(env()->isolate()); - // Since async_hooks is experimental, do only perform the check // when async_hooks is enabled. if (fields_[kCheck] > 0) { @@ -152,10 +150,6 @@ inline void AsyncHooks::push_async_context(double async_id, async_id_fields_[kExecutionAsyncId] = async_id; async_id_fields_[kTriggerAsyncId] = trigger_async_id; -#ifdef DEBUG - for (uint32_t i = offset; i < native_execution_async_resources_.size(); i++) - CHECK(native_execution_async_resources_[i].IsEmpty()); -#endif native_execution_async_resources_.resize(offset + 1); native_execution_async_resources_[offset].Reset(env()->isolate(), resource); } @@ -192,13 +186,6 @@ inline bool AsyncHooks::pop_async_context(double async_id) { if (LIKELY(offset < native_execution_async_resources_.size() && !native_execution_async_resources_[offset].IsEmpty())) { -#ifdef DEBUG - for (uint32_t i = offset + 1; - i < native_execution_async_resources_.size(); - i++) { - CHECK(native_execution_async_resources_[i].IsEmpty()); - } -#endif native_execution_async_resources_.resize(offset); if (native_execution_async_resources_.size() < native_execution_async_resources_.capacity() / 2 && @@ -208,6 +195,7 @@ inline bool AsyncHooks::pop_async_context(double async_id) { } if (UNLIKELY(js_execution_async_resources()->Length() > offset)) { + v8::HandleScope handle_scope(env()->isolate()); USE(js_execution_async_resources()->Set( env()->context(), env()->length_string(), From 078c42719d4350b4f2bf81d9a40e40eee5a8a3f1 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 14 Jul 2020 00:53:45 +0200 Subject: [PATCH 3/4] fixup! fixup! async_hooks: improve resource stack performance --- lib/internal/async_hooks.js | 2 +- lib/internal/process/execution.js | 6 +++--- src/api/callback.cc | 13 ++++++++----- src/async_wrap.cc | 3 +-- src/env-inl.h | 22 ++++++++++++++++++++-- 5 files changed, 33 insertions(+), 13 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 8ed220a1a47271..f4d4f1da49c4ca 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -500,7 +500,7 @@ function hasAsyncIdStack() { function pushAsyncContext(asyncId, triggerAsyncId, resource) { const offset = async_hook_fields[kStackLength]; if (offset * 2 >= async_wrap.async_ids_stack.length) - return pushAsyncContext_(asyncId, triggerAsyncId, resource); + return pushAsyncContext_(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]; execution_async_resources[offset] = resource; diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index 0d6b5cdf73cf23..08d6ec8c6ea906 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -190,10 +190,10 @@ function createOnGlobalUncaughtException() { do { emitAfter(executionAsyncId()); } while (hasAsyncIdStack()); - // Or completely empty the id stack. - } else { - clearAsyncIdStack(); } + // And completely empty the id stack, including anything that may be + // cached on the native side. + clearAsyncIdStack(); return true; }; diff --git a/src/api/callback.cc b/src/api/callback.cc index 99c53c55a2a544..2bb34b088f74e1 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -160,13 +160,16 @@ MaybeLocal InternalMakeCallback(Environment* env, Local hook_cb = env->async_hooks_callback_trampoline(); int flags = InternalCallbackScope::kNoFlags; - int hook_count = 0; + bool use_async_hooks_trampoline = false; AsyncHooks* async_hooks = env->async_hooks(); if (!hook_cb.IsEmpty()) { + // Use the callback trampoline if there are any before or after hooks, or + // we can expect some kind of usage of async_hooks.executionAsyncResource(). flags = InternalCallbackScope::kSkipAsyncHooks; - hook_count = async_hooks->fields()[AsyncHooks::kBefore] + - async_hooks->fields()[AsyncHooks::kAfter] + - async_hooks->fields()[AsyncHooks::kUsesExecutionAsyncResource]; + use_async_hooks_trampoline = + async_hooks->fields()[AsyncHooks::kBefore] + + async_hooks->fields()[AsyncHooks::kAfter] + + async_hooks->fields()[AsyncHooks::kUsesExecutionAsyncResource] > 0; } InternalCallbackScope scope(env, resource, asyncContext, flags); @@ -176,7 +179,7 @@ MaybeLocal InternalMakeCallback(Environment* env, MaybeLocal ret; - if (hook_count != 0) { + if (use_async_hooks_trampoline) { MaybeStackBuffer, 16> args(3 + argc); args[0] = v8::Number::New(env->isolate(), asyncContext.async_id); args[1] = resource; diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 3147e4da884e1f..09775db79a8d97 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -502,8 +502,7 @@ void AsyncWrap::PushAsyncContext(const FunctionCallbackInfo& args) { // then the checks in push_async_ids() and pop_async_id() will. double async_id = args[0]->NumberValue(env->context()).FromJust(); double trigger_async_id = args[1]->NumberValue(env->context()).FromJust(); - env->async_hooks()->push_async_context( - async_id, trigger_async_id, args[2].As()); + env->async_hooks()->push_async_context(async_id, trigger_async_id, {}); } diff --git a/src/env-inl.h b/src/env-inl.h index ee01fb1c83b8d9..f7b4ef4c4a30e6 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -150,8 +150,19 @@ inline void AsyncHooks::push_async_context(double async_id, async_id_fields_[kExecutionAsyncId] = async_id; async_id_fields_[kTriggerAsyncId] = trigger_async_id; - native_execution_async_resources_.resize(offset + 1); - native_execution_async_resources_[offset].Reset(env()->isolate(), resource); +#ifdef DEBUG + for (uint32_t i = offset; i < native_execution_async_resources_.size(); i++) + CHECK(native_execution_async_resources_[i].IsEmpty()); +#endif + + // When this call comes from JS (as a way of increasing the stack size), + // `resource` will be empty, because JS caches these values anyway, and + // we should avoid creating strong global references that might keep + // these JS resource objects alive longer than necessary. + if (!resource.IsEmpty()) { + native_execution_async_resources_.resize(offset + 1); + native_execution_async_resources_[offset].Reset(env()->isolate(), resource); + } } // Remember to keep this code aligned with popAsyncContext() in JS. @@ -186,6 +197,13 @@ inline bool AsyncHooks::pop_async_context(double async_id) { if (LIKELY(offset < native_execution_async_resources_.size() && !native_execution_async_resources_[offset].IsEmpty())) { +#ifdef DEBUG + for (uint32_t i = offset + 1; + i < native_execution_async_resources_.size(); + i++) { + CHECK(native_execution_async_resources_[i].IsEmpty()); + } +#endif native_execution_async_resources_.resize(offset); if (native_execution_async_resources_.size() < native_execution_async_resources_.capacity() / 2 && From 5ddee99f3554b5747b0c9d0ce8687d5cca99e269 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 14 Jul 2020 01:28:29 +0200 Subject: [PATCH 4/4] fixup! fixup! fixup! async_hooks: improve resource stack performance --- src/env.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/env.h b/src/env.h index ee2a66a6c70ec2..f7771e8acd7765 100644 --- a/src/env.h +++ b/src/env.h @@ -678,6 +678,10 @@ class AsyncHooks : public MemoryRetainer { inline AliasedFloat64Array& async_id_fields(); inline AliasedFloat64Array& async_ids_stack(); inline v8::Local js_execution_async_resources(); + // Returns the native executionAsyncResource value at stack index `index`. + // Resources provided on the JS side are not stored on the native stack, + // in which case an empty `Local<>` is returned. + // The `js_execution_async_resources` array contains the value in that case. inline v8::Local native_execution_async_resource(size_t index); inline v8::Local provider_string(int idx);