diff options
author | Ruben Bridgewater <ruben@bridgewater.de> | 2018-05-19 00:25:07 +0200 |
---|---|---|
committer | Ruben Bridgewater <ruben@bridgewater.de> | 2018-07-16 16:45:48 +0200 |
commit | bd1f355fc5805c8307da85a16d1a385e5c48813a (patch) | |
tree | 91cf2d7334ab07d87579eaceff8f956d1a45642c | |
parent | b70367e8cf0f4eae8a4f0c92b1a8a1fa59fec288 (diff) | |
download | android-node-v8-bd1f355fc5805c8307da85a16d1a385e5c48813a.tar.gz android-node-v8-bd1f355fc5805c8307da85a16d1a385e5c48813a.tar.bz2 android-node-v8-bd1f355fc5805c8307da85a16d1a385e5c48813a.zip |
lib,src: replace all C++ promises with JS promises
C++ promises can not be properly optimized by V8. They also behave
a tiny bit different than "regular" promises.
PR-URL: https://github.com/nodejs/node/pull/20830
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
-rw-r--r-- | lib/child_process.js | 23 | ||||
-rw-r--r-- | lib/fs.js | 5 | ||||
-rw-r--r-- | lib/internal/http2/core.js | 9 | ||||
-rw-r--r-- | lib/internal/util.js | 20 | ||||
-rw-r--r-- | lib/timers.js | 17 | ||||
-rw-r--r-- | src/node_util.cc | 35 | ||||
-rw-r--r-- | test/parallel/test-promise-internal-creation.js | 41 |
7 files changed, 27 insertions, 123 deletions
diff --git a/lib/child_process.js b/lib/child_process.js index 5c315290c2..a790e85ee2 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -26,8 +26,6 @@ const { deprecate, convertToValidSignal, getSystemErrorName } = require('internal/util'); const { isUint8Array } = require('internal/util/types'); -const { createPromise, - promiseResolve, promiseReject } = process.binding('util'); const debug = util.debuglog('child_process'); const { Buffer } = require('buffer'); const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); @@ -152,18 +150,17 @@ exports.exec = function exec(command /* , options, callback */) { const customPromiseExecFunction = (orig) => { return (...args) => { - const promise = createPromise(); - - orig(...args, (err, stdout, stderr) => { - if (err !== null) { - err.stdout = stdout; - err.stderr = stderr; - promiseReject(promise, err); - } else { - promiseResolve(promise, { stdout, stderr }); - } + return new Promise((resolve, reject) => { + orig(...args, (err, stdout, stderr) => { + if (err !== null) { + err.stdout = stdout; + err.stderr = stderr; + reject(err); + } else { + resolve({ stdout, stderr }); + } + }); }); - return promise; }; }; @@ -214,10 +214,7 @@ function exists(path, callback) { Object.defineProperty(exists, internalUtil.promisify.custom, { value: (path) => { - const { createPromise, promiseResolve } = process.binding('util'); - const promise = createPromise(); - fs.exists(path, (exists) => promiseResolve(promise, exists)); - return promise; + return new Promise((resolve) => fs.exists(path, resolve)); } }); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index db251caf5f..be4545ecc3 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -113,7 +113,6 @@ const { isArrayBufferView } = require('internal/util/types'); const { FileHandle } = process.binding('fs'); const binding = process.binding('http2'); const { ShutdownWrap } = process.binding('stream_wrap'); -const { createPromise, promiseResolve } = process.binding('util'); const { UV_EOF } = process.binding('uv'); const { StreamPipe } = internalBinding('stream_pipe'); @@ -2747,11 +2746,9 @@ function connect(authority, options, listener) { // Support util.promisify Object.defineProperty(connect, promisify.custom, { value: (authority, options) => { - const promise = createPromise(); - const server = connect(authority, - options, - () => promiseResolve(promise, server)); - return promise; + return new Promise((resolve) => { + const server = connect(authority, options, () => resolve(server)); + }); } }); diff --git a/lib/internal/util.js b/lib/internal/util.js index bc513e7fed..49dad0c6c3 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -8,10 +8,7 @@ const { const { signals } = process.binding('constants').os; const { - createPromise, getHiddenValue, - promiseResolve, - promiseReject, setHiddenValue, arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex, decorated_private_symbol: kDecoratedPrivateSymbolIndex @@ -276,24 +273,21 @@ function promisify(original) { const argumentNames = original[kCustomPromisifyArgsSymbol]; function fn(...args) { - const promise = createPromise(); - try { + return new Promise((resolve, reject) => { original.call(this, ...args, (err, ...values) => { if (err) { - promiseReject(promise, err); - } else if (argumentNames !== undefined && values.length > 1) { + return reject(err); + } + if (argumentNames !== undefined && values.length > 1) { const obj = {}; for (var i = 0; i < argumentNames.length; i++) obj[argumentNames[i]] = values[i]; - promiseResolve(promise, obj); + resolve(obj); } else { - promiseResolve(promise, values[0]); + resolve(values[0]); } }); - } catch (err) { - promiseReject(promise, err); - } - return promise; + }); } Object.setPrototypeOf(fn, Object.getPrototypeOf(original)); diff --git a/lib/timers.js b/lib/timers.js index ad2d89d7bf..dc9b875ec3 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -41,7 +41,6 @@ const { validateTimerDuration } = require('internal/timers'); const internalUtil = require('internal/util'); -const { createPromise, promiseResolve } = process.binding('util'); const util = require('util'); const { ERR_INVALID_CALLBACK } = require('internal/errors').codes; const debug = util.debuglog('timer'); @@ -439,11 +438,9 @@ function setTimeout(callback, after, arg1, arg2, arg3) { } setTimeout[internalUtil.promisify.custom] = function(after, value) { - const promise = createPromise(); - const timeout = new Timeout(promise, after, [value], false); - active(timeout); - - return promise; + return new Promise((resolve) => { + active(new Timeout(resolve, after, [value], false)); + }); }; exports.setTimeout = setTimeout; @@ -452,7 +449,7 @@ exports.setTimeout = setTimeout; function ontimeout(timer) { const args = timer._timerArgs; if (typeof timer._onTimeout !== 'function') - return promiseResolve(timer._onTimeout, args[0]); + return Promise.resolve(timer._onTimeout, args[0]); if (!args) timer._onTimeout(); else @@ -659,7 +656,7 @@ function tryOnImmediate(immediate, oldTail, count, refCount) { function runCallback(timer) { const argv = timer._argv; if (typeof timer._onImmediate !== 'function') - return promiseResolve(timer._onImmediate, argv[0]); + return Promise.resolve(timer._onImmediate, argv[0]); if (!argv) return timer._onImmediate(); Reflect.apply(timer._onImmediate, timer, argv); @@ -738,9 +735,7 @@ function setImmediate(callback, arg1, arg2, arg3) { } setImmediate[internalUtil.promisify.custom] = function(value) { - const promise = createPromise(); - new Immediate(promise, [value]); - return promise; + return new Promise((resolve) => new Immediate(resolve, [value])); }; exports.setImmediate = setImmediate; diff --git a/src/node_util.cc b/src/node_util.cc index ef7dc8a818..9f31786b32 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -9,7 +9,6 @@ using v8::Context; using v8::FunctionCallbackInfo; using v8::Integer; using v8::Local; -using v8::Maybe; using v8::Object; using v8::Private; using v8::Promise; @@ -127,36 +126,6 @@ void WatchdogHasPendingSigint(const FunctionCallbackInfo<Value>& args) { args.GetReturnValue().Set(ret); } - -void CreatePromise(const FunctionCallbackInfo<Value>& args) { - Local<Context> context = args.GetIsolate()->GetCurrentContext(); - auto maybe_resolver = Promise::Resolver::New(context); - if (!maybe_resolver.IsEmpty()) - args.GetReturnValue().Set(maybe_resolver.ToLocalChecked()); -} - - -void PromiseResolve(const FunctionCallbackInfo<Value>& args) { - Local<Context> context = args.GetIsolate()->GetCurrentContext(); - Local<Value> promise = args[0]; - CHECK(promise->IsPromise()); - if (promise.As<Promise>()->State() != Promise::kPending) return; - Local<Promise::Resolver> resolver = promise.As<Promise::Resolver>(); // sic - Maybe<bool> ret = resolver->Resolve(context, args[1]); - args.GetReturnValue().Set(ret.FromMaybe(false)); -} - - -void PromiseReject(const FunctionCallbackInfo<Value>& args) { - Local<Context> context = args.GetIsolate()->GetCurrentContext(); - Local<Value> promise = args[0]; - CHECK(promise->IsPromise()); - if (promise.As<Promise>()->State() != Promise::kPending) return; - Local<Promise::Resolver> resolver = promise.As<Promise::Resolver>(); // sic - Maybe<bool> ret = resolver->Reject(context, args[1]); - args.GetReturnValue().Set(ret.FromMaybe(false)); -} - void SafeGetenv(const FunctionCallbackInfo<Value>& args) { CHECK(args[0]->IsString()); Utf8Value strenvtag(args.GetIsolate(), args[0]); @@ -211,10 +180,6 @@ void Initialize(Local<Object> target, env->SetMethodNoSideEffect(target, "watchdogHasPendingSigint", WatchdogHasPendingSigint); - env->SetMethodNoSideEffect(target, "createPromise", CreatePromise); - env->SetMethod(target, "promiseResolve", PromiseResolve); - env->SetMethod(target, "promiseReject", PromiseReject); - env->SetMethod(target, "safeGetenv", SafeGetenv); } diff --git a/test/parallel/test-promise-internal-creation.js b/test/parallel/test-promise-internal-creation.js deleted file mode 100644 index 7142c872d9..0000000000 --- a/test/parallel/test-promise-internal-creation.js +++ /dev/null @@ -1,41 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const { - createPromise, promiseResolve, promiseReject -} = process.binding('util'); -const { inspect } = require('util'); - -common.crashOnUnhandledRejection(); - -{ - const promise = createPromise(); - assert.strictEqual(inspect(promise), 'Promise { <pending> }'); - promiseResolve(promise, 42); - assert.strictEqual(inspect(promise), 'Promise { 42 }'); - promise.then(common.mustCall((value) => { - assert.strictEqual(value, 42); - })); -} - -{ - const promise = createPromise(); - const error = new Error('foobar'); - promiseReject(promise, error); - assert(inspect(promise).includes('<rejected> Error: foobar')); - promise.catch(common.mustCall((value) => { - assert.strictEqual(value, error); - })); -} - -{ - const promise = createPromise(); - const error = new Error('foobar'); - promiseReject(promise, error); - assert(inspect(promise).includes('<rejected> Error: foobar')); - promiseResolve(promise, 42); - assert(inspect(promise).includes('<rejected> Error: foobar')); - promise.catch(common.mustCall((value) => { - assert.strictEqual(value, error); - })); -} |