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>
This commit is contained in:
Ruben Bridgewater 2018-05-19 00:25:07 +02:00
parent b70367e8cf
commit bd1f355fc5
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
7 changed files with 27 additions and 123 deletions

View File

@ -26,8 +26,6 @@ const {
deprecate, convertToValidSignal, getSystemErrorName deprecate, convertToValidSignal, getSystemErrorName
} = require('internal/util'); } = require('internal/util');
const { isUint8Array } = require('internal/util/types'); const { isUint8Array } = require('internal/util/types');
const { createPromise,
promiseResolve, promiseReject } = process.binding('util');
const debug = util.debuglog('child_process'); const debug = util.debuglog('child_process');
const { Buffer } = require('buffer'); const { Buffer } = require('buffer');
const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
@ -152,18 +150,17 @@ exports.exec = function exec(command /* , options, callback */) {
const customPromiseExecFunction = (orig) => { const customPromiseExecFunction = (orig) => {
return (...args) => { return (...args) => {
const promise = createPromise(); return new Promise((resolve, reject) => {
orig(...args, (err, stdout, stderr) => {
orig(...args, (err, stdout, stderr) => { if (err !== null) {
if (err !== null) { err.stdout = stdout;
err.stdout = stdout; err.stderr = stderr;
err.stderr = stderr; reject(err);
promiseReject(promise, err); } else {
} else { resolve({ stdout, stderr });
promiseResolve(promise, { stdout, stderr }); }
} });
}); });
return promise;
}; };
}; };

View File

@ -214,10 +214,7 @@ function exists(path, callback) {
Object.defineProperty(exists, internalUtil.promisify.custom, { Object.defineProperty(exists, internalUtil.promisify.custom, {
value: (path) => { value: (path) => {
const { createPromise, promiseResolve } = process.binding('util'); return new Promise((resolve) => fs.exists(path, resolve));
const promise = createPromise();
fs.exists(path, (exists) => promiseResolve(promise, exists));
return promise;
} }
}); });

View File

@ -113,7 +113,6 @@ const { isArrayBufferView } = require('internal/util/types');
const { FileHandle } = process.binding('fs'); const { FileHandle } = process.binding('fs');
const binding = process.binding('http2'); const binding = process.binding('http2');
const { ShutdownWrap } = process.binding('stream_wrap'); const { ShutdownWrap } = process.binding('stream_wrap');
const { createPromise, promiseResolve } = process.binding('util');
const { UV_EOF } = process.binding('uv'); const { UV_EOF } = process.binding('uv');
const { StreamPipe } = internalBinding('stream_pipe'); const { StreamPipe } = internalBinding('stream_pipe');
@ -2747,11 +2746,9 @@ function connect(authority, options, listener) {
// Support util.promisify // Support util.promisify
Object.defineProperty(connect, promisify.custom, { Object.defineProperty(connect, promisify.custom, {
value: (authority, options) => { value: (authority, options) => {
const promise = createPromise(); return new Promise((resolve) => {
const server = connect(authority, const server = connect(authority, options, () => resolve(server));
options, });
() => promiseResolve(promise, server));
return promise;
} }
}); });

View File

@ -8,10 +8,7 @@ const {
const { signals } = process.binding('constants').os; const { signals } = process.binding('constants').os;
const { const {
createPromise,
getHiddenValue, getHiddenValue,
promiseResolve,
promiseReject,
setHiddenValue, setHiddenValue,
arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex, arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex,
decorated_private_symbol: kDecoratedPrivateSymbolIndex decorated_private_symbol: kDecoratedPrivateSymbolIndex
@ -276,24 +273,21 @@ function promisify(original) {
const argumentNames = original[kCustomPromisifyArgsSymbol]; const argumentNames = original[kCustomPromisifyArgsSymbol];
function fn(...args) { function fn(...args) {
const promise = createPromise(); return new Promise((resolve, reject) => {
try {
original.call(this, ...args, (err, ...values) => { original.call(this, ...args, (err, ...values) => {
if (err) { if (err) {
promiseReject(promise, err); return reject(err);
} else if (argumentNames !== undefined && values.length > 1) { }
if (argumentNames !== undefined && values.length > 1) {
const obj = {}; const obj = {};
for (var i = 0; i < argumentNames.length; i++) for (var i = 0; i < argumentNames.length; i++)
obj[argumentNames[i]] = values[i]; obj[argumentNames[i]] = values[i];
promiseResolve(promise, obj); resolve(obj);
} else { } else {
promiseResolve(promise, values[0]); resolve(values[0]);
} }
}); });
} catch (err) { });
promiseReject(promise, err);
}
return promise;
} }
Object.setPrototypeOf(fn, Object.getPrototypeOf(original)); Object.setPrototypeOf(fn, Object.getPrototypeOf(original));

View File

@ -41,7 +41,6 @@ const {
validateTimerDuration validateTimerDuration
} = require('internal/timers'); } = require('internal/timers');
const internalUtil = require('internal/util'); const internalUtil = require('internal/util');
const { createPromise, promiseResolve } = process.binding('util');
const util = require('util'); const util = require('util');
const { ERR_INVALID_CALLBACK } = require('internal/errors').codes; const { ERR_INVALID_CALLBACK } = require('internal/errors').codes;
const debug = util.debuglog('timer'); const debug = util.debuglog('timer');
@ -439,11 +438,9 @@ function setTimeout(callback, after, arg1, arg2, arg3) {
} }
setTimeout[internalUtil.promisify.custom] = function(after, value) { setTimeout[internalUtil.promisify.custom] = function(after, value) {
const promise = createPromise(); return new Promise((resolve) => {
const timeout = new Timeout(promise, after, [value], false); active(new Timeout(resolve, after, [value], false));
active(timeout); });
return promise;
}; };
exports.setTimeout = setTimeout; exports.setTimeout = setTimeout;
@ -452,7 +449,7 @@ exports.setTimeout = setTimeout;
function ontimeout(timer) { function ontimeout(timer) {
const args = timer._timerArgs; const args = timer._timerArgs;
if (typeof timer._onTimeout !== 'function') if (typeof timer._onTimeout !== 'function')
return promiseResolve(timer._onTimeout, args[0]); return Promise.resolve(timer._onTimeout, args[0]);
if (!args) if (!args)
timer._onTimeout(); timer._onTimeout();
else else
@ -659,7 +656,7 @@ function tryOnImmediate(immediate, oldTail, count, refCount) {
function runCallback(timer) { function runCallback(timer) {
const argv = timer._argv; const argv = timer._argv;
if (typeof timer._onImmediate !== 'function') if (typeof timer._onImmediate !== 'function')
return promiseResolve(timer._onImmediate, argv[0]); return Promise.resolve(timer._onImmediate, argv[0]);
if (!argv) if (!argv)
return timer._onImmediate(); return timer._onImmediate();
Reflect.apply(timer._onImmediate, timer, argv); Reflect.apply(timer._onImmediate, timer, argv);
@ -738,9 +735,7 @@ function setImmediate(callback, arg1, arg2, arg3) {
} }
setImmediate[internalUtil.promisify.custom] = function(value) { setImmediate[internalUtil.promisify.custom] = function(value) {
const promise = createPromise(); return new Promise((resolve) => new Immediate(resolve, [value]));
new Immediate(promise, [value]);
return promise;
}; };
exports.setImmediate = setImmediate; exports.setImmediate = setImmediate;

View File

@ -9,7 +9,6 @@ using v8::Context;
using v8::FunctionCallbackInfo; using v8::FunctionCallbackInfo;
using v8::Integer; using v8::Integer;
using v8::Local; using v8::Local;
using v8::Maybe;
using v8::Object; using v8::Object;
using v8::Private; using v8::Private;
using v8::Promise; using v8::Promise;
@ -127,36 +126,6 @@ void WatchdogHasPendingSigint(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(ret); 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) { void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsString()); CHECK(args[0]->IsString());
Utf8Value strenvtag(args.GetIsolate(), args[0]); Utf8Value strenvtag(args.GetIsolate(), args[0]);
@ -211,10 +180,6 @@ void Initialize(Local<Object> target,
env->SetMethodNoSideEffect(target, "watchdogHasPendingSigint", env->SetMethodNoSideEffect(target, "watchdogHasPendingSigint",
WatchdogHasPendingSigint); WatchdogHasPendingSigint);
env->SetMethodNoSideEffect(target, "createPromise", CreatePromise);
env->SetMethod(target, "promiseResolve", PromiseResolve);
env->SetMethod(target, "promiseReject", PromiseReject);
env->SetMethod(target, "safeGetenv", SafeGetenv); env->SetMethod(target, "safeGetenv", SafeGetenv);
} }

View File

@ -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);
}));
}