Skip to content

Commit

Permalink
Entangled expired lanes with SyncLane (facebook#21083)
Browse files Browse the repository at this point in the history 
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.)
  • Loading branch information
@acdlite @koto
acdlite authored and koto committed Jun 15, 2021
1 parent 0deb8d2 commit ffc9e0c
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 101 deletions.
70 changes: 38 additions & 32 deletions packages/react-reconciler/src/ReactFiberLane.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,42 +286,34 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
let nextLanes = NoLanes;
let nextLanePriority = NoLanePriority;

const expiredLanes = root.expiredLanes;
const suspendedLanes = root.suspendedLanes;
const pingedLanes = root.pingedLanes;

// Check if any work has expired.
if (expiredLanes !== NoLanes) {
// TODO: Should entangle with SyncLane
nextLanes = expiredLanes;
nextLanePriority = return_highestLanePriority = SyncLanePriority;
} else {
// Do not work on any idle work until all the non-idle work has finished,
// even if the work is suspended.
const nonIdlePendingLanes = pendingLanes & NonIdleLanes;
if (nonIdlePendingLanes !== NoLanes) {
const nonIdleUnblockedLanes = nonIdlePendingLanes & ~suspendedLanes;
if (nonIdleUnblockedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(nonIdleUnblockedLanes);
// Do not work on any idle work until all the non-idle work has finished,
// even if the work is suspended.
const nonIdlePendingLanes = pendingLanes & NonIdleLanes;
if (nonIdlePendingLanes !== NoLanes) {
const nonIdleUnblockedLanes = nonIdlePendingLanes & ~suspendedLanes;
if (nonIdleUnblockedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(nonIdleUnblockedLanes);
nextLanePriority = return_highestLanePriority;
} else {
const nonIdlePingedLanes = nonIdlePendingLanes & pingedLanes;
if (nonIdlePingedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
nextLanePriority = return_highestLanePriority;
} else {
const nonIdlePingedLanes = nonIdlePendingLanes & pingedLanes;
if (nonIdlePingedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
nextLanePriority = return_highestLanePriority;
}
}
}
} else {
// The only remaining work is Idle.
const unblockedLanes = pendingLanes & ~suspendedLanes;
if (unblockedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(unblockedLanes);
nextLanePriority = return_highestLanePriority;
} else {
// The only remaining work is Idle.
const unblockedLanes = pendingLanes & ~suspendedLanes;
if (unblockedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(unblockedLanes);
if (pingedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(pingedLanes);
nextLanePriority = return_highestLanePriority;
} else {
if (pingedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(pingedLanes);
nextLanePriority = return_highestLanePriority;
}
}
}
}
Expand Down Expand Up @@ -463,6 +455,7 @@ export function markStarvedLanesAsExpired(
// expiration time. If so, we'll assume the update is being starved and mark
// it as expired to force it to finish.
let lanes = pendingLanes;
let expiredLanes = 0;
while (lanes > 0) {
const index = pickArbitraryLaneIndex(lanes);
const lane = 1 << index;
Expand All @@ -481,11 +474,15 @@ export function markStarvedLanesAsExpired(
}
} else if (expirationTime <= currentTime) {
// This lane expired
root.expiredLanes |= lane;
expiredLanes |= lane;
}

lanes &= ~lane;
}

if (expiredLanes !== 0) {
markRootExpired(root, expiredLanes);
}
}

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

export function markRootExpired(root: FiberRoot, expiredLanes: Lanes) {
root.expiredLanes |= expiredLanes & root.pendingLanes;
const entanglements = root.entanglements;
const SyncLaneIndex = 0;
entanglements[SyncLaneIndex] |= expiredLanes;
root.entangledLanes |= SyncLane;
root.pendingLanes |= SyncLane;
}

export function areLanesExpired(root: FiberRoot, lanes: Lanes) {
const SyncLaneIndex = 0;
const entanglements = root.entanglements;
return (entanglements[SyncLaneIndex] & lanes) !== NoLanes;
}

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

root.expiredLanes &= remainingLanes;
root.mutableReadLanes &= remainingLanes;

root.entangledLanes &= remainingLanes;
Expand Down
70 changes: 38 additions & 32 deletions packages/react-reconciler/src/ReactFiberLane.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,42 +286,34 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
let nextLanes = NoLanes;
let nextLanePriority = NoLanePriority;

const expiredLanes = root.expiredLanes;
const suspendedLanes = root.suspendedLanes;
const pingedLanes = root.pingedLanes;

// Check if any work has expired.
if (expiredLanes !== NoLanes) {
// TODO: Should entangle with SyncLane
nextLanes = expiredLanes;
nextLanePriority = return_highestLanePriority = SyncLanePriority;
} else {
// Do not work on any idle work until all the non-idle work has finished,
// even if the work is suspended.
const nonIdlePendingLanes = pendingLanes & NonIdleLanes;
if (nonIdlePendingLanes !== NoLanes) {
const nonIdleUnblockedLanes = nonIdlePendingLanes & ~suspendedLanes;
if (nonIdleUnblockedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(nonIdleUnblockedLanes);
// Do not work on any idle work until all the non-idle work has finished,
// even if the work is suspended.
const nonIdlePendingLanes = pendingLanes & NonIdleLanes;
if (nonIdlePendingLanes !== NoLanes) {
const nonIdleUnblockedLanes = nonIdlePendingLanes & ~suspendedLanes;
if (nonIdleUnblockedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(nonIdleUnblockedLanes);
nextLanePriority = return_highestLanePriority;
} else {
const nonIdlePingedLanes = nonIdlePendingLanes & pingedLanes;
if (nonIdlePingedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
nextLanePriority = return_highestLanePriority;
} else {
const nonIdlePingedLanes = nonIdlePendingLanes & pingedLanes;
if (nonIdlePingedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
nextLanePriority = return_highestLanePriority;
}
}
}
} else {
// The only remaining work is Idle.
const unblockedLanes = pendingLanes & ~suspendedLanes;
if (unblockedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(unblockedLanes);
nextLanePriority = return_highestLanePriority;
} else {
// The only remaining work is Idle.
const unblockedLanes = pendingLanes & ~suspendedLanes;
if (unblockedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(unblockedLanes);
if (pingedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(pingedLanes);
nextLanePriority = return_highestLanePriority;
} else {
if (pingedLanes !== NoLanes) {
nextLanes = getHighestPriorityLanes(pingedLanes);
nextLanePriority = return_highestLanePriority;
}
}
}
}
Expand Down Expand Up @@ -463,6 +455,7 @@ export function markStarvedLanesAsExpired(
// expiration time. If so, we'll assume the update is being starved and mark
// it as expired to force it to finish.
let lanes = pendingLanes;
let expiredLanes = 0;
while (lanes > 0) {
const index = pickArbitraryLaneIndex(lanes);
const lane = 1 << index;
Expand All @@ -481,11 +474,15 @@ export function markStarvedLanesAsExpired(
}
} else if (expirationTime <= currentTime) {
// This lane expired
root.expiredLanes |= lane;
expiredLanes |= lane;
}

lanes &= ~lane;
}

if (expiredLanes !== 0) {
markRootExpired(root, expiredLanes);
}
}

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

export function markRootExpired(root: FiberRoot, expiredLanes: Lanes) {
root.expiredLanes |= expiredLanes & root.pendingLanes;
const entanglements = root.entanglements;
const SyncLaneIndex = 0;
entanglements[SyncLaneIndex] |= expiredLanes;
root.entangledLanes |= SyncLane;
root.pendingLanes |= SyncLane;
}

export function areLanesExpired(root: FiberRoot, lanes: Lanes) {
const SyncLaneIndex = 0;
const entanglements = root.entanglements;
return (entanglements[SyncLaneIndex] & lanes) !== NoLanes;
}

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

root.expiredLanes &= remainingLanes;
root.mutableReadLanes &= remainingLanes;

root.entangledLanes &= remainingLanes;
Expand Down
1 change: 0 additions & 1 deletion packages/react-reconciler/src/ReactFiberRoot.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ function FiberRootNode(containerInfo, tag, hydrate) {
this.pendingLanes = NoLanes;
this.suspendedLanes = NoLanes;
this.pingedLanes = NoLanes;
this.expiredLanes = NoLanes;
this.mutableReadLanes = NoLanes;
this.finishedLanes = NoLanes;

Expand Down
1 change: 0 additions & 1 deletion packages/react-reconciler/src/ReactFiberRoot.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ function FiberRootNode(containerInfo, tag, hydrate) {
this.pendingLanes = NoLanes;
this.suspendedLanes = NoLanes;
this.pingedLanes = NoLanes;
this.expiredLanes = NoLanes;
this.mutableReadLanes = NoLanes;
this.finishedLanes = NoLanes;

Expand Down
17 changes: 11 additions & 6 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ import {
markRootExpired,
markRootFinished,
lanePriorityToSchedulerPriority,
areLanesExpired,
} from './ReactFiberLane.new';
import {
DiscreteEventPriority,
Expand Down Expand Up @@ -966,7 +967,7 @@ function performSyncWorkOnRoot(root) {
let exitStatus;
if (
root === workInProgressRoot &&
includesSomeLane(root.expiredLanes, workInProgressRootRenderLanes)
areLanesExpired(root, workInProgressRootRenderLanes)
) {
// There's a partial tree, and at least one of its lanes has expired. Finish
// rendering it before rendering the rest of the expired work.
Expand Down Expand Up @@ -1022,12 +1023,16 @@ function performSyncWorkOnRoot(root) {
return null;
}

// TODO: Do we still need this API? I think we can delete it. Was only used
// internally.
export function flushRoot(root: FiberRoot, lanes: Lanes) {
markRootExpired(root, lanes);
ensureRootIsScheduled(root, now());
if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
resetRenderTimer();
flushSyncCallbackQueue();
if (lanes !== NoLanes) {
markRootExpired(root, lanes);
ensureRootIsScheduled(root, now());
if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
resetRenderTimer();
flushSyncCallbackQueue();
}
}
}

Expand Down
17 changes: 11 additions & 6 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ import {
markRootExpired,
markRootFinished,
lanePriorityToSchedulerPriority,
areLanesExpired,
} from './ReactFiberLane.old';
import {
DiscreteEventPriority,
Expand Down Expand Up @@ -966,7 +967,7 @@ function performSyncWorkOnRoot(root) {
let exitStatus;
if (
root === workInProgressRoot &&
includesSomeLane(root.expiredLanes, workInProgressRootRenderLanes)
areLanesExpired(root, workInProgressRootRenderLanes)
) {
// There's a partial tree, and at least one of its lanes has expired. Finish
// rendering it before rendering the rest of the expired work.
Expand Down Expand Up @@ -1022,12 +1023,16 @@ function performSyncWorkOnRoot(root) {
return null;
}

// TODO: Do we still need this API? I think we can delete it. Was only used
// internally.
export function flushRoot(root: FiberRoot, lanes: Lanes) {
markRootExpired(root, lanes);
ensureRootIsScheduled(root, now());
if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
resetRenderTimer();
flushSyncCallbackQueue();
if (lanes !== NoLanes) {
markRootExpired(root, lanes);
ensureRootIsScheduled(root, now());
if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
resetRenderTimer();
flushSyncCallbackQueue();
}
}
}

Expand Down
1 change: 0 additions & 1 deletion packages/react-reconciler/src/ReactInternalTypes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,6 @@ type BaseFiberRootProperties = {|
pendingLanes: Lanes,
suspendedLanes: Lanes,
pingedLanes: Lanes,
expiredLanes: Lanes,
mutableReadLanes: Lanes,

finishedLanes: Lanes,
Expand Down
30 changes: 8 additions & 22 deletions packages/react-reconciler/src/__tests__/ReactExpiration-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -682,28 +682,14 @@ describe('ReactExpiration', () => {
updateHighPri();

// Both normal pri updates should have expired.
if (gate(flags => flags.FIXME)) {
// The sync update and the expired normal pri updates render in a
// single batch.
expect(Scheduler).toHaveYielded([
'Sibling',
'High pri: 1',
'Normal pri: 2',
'Sibling',
]);
} else {
expect(Scheduler).toHaveYielded([
'Sibling',
'High pri: 0',
'Normal pri: 2',
'Sibling',
// TODO: This is the sync update. We should have rendered it in the same
// batch as the expired update.
'High pri: 1',
'Normal pri: 2',
'Sibling',
]);
}
// The sync update and the expired normal pri updates render in a
// single batch.
expect(Scheduler).toHaveYielded([
'Sibling',
'High pri: 1',
'Normal pri: 2',
'Sibling',
]);
});
});

Expand Down

0 comments on commit ffc9e0c

Please sign in to comment.