Skip to content

Commit

Permalink
Don't shift interleaved updates to separate lane
Browse files Browse the repository at this point in the history
Now that interleaved updates are added to a special queue, we no longer
need to shift them into their own lane. We can add to a lane that's
already in the middle of rendering without risk of tearing.

See facebook#20615 for more background.

I've only changed this in the new fork, and only behind the
enableTransitionEntanglements flag.

Most of this commit involves updating tests. The "shift-to-a-new" lane
trick was intentionally used in a handful of tests where two or more
updates need to be scheduled in different lanes. Most of these tests
were written before `startTransition` existed, and all of them were
written before transitions were assigned arbitrary lanes.

So I ported these tests to use `startTransition` instead, which is the
idiomatic way to mark an update as parallel.

I didn't change the old fork at all. Writing these tests in such a way
that they also pass in the old fork actually revealed a few flaws in the
current implementation regarding interrupting a suspended refresh
transition early, which is a good reminder that we should be writing our
tests using idiomatic patterns as much as we possibly can.
  • Loading branch information
acdlite committed Feb 8, 2021
1 parent a014c91 commit 998cbb5
Show file tree
Hide file tree
Showing 15 changed files with 505 additions and 362 deletions.
9 changes: 5 additions & 4 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2029,10 +2029,11 @@ function dispatchAction<S, A>(

// Entangle the new transition lane with the other transition lanes.
const newQueueLanes = mergeLanes(queueLanes, lane);
if (newQueueLanes !== queueLanes) {
queue.lanes = newQueueLanes;
markRootEntangled(root, newQueueLanes);
}
queue.lanes = newQueueLanes;
// Even if queue.lanes already include lane, we don't know for certain if
// the lane finished since the last time we entangled it. So we need to
// entangle it again, just to be sure.
markRootEntangled(root, newQueueLanes);
}
}

Expand Down
9 changes: 5 additions & 4 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -2029,10 +2029,11 @@ function dispatchAction<S, A>(

// Entangle the new transition lane with the other transition lanes.
const newQueueLanes = mergeLanes(queueLanes, lane);
if (newQueueLanes !== queueLanes) {
queue.lanes = newQueueLanes;
markRootEntangled(root, newQueueLanes);
}
queue.lanes = newQueueLanes;
// Even if queue.lanes already include lane, we don't know for certain if
// the lane finished since the last time we entangled it. So we need to
// entangle it again, just to be sure.
markRootEntangled(root, newQueueLanes);
}
}

Expand Down
136 changes: 86 additions & 50 deletions packages/react-reconciler/src/ReactFiberLane.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ export type Lane = number;
export type LaneMap<T> = Array<T>;

import invariant from 'shared/invariant';
import {enableCache} from 'shared/ReactFeatureFlags';
import {
enableCache,
enableTransitionEntanglement,
} from 'shared/ReactFeatureFlags';

