Skip to content

Commit d3933f2

Browse files
acdliteacdlite
committed
Don't shift interleaved updates to separate lane
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.
1 parent a014c91 commit d3933f2

15 files changed

+496
-354
lines changed

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2029,10 +2029,11 @@ function dispatchAction<S, A>(
20292029

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

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2029,10 +2029,11 @@ function dispatchAction<S, A>(
20292029

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

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

Lines changed: 84 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,10 @@ export type Lane = number;
3636
export type LaneMap<T> = Array<T>;
3737

3838
import invariant from 'shared/invariant';
39-
import {enableCache} from 'shared/ReactFeatureFlags';
39+
import {
40+
enableCache,
41+
enableTransitionEntanglement,
42+
} from 'shared/ReactFeatureFlags';
4043

4144
import {
4245
ImmediatePriority as ImmediateSchedulerPriority,
@@ -92,11 +95,12 @@ export const DefaultLanes: Lanes = /* */ 0b0000000000000000000
9295

9396
const TransitionHydrationLane: Lane = /* */ 0b0000000000000000001000000000000;
9497
const TransitionLanes: Lanes = /* */ 0b0000000001111111110000000000000;
95-
const SomeTransitionLane: Lane = /* */ 0b0000000000000000010000000000000;
98+
const FirstTransitionLane: Lane = /* */ 0b0000000000000000010000000000000;
9699

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

99-
export const SomeRetryLane: Lanes = /* */ 0b0000010000000000000000000000000;
102+
const FirstRetryLane: Lanes = /* */ 0b0000000010000000000000000000000;
103+
export const SomeRetryLane = FirstRetryLane;
100104

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

@@ -111,7 +115,7 @@ export const NoTimestamp = -1;
111115

112116
let currentUpdateLanePriority: LanePriority = NoLanePriority;
113117

114-
let nextTransitionLane: Lane = SomeTransitionLane;
118+
let nextTransitionLane: Lane = FirstTransitionLane;
115119
let nextRetryLane: Lane = SomeRetryLane;
116120

117121
export function getCurrentUpdateLanePriority(): LanePriority {
@@ -498,56 +502,88 @@ export function findUpdateLane(
498502
lanePriority: LanePriority,
499503
wipLanes: Lanes,
500504
): Lane {
501-
switch (lanePriority) {
502-
case NoLanePriority:
503-
break;
504-
case SyncLanePriority:
505-
return SyncLane;
506-
case SyncBatchedLanePriority:
507-
return SyncBatchedLane;
508-
case InputDiscreteLanePriority: {
509-
const lane = pickArbitraryLane(InputDiscreteLanes & ~wipLanes);
510-
if (lane === NoLane) {
511-
// Shift to the next priority level
512-
return findUpdateLane(InputContinuousLanePriority, wipLanes);
505+
if (enableTransitionEntanglement) {
506+
// Ignore wipLanes. Always assign to the same bit per priority.
507+
switch (lanePriority) {
508+
case NoLanePriority:
509+
break;
510+
case SyncLanePriority:
511+
return SyncLane;
512+
case SyncBatchedLanePriority:
513+
return SyncBatchedLane;
514+
case InputDiscreteLanePriority: {
515+
return pickArbitraryLane(InputDiscreteLanes);
513516
}
514-
return lane;
515-
}
516-
case InputContinuousLanePriority: {
517-
const lane = pickArbitraryLane(InputContinuousLanes & ~wipLanes);
518-
if (lane === NoLane) {
519-
// Shift to the next priority level
520-
return findUpdateLane(DefaultLanePriority, wipLanes);
517+
case InputContinuousLanePriority: {
518+
return pickArbitraryLane(InputContinuousLanes);
519+
}
520+
case DefaultLanePriority: {
521+
return pickArbitraryLane(DefaultLanes);
521522
}
522-
return lane;
523+
case TransitionPriority: // Should be handled by findTransitionLane instead
524+
case RetryLanePriority: // Should be handled by findRetryLane instead
525+
break;
526+
case IdleLanePriority:
527+
return pickArbitraryLane(IdleLanes);
528+
default:
529+
// The remaining priorities are not valid for updates
530+
break;
523531
}
524-
case DefaultLanePriority: {
525-
let lane = pickArbitraryLane(DefaultLanes & ~wipLanes);
526-
if (lane === NoLane) {
527-
// If all the default lanes are already being worked on, look for a
528-
// lane in the transition range.
529-
lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
532+
} else {
533+
// Old behavior that uses wipLanes to shift interleaved updates into a
534+
// separate lane. This is no longer needed because we put interleaved
535+
// updates on a special queue.
536+
switch (lanePriority) {
537+
case NoLanePriority:
538+
break;
539+
case SyncLanePriority:
540+
return SyncLane;
541+
case SyncBatchedLanePriority:
542+
return SyncBatchedLane;
543+
case InputDiscreteLanePriority: {
544+
const lane = pickArbitraryLane(InputDiscreteLanes & ~wipLanes);
530545
if (lane === NoLane) {
531-
// All the transition lanes are taken, too. This should be very
532-
// rare, but as a last resort, pick a default lane. This will have
533-
// the effect of interrupting the current work-in-progress render.
534-
lane = pickArbitraryLane(DefaultLanes);
546+
// Shift to the next priority level
547+
return findUpdateLane(InputContinuousLanePriority, wipLanes);
535548
}
549+
return lane;
536550
}
537-
return lane;
538-
}
539-
case TransitionPriority: // Should be handled by findTransitionLane instead
540-
case RetryLanePriority: // Should be handled by findRetryLane instead
541-
break;
542-
case IdleLanePriority:
543-
let lane = pickArbitraryLane(IdleLanes & ~wipLanes);
544-
if (lane === NoLane) {
545-
lane = pickArbitraryLane(IdleLanes);
551+
case InputContinuousLanePriority: {
552+
const lane = pickArbitraryLane(InputContinuousLanes & ~wipLanes);
553+
if (lane === NoLane) {
554+
// Shift to the next priority level
555+
return findUpdateLane(DefaultLanePriority, wipLanes);
556+
}
557+
return lane;
546558
}
547-
return lane;
548-
default:
549-
// The remaining priorities are not valid for updates
550-
break;
559+
case DefaultLanePriority: {
560+
let lane = pickArbitraryLane(DefaultLanes & ~wipLanes);
561+
if (lane === NoLane) {
562+
// If all the default lanes are already being worked on, look for a
563+
// lane in the transition range.
564+
lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
565+
if (lane === NoLane) {
566+
// All the transition lanes are taken, too. This should be very
567+
// rare, but as a last resort, pick a default lane. This will have
568+
// the effect of interrupting the current work-in-progress render.
569+
lane = pickArbitraryLane(DefaultLanes);
570+
}
571+
}
572+
return lane;
573+
}
574+
case TransitionPriority: // Should be handled by findTransitionLane instead
575+
case RetryLanePriority: // Should be handled by findRetryLane instead
576+
break;
577+
case IdleLanePriority:
578+
let lane = pickArbitraryLane(IdleLanes & ~wipLanes);
579+
if (lane === NoLane) {
580+
lane = pickArbitraryLane(IdleLanes);
581+
}
582+
return lane;
583+
default:
584+
// The remaining priorities are not valid for updates
585+
break;
586+
}
551587
}
552588
invariant(
553589
false,
@@ -563,7 +599,7 @@ export function claimNextTransitionLane(): Lane {
563599
const lane = nextTransitionLane;
564600
nextTransitionLane <<= 1;
565601
if ((nextTransitionLane & TransitionLanes) === 0) {
566-
nextTransitionLane = SomeTransitionLane;
602+
nextTransitionLane = FirstTransitionLane;
567603
}
568604
return lane;
569605
}

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

Lines changed: 79 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,10 @@ export type Lane = number;
3636
export type LaneMap<T> = Array<T>;
3737

3838
import invariant from 'shared/invariant';
39-
import {enableCache} from 'shared/ReactFeatureFlags';
39+
import {
40+
enableCache,
41+
enableTransitionEntanglement,
42+
} from 'shared/ReactFeatureFlags';
4043

4144
import {
4245
ImmediatePriority as ImmediateSchedulerPriority,
@@ -498,56 +501,88 @@ export function findUpdateLane(
498501
lanePriority: LanePriority,
499502
wipLanes: Lanes,
500503
): Lane {
501-
switch (lanePriority) {
502-
case NoLanePriority:
503-
break;
504-
case SyncLanePriority:
505-
return SyncLane;
506-
case SyncBatchedLanePriority:
507-
return SyncBatchedLane;
508-
case InputDiscreteLanePriority: {
509-
const lane = pickArbitraryLane(InputDiscreteLanes & ~wipLanes);
510-
if (lane === NoLane) {
511-
// Shift to the next priority level
512-
return findUpdateLane(InputContinuousLanePriority, wipLanes);
504+
if (enableTransitionEntanglement) {
505+
// Ignore wipLanes. Always assign to the same bit per priority.
506+
switch (lanePriority) {
507+
case NoLanePriority:
508+
break;
509+
case SyncLanePriority:
510+
return SyncLane;
511+
case SyncBatchedLanePriority:
512+
return SyncBatchedLane;
513+
case InputDiscreteLanePriority: {
514+
return pickArbitraryLane(InputDiscreteLanes);
513515
}
514-
return lane;
515-
}
516-
case InputContinuousLanePriority: {
517-
const lane = pickArbitraryLane(InputContinuousLanes & ~wipLanes);
518-
if (lane === NoLane) {
519-
// Shift to the next priority level
520-
return findUpdateLane(DefaultLanePriority, wipLanes);
516+
case InputContinuousLanePriority: {
517+
return pickArbitraryLane(InputContinuousLanes);
518+
}
519+
case DefaultLanePriority: {
520+
return pickArbitraryLane(DefaultLanes);
521521
}
522-
return lane;
522+
case TransitionPriority: // Should be handled by findTransitionLane instead
523+
case RetryLanePriority: // Should be handled by findRetryLane instead
524+
break;
525+
case IdleLanePriority:
526+
return pickArbitraryLane(IdleLanes);
527+
default:
528+
// The remaining priorities are not valid for updates
529+
break;
523530
}
524-
case DefaultLanePriority: {
525-
let lane = pickArbitraryLane(DefaultLanes & ~wipLanes);
526-
if (lane === NoLane) {
527-
// If all the default lanes are already being worked on, look for a
528-
// lane in the transition range.
529-
lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
531+
} else {
532+
// Old behavior that uses wipLanes to shift interleaved updates into a
533+
// separate lane. This is no longer needed because we put interleaved
534+
// updates on a special queue.
535+
switch (lanePriority) {
536+
case NoLanePriority:
537+
break;
538+
case SyncLanePriority:
539+
return SyncLane;
540+
case SyncBatchedLanePriority:
541+
return SyncBatchedLane;
542+
case InputDiscreteLanePriority: {
543+
const lane = pickArbitraryLane(InputDiscreteLanes & ~wipLanes);
530544
if (lane === NoLane) {
531-
// All the transition lanes are taken, too. This should be very
532-
// rare, but as a last resort, pick a default lane. This will have
533-
// the effect of interrupting the current work-in-progress render.
534-
lane = pickArbitraryLane(DefaultLanes);
545+
// Shift to the next priority level
546+
return findUpdateLane(InputContinuousLanePriority, wipLanes);
535547
}
548+
return lane;
536549
}
537-
return lane;
538-
}
539-
case TransitionPriority: // Should be handled by findTransitionLane instead
540-
case RetryLanePriority: // Should be handled by findRetryLane instead
541-
break;
542-
case IdleLanePriority:
543-
let lane = pickArbitraryLane(IdleLanes & ~wipLanes);
544-
if (lane === NoLane) {
545-
lane = pickArbitraryLane(IdleLanes);
550+
case InputContinuousLanePriority: {
551+
const lane = pickArbitraryLane(InputContinuousLanes & ~wipLanes);
552+
if (lane === NoLane) {
553+
// Shift to the next priority level
554+
return findUpdateLane(DefaultLanePriority, wipLanes);
555+
}
556+
return lane;
546557
}
547-
return lane;
548-
default:
549-
// The remaining priorities are not valid for updates
550-
break;
558+
case DefaultLanePriority: {
559+
let lane = pickArbitraryLane(DefaultLanes & ~wipLanes);
560+
if (lane === NoLane) {
561+
// If all the default lanes are already being worked on, look for a
562+
// lane in the transition range.
563+
lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
564+
if (lane === NoLane) {
565+
// All the transition lanes are taken, too. This should be very
566+
// rare, but as a last resort, pick a default lane. This will have
567+
// the effect of interrupting the current work-in-progress render.
568+
lane = pickArbitraryLane(DefaultLanes);
569+
}
570+
}
571+
return lane;
572+
}
573+
case TransitionPriority: // Should be handled by findTransitionLane instead
574+
case RetryLanePriority: // Should be handled by findRetryLane instead
575+
break;
576+
case IdleLanePriority:
577+
let lane = pickArbitraryLane(IdleLanes & ~wipLanes);
578+
if (lane === NoLane) {
579+
lane = pickArbitraryLane(IdleLanes);
580+
}
581+
return lane;
582+
default:
583+
// The remaining priorities are not valid for updates
584+
break;
585+
}
551586
}
552587
invariant(
553588
false,

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
disableSchedulerTimeoutInWorkLoop,
3535
enableDoubleInvokingEffects,
3636
skipUnmountedBoundaries,
37+
enableTransitionEntanglement,
3738
} from 'shared/ReactFeatureFlags';
3839
import ReactSharedInternals from 'shared/ReactSharedInternals';
3940
import invariant from 'shared/invariant';
@@ -832,6 +833,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
832833
let exitStatus = renderRootConcurrent(root, lanes);
833834

834835
if (
836+
!enableTransitionEntanglement &&
835837
includesSomeLane(
836838
workInProgressRootIncludedLanes,
837839
workInProgressRootUpdatedLanes,
@@ -1037,6 +1039,7 @@ function performSyncWorkOnRoot(root) {
10371039
lanes = workInProgressRootRenderLanes;
10381040
exitStatus = renderRootSync(root, lanes);
10391041
if (
1042+
!enableTransitionEntanglement &&
10401043
includesSomeLane(
10411044
workInProgressRootIncludedLanes,
10421045
workInProgressRootUpdatedLanes,

0 commit comments

Comments
 (0)