Skip to content

Commit d13ba81

Browse files
acdliteacdlite
committed
Move suspended render logic to ensureRootIsScheduled (#26328)
When the work loop is suspended, we shouldn't schedule a new render task until the promise has resolved. When I originally implemented this, I wasn't sure where to put this logic — `ensureRootIsScheduled` is the more natural place for it, but that's also a really hot path, so I chose to do it elsewhere, and left a TODO to reconsider later. Now it's later. I'm working on a refactor to move the `ensureRootIsScheduled` call to always happen in a microtask, so that if there are multiple updates/pings in a single event, they get batched into a single operation. Which means I can put the logic in that function where it belongs. DiffTrain build for [6e1756a](6e1756a)
1 parent 4b89fa0 commit d13ba81

28 files changed

+294
-214
lines changed

‎compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1528c5ccdf5c61a08adab31116156df6503e26ce
1+
6e1756a5a9bb0d95ec17983f7dc38cd2c2663b39

‎compiled/facebook-www/REVISION_TRANSFORMS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1528c5ccdf5c61a08adab31116156df6503e26ce
1+
6e1756a5a9bb0d95ec17983f7dc38cd2c2663b39

‎compiled/facebook-www/React-dev.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if (
2727
}
2828
"use strict";
2929

30-
var ReactVersion = "18.3.0-www-classic-1528c5ccd-20230306";
30+
var ReactVersion = "18.3.0-www-classic-6e1756a5a-20230306";
3131

3232
// ATTENTION
3333
// When adding new symbols to this file,

‎compiled/facebook-www/React-dev.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if (
2727
}
2828
"use strict";
2929

30-
var ReactVersion = "18.3.0-www-modern-1528c5ccd-20230306";
30+
var ReactVersion = "18.3.0-www-modern-6e1756a5a-20230306";
3131

3232
// ATTENTION
3333
// When adding new symbols to this file,

‎compiled/facebook-www/React-prod.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,4 +646,4 @@ exports.useSyncExternalStore = function (
646646
);
647647
};
648648
exports.useTransition = useTransition;
649-
exports.version = "18.3.0-www-classic-1528c5ccd-20230306";
649+
exports.version = "18.3.0-www-classic-6e1756a5a-20230306";

‎compiled/facebook-www/React-prod.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -638,4 +638,4 @@ exports.useSyncExternalStore = function (
638638
);
639639
};
640640
exports.useTransition = useTransition;
641-
exports.version = "18.3.0-www-modern-1528c5ccd-20230306";
641+
exports.version = "18.3.0-www-modern-6e1756a5a-20230306";

‎compiled/facebook-www/React-profiling.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ exports.useSyncExternalStore = function (
657657
);
658658
};
659659
exports.useTransition = useTransition;
660-
exports.version = "18.3.0-www-classic-1528c5ccd-20230306";
660+
exports.version = "18.3.0-www-classic-6e1756a5a-20230306";
661661

662662
/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
663663
if (

‎compiled/facebook-www/React-profiling.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ exports.useSyncExternalStore = function (
649649
);
650650
};
651651
exports.useTransition = useTransition;
652-
exports.version = "18.3.0-www-modern-1528c5ccd-20230306";
652+
exports.version = "18.3.0-www-modern-6e1756a5a-20230306";
653653

654654
/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
655655
if (

‎compiled/facebook-www/ReactART-dev.classic.js

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
6969
return self;
7070
}
7171

72-
var ReactVersion = "18.3.0-www-classic-1528c5ccd-20230306";
72+
var ReactVersion = "18.3.0-www-classic-6e1756a5a-20230306";
7373

7474
var LegacyRoot = 0;
7575
var ConcurrentRoot = 1;
@@ -22862,7 +22862,7 @@ var SuspendedOnError = 1;
2286222862
var SuspendedOnData = 2;
2286322863
var SuspendedOnImmediate = 3;
2286422864
var SuspendedOnDeprecatedThrowPromise = 4;
22865-
var SuspendedAndReadyToUnwind = 5;
22865+
var SuspendedAndReadyToContinue = 5;
2286622866
var SuspendedOnHydration = 6; // When this is true, the work-in-progress fiber just suspended (or errored) and
2286722867
// we've yet to unwind the stack. In some cases, we may yield to the main thread
2286822868
// after this happens. If the fiber is pinged before we resume, we can retry
@@ -23363,6 +23363,17 @@ function ensureRootIsScheduled(root, currentTime) {
2336323363
root.callbackNode = null;
2336423364
root.callbackPriority = NoLane;
2336523365
return;
23366+
} // If this root is currently suspended and waiting for data to resolve, don't
23367+
// schedule a task to render it. We'll either wait for a ping, or wait to
23368+
// receive an update.
23369+
23370+
if (
23371+
workInProgressSuspendedReason === SuspendedOnData &&
23372+
workInProgressRoot === root
23373+
) {
23374+
root.callbackPriority = NoLane;
23375+
root.callbackNode = null;
23376+
return;
2336623377
} // We use the highest priority lane to represent the priority of the callback.
2336723378