import {
ImmediatePriority as ImmediateSchedulerPriority,
Expand Down Expand Up @@ -92,11 +95,12 @@ export const DefaultLanes: Lanes = /* */ 0b0000000000000000000

const TransitionHydrationLane: Lane = /* */ 0b0000000000000000001000000000000;
const TransitionLanes: Lanes = /* */ 0b0000000001111111110000000000000;
const SomeTransitionLane: Lane = /* */ 0b0000000000000000010000000000000;
const FirstTransitionLane: Lane = /* */ 0b0000000000000000010000000000000;

const RetryLanes: Lanes = /* */ 0b0000011110000000000000000000000;

export const SomeRetryLane: Lanes = /* */ 0b0000010000000000000000000000000;
const FirstRetryLane: Lanes = /* */ 0b0000000010000000000000000000000;
export const SomeRetryLane: Lane = FirstRetryLane;

export const SelectiveHydrationLane: Lane = /* */ 0b0000100000000000000000000000000;

Expand All @@ -111,8 +115,8 @@ export const NoTimestamp = -1;

let currentUpdateLanePriority: LanePriority = NoLanePriority;

let nextTransitionLane: Lane = SomeTransitionLane;
let nextRetryLane: Lane = SomeRetryLane;
let nextTransitionLane: Lane = FirstTransitionLane;
let nextRetryLane: Lane = FirstRetryLane;

export function getCurrentUpdateLanePriority(): LanePriority {
return currentUpdateLanePriority;
Expand Down Expand Up @@ -498,56 +502,88 @@ export function findUpdateLane(
lanePriority: LanePriority,
wipLanes: Lanes,
): Lane {
switch (lanePriority) {
case NoLanePriority:
break;
case SyncLanePriority:
return SyncLane;
case SyncBatchedLanePriority:
return SyncBatchedLane;
case InputDiscreteLanePriority: {
const lane = pickArbitraryLane(InputDiscreteLanes & ~wipLanes);
if (lane === NoLane) {
// Shift to the next priority level
return findUpdateLane(InputContinuousLanePriority, wipLanes);
if (enableTransitionEntanglement) {
// Ignore wipLanes. Always assign to the same bit per priority.
switch (lanePriority) {
case NoLanePriority:
break;
case SyncLanePriority:
return SyncLane;
case SyncBatchedLanePriority:
return SyncBatchedLane;
case InputDiscreteLanePriority: {
return pickArbitraryLane(InputDiscreteLanes);
}
return lane;
}
case InputContinuousLanePriority: {
const lane = pickArbitraryLane(InputContinuousLanes & ~wipLanes);
if (lane === NoLane) {
// Shift to the next priority level
return findUpdateLane(DefaultLanePriority, wipLanes);
case InputContinuousLanePriority: {
return pickArbitraryLane(InputContinuousLanes);
}
case DefaultLanePriority: {
return pickArbitraryLane(DefaultLanes);
}
return lane;
case TransitionPriority: // Should be handled by findTransitionLane instead
case RetryLanePriority: // Should be handled by findRetryLane instead
break;
case IdleLanePriority:
return pickArbitraryLane(IdleLanes);
default:
// The remaining priorities are not valid for updates
break;
}
case DefaultLanePriority: {
let lane = pickArbitraryLane(DefaultLanes & ~wipLanes);
if (lane === NoLane) {
// If all the default lanes are already being worked on, look for a
// lane in the transition range.
lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
} else {
// Old behavior that uses wipLanes to shift interleaved updates into a
// separate lane. This is no longer needed because we put interleaved
// updates on a special queue.
switch (lanePriority) {
case NoLanePriority:
break;
case SyncLanePriority:
return SyncLane;
case SyncBatchedLanePriority:
return SyncBatchedLane;
case InputDiscreteLanePriority: {
const lane = pickArbitraryLane(InputDiscreteLanes & ~wipLanes);
if (lane === NoLane) {
// All the transition lanes are taken, too. This should be very
// rare, but as a last resort, pick a default lane. This will have
// the effect of interrupting the current work-in-progress render.
lane = pickArbitraryLane(DefaultLanes);
// Shift to the next priority level
return findUpdateLane(InputContinuousLanePriority, wipLanes);
}
return lane;
}
return lane;
}
case TransitionPriority: // Should be handled by findTransitionLane instead
case RetryLanePriority: // Should be handled by findRetryLane instead
break;
case IdleLanePriority:
let lane = pickArbitraryLane(IdleLanes & ~wipLanes);
if (lane === NoLane) {
lane = pickArbitraryLane(IdleLanes);
case InputContinuousLanePriority: {
const lane = pickArbitraryLane(InputContinuousLanes & ~wipLanes);
if (lane === NoLane) {
// Shift to the next priority level
return findUpdateLane(DefaultLanePriority, wipLanes);
}
return lane;
}
return lane;
default:
// The remaining priorities are not valid for updates
break;
case DefaultLanePriority: {
let lane = pickArbitraryLane(DefaultLanes & ~wipLanes);
if (lane === NoLane) {
// If all the default lanes are already being worked on, look for a
// lane in the transition range.
lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
if (lane === NoLane) {
// All the transition lanes are taken, too. This should be very
// rare, but as a last resort, pick a default lane. This will have
// the effect of interrupting the current work-in-progress render.
lane = pickArbitraryLane(DefaultLanes);
}
}
return lane;
}
case TransitionPriority: // Should be handled by findTransitionLane instead
case RetryLanePriority: // Should be handled by findRetryLane instead
break;
case IdleLanePriority:
let lane = pickArbitraryLane(IdleLanes & ~wipLanes);
if (lane === NoLane) {
lane = pickArbitraryLane(IdleLanes);
}
return lane;
default:
// The remaining priorities are not valid for updates
break;
}
}
invariant(
false,
Expand All @@ -563,7 +599,7 @@ export function claimNextTransitionLane(): Lane {
const lane = nextTransitionLane;
nextTransitionLane <<= 1;
if ((nextTransitionLane & TransitionLanes) === 0) {
nextTransitionLane = SomeTransitionLane;
nextTransitionLane = FirstTransitionLane;
}
return lane;
}
Expand All @@ -572,7 +608,7 @@ export function claimNextRetryLane(): Lane {
const lane = nextRetryLane;
nextRetryLane <<= 1;
if ((nextRetryLane & RetryLanes) === 0) {
nextRetryLane = SomeRetryLane;
nextRetryLane = FirstRetryLane;
}
return lane;
}
Expand Down
Loading

0 comments on commit 998cbb5

Please sign in to comment.