Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 9817e40

Browse filesBrowse files
BridgeARtargos
authored andcommitted
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 <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> Backport-PR-URL: #21879 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent b98bf82 commit 9817e40
Copy full SHA for 9817e40

File tree

Expand file treeCollapse file tree

7 files changed

+27
-123
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+27
-123
lines changed
Open diff view settings
Collapse file

‎lib/child_process.js‎

Copy file name to clipboardExpand all lines: lib/child_process.js
+10-13Lines 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

Collapse file

‎lib/fs.js‎

Copy file name to clipboardExpand all lines: lib/fs.js
+1-4Lines 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

Collapse file

‎lib/internal/http2/core.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/core.js
+3-6Lines 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

Collapse file

‎lib/internal/util.js‎

Copy file name to clipboardExpand all lines: lib/internal/util.js
+7-13Lines 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));
Collapse file

‎lib/timers.js‎

Copy file name to clipboardExpand all lines: lib/timers.js
+6-11Lines 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;
Collapse file

‎src/node_util.cc‎

Copy file name to clipboardExpand all lines: src/node_util.cc
-35Lines 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

Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-promise-internal-creation.js
-41Lines changed: 0 additions & 41 deletions
This file was deleted.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.