Skip to content

Commit bd1f355

Browse files
BridgeARBridgeAR
committed
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: #20830 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Benedikt Meurer <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
1 parent b70367e commit bd1f355

7 files changed

Lines changed: 27 additions & 123 deletions

File tree

‎lib/child_process.js‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ const {
2626
deprecate, convertToValidSignal, getSystemErrorName
2727
} = require('internal/util');
2828
const { isUint8Array } = require('internal/util/types');
29-
const { createPromise,
30-
promiseResolve, promiseReject } = process.binding('util');
3129
const debug = util.debuglog('child_process');
3230
const { Buffer } = require('buffer');
3331
const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
@@ -152,18 +150,17 @@ exports.exec = function exec(command /* , options, callback */) {
152150

153151
const customPromiseExecFunction = (orig) => {
154152
return (...args) => {
155-
const promise = createPromise();
156-
157-
orig(...args, (err, stdout, stderr) => {
158-
if (err !== null) {
159-
err.stdout = stdout;
160-
err.stderr = stderr;
161-
promiseReject(promise, err);
162-
} else {
163-
promiseResolve(promise, { stdout, stderr });
164-
}
153+
return new Promise((resolve, reject) => {
154+
orig(...args, (err, stdout, stderr) => {
155+
if (err !== null) {
156+
err.stdout = stdout;
157+
err.stderr = stderr;
158+
reject(err);
159+
} else {
160+
resolve({ stdout, stderr });
161+
}
162+
});
165163
});
166-
return promise;
167164
};
168165
};
169166

‎lib/fs.js‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,10 +214,7 @@ function exists(path, callback) {
214214

215215
Object.defineProperty(exists, internalUtil.promisify.custom, {
216216
value: (path) => {
217-
const { createPromise, promiseResolve } = process.binding('util');
218-
const promise = createPromise();
219-
fs.exists(path, (exists) => promiseResolve(promise, exists));
220-
return promise;
217+
return new Promise((resolve) => fs.exists(path, resolve));
221218
}
222219
});
223220

‎lib/internal/http2/core.js‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ const { isArrayBufferView } = require('internal/util/types');
113113
const { FileHandle } = process.binding('fs');
114114
const binding = process.binding('http2');
115115
const { ShutdownWrap } = process.binding('stream_wrap');
116-
const { createPromise, promiseResolve } = process.binding('util');
117116
const { UV_EOF } = process.binding('uv');
118117

119118
const { StreamPipe } = internalBinding('stream_pipe');
@@ -2747,11 +2746,9 @@ function connect(authority, options, listener) {
27472746
// Support util.promisify
27482747
Object.defineProperty(connect, promisify.custom, {
27492748
value: (authority, options) => {
2750-
const promise = createPromise();
2751-
const server = connect(authority,
2752-
options,
2753-
() => promiseResolve(promise, server));
2754-
return promise;
2749+
return new Promise((resolve) => {
2750+
const server = connect(authority, options, () => resolve(server));
2751+
});
27552752
}
27562753
});
27572754

‎lib/internal/util.js‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,7 @@ const {
88
const { signals } = process.binding('constants').os;
99

1010
const {
11-
createPromise,
1211
getHiddenValue,
13-
promiseResolve,
14-
promiseReject,
1512
setHiddenValue,
1613
arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex,
1714
decorated_private_symbol: kDecoratedPrivateSymbolIndex
@@ -276,24 +273,21 @@ function promisify(original) {
276273
const argumentNames = original[kCustomPromisifyArgsSymbol];
277274

278275
function fn(...args) {
279-
const promise = createPromise();
280-
try {
276+
return new Promise((resolve, reject) => {
281277
original.call(this, ...args, (err, ...values) => {
282278
if (err) {
283-
promiseReject(promise, err);
284-
} else if (argumentNames !== undefined && values.length > 1) {
279+
return reject(err);
280+
}
281+
if (argumentNames !== undefined && values.length > 1) {
285282
const obj = {};
286283
for (var i = 0; i < argumentNames.length; i++)
287284
obj[argumentNames[i]] = values[i];
288-
promiseResolve(promise, obj);
285+
resolve(obj);
289286
} else {
290-
promiseResolve(promise, values[0]);
287+
resolve(values[0]);
291288
}
292289
});
293-
} catch (err) {
294-
promiseReject(promise, err);
295-
}
296-
return promise;
290+
});
297291
}
298292

299293
Object.setPrototypeOf(fn, Object.getPrototypeOf(original));

‎lib/timers.js‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ const {
4141
validateTimerDuration
4242
} = require('internal/timers');
4343
const internalUtil = require('internal/util');
44-
const { createPromise, promiseResolve } = process.binding('util');
4544
const util = require('util');
4645
const { ERR_INVALID_CALLBACK } = require('internal/errors').codes;
4746
const debug = util.debuglog('timer');
@@ -439,11 +438,9 @@ function setTimeout(callback, after, arg1, arg2, arg3) {
439438
}
440439

441440
setTimeout[internalUtil.promisify.custom] = function(after, value) {
442-
const promise = createPromise();
443-
const timeout = new Timeout(promise, after, [value], false);
444-
active(timeout);
445-
446-
return promise;
441+
return new Promise((resolve) => {
442+
active(new Timeout(resolve, after, [value], false));
443+
});
447444
};
448445

449446
exports.setTimeout = setTimeout;
@@ -452,7 +449,7 @@ exports.setTimeout = setTimeout;
452449
function ontimeout(timer) {
453450
const args = timer._timerArgs;
454451
if (typeof timer._onTimeout !== 'function')
455-
return promiseResolve(timer._onTimeout, args[0]);
452+
return Promise.resolve(timer._onTimeout, args[0]);
456453
if (!args)
457454
timer._onTimeout();
458455
else
@@ -659,7 +656,7 @@ function tryOnImmediate(immediate, oldTail, count, refCount) {
659656
function runCallback(timer) {
660657
const argv = timer._argv;
661658
if (typeof timer._onImmediate !== 'function')
662-
return promiseResolve(timer._onImmediate, argv[0]);
659+
return Promise.resolve(timer._onImmediate, argv[0]);
663660
if (!argv)
664661
return timer._onImmediate();
665662
Reflect.apply(timer._onImmediate, timer, argv);
@@ -738,9 +735,7 @@ function setImmediate(callback, arg1, arg2, arg3) {
738735
}
739736

740737
setImmediate[internalUtil.promisify.custom] = function(value) {
741-
const promise = createPromise();
742-
new Immediate(promise, [value]);
743-
return promise;
738+
return new Promise((resolve) => new Immediate(resolve, [value]));
744739
};
745740

746741
exports.setImmediate = setImmediate;

‎src/node_util.cc‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ using v8::Context;
99
using v8::FunctionCallbackInfo;
1010
using v8::Integer;
1111
using v8::Local;
12-
using v8::Maybe;
1312
using v8::Object;
1413
using v8::Private;
1514
using v8::Promise;
@@ -127,36 +126,6 @@ void WatchdogHasPendingSigint(const FunctionCallbackInfo<Value>& args) {
127126
args.GetReturnValue().Set(ret);
128127
}
129128

130-
131-
void CreatePromise(const FunctionCallbackInfo<Value>& args) {
132-
Local<Context> context = args.GetIsolate()->GetCurrentContext();
133-
auto maybe_resolver = Promise::Resolver::New(context);
134-
if (!maybe_resolver.IsEmpty())
135-
args.GetReturnValue().Set(maybe_resolver.ToLocalChecked());
136-
}
137-
138-
139-
void PromiseResolve(const FunctionCallbackInfo<Value>& args) {
140-
Local<Context> context = args.GetIsolate()->GetCurrentContext();
141-
Local<Value> promise = args[0];
142-
CHECK(promise->IsPromise());
143-
if (promise.As<Promise>()->State() != Promise::kPending) return;
144-
Local<Promise::Resolver> resolver = promise.As<Promise::Resolver>(); // sic
145-
Maybe<bool> ret = resolver->Resolve(context, args[1]);
146-
args.GetReturnValue().Set(ret.FromMaybe(false));
147-
}
148-
149-
150-
void PromiseReject(const FunctionCallbackInfo<Value>& args) {
151-
Local<Context> context = args.GetIsolate()->GetCurrentContext();
152-
Local<Value> promise = args[0];
153-
CHECK(promise->IsPromise());
154-
if (promise.As<Promise>()->State() != Promise::kPending) return;
155-
Local<Promise::Resolver> resolver = promise.As<Promise::Resolver>(); // sic
156-
Maybe<bool> ret = resolver->Reject(context, args[1]);
157-
args.GetReturnValue().Set(ret.FromMaybe(false));
158-
}
159-
160129
void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
161130
CHECK(args[0]->IsString());
162131
Utf8Value strenvtag(args.GetIsolate(), args[0]);
@@ -211,10 +180,6 @@ void Initialize(Local<Object> target,
211180
env->SetMethodNoSideEffect(target, "watchdogHasPendingSigint",
212181
WatchdogHasPendingSigint);
213182

214-
env->SetMethodNoSideEffect(target, "createPromise", CreatePromise);
215-
env->SetMethod(target, "promiseResolve", PromiseResolve);
216-
env->SetMethod(target, "promiseReject", PromiseReject);
217-
218183
env->SetMethod(target, "safeGetenv", SafeGetenv);
219184
}
220185

‎test/parallel/test-promise-internal-creation.js‎

Lines changed: 0 additions & 41 deletions
This file was deleted.

0 commit comments

Comments
 (0)