Skip to content

Commit 05ec0d7

Browse files
acdliteacdlite
authored
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.)
1 parent 03ede83 commit 05ec0d7

File tree

8 files changed

+106
-101
lines changed

8 files changed

+106
-101
lines changed

‎packages/react-reconciler/src/ReactFiberLane.new.js

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -286,42 +286,34 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
286286
let nextLanes = NoLanes;
287287
let nextLanePriority = NoLanePriority;
288288

289-
const expiredLanes = root.expiredLanes;
290289
const suspendedLanes = root.suspendedLanes;
291290
const pingedLanes = root.pingedLanes;
292291

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);
306304
nextLanePriority = return_highestLanePriority;
307-
} else {
308-
const nonIdlePingedLanes = nonIdlePendingLanes & pingedLanes;
309-
if (nonIdlePingedLanes !== NoLanes) {
310-
nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
311-
nextLanePriority = return_highestLanePriority;
312-
}
313305
}
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;
314313
} 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);
319316
nextLanePriority = return_highestLanePriority;
320-
} else {
321-
if (pingedLanes !== NoLanes) {
322-
nextLanes = getHighestPriorityLanes(pingedLanes);
323-
nextLanePriority = return_highestLanePriority;
324-
}
325317
}
326318
}
327319
}
@@ -463,6 +455,7 @@ export function markStarvedLanesAsExpired(
463455
// expiration time. If so, we'll assume the update is being starved and mark
464456
// it as expired to force it to finish.
465457
let lanes = pendingLanes;
458+
let expiredLanes = 0;
466459
while (lanes > 0) {
467460
const index = pickArbitraryLaneIndex(lanes);
468461
const lane = 1 << index;
@@ -481,11 +474,15 @@ export function markStarvedLanesAsExpired(
481474
}
482475
} else if (expirationTime <= currentTime) {
483476
// This lane expired
484-
root.expiredLanes |= lane;
477+
expiredLanes |= lane;
485478
}
486479

487480
lanes &= ~lane;
488481
}
482+
483+
if (expiredLanes !== 0) {
484+
markRootExpired(root, expiredLanes);
485+
}
489486
}
490487

491488
// This returns the highest priority pending lanes regardless of whether they
@@ -668,7 +665,17 @@ export function markRootPinged(
668665
}
669666

670667
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;
672679
}
673680

674681
export function markRootMutableRead(root: FiberRoot, updateLane: Lane) {
@@ -684,7 +691,6 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
684691
root.suspendedLanes = 0;
685692
root.pingedLanes = 0;
686693

687-
root.expiredLanes &= remainingLanes;
688694
root.mutableReadLanes &= remainingLanes;
689695

690696
root.entangledLanes &= remainingLanes;

‎packages/react-reconciler/src/ReactFiberLane.old.js

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -286,42 +286,34 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
286286
let nextLanes = NoLanes;
287287
let nextLanePriority = NoLanePriority;
288288

289-
const expiredLanes = root.expiredLanes;
290289
const suspendedLanes = root.suspendedLanes;
291290
const pingedLanes = root.pingedLanes;
292291

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);
306304
nextLanePriority = return_highestLanePriority;
307-
} else {
308-
const nonIdlePingedLanes = nonIdlePendingLanes & pingedLanes;
309-
if (nonIdlePingedLanes !== NoLanes) {
310-
nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
311-
nextLanePriority = return_highestLanePriority;
312-
}
313305
}
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;
314313
} 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);
319316
nextLanePriority = return_highestLanePriority;
320-
} else {
321-
if (pingedLanes !== NoLanes) {
322-
nextLanes = getHighestPriorityLanes(pingedLanes);
323-
nextLanePriority = return_highestLanePriority;
324-
}
325317
}
326318
}
327319
}
@@ -463,6 +455,7 @@ export function markStarvedLanesAsExpired(
463455
// expiration time. If so, we'll assume the update is being starved and mark
464456
// it as expired to force it to finish.
465457
let lanes = pendingLanes;
458+
let expiredLanes = 0;
466459
while (lanes > 0) {
467460
const index = pickArbitraryLaneIndex(lanes);
468461
const lane = 1 << index;
@@ -481,11 +474,15 @@ export function markStarvedLanesAsExpired(
481474
}
482475
} else if (expirationTime <= currentTime) {
483476
// This lane expired
484-
root.expiredLanes |= lane;
477+
expiredLanes |= lane;
485478
}
486479

487480
lanes &= ~lane;
488481
}
482+
483+
if (expiredLanes !== 0) {
484+
markRootExpired(root, expiredLanes);
485+
}
489486
}
490487

491488
// This returns the highest priority pending lanes regardless of whether they
@@ -668,7 +665,17 @@ export function markRootPinged(
668665
}
669666

670667
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;
672679
}
673680

674681
export function markRootMutableRead(root: FiberRoot, updateLane: Lane) {
@@ -684,7 +691,6 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
684691
root.suspendedLanes = 0;
685692
root.pingedLanes = 0;
686693

687-
root.expiredLanes &= remainingLanes;
688694
root.mutableReadLanes &= remainingLanes;
689695

690696
root.entangledLanes &= remainingLanes;

‎packages/react-reconciler/src/ReactFiberRoot.new.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ function FiberRootNode(containerInfo, tag, hydrate) {
4848
this.pendingLanes = NoLanes;
4949
this.suspendedLanes = NoLanes;
5050
this.pingedLanes = NoLanes;
51-
this.expiredLanes = NoLanes;
5251
this.mutableReadLanes = NoLanes;
5352
this.finishedLanes = NoLanes;
5453

‎packages/react-reconciler/src/ReactFiberRoot.old.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ function FiberRootNode(containerInfo, tag, hydrate) {
4848
this.pendingLanes = NoLanes;
4949
this.suspendedLanes = NoLanes;
5050
this.pingedLanes = NoLanes;
51-
this.expiredLanes = NoLanes;
5251
this.mutableReadLanes = NoLanes;
5352
this.finishedLanes = NoLanes;
5453

‎packages/react-reconciler/src/ReactFiberWorkLoop.new.js

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ import {
157157
markRootExpired,
158158
markRootFinished,
159159
lanePriorityToSchedulerPriority,
160+
areLanesExpired,
160161
} from './ReactFiberLane.new';
161162
import {
162163
DiscreteEventPriority,
@@ -966,7 +967,7 @@ function performSyncWorkOnRoot(root) {
966967
let exitStatus;
967968
if (
968969
root === workInProgressRoot &&
969-
includesSomeLane(root.expiredLanes, workInProgressRootRenderLanes)
970+
areLanesExpired(root, workInProgressRootRenderLanes)
970971
) {
971972
// There's a partial tree, and at least one of its lanes has expired. Finish
972973
// rendering it before rendering the rest of the expired work.
@@ -1022,12 +1023,16 @@ function performSyncWorkOnRoot(root) {
10221023
return null;
10231024
}
10241025

1026+
// TODO: Do we still need this API? I think we can delete it. Was only used
1027+
// internally.
10251028
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+
}
10311036
}
10321037
}
10331038

‎packages/react-reconciler/src/ReactFiberWorkLoop.old.js

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ import {
157157
markRootExpired,
158158
markRootFinished,
159159
lanePriorityToSchedulerPriority,
160+
areLanesExpired,
160161
} from './ReactFiberLane.old';
161162
import {
162163
DiscreteEventPriority,
@@ -966,7 +967,7 @@ function performSyncWorkOnRoot(root) {
966967
let exitStatus;
967968
if (
968969
root === workInProgressRoot &&
969-
includesSomeLane(root.expiredLanes, workInProgressRootRenderLanes)
970+
areLanesExpired(root, workInProgressRootRenderLanes)
970971
) {
971972
// There's a partial tree, and at least one of its lanes has expired. Finish
972973
// rendering it before rendering the rest of the expired work.
@@ -1022,12 +1023,16 @@ function performSyncWorkOnRoot(root) {
10221023
return null;
10231024
}
10241025

1026+
// TODO: Do we still need this API? I think we can delete it. Was only used
1027+
// internally.
10251028
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+
}
10311036
}
10321037
}
10331038

‎packages/react-reconciler/src/ReactInternalTypes.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,6 @@ type BaseFiberRootProperties = {|
230230
pendingLanes: Lanes,
231231
suspendedLanes: Lanes,
232232
pingedLanes: Lanes,
233-
expiredLanes: Lanes,
234233
mutableReadLanes: Lanes,
235234

236235
finishedLanes: Lanes,

‎packages/react-reconciler/src/__tests__/ReactExpiration-test.js

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -682,28 +682,14 @@ describe('ReactExpiration', () => {
682682
updateHighPri();
683683

684684
// 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+
]);
707693
});
708694
});
709695

0 commit comments

Comments
 (0)