@samitouri / QOS-React-2 / commits / 965d08cfff

Add dedicated LanePriority for Suspense retries (#19287)

A "retry" is a special type of update that attempts to flip a Suspense boundary from its placeholder state back to its primary/resolved state. Currently, retries are given default priority, using the same algorithm as normal updates and occupying range of lanes. This adds a new range of lanes dedicated specifically to retries, and gives them lower priority than normal updates. A couple of existing tests were affected because retries are no longer batched with normal updates; they commit in separate batches. Not totally satisfied with this design, but I think things will snap more into place once the rest of the Lanes changes (like how we handle parallel Suspense transitions) have settled.

Andrew Clark committed Jul 8, 2020 at 23:09 UTC 965d08cfff50609eed5d3f2317145f7be762423d

6 files changed +181 -35

packages/react-reconciler/src/ReactFiberLane.js

+62 -26

@@ -26,7 +26,8 @@ export opaque type LanePriority =

26 | 13

27 | 14

28 | 15

29 - | 16;

29 + | 16

30 + | 17;

31 export opaque type Lanes = number;

32 export opaque type Lane = number;

33 export opaque type LaneMap<T> = Array<T>;

@@ -42,23 +43,25 @@ import {

43 NoPriority as NoSchedulerPriority,

44 } from './SchedulerWithReactIntegration.new';

45 -export const SyncLanePriority: LanePriority = 16;

46 -const SyncBatchedLanePriority: LanePriority = 15;

46 +export const SyncLanePriority: LanePriority = 17;

47 +const SyncBatchedLanePriority: LanePriority = 16;

48 -const InputDiscreteHydrationLanePriority: LanePriority = 14;

49 -export const InputDiscreteLanePriority: LanePriority = 13;

49 +const InputDiscreteHydrationLanePriority: LanePriority = 15;

50 +export const InputDiscreteLanePriority: LanePriority = 14;

51 -const InputContinuousHydrationLanePriority: LanePriority = 12;

52 -export const InputContinuousLanePriority: LanePriority = 11;

52 +const InputContinuousHydrationLanePriority: LanePriority = 13;

53 +export const InputContinuousLanePriority: LanePriority = 12;

54 -const DefaultHydrationLanePriority: LanePriority = 10;

55 -export const DefaultLanePriority: LanePriority = 9;

55 +const DefaultHydrationLanePriority: LanePriority = 11;

56 +export const DefaultLanePriority: LanePriority = 10;

57 -const TransitionShortHydrationLanePriority: LanePriority = 8;

58 -export const TransitionShortLanePriority: LanePriority = 7;

58 +const TransitionShortHydrationLanePriority: LanePriority = 9;

59 +export const TransitionShortLanePriority: LanePriority = 8;

60 -const TransitionLongHydrationLanePriority: LanePriority = 6;

61 -export const TransitionLongLanePriority: LanePriority = 5;

61 +const TransitionLongHydrationLanePriority: LanePriority = 7;

62 +export const TransitionLongLanePriority: LanePriority = 6;

63 +

64 +const RetryLanePriority: LanePriority = 5;

66 const SelectiveHydrationLanePriority: LanePriority = 4;

@@ -90,25 +93,30 @@ const InputContinuousUpdateRangeStart = 6;

93 const InputContinuousUpdateRangeEnd = 8;

95 export const DefaultHydrationLane: Lane = /* */ 0b0000000000000000000000100000000;

93 -const DefaultLanes: Lanes = /* */ 0b0000000000000000011111100000000;

96 +export const DefaultLanes: Lanes = /* */ 0b0000000000000000000111100000000;

97 const DefaultUpdateRangeStart = 9;

95 -const DefaultUpdateRangeEnd = 14;

98 +const DefaultUpdateRangeEnd = 12;

99 +

100 +const TransitionShortHydrationLane: Lane = /* */ 0b0000000000000000001000000000000;

101 +const TransitionShortLanes: Lanes = /* */ 0b0000000000000011111000000000000;

102 +const TransitionShortUpdateRangeStart = 13;

103 +const TransitionShortUpdateRangeEnd = 17;

104

97 -const TransitionShortHydrationLane: Lane = /* */ 0b0000000000000000100000000000000;

98 -const TransitionShortLanes: Lanes = /* */ 0b0000000000011111100000000000000;

99 -const TransitionShortUpdateRangeStart = 15;

100 -const TransitionShortUpdateRangeEnd = 20;

105 +const TransitionLongHydrationLane: Lane = /* */ 0b0000000000000100000000000000000;

106 +const TransitionLongLanes: Lanes = /* */ 0b0000000001111100000000000000000;

107 +const TransitionLongUpdateRangeStart = 18;

108 +const TransitionLongUpdateRangeEnd = 22;

109

102 -const TransitionLongHydrationLane: Lane = /* */ 0b0000000000100000000000000000000;

103 -const TransitionLongLanes: Lanes = /* */ 0b0000011111100000000000000000000;

104 -const TransitionLongUpdateRangeStart = 21;

105 -const TransitionLongUpdateRangeEnd = 26;

110 +// Includes all updates. Except Idle updates, which have special semantics.

111 +const UpdateRangeEnd = TransitionLongUpdateRangeEnd;

112

107 -export const SelectiveHydrationLane: Lane = /* */ 0b0000110000000000000000000000000;

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

114 +const RetryRangeStart = 22;

115 +const RetryRangeEnd = 26;

116 +

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

118 const SelectiveHydrationRangeEnd = 27;

119

110 -// Includes all non-Idle updates

111 -const UpdateRangeEnd = 27;

120 const NonIdleLanes = /* */ 0b0000111111111111111111111111111;

121

122 export const IdleHydrationLane: Lane = /* */ 0b0001000000000000000000000000000;

@@ -206,6 +214,12 @@ function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes {

214 return transitionLongLanes;

215 }

216 }

217 + const retryLanes = RetryLanes & lanes;

218 + if (retryLanes !== NoLanes) {

219 + return_highestLanePriority = RetryLanePriority;

220 + return_updateRangeEnd = RetryRangeEnd;

221 + return retryLanes;

222 + }

223 if (lanes & SelectiveHydrationLane) {

224 return_highestLanePriority = SelectiveHydrationLanePriority;

225 return_updateRangeEnd = SelectiveHydrationRangeEnd;

@@ -275,6 +289,7 @@ export function lanePriorityToSchedulerPriority(

289 case TransitionLongHydrationLanePriority:

290 case TransitionLongLanePriority:

291 case SelectiveHydrationLanePriority:

292 + case RetryLanePriority:

293 return NormalSchedulerPriority;

294 case IdleHydrationLanePriority:

295 case IdleLanePriority:

@@ -537,6 +552,9 @@ export function findUpdateLane(

552 case TransitionLongLanePriority:

553 // Should be handled by findTransitionLane instead

554 break;

555 + case RetryLanePriority:

556 + // Should be handled by findRetryLane instead

557 + break;

558 case IdleLanePriority:

559 let lane = findLane(IdleUpdateRangeStart, IdleUpdateRangeEnd, wipLanes);

560 if (lane === NoLane) {

@@ -604,6 +622,19 @@ export function findTransitionLane(

622 );

623 }

624

625 +// To ensure consistency across multiple updates in the same event, this should

626 +// be pure function, so that it always returns the same lane for given inputs.

627 +export function findRetryLane(wipLanes: Lanes): Lane {

628 + // This is a fork of `findUpdateLane` designed specifically for Suspense

629 + // "retries" — a special update that attempts to flip a Suspense boundary

630 + // from its placeholder state to its primary/resolved state.

631 + let lane = findLane(RetryRangeStart, RetryRangeEnd, wipLanes);

632 + if (lane === NoLane) {

633 + lane = pickArbitraryLane(RetryLanes);

634 + }

635 + return lane;

636 +}

637 +

638 function findLane(start, end, skipLanes) {

639 // This finds the first bit between the `start` and `end` positions that isn't

640 // in `skipLanes`.

@@ -808,6 +839,11 @@ export function getBumpedLaneForHydration(

839 case TransitionLongLanePriority:

840 lane = TransitionLongHydrationLane;

841 break;

842 + case RetryLanePriority:

843 + // Shouldn't be reachable under normal circumstances, so there's no

844 + // dedicated lane for retry priority. Use the one for long transitions.

845 + lane = TransitionLongHydrationLane;

846 + break;

847 case SelectiveHydrationLanePriority:

848 lane = SelectiveHydrationLane;

849 break;

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

+24 -4

@@ -125,6 +125,7 @@ import {

125 NoTimestamp,

126 findUpdateLane,

127 findTransitionLane,

128 + findRetryLane,

129 includesSomeLane,

130 isSubsetOfLanes,

131 mergeLanes,

@@ -472,6 +473,28 @@ export function requestUpdateLane(

473 return lane;

474 }

475

476 +function requestRetryLane(fiber: Fiber) {

477 + // This is a fork of `requestUpdateLane` designed specifically for Suspense

478 + // "retries" — a special update that attempts to flip a Suspense boundary

479 + // from its placeholder state to its primary/resolved state.

480 +

481 + // Special cases

482 + const mode = fiber.mode;

483 + if ((mode & BlockingMode) === NoMode) {

484 + return (SyncLane: Lane);

485 + } else if ((mode & ConcurrentMode) === NoMode) {

486 + return getCurrentPriorityLevel() === ImmediateSchedulerPriority

487 + ? (SyncLane: Lane)

488 + : (SyncBatchedLane: Lane);

489 + }

490 +

491 + // See `requestUpdateLane` for explanation of `currentEventWipLanes`

492 + if (currentEventWipLanes === NoLanes) {

493 + currentEventWipLanes = workInProgressRootIncludedLanes;

494 + }

495 + return findRetryLane(currentEventWipLanes);

496 +}

497 +

498 export function scheduleUpdateOnFiber(

499 fiber: Fiber,

500 lane: Lane,

@@ -2708,10 +2731,7 @@ function retryTimedOutBoundary(boundaryFiber: Fiber, retryLane: Lane) {

2731 // suspended it has resolved, which means at least part of the tree was

2732 // likely unblocked. Try rendering again, at a new expiration time.

2733 if (retryLane === NoLane) {

2711 - const suspenseConfig = null; // Retries don't carry over the already committed update.

2712 - // TODO: Should retries get their own lane? Maybe it could share with

2713 - // transitions.

2714 - retryLane = requestUpdateLane(boundaryFiber, suspenseConfig);

2734 + retryLane = requestRetryLane(boundaryFiber);

2735 }

2736 // TODO: Special case idle priority?

2737 const eventTime = requestEventTime();

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

+24 -4

@@ -148,6 +148,7 @@ import {

148 NoTimestamp,

149 findUpdateLane,

150 findTransitionLane,

151 + findRetryLane,

152 includesSomeLane,

153 isSubsetOfLanes,

154 mergeLanes,

@@ -495,6 +496,28 @@ export function requestUpdateLane(

496 return lane;

497 }

498

499 +function requestRetryLane(fiber: Fiber) {

500 + // This is a fork of `requestUpdateLane` designed specifically for Suspense

501 + // "retries" — a special update that attempts to flip a Suspense boundary

502 + // from its placeholder state to its primary/resolved state.

503 +

504 + // Special cases

505 + const mode = fiber.mode;

506 + if ((mode & BlockingMode) === NoMode) {

507 + return (SyncLane: Lane);

508 + } else if ((mode & ConcurrentMode) === NoMode) {

509 + return getCurrentPriorityLevel() === ImmediateSchedulerPriority

510 + ? (SyncLane: Lane)

511 + : (SyncBatchedLane: Lane);

512 + }

513 +

514 + // See `requestUpdateLane` for explanation of `currentEventWipLanes`

515 + if (currentEventWipLanes === NoLanes) {

516 + currentEventWipLanes = workInProgressRootIncludedLanes;

517 + }

518 + return findRetryLane(currentEventWipLanes);

519 +}

520 +

521 export function scheduleUpdateOnFiber(

522 fiber: Fiber,

523 lane: Lane,

@@ -2855,10 +2878,7 @@ function retryTimedOutBoundary(boundaryFiber: Fiber, retryLane: Lane) {

2878 // suspended it has resolved, which means at least part of the tree was

2879 // likely unblocked. Try rendering again, at a new expiration time.

2880 if (retryLane === NoLane) {

2858 - const suspenseConfig = null; // Retries don't carry over the already committed update.

2859 - // TODO: Should retries get their own lane? Maybe it could share with

2860 - // transitions.

2861 - retryLane = requestUpdateLane(boundaryFiber, suspenseConfig);

2881 + retryLane = requestRetryLane(boundaryFiber);

2882 }

2883 // TODO: Special case idle priority?

2884 const eventTime = requestEventTime();

packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js

@@ -219,6 +219,11 @@ describe('ReactSuspense', () => {

219 await resolve1();

220 ReactNoop.render(element);

221 expect(Scheduler).toFlushWithoutYielding();

222 +

223 + // Force fallback to commit.

224 + // TODO: Should be able to use `act` here.

225 + jest.runAllTimers();

226 +

227 expect(ReactNoop.getChildren()).toEqual([

228 text('Waiting Tier 2'),

229 text('Done'),

packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js

+66

@@ -623,6 +623,17 @@ describe('ReactSuspenseWithNoopRenderer', () => {

623 // The new reconciler batches everything together, so it finishes without

624 // suspending again.

625 'Sibling',

626 +

627 + // NOTE: The final of the update got pushed into a lower priority range of

628 + // lanes, leading to the extra intermediate render. This is because when

629 + // we schedule the fourth update, we're already in the middle of rendering

630 + // the three others. Since there are only three lanes in the default

631 + // range, the fourth lane is shifted to slightly lower priority. This

632 + // could easily change when we tweak our batching heuristics. Ideally,

633 + // they'd all have default priority and render in a single batch.

634 + 'Suspend! [Step 3]',

635 + 'Sibling',

636 +

637 'Step 4',

638 ]);

639 });

@@ -3896,4 +3907,59 @@ describe('ReactSuspenseWithNoopRenderer', () => {

3907 expect(Scheduler).toHaveYielded(['Promise resolved [B]', 'B']);

3908 expect(root).toMatchRenderedOutput();

3909 });

3910 +

3911 + it('retries have lower priority than normal updates', async () => {

3912 + const {useState} = React;

3913 +

3914 + let setText;

3915 + function UpdatingText() {

3916 + const [text, _setText] = useState('A');

3917 + setText = _setText;

3918 + return <Text text={text} />;

3919 + }

3920 +

3921 + const root = ReactNoop.createRoot();

3922 + await ReactNoop.act(async () => {

3923 + root.render(

3924 + <>

3925 + <UpdatingText />

3926 + <Suspense fallback={<Text text="Loading..." />}>

3927 + <AsyncText text="Async" />

3928 + </Suspense>

3929 + </>,

3930 + );

3931 + });

3932 + expect(Scheduler).toHaveYielded(['A', 'Suspend! [Async]', 'Loading...']);

3933 + expect(root).toMatchRenderedOutput(

3934 + <>

3935 +

3936 +

3937 + </>,

3938 + );

3939 +

3940 + await ReactNoop.act(async () => {

3941 + // Resolve the promise. This will trigger a retry.

3942 + await resolveText('Async');

3943 + expect(Scheduler).toHaveYielded(['Promise resolved [Async]']);

3944 + // Before the retry happens, schedule a new update.

3945 + setText('B');

3946 +

3947 + // The update should be allowed to finish before the retry is attempted.

3948 + expect(Scheduler).toFlushUntilNextPaint(['B']);

3949 + expect(root).toMatchRenderedOutput(

3950 + <>

3951 +

3952 +

3953 + </>,

3954 + );

3955 + });

3956 + // Then do the retry.

3957 + expect(Scheduler).toHaveYielded(['Async']);

3958 + expect(root).toMatchRenderedOutput(

3959 + <>

3960 +

3961 +

3962 + </>,

3963 + );

3964 + });

3965 });

packages/react/src/__tests__/ReactProfiler-test.internal.js

-1

@@ -3923,7 +3923,6 @@ describe('Profiler', () => {

3923

3924 expect(onPostCommit).toHaveBeenCalledTimes(1);

3925 expect(onPostCommit.mock.calls[0][4]).toMatchInteractions([

3926 - initialRenderInteraction,

3926 highPriUpdateInteraction,

3927 ]);

3928