Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(100)

Unified Diff: src/js/promise.js

Issue 2512103002: [async-await] Don't create resolving callbacks for throwaway promises (Closed)
Patch Set: Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/js/async-await.js ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/js/promise.js
diff --git a/src/js/promise.js b/src/js/promise.js
index d7e25124708737f8c5031e8d86d0d20b31176be1..ff4358a81086939309a872e8dbbeccc0ec66ad27 100644
--- a/src/js/promise.js
+++ b/src/js/promise.js
@@ -293,10 +293,22 @@ function DoRejectPromise(promise, reason) {
// ES#sec-newpromisecapability
// NewPromiseCapability ( C )
-function NewPromiseCapability(C, debugEvent) {
+function NewPromiseCapability(C, debugEvent, doNotCreateCallbacks) {
Dan Ehrenberg 2016/11/18 08:05:38 Style suggestion: Rather than making this extra bo
gsathya 2016/11/18 08:34:28 Do you mean a function like -- NewPromiseCapabilit
Dan Ehrenberg 2016/11/18 08:55:37 I was imagining that all of that would be part of
if (C === GlobalPromise) {
// Optimized case, avoid extra closure.
var promise = PromiseCreate();
+
+ // The resultCapability.promise is only ever fulfilled internally,
+ // so we don't need the closures to protect against accidentally
+ // calling them multiple times.
caitp 2016/11/18 05:47:15 I like that this is all documented so nicely. Sinc
gsathya 2016/11/18 06:02:28 This is used by PromiseThen as well which is why I
Dan Ehrenberg 2016/11/18 08:05:38 Could you mention this in the commit message? Does
gsathya 2016/11/18 08:34:27 This isn't a new change. This piece of code was sp
+ if (doNotCreateCallbacks) {
+ return {
+ promise: promise,
+ resolve: UNDEFINED,
+ reject: UNDEFINED
+ };
+ }
+
// TODO(gsathya): Remove container for callbacks when this is
// moved to CPP/TF.
var callbacks = %create_resolving_functions(promise, debugEvent);
@@ -380,23 +392,12 @@ function PromiseThen(onResolve, onReject) {
}
var constructor = SpeciesConstructor(this, GlobalPromise);
- var resultCapability;
-
- // The resultCapability.promise is only ever fulfilled internally,
- // so we don't need the closures to protect against accidentally
- // calling them multiple times.
- if (constructor === GlobalPromise) {
- // TODO(gsathya): Combine this into NewPromiseCapability.
- resultCapability = {
- promise: PromiseCreate(),
- resolve: UNDEFINED,
- reject: UNDEFINED
- };
- } else {
- // Pass false for debugEvent so .then chaining does not trigger
- // redundant ExceptionEvents.
- resultCapability = NewPromiseCapability(constructor, false);
- }
+
+ // Pass false for debugEvent so .then chaining does not trigger
+ // redundant ExceptionEvents.
+ var resultCapability = NewPromiseCapability(
+ constructor, false, constructor === GlobalPromise);
Dan Ehrenberg 2016/11/18 08:05:38 Not sure why you do this comparison. Seems like av
gsathya 2016/11/18 08:34:27 Yes, but this comparison is required to not create
Dan Ehrenberg 2016/11/18 08:55:37 I'd suggest factoring all this logic into NewPromi
+
return PerformPromiseThen(this, onResolve, onReject, resultCapability);
}
@@ -423,8 +424,10 @@ function PromiseResolve(x) {
return promise;
}
- // debugEvent is not so meaningful here as it will be resolved
- var promiseCapability = NewPromiseCapability(this, true);
+ // debugEvent is not so meaningful here as it will be resolved.
+ // We have already addressed the case where we don't need callbacks created,
+ // so we pass false to create the callbacks.
+ var promiseCapability = NewPromiseCapability(this, true, false);
%_Call(promiseCapability.resolve, UNDEFINED, x);
return promiseCapability.promise;
}
@@ -438,7 +441,8 @@ function PromiseAll(iterable) {
// false debugEvent so that forwarding the rejection through all does not
// trigger redundant ExceptionEvents
- var deferred = NewPromiseCapability(this, false);
+ // We need to create the resolving callbacks for the promise.
+ var deferred = NewPromiseCapability(this, false, false);
var resolutions = new InternalArray();
var count;
@@ -502,7 +506,8 @@ function PromiseRace(iterable) {
// false debugEvent so that forwarding the rejection through race does not
// trigger redundant ExceptionEvents
- var deferred = NewPromiseCapability(this, false);
+ // We need to create the resolving callbacks for the promise.
+ var deferred = NewPromiseCapability(this, false, false);
// For catch prediction, don't treat the .then calls as handling it;
// instead, recurse outwards.
« no previous file with comments | « src/js/async-await.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698