From 849f22309a3e612dd4b9483db32713f7e759215b Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Thu, 25 May 2017 11:51:56 -0700 Subject: [PATCH] async_wrap,src: wrap promises directly Promises do not have any internal fields by default. V8 recently added the capability of configuring the number of internal fields on promises. This change adds an internal field to promises allowing promises to be wrapped directly by the PromiseWrap object. In addition to cleaner code this avoids an extra object allocation per promise and speeds up promise creation with async_hooks enabled by ~2x. PR-URL: https://github.com/nodejs/node/pull/13242 Ref: https://github.com/nodejs/node/pull/13224 Reviewed-By: Andreas Madsen Reviewed-By: Anna Henningsen --- configure | 1 + src/async-wrap.cc | 31 +++---------------------------- src/env.h | 3 --- 3 files changed, 4 insertions(+), 31 deletions(-) diff --git a/configure b/configure index 7ce5e964e5..01aacb8ed0 100755 --- a/configure +++ b/configure @@ -954,6 +954,7 @@ def configure_v8(o): o['variables']['v8_no_strict_aliasing'] = 1 # Work around compiler bugs. o['variables']['v8_optimized_debug'] = 0 # Compile with -O0 in debug builds. o['variables']['v8_random_seed'] = 0 # Use a random seed for hash tables. + o['variables']['v8_promise_internal_field_count'] = 1 # Add internal field to promises for async hooks. o['variables']['v8_use_snapshot'] = 'false' if options.without_snapshot else 'true' o['variables']['node_use_v8_platform'] = b(not options.without_v8_platform) o['variables']['node_use_bundled_v8'] = b(not options.without_bundled_v8) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 2efe0e23ec..696d275a6e 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -294,32 +294,12 @@ static void PromiseHook(PromiseHookType type, Local promise, Local context = promise->CreationContext(); Environment* env = Environment::GetCurrent(context); if (type == PromiseHookType::kInit) { - // Unfortunately, promises don't have internal fields. Need a surrogate that - // async wrap can wrap. - Local obj = - env->async_hooks_promise_object()->NewInstance(context).ToLocalChecked(); - PromiseWrap* wrap = new PromiseWrap(env, obj); - v8::PropertyAttribute hidden = - static_cast(v8::ReadOnly - | v8::DontDelete - | v8::DontEnum); - promise->DefineOwnProperty(context, - env->promise_wrap(), - v8::External::New(env->isolate(), wrap), - hidden).FromJust(); - // The async tag will be destroyed at the same time as the promise as the - // only reference to it is held by the promise. This allows the promise - // wrap instance to be notified when the promise is destroyed. - promise->DefineOwnProperty(context, - env->promise_async_tag(), - obj, hidden).FromJust(); + PromiseWrap* wrap = new PromiseWrap(env, promise); + wrap->MakeWeak(wrap); } else if (type == PromiseHookType::kResolve) { // TODO(matthewloring): need to expose this through the async hooks api. } - Local external_wrap = - promise->Get(context, env->promise_wrap()).ToLocalChecked(); - PromiseWrap* wrap = - static_cast(external_wrap.As()->Value()); + PromiseWrap* wrap = Unwrap(promise); CHECK_NE(wrap, nullptr); if (type == PromiseHookType::kBefore) { PreCallbackExecution(wrap, false); @@ -415,11 +395,6 @@ void AsyncWrap::Initialize(Local target, env->SetMethod(target, "clearIdStack", ClearIdStack); env->SetMethod(target, "addIdToDestroyList", QueueDestroyId); - Local promise_object_template = - v8::ObjectTemplate::New(env->isolate()); - promise_object_template->SetInternalFieldCount(1); - env->set_async_hooks_promise_object(promise_object_template); - v8::PropertyAttribute ReadOnlyDontDelete = static_cast(v8::ReadOnly | v8::DontDelete); diff --git a/src/env.h b/src/env.h index 11c957d2be..c8c8232cc0 100644 --- a/src/env.h +++ b/src/env.h @@ -195,8 +195,6 @@ namespace node { V(preference_string, "preference") \ V(priority_string, "priority") \ V(produce_cached_data_string, "produceCachedData") \ - V(promise_wrap, "_promise_async_wrap") \ - V(promise_async_tag, "_promise_async_wrap_tag") \ V(raw_string, "raw") \ V(read_host_object_string, "_readHostObject") \ V(readable_string, "readable") \ @@ -258,7 +256,6 @@ namespace node { V(async_hooks_init_function, v8::Function) \ V(async_hooks_before_function, v8::Function) \ V(async_hooks_after_function, v8::Function) \ - V(async_hooks_promise_object, v8::ObjectTemplate) \ V(binding_cache_object, v8::Object) \ V(buffer_constructor_function, v8::Function) \ V(buffer_prototype_object, v8::Object) \