2336823379
var newCallbackPriority = getHighestPriorityLane(nextLanes); // Check if there's an existing task. We may be able to reuse it.
@@ -23603,21 +23614,6 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
2360323614
if (root.callbackNode === originalCallbackNode) {
2360423615
// The task node scheduled for this root is the same one that's
2360523616
// currently executed. Need to return a continuation.
23606-
if (
23607-
workInProgressSuspendedReason === SuspendedOnData &&
23608-
workInProgressRoot === root
23609-
) {
23610-
// Special case: The work loop is currently suspended and waiting for
23611-
// data to resolve. Unschedule the current task.
23612-
//
23613-
// TODO: The factoring is a little weird. Arguably this should be checked
23614-
// in ensureRootIsScheduled instead. I went back and forth, not totally
23615-
// sure yet.
23616-
root.callbackPriority = NoLane;
23617-
root.callbackNode = null;
23618-
return null;
23619-
}
23620-
2362123617
return performConcurrentWorkOnRoot.bind(null, root);
2362223618
}
2362323619

@@ -24199,7 +24195,7 @@ function handleThrow(root, thrownValue) {
2419924195
case SuspendedOnData:
2420024196
case SuspendedOnImmediate:
2420124197
case SuspendedOnDeprecatedThrowPromise:
24202-
case SuspendedAndReadyToUnwind: {
24198+
case SuspendedAndReadyToContinue: {
2420324199
var wakeable = thrownValue;
2420424200
markComponentSuspended(
2420524201
erroredWork,
@@ -24538,6 +24534,17 @@ function renderRootConcurrent(root, lanes) {
2453824534
// have added a listener until right here.
2453924535

2454024536
var onResolution = function () {
24537+
// Check if the root is still suspended on this promise.
24538+
if (
24539+
workInProgressSuspendedReason === SuspendedOnData &&
24540+
workInProgressRoot === root
24541+
) {
24542+
// Mark the root as ready to continue rendering.
24543+
workInProgressSuspendedReason = SuspendedAndReadyToContinue;
24544+
} // Ensure the root is scheduled. We should do this even if we're
24545+
// currently working on a different root, so that we resume
24546+
// rendering later.
24547+
2454124548
ensureRootIsScheduled(root, now$1());
2454224549
};
2454324550

@@ -24549,11 +24556,11 @@ function renderRootConcurrent(root, lanes) {
2454924556
// If this fiber just suspended, it's possible the data is already
2455024557
// cached. Yield to the main thread to give it a chance to ping. If
2455124558
// it does, we can retry immediately without unwinding the stack.
24552-
workInProgressSuspendedReason = SuspendedAndReadyToUnwind;
24559+
workInProgressSuspendedReason = SuspendedAndReadyToContinue;
2455324560
break outer;
2455424561
}
2455524562

24556-
case SuspendedAndReadyToUnwind: {
24563+
case SuspendedAndReadyToContinue: {
2455724564
var _thenable = thrownValue;
2455824565

2455924566
if (isThenableResolved(_thenable)) {

‎compiled/facebook-www/ReactART-dev.modern.js

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
6969
return self;
7070
}
7171

72-
var ReactVersion = "18.3.0-www-modern-1528c5ccd-20230306";
72+
var ReactVersion = "18.3.0-www-modern-6e1756a5a-20230306";
7373

7474
var LegacyRoot = 0;
7575
var ConcurrentRoot = 1;
@@ -22522,7 +22522,7 @@ var SuspendedOnError = 1;
2252222522
var SuspendedOnData = 2;
2252322523
var SuspendedOnImmediate = 3;
2252422524
var SuspendedOnDeprecatedThrowPromise = 4;
22525-
var SuspendedAndReadyToUnwind = 5;
22525+
var SuspendedAndReadyToContinue = 5;
2252622526
var SuspendedOnHydration = 6; // When this is true, the work-in-progress fiber just suspended (or errored) and
2252722527
// we've yet to unwind the stack. In some cases, we may yield to the main thread
2252822528
// after this happens. If the fiber is pinged before we resume, we can retry
@@ -23023,6 +23023,17 @@ function ensureRootIsScheduled(root, currentTime) {
2302323023
root.callbackNode = null;
2302423024
root.callbackPriority = NoLane;
2302523025
return;
23026+
} // If this root is currently suspended and waiting for data to resolve, don't
23027+
// schedule a task to render it. We'll either wait for a ping, or wait to
23028+
// receive an update.
23029+
23030+
if (
23031+
workInProgressSuspendedReason === SuspendedOnData &&
23032+
workInProgressRoot === root
23033+
) {
23034+
root.callbackPriority = NoLane;
23035+
root.callbackNode = null;
23036+
return;
2302623037
} // We use the highest priority lane to represent the priority of the callback.
2302723038

2302823039
var newCallbackPriority = getHighestPriorityLane(nextLanes); // Check if there's an existing task. We may be able to reuse it.
@@ -23263,21 +23274,6 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
2326323274
if (root.callbackNode === originalCallbackNode) {
2326423275
// The task node scheduled for this root is the same one that's
2326523276
// currently executed. Need to return a continuation.
23266-
if (
23267-
workInProgressSuspendedReason === SuspendedOnData &&
23268-
workInProgressRoot === root
23269-
) {
23270-
// Special case: The work loop is currently suspended and waiting for
23271-
// data to resolve. Unschedule the current task.
23272-
//
23273-
// TODO: The factoring is a little weird. Arguably this should be checked
23274-
// in ensureRootIsScheduled instead. I went back and forth, not totally
23275-
// sure yet.
23276-
root.callbackPriority = NoLane;
23277-
root.callbackNode = null;
23278-
return null;
23279-
}
23280-
2328123277
return performConcurrentWorkOnRoot.bind(null, root);
2328223278
}
2328323279

@@ -23859,7 +23855,7 @@ function handleThrow(root, thrownValue) {
2385923855
case SuspendedOnData:
2386023856
case SuspendedOnImmediate:
2386123857
case SuspendedOnDeprecatedThrowPromise:
23862-
case SuspendedAndReadyToUnwind: {
23858+
case SuspendedAndReadyToContinue: {
2386323859
var wakeable = thrownValue;
2386423860
markComponentSuspended(
2386523861
erroredWork,
@@ -24198,6 +24194,17 @@ function renderRootConcurrent(root, lanes) {
2419824194
// have added a listener until right here.
2419924195

2420024196
var onResolution = function () {
24197+
// Check if the root is still suspended on this promise.
24198+
if (
24199+
workInProgressSuspendedReason === SuspendedOnData &&
24200+
workInProgressRoot === root
24201+
) {
24202+
// Mark the root as ready to continue rendering.
24203+
workInProgressSuspendedReason = SuspendedAndReadyToContinue;
24204+
} // Ensure the root is scheduled. We should do this even if we're
24205+
// currently working on a different root, so that we resume
24206+
// rendering later.
24207+
2420124208
ensureRootIsScheduled(root, now$1());
2420224209
};
2420324210

@@ -24209,11 +24216,11 @@ function renderRootConcurrent(root, lanes) {
2420924216
// If this fiber just suspended, it's possible the data is already
2421024217
// cached. Yield to the main thread to give it a chance to ping. If
2421124218
// it does, we can retry immediately without unwinding the stack.
24212-
workInProgressSuspendedReason = SuspendedAndReadyToUnwind;
24219+
workInProgressSuspendedReason = SuspendedAndReadyToContinue;
2421324220
break outer;
2421424221
}
2421524222

24216-
case SuspendedAndReadyToUnwind: {
24223+
case SuspendedAndReadyToContinue: {
2421724224
var _thenable = thrownValue;
2421824225

2421924226
if (isThenableResolved(_thenable)) {

‎compiled/facebook-www/ReactART-prod.classic.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7723,6 +7723,8 @@ function ensureRootIsScheduled(root, currentTime) {
77237723
null !== existingCallbackNode && cancelCallback$1(existingCallbackNode),
77247724
(root.callbackNode = null),
77257725
(root.callbackPriority = 0);
7726+
else if (2 === workInProgressSuspendedReason && workInProgressRoot === root)
7727+
(root.callbackPriority = 0), (root.callbackNode = null);
77267728
else if (
77277729
((currentTime = suspendedLanes & -suspendedLanes),
77287730
root.callbackPriority !== currentTime)
@@ -7936,9 +7938,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
79367938
}
79377939
ensureRootIsScheduled(root, now());
79387940
return root.callbackNode === originalCallbackNode
7939-
? 2 === workInProgressSuspendedReason && workInProgressRoot === root
7940-
? ((root.callbackPriority = 0), (root.callbackNode = null))
7941-
: performConcurrentWorkOnRoot.bind(null, root)
7941+
? performConcurrentWorkOnRoot.bind(null, root)
79427942
: null;
79437943
}
79447944
function recoverFromConcurrentError(
@@ -8229,6 +8229,9 @@ function renderRootConcurrent(root, lanes) {
82298229
break;
82308230
}
82318231
lanes = function () {
8232+
2 === workInProgressSuspendedReason &&
8233+
workInProgressRoot === root &&
8234+
(workInProgressSuspendedReason = 5);
82328235
ensureRootIsScheduled(root, now());
82338236
};
82348237
thrownValue.then(lanes, lanes);
@@ -9780,7 +9783,7 @@ var slice = Array.prototype.slice,
97809783
return null;
97819784
},
97829785
bundleType: 0,
9783-
version: "18.3.0-www-classic-1528c5ccd-20230306",
9786+
version: "18.3.0-www-classic-6e1756a5a-20230306",
97849787
rendererPackageName: "react-art"
97859788
};
97869789
var internals$jscomp$inline_1300 = {
@@ -9811,7 +9814,7 @@ var internals$jscomp$inline_1300 = {
98119814
scheduleRoot: null,
98129815
setRefreshHandler: null,
98139816
getCurrentFiber: null,
9814-
reconcilerVersion: "18.3.0-next-1528c5ccd-20230306"
9817+
reconcilerVersion: "18.3.0-next-6e1756a5a-20230306"
98159818
};
98169819
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
98179820
var hook$jscomp$inline_1301 = __REACT_DEVTOOLS_GLOBAL_HOOK__;

‎compiled/facebook-www/ReactART-prod.modern.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7455,6 +7455,8 @@ function ensureRootIsScheduled(root, currentTime) {
74557455
null !== existingCallbackNode && cancelCallback$1(existingCallbackNode),
74567456
(root.callbackNode = null),
74577457
(root.callbackPriority = 0);
7458+
else if (2 === workInProgressSuspendedReason && workInProgressRoot === root)
7459+
(root.callbackPriority = 0), (root.callbackNode = null);
74587460
else if (
74597461
((currentTime = suspendedLanes & -suspendedLanes),
74607462
root.callbackPriority !== currentTime)
@@ -7668,9 +7670,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
76687670
}
76697671
ensureRootIsScheduled(root, now());
76707672
return root.callbackNode === originalCallbackNode
7671-
? 2 === workInProgressSuspendedReason && workInProgressRoot === root
7672-
? ((root.callbackPriority = 0), (root.callbackNode = null))
7673-
: performConcurrentWorkOnRoot.bind(null, root)
7673+
? performConcurrentWorkOnRoot.bind(null, root)
76747674
: null;
76757675
}
76767676
function recoverFromConcurrentError(
@@ -7961,6 +7961,9 @@ function renderRootConcurrent(root, lanes) {
79617961
break;
79627962
}
79637963
lanes = function () {
7964+
2 === workInProgressSuspendedReason &&
7965+
workInProgressRoot === root &&
7966+
(workInProgressSuspendedReason = 5);
79647967
ensureRootIsScheduled(root, now());
79657968
};
79667969
thrownValue.then(lanes, lanes);
@@ -9445,7 +9448,7 @@ var slice = Array.prototype.slice,
94459448
return null;
94469449
},
94479450
bundleType: 0,
9448-
version: "18.3.0-www-modern-1528c5ccd-20230306",
9451+
version: "18.3.0-www-modern-6e1756a5a-20230306",
94499452
rendererPackageName: "react-art"
94509453
};
94519454
var internals$jscomp$inline_1280 = {
@@ -9476,7 +9479,7 @@ var internals$jscomp$inline_1280 = {
94769479
scheduleRoot: null,
94779480
setRefreshHandler: null,
94789481
getCurrentFiber: null,
9479-
reconcilerVersion: "18.3.0-next-1528c5ccd-20230306"
9482+
reconcilerVersion: "18.3.0-next-6e1756a5a-20230306"
94809483
};
94819484
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
94829485
var hook$jscomp$inline_1281 = __REACT_DEVTOOLS_GLOBAL_HOOK__;

0 commit comments

Comments
 (0)