@samitouri / QOS-React-2 / commits / 05ec0d7646

Entangled expired lanes with SyncLane (#21083)

Makes the implementation simpler. Expiration is now a special case of entanglement. Also fixes an issue where expired lanes weren't batched with normal sync updates. (See deleted TODO comment in test.)

Andrew Clark committed Mar 25, 2021 at 11:24 UTC 05ec0d7646b463dcbd64f3a2575f4bb992f7e423
8 files changed +106 -101
packages/react-reconciler/src/ReactFiberLane.new.js
+38 -32
@@ -286,42 +286,34 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
286 let nextLanes = NoLanes;
287 let nextLanePriority = NoLanePriority;
288
289 - const expiredLanes = root.expiredLanes;
289 const suspendedLanes = root.suspendedLanes;
290 const pingedLanes = root.pingedLanes;
291
293 - // Check if any work has expired.
294 - if (expiredLanes !== NoLanes) {
295 - // TODO: Should entangle with SyncLane
296 - nextLanes = expiredLanes;
297 - nextLanePriority = return_highestLanePriority = SyncLanePriority;
298 - } else {
299 - // Do not work on any idle work until all the non-idle work has finished,
300 - // even if the work is suspended.
301 - const nonIdlePendingLanes = pendingLanes & NonIdleLanes;
302 - if (nonIdlePendingLanes !== NoLanes) {
303 - const nonIdleUnblockedLanes = nonIdlePendingLanes & ~suspendedLanes;
304 - if (nonIdleUnblockedLanes !== NoLanes) {
305 - nextLanes = getHighestPriorityLanes(nonIdleUnblockedLanes);
292 + // Do not work on any idle work until all the non-idle work has finished,
293 + // even if the work is suspended.
294 + const nonIdlePendingLanes = pendingLanes & NonIdleLanes;
295 + if (nonIdlePendingLanes !== NoLanes) {
296 + const nonIdleUnblockedLanes = nonIdlePendingLanes & ~suspendedLanes;
297 + if (nonIdleUnblockedLanes !== NoLanes) {
298 + nextLanes = getHighestPriorityLanes(nonIdleUnblockedLanes);
299 + nextLanePriority = return_highestLanePriority;
300 + } else {
301 + const nonIdlePingedLanes = nonIdlePendingLanes & pingedLanes;
302 + if (nonIdlePingedLanes !== NoLanes) {
303 + nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
304 nextLanePriority = return_highestLanePriority;
307 - } else {
308 - const nonIdlePingedLanes = nonIdlePendingLanes & pingedLanes;
309 - if (nonIdlePingedLanes !== NoLanes) {
310 - nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
311 - nextLanePriority = return_highestLanePriority;
312 - }
305 }
306 + }
307 + } else {
308 + // The only remaining work is Idle.
309 + const unblockedLanes = pendingLanes & ~suspendedLanes;
310 + if (unblockedLanes !== NoLanes) {
311 + nextLanes = getHighestPriorityLanes(unblockedLanes);
312 + nextLanePriority = return_highestLanePriority;
313 } else {
315 - // The only remaining work is Idle.
316 - const unblockedLanes = pendingLanes & ~suspendedLanes;
317 - if (unblockedLanes !== NoLanes) {
318 - nextLanes = getHighestPriorityLanes(unblockedLanes);
314 + if (pingedLanes !== NoLanes) {
315 + nextLanes = getHighestPriorityLanes(pingedLanes);
316 nextLanePriority = return_highestLanePriority;
320 - } else {
321 - if (pingedLanes !== NoLanes) {
322 - nextLanes = getHighestPriorityLanes(pingedLanes);
323 - nextLanePriority = return_highestLanePriority;
324 - }
317 }
318 }
319 }
@@ -463,6 +455,7 @@ export function markStarvedLanesAsExpired(
455 // expiration time. If so, we'll assume the update is being starved and mark
456 // it as expired to force it to finish.
457 let lanes = pendingLanes;
458 + let expiredLanes = 0;
459 while (lanes > 0) {
460 const index = pickArbitraryLaneIndex(lanes);
461 const lane = 1 << index;
@@ -481,11 +474,15 @@ export function markStarvedLanesAsExpired(
474 }
475 } else if (expirationTime <= currentTime) {
476 // This lane expired
484 - root.expiredLanes |= lane;
477 + expiredLanes |= lane;
478 }
479
480 lanes &= ~lane;
481 }
482 +
483 + if (expiredLanes !== 0) {
484 + markRootExpired(root, expiredLanes);
485 + }
486 }
487
488 // This returns the highest priority pending lanes regardless of whether they
@@ -668,7 +665,17 @@ export function markRootPinged(
665 }
666
667 export function markRootExpired(root: FiberRoot, expiredLanes: Lanes) {
671 - root.expiredLanes |= expiredLanes & root.pendingLanes;
668 + const entanglements = root.entanglements;
669 + const SyncLaneIndex = 0;
670 + entanglements[SyncLaneIndex] |= expiredLanes;
671 + root.entangledLanes |= SyncLane;
672 + root.pendingLanes |= SyncLane;
673 +}
674 +
675 +export function areLanesExpired(root: FiberRoot, lanes: Lanes) {
676 + const SyncLaneIndex = 0;
677 + const entanglements = root.entanglements;
678 + return (entanglements[SyncLaneIndex] & lanes) !== NoLanes;
679 }
680
681 export function markRootMutableRead(root: FiberRoot, updateLane: Lane) {
@@ -684,7 +691,6 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
691 root.suspendedLanes = 0;
692 root.pingedLanes = 0;
693
687 - root.expiredLanes &= remainingLanes;
694 root.mutableReadLanes &= remainingLanes;
695
696 root.entangledLanes &= remainingLanes;
packages/react-reconciler/src/ReactFiberLane.old.js
+38 -32
@@ -286,42 +286,34 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
286 let nextLanes = NoLanes;
287 let nextLanePriority = NoLanePriority;
288
289 - const expiredLanes = root.expiredLanes;
289 const suspendedLanes = root.suspendedLanes;
290 const pingedLanes = root.pingedLanes;
291
293 - // Check if any work has expired.
294 - if (expiredLanes !== NoLanes) {
295 - // TODO: Should entangle with SyncLane
296 - nextLanes = expiredLanes;
297 - nextLanePriority = return_highestLanePriority = SyncLanePriority;
298 - } else {
299 - // Do not work on any idle work until all the non-idle work has finished,
300 - // even if the work is suspended.
301 - const nonIdlePendingLanes = pendingLanes & NonIdleLanes;
302 - if (nonIdlePendingLanes !== NoLanes) {
303 - const nonIdleUnblockedLanes = nonIdlePendingLanes & ~suspendedLanes;
304 - if (nonIdleUnblockedLanes !== NoLanes) {
305 - nextLanes = getHighestPriorityLanes(nonIdleUnblockedLanes);
292 + // Do not work on any idle work until all the non-idle work has finished,
293 + // even if the work is suspended.
294 + const nonIdlePendingLanes = pendingLanes & NonIdleLanes;
295 + if (nonIdlePendingLanes !== NoLanes) {
296 + const nonIdleUnblockedLanes = nonIdlePendingLanes & ~suspendedLanes;
297 + if (nonIdleUnblockedLanes !== NoLanes) {
298 + nextLanes = getHighestPriorityLanes(nonIdleUnblockedLanes);
299 + nextLanePriority = return_highestLanePriority;
300 + } else {
301 + const nonIdlePingedLanes = nonIdlePendingLanes & pingedLanes;
302 + if (nonIdlePingedLanes !== NoLanes) {
303 + nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
304 nextLanePriority = return_highestLanePriority;
307 - } else {
308 - const nonIdlePingedLanes = nonIdlePendingLanes & pingedLanes;
309 - if (nonIdlePingedLanes !== NoLanes) {
310 - nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
311 - nextLanePriority = return_highestLanePriority;
312 - }
305 }
306 + }
307 + } else {
308 + // The only remaining work is Idle.
309 + const unblockedLanes = pendingLanes & ~suspendedLanes;
310 + if (unblockedLanes !== NoLanes) {
311 + nextLanes = getHighestPriorityLanes(unblockedLanes);
312 + nextLanePriority = return_highestLanePriority;
313 } else {
315 - // The only remaining work is Idle.
316 - const unblockedLanes = pendingLanes & ~suspendedLanes;
317 - if (unblockedLanes !== NoLanes) {
318 - nextLanes = getHighestPriorityLanes(unblockedLanes);
314 + if (pingedLanes !== NoLanes) {
315 + nextLanes = getHighestPriorityLanes(pingedLanes);
316 nextLanePriority = return_highestLanePriority;
320 - } else {
321 - if (pingedLanes !== NoLanes) {
322 - nextLanes = getHighestPriorityLanes(pingedLanes);
323 - nextLanePriority = return_highestLanePriority;
324 - }
317 }
318 }
319 }
@@ -463,6 +455,7 @@ export function markStarvedLanesAsExpired(
455 // expiration time. If so, we'll assume the update is being starved and mark
456 // it as expired to force it to finish.
457 let lanes = pendingLanes;
458 + let expiredLanes = 0;
459 while (lanes > 0) {
460 const index = pickArbitraryLaneIndex(lanes);
461 const lane = 1 << index;
@@ -481,11 +474,15 @@ export function markStarvedLanesAsExpired(
474 }
475 } else if (expirationTime <= currentTime) {
476 // This lane expired
484 - root.expiredLanes |= lane;
477 + expiredLanes |= lane;
478 }
479
480 lanes &= ~lane;
481 }
482 +
483 + if (expiredLanes !== 0) {
484 + markRootExpired(root, expiredLanes);
485 + }
486 }
487
488 // This returns the highest priority pending lanes regardless of whether they
@@ -668,7 +665,17 @@ export function markRootPinged(
665 }
666
667 export function markRootExpired(root: FiberRoot, expiredLanes: Lanes) {
671 - root.expiredLanes |= expiredLanes & root.pendingLanes;
668 + const entanglements = root.entanglements;
669 + const SyncLaneIndex = 0;
670 + entanglements[SyncLaneIndex] |= expiredLanes;
671 + root.entangledLanes |= SyncLane;
672 + root.pendingLanes |= SyncLane;
673 +}
674 +
675 +export function areLanesExpired(root: FiberRoot, lanes: Lanes) {
676 + const SyncLaneIndex = 0;
677 + const entanglements = root.entanglements;
678 + return (entanglements[SyncLaneIndex] & lanes) !== NoLanes;
679 }
680
681 export function markRootMutableRead(root: FiberRoot, updateLane: Lane) {
@@ -684,7 +691,6 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
691 root.suspendedLanes = 0;
692 root.pingedLanes = 0;
693
687 - root.expiredLanes &= remainingLanes;
694 root.mutableReadLanes &= remainingLanes;
695
696 root.entangledLanes &= remainingLanes;
packages/react-reconciler/src/ReactFiberRoot.new.js
-1
@@ -48,7 +48,6 @@ function FiberRootNode(containerInfo, tag, hydrate) {
48 this.pendingLanes = NoLanes;
49 this.suspendedLanes = NoLanes;
50 this.pingedLanes = NoLanes;
51 - this.expiredLanes = NoLanes;
51 this.mutableReadLanes = NoLanes;
52 this.finishedLanes = NoLanes;
53
packages/react-reconciler/src/ReactFiberRoot.old.js
-1
@@ -48,7 +48,6 @@ function FiberRootNode(containerInfo, tag, hydrate) {
48 this.pendingLanes = NoLanes;
49 this.suspendedLanes = NoLanes;
50 this.pingedLanes = NoLanes;
51 - this.expiredLanes = NoLanes;
51 this.mutableReadLanes = NoLanes;
52 this.finishedLanes = NoLanes;
53
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+11 -6
@@ -157,6 +157,7 @@ import {
157 markRootExpired,
158 markRootFinished,
159 lanePriorityToSchedulerPriority,
160 + areLanesExpired,
161 } from './ReactFiberLane.new';
162 import {
163 DiscreteEventPriority,
@@ -966,7 +967,7 @@ function performSyncWorkOnRoot(root) {
967 let exitStatus;
968 if (
969 root === workInProgressRoot &&
969 - includesSomeLane(root.expiredLanes, workInProgressRootRenderLanes)
970 + areLanesExpired(root, workInProgressRootRenderLanes)
971 ) {
972 // There's a partial tree, and at least one of its lanes has expired. Finish
973 // rendering it before rendering the rest of the expired work.
@@ -1022,12 +1023,16 @@ function performSyncWorkOnRoot(root) {
1023 return null;
1024 }
1025
1026 +// TODO: Do we still need this API? I think we can delete it. Was only used
1027 +// internally.
1028 export function flushRoot(root: FiberRoot, lanes: Lanes) {
1026 - markRootExpired(root, lanes);
1027 - ensureRootIsScheduled(root, now());
1028 - if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
1029 - resetRenderTimer();
1030 - flushSyncCallbackQueue();
1029 + if (lanes !== NoLanes) {
1030 + markRootExpired(root, lanes);
1031 + ensureRootIsScheduled(root, now());
1032 + if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
1033 + resetRenderTimer();
1034 + flushSyncCallbackQueue();
1035 + }
1036 }
1037 }
1038
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+11 -6
@@ -157,6 +157,7 @@ import {
157 markRootExpired,
158 markRootFinished,
159 lanePriorityToSchedulerPriority,
160 + areLanesExpired,
161 } from './ReactFiberLane.old';
162 import {
163 DiscreteEventPriority,
@@ -966,7 +967,7 @@ function performSyncWorkOnRoot(root) {
967 let exitStatus;
968 if (
969 root === workInProgressRoot &&
969 - includesSomeLane(root.expiredLanes, workInProgressRootRenderLanes)
970 + areLanesExpired(root, workInProgressRootRenderLanes)
971 ) {
972 // There's a partial tree, and at least one of its lanes has expired. Finish
973 // rendering it before rendering the rest of the expired work.
@@ -1022,12 +1023,16 @@ function performSyncWorkOnRoot(root) {
1023 return null;
1024 }
1025
1026 +// TODO: Do we still need this API? I think we can delete it. Was only used
1027 +// internally.
1028 export function flushRoot(root: FiberRoot, lanes: Lanes) {
1026 - markRootExpired(root, lanes);
1027 - ensureRootIsScheduled(root, now());
1028 - if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
1029 - resetRenderTimer();
1030 - flushSyncCallbackQueue();
1029 + if (lanes !== NoLanes) {
1030 + markRootExpired(root, lanes);
1031 + ensureRootIsScheduled(root, now());
1032 + if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
1033 + resetRenderTimer();
1034 + flushSyncCallbackQueue();
1035 + }
1036 }
1037 }
1038
packages/react-reconciler/src/ReactInternalTypes.js
-1
@@ -230,7 +230,6 @@ type BaseFiberRootProperties = {|
230 pendingLanes: Lanes,
231 suspendedLanes: Lanes,
232 pingedLanes: Lanes,
233 - expiredLanes: Lanes,
233 mutableReadLanes: Lanes,
234
235 finishedLanes: Lanes,
packages/react-reconciler/src/__tests__/ReactExpiration-test.js
+8 -22
@@ -682,28 +682,14 @@ describe('ReactExpiration', () => {
682 updateHighPri();
683
684 // Both normal pri updates should have expired.
685 - if (gate(flags => flags.FIXME)) {
686 - // The sync update and the expired normal pri updates render in a
687 - // single batch.
688 - expect(Scheduler).toHaveYielded([
689 - 'Sibling',
690 - 'High pri: 1',
691 - 'Normal pri: 2',
692 - 'Sibling',
693 - ]);
694 - } else {
695 - expect(Scheduler).toHaveYielded([
696 - 'Sibling',
697 - 'High pri: 0',
698 - 'Normal pri: 2',
699 - 'Sibling',
700 - // TODO: This is the sync update. We should have rendered it in the same
701 - // batch as the expired update.
702 - 'High pri: 1',
703 - 'Normal pri: 2',
704 - 'Sibling',
705 - ]);
706 - }
685 + // The sync update and the expired normal pri updates render in a
686 + // single batch.
687 + expect(Scheduler).toHaveYielded([
688 + 'Sibling',
689 + 'High pri: 1',
690 + 'Normal pri: 2',
691 + 'Sibling',
692 + ]);
693 });
694 });
695