diff options
author | Andreas Madsen <amwebdk@gmail.com> | 2018-10-12 09:56:26 +0300 |
---|---|---|
committer | George Adams <george.adams@uk.ibm.com> | 2018-10-15 21:40:51 +0300 |
commit | 1a2cf6696fa59c9723c423c4ff4c1167ab82155c (patch) | |
tree | f693eb6f3a152a7191e344ba79f8857ebcd834fb /src/async_wrap.cc | |
parent | 561e30d9ef581e86b36318fe22ebd1e82ab88754 (diff) |
async_hooks: remove promise object from resource
While it doesn't make any difference now. In the future PromiseHooks
could be refactored to provide an asyncId instead of the promise object.
That would make escape analysis on promises possible.
Escape analysis on promises could lead to a more efficient destroy hook,
if provide by PromiseHooks as well. But at the very least would allow
the destroy hook to be emitted earlier. The destroy hook not being
emitted on promises frequent enough is a known and reported issue.
See https://github.com/nodejs/node/issues/14446 and
https://github.com/Jeff-Lewis/cls-hooked/issues/11.
While all this is speculation for now, it all depends on the promise
object not being a part of the PromiseWrap resource object.
Ref: https://github.com/nodejs/node/issues/14446
Ref: https://github.com/nodejs/diagnostics/issues/188
PR-URL: https://github.com/nodejs/node/pull/23443
Refs: https://github.com/nodejs/node/issues/14446
Refs: https://github.com/nodejs/diagnostics/issues/188
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
Diffstat (limited to 'src/async_wrap.cc')
-rw-r--r-- | src/async_wrap.cc | 16 |
1 files changed, 2 insertions, 14 deletions
diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 29bd64f2c5e..17636ceb31c 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -185,16 +185,13 @@ class PromiseWrap : public AsyncWrap { SET_MEMORY_INFO_NAME(PromiseWrap) SET_SELF_SIZE(PromiseWrap) - static constexpr int kPromiseField = 1; - static constexpr int kIsChainedPromiseField = 2; - static constexpr int kInternalFieldCount = 3; + static constexpr int kIsChainedPromiseField = 1; + static constexpr int kInternalFieldCount = 2; static PromiseWrap* New(Environment* env, Local<Promise> promise, PromiseWrap* parent_wrap, bool silent); - static void GetPromise(Local<String> property, - const PropertyCallbackInfo<Value>& info); static void getIsChainedPromise(Local<String> property, const PropertyCallbackInfo<Value>& info); }; @@ -205,7 +202,6 @@ PromiseWrap* PromiseWrap::New(Environment* env, bool silent) { Local<Object> object = env->promise_wrap_template() ->NewInstance(env->context()).ToLocalChecked(); - object->SetInternalField(PromiseWrap::kPromiseField, promise); object->SetInternalField(PromiseWrap::kIsChainedPromiseField, parent_wrap != nullptr ? v8::True(env->isolate()) : @@ -215,11 +211,6 @@ PromiseWrap* PromiseWrap::New(Environment* env, return new PromiseWrap(env, object, silent); } -void PromiseWrap::GetPromise(Local<String> property, - const PropertyCallbackInfo<Value>& info) { - info.GetReturnValue().Set(info.Holder()->GetInternalField(kPromiseField)); -} - void PromiseWrap::getIsChainedPromise(Local<String> property, const PropertyCallbackInfo<Value>& info) { info.GetReturnValue().Set( @@ -316,9 +307,6 @@ static void SetupHooks(const FunctionCallbackInfo<Value>& args) { promise_wrap_template->SetInternalFieldCount( PromiseWrap::kInternalFieldCount); promise_wrap_template->SetAccessor( - FIXED_ONE_BYTE_STRING(env->isolate(), "promise"), - PromiseWrap::GetPromise); - promise_wrap_template->SetAccessor( FIXED_ONE_BYTE_STRING(env->isolate(), "isChainedPromise"), PromiseWrap::getIsChainedPromise); env->set_promise_wrap_template(promise_wrap_template); |