Skip to content

Commit fb1d667

Browse files
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.
1 parent 57e3015 commit fb1d667

File tree

7 files changed

+27
-123
lines changed

7 files changed

+27
-123
lines changed

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

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

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;
@@ -140,36 +139,6 @@ void WatchdogHasPendingSigint(const FunctionCallbackInfo<Value>& args) {
140139
args.GetReturnValue().Set(ret);
141140
}
142141

143-
144-
void CreatePromise(const FunctionCallbackInfo<Value>& args) {
145-
Local<Context> context = args.GetIsolate()->GetCurrentContext();
146-
auto maybe_resolver = Promise::Resolver::New(context);
147-
if (!maybe_resolver.IsEmpty())
148-
args.GetReturnValue().Set(maybe_resolver.ToLocalChecked());
149-
}
150-
151-
152-
void PromiseResolve(const FunctionCallbackInfo<Value>& args) {
153-
Local<Context> context = args.GetIsolate()->GetCurrentContext();
154-
Local<Value> promise = args[0];
155-
CHECK(promise->IsPromise());
156-
if (promise.As<Promise>()->State() != Promise::kPending) return;
157-
Local<Promise::Resolver> resolver = promise.As<Promise::Resolver>(); // sic
158-
Maybe<bool> ret = resolver->Resolve(context, args[1]);
159-
args.GetReturnValue().Set(ret.FromMaybe(false));
160-
}
161-
162-
163-
void PromiseReject(const FunctionCallbackInfo<Value>& args) {
164-
Local<Context> context = args.GetIsolate()->GetCurrentContext();
165-
Local<Value> promise = args[0];
166-
CHECK(promise->IsPromise());
167-
if (promise.As<Promise>()->State() != Promise::kPending) return;
168-
Local<Promise::Resolver> resolver = promise.As<Promise::Resolver>(); // sic
169-
Maybe<bool> ret = resolver->Reject(context, args[1]);
170-
args.GetReturnValue().Set(ret.FromMaybe(false));
171-
}
172-
173142
void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
174143
CHECK(args[0]->IsString());
175144
Utf8Value strenvtag(args.GetIsolate(), args[0]);
@@ -224,10 +193,6 @@ void Initialize(Local<Object> target,
224193
env->SetMethodNoSideEffect(target, "watchdogHasPendingSigint",
225194
WatchdogHasPendingSigint);
226195

227-
env->SetMethodNoSideEffect(target, "createPromise", CreatePromise);
228-
env->SetMethod(target, "promiseResolve", PromiseResolve);
229-
env->SetMethod(target, "promiseReject", PromiseReject);
230-
231196
env->SetMethod(target, "safeGetenv", SafeGetenv);
232197
}
233198

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

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

0 commit comments

Comments
 (0)