Skip to content

Commit

Permalink
async_wrap,src: wrap promises directly
Browse files Browse the repository at this point in the history
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: #13242
Ref: #13224
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
Matt Loring authored and jasnell committed May 28, 2017
1 parent 0be029e commit 4a8ea63
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 31 deletions.
1 change: 1 addition & 0 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
31 changes: 3 additions & 28 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -294,32 +294,12 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise,
Local<Context> 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<Object> obj =
env->async_hooks_promise_object()->NewInstance(context).ToLocalChecked();
PromiseWrap* wrap = new PromiseWrap(env, obj);
v8::PropertyAttribute hidden =
static_cast<v8::PropertyAttribute>(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<v8::Value> external_wrap =
promise->Get(context, env->promise_wrap()).ToLocalChecked();
PromiseWrap* wrap =
static_cast<PromiseWrap*>(external_wrap.As<v8::External>()->Value());
PromiseWrap* wrap = Unwrap<PromiseWrap>(promise);
CHECK_NE(wrap, nullptr);
if (type == PromiseHookType::kBefore) {
PreCallbackExecution(wrap, false);
Expand Down Expand Up @@ -415,11 +395,6 @@ void AsyncWrap::Initialize(Local<Object> target,
env->SetMethod(target, "clearIdStack", ClearIdStack);
env->SetMethod(target, "addIdToDestroyList", QueueDestroyId);

Local<v8::ObjectTemplate> 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::PropertyAttribute>(v8::ReadOnly | v8::DontDelete);

Expand Down
3 changes: 0 additions & 3 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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") \
Expand Down Expand Up @@ -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) \
Expand Down

0 comments on commit 4a8ea63

Please sign in to comment.