Skip to content

Commit 9817e40

Browse files
BridgeARtargos
BridgeAR
authored and
targos
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]> Backport-PR-URL: #21879 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
1 parent b98bf82 commit 9817e40

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
@@ -207,10 +207,7 @@ function exists(path, callback) {
207207

208208
Object.defineProperty(exists, internalUtil.promisify.custom, {
209209
value: (path) => {
210-
const { createPromise, promiseResolve } = process.binding('util');
211-
const promise = createPromise();
212-
fs.exists(path, (exists) => promiseResolve(promise, exists));
213-
return promise;
210+
return new Promise((resolve) => fs.exists(path, resolve));
214211
}
215212
});
216213

‎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
@@ -34,7 +34,6 @@ const {
3434
validateTimerDuration
3535
} = require('internal/timers');
3636
const internalUtil = require('internal/util');
37-
const { createPromise, promiseResolve } = process.binding('util');
3837
const assert = require('assert');
3938
const util = require('util');
4039
const { ERR_INVALID_CALLBACK } = require('internal/errors').codes;
@@ -407,11 +406,9 @@ function setTimeout(callback, after, arg1, arg2, arg3) {
407406
}
408407

409408
setTimeout[internalUtil.promisify.custom] = function(after, value) {
410-
const promise = createPromise();
411-
const timeout = new Timeout(promise, after, [value], false, false);
412-
active(timeout);
413-
414-
return promise;
409+
return new Promise((resolve) => {
410+
active(new Timeout(resolve, after, [value], false, false));
411+
});
415412
};
416413

417414
exports.setTimeout = setTimeout;
@@ -420,7 +417,7 @@ exports.setTimeout = setTimeout;
420417
function ontimeout(timer, start) {
421418
const args = timer._timerArgs;
422419
if (typeof timer._onTimeout !== 'function')
423-
return promiseResolve(timer._onTimeout, args[0]);
420+
return Promise.resolve(timer._onTimeout, args[0]);
424421
if (start === undefined && timer._repeat)
425422
start = TimerWrap.now();
426423
if (!args)
@@ -691,7 +688,7 @@ function tryOnImmediate(immediate, oldTail, count, refCount) {
691688
function runCallback(timer) {
692689
const argv = timer._argv;
693690
if (typeof timer._onImmediate !== 'function')
694-
return promiseResolve(timer._onImmediate, argv[0]);
691+
return Promise.resolve(timer._onImmediate, argv[0]);
695692
if (!argv)
696693
return timer._onImmediate();
697694
Reflect.apply(timer._onImmediate, timer, argv);
@@ -766,9 +763,7 @@ function setImmediate(callback, arg1, arg2, arg3) {
766763
}
767764

768765
setImmediate[internalUtil.promisify.custom] = function(value) {
769-
const promise = createPromise();
770-
new Immediate(promise, [value]);
771-
return promise;
766+
return new Promise((resolve) => new Immediate(resolve, [value]));
772767
};
773768

774769
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)