@samitouri / QOS-React-2 / commits / 48740429b4

Expiration: Do nothing except disable time slicing (#21345)

We have a feature called "expiration" whose purpose is to prevent a concurrent update from being starved by higher priority events. If a lane is CPU-bound for too long, we finish the rest of the work synchronously without allowing further interruptions. In the current implementation, we do this in sort of a roundabout way: once a lane is determined to have expired, we entangle it with SyncLane and switch to the synchronous work loop. There are a few flaws with the approach. One is that SyncLane has a particular semantic meaning besides its non-yieldiness. For example, `flushSync` will force remaining Sync work to finish; currently, that also includes expired work, which isn't an intended behavior, but rather an artifact of the implementation. An event worse example is that passive effects triggered by a Sync update are flushed synchronously, before paint, so that its result is guaranteed to be observed by the next discrete event. But expired work has no such requirement: we're flushing expired effects before paint unnecessarily. Aside from the behaviorial implications, the current implementation has proven to be fragile: more than once, we've accidentally regressed performance due to a subtle change in how expiration is handled. This PR aims to radically simplify how we model starvation protection by scaling back the implementation as much as possible. In this new model, if a lane is expired, we disable time slicing. That's it. We don't entangle it with SyncLane. The only thing we do is skip the call to `shouldYield` in between each time slice. This is identical to how we model synchronous-by-default updates in React 18.

Andrew Clark committed Apr 24, 2021 at 18:32 UTC 48740429b4a74e984193e4e2d364d461e4fdc3be
10 files changed +213 -294
packages/react-reconciler/src/ReactFiberLane.new.js
+16 -22
@@ -401,7 +401,6 @@ export function markStarvedLanesAsExpired(
401 // expiration time. If so, we'll assume the update is being starved and mark
402 // it as expired to force it to finish.
403 let lanes = pendingLanes;
404 - let expiredLanes = 0;
404 while (lanes > 0) {
405 const index = pickArbitraryLaneIndex(lanes);
406 const lane = 1 << index;
@@ -420,15 +419,11 @@ export function markStarvedLanesAsExpired(
419 }
420 } else if (expirationTime <= currentTime) {
421 // This lane expired
423 - expiredLanes |= lane;
422 + root.expiredLanes |= lane;
423 }
424
425 lanes &= ~lane;
426 }
428 -
429 - if (expiredLanes !== 0) {
430 - markRootExpired(root, expiredLanes);
431 - }
427 }
428
429 // This returns the highest priority pending lanes regardless of whether they
@@ -459,16 +454,22 @@ export function includesOnlyTransitions(lanes: Lanes) {
454 }
455
456 export function shouldTimeSlice(root: FiberRoot, lanes: Lanes) {
462 - if (!enableSyncDefaultUpdates) {
457 + if ((lanes & root.expiredLanes) !== NoLanes) {
458 + // At least one of these lanes expired. To prevent additional starvation,
459 + // finish rendering without yielding execution.
460 + return false;
461 + }
462 + if (enableSyncDefaultUpdates) {
463 + const SyncDefaultLanes =
464 + InputContinuousHydrationLane |
465 + InputContinuousLane |
466 + DefaultHydrationLane |
467 + DefaultLane;
468 + // TODO: Check for root override, once that lands
469 + return (lanes & SyncDefaultLanes) === NoLanes;
470 + } else {
471 return true;
472 }
465 - const SyncDefaultLanes =
466 - InputContinuousHydrationLane |
467 - InputContinuousLane |
468 - DefaultHydrationLane |
469 - DefaultLane;
470 - // TODO: Check for root override, once that lands
471 - return (lanes & SyncDefaultLanes) === NoLanes;
473 }
474
475 export function isTransitionLane(lane: Lane) {
@@ -613,14 +614,6 @@ export function markRootPinged(
614 root.pingedLanes |= root.suspendedLanes & pingedLanes;
615 }
616
616 -export function markRootExpired(root: FiberRoot, expiredLanes: Lanes) {
617 - const entanglements = root.entanglements;
618 - const SyncLaneIndex = 0;
619 - entanglements[SyncLaneIndex] |= expiredLanes;
620 - root.entangledLanes |= SyncLane;
621 - root.pendingLanes |= SyncLane;
622 -}
623 -
617 export function markRootMutableRead(root: FiberRoot, updateLane: Lane) {
618 root.mutableReadLanes |= updateLane & root.pendingLanes;
619 }
@@ -634,6 +627,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
627 root.suspendedLanes = 0;
628 root.pingedLanes = 0;
629
630 + root.expiredLanes &= remainingLanes;
631 root.mutableReadLanes &= remainingLanes;
632
633 root.entangledLanes &= remainingLanes;
packages/react-reconciler/src/ReactFiberLane.old.js
+16 -22
@@ -401,7 +401,6 @@ export function markStarvedLanesAsExpired(
401 // expiration time. If so, we'll assume the update is being starved and mark
402 // it as expired to force it to finish.
403 let lanes = pendingLanes;
404 - let expiredLanes = 0;
404 while (lanes > 0) {
405 const index = pickArbitraryLaneIndex(lanes);
406 const lane = 1 << index;
@@ -420,15 +419,11 @@ export function markStarvedLanesAsExpired(
419 }
420 } else if (expirationTime <= currentTime) {
421 // This lane expired
423 - expiredLanes |= lane;
422 + root.expiredLanes |= lane;
423 }
424
425 lanes &= ~lane;
426 }
428 -
429 - if (expiredLanes !== 0) {
430 - markRootExpired(root, expiredLanes);
431 - }
427 }
428
429 // This returns the highest priority pending lanes regardless of whether they
@@ -459,16 +454,22 @@ export function includesOnlyTransitions(lanes: Lanes) {
454 }
455
456 export function shouldTimeSlice(root: FiberRoot, lanes: Lanes) {
462 - if (!enableSyncDefaultUpdates) {
457 + if ((lanes & root.expiredLanes) !== NoLanes) {
458 + // At least one of these lanes expired. To prevent additional starvation,
459 + // finish rendering without yielding execution.
460 + return false;
461 + }
462 + if (enableSyncDefaultUpdates) {
463 + const SyncDefaultLanes =
464 + InputContinuousHydrationLane |
465 + InputContinuousLane |
466 + DefaultHydrationLane |
467 + DefaultLane;
468 + // TODO: Check for root override, once that lands
469 + return (lanes & SyncDefaultLanes) === NoLanes;
470 + } else {
471 return true;
472 }
465 - const SyncDefaultLanes =
466 - InputContinuousHydrationLane |
467 - InputContinuousLane |
468 - DefaultHydrationLane |
469 - DefaultLane;
470 - // TODO: Check for root override, once that lands
471 - return (lanes & SyncDefaultLanes) === NoLanes;
473 }
474
475 export function isTransitionLane(lane: Lane) {
@@ -613,14 +614,6 @@ export function markRootPinged(
614 root.pingedLanes |= root.suspendedLanes & pingedLanes;
615 }
616
616 -export function markRootExpired(root: FiberRoot, expiredLanes: Lanes) {
617 - const entanglements = root.entanglements;
618 - const SyncLaneIndex = 0;
619 - entanglements[SyncLaneIndex] |= expiredLanes;
620 - root.entangledLanes |= SyncLane;
621 - root.pendingLanes |= SyncLane;
622 -}
623 -
617 export function markRootMutableRead(root: FiberRoot, updateLane: Lane) {
618 root.mutableReadLanes |= updateLane & root.pendingLanes;
619 }
@@ -634,6 +627,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
627 root.suspendedLanes = 0;
628 root.pingedLanes = 0;
629
630 + root.expiredLanes &= remainingLanes;
631 root.mutableReadLanes &= remainingLanes;
632
633 root.entangledLanes &= remainingLanes;
packages/react-reconciler/src/ReactFiberRoot.new.js
+1
@@ -50,6 +50,7 @@ function FiberRootNode(containerInfo, tag, hydrate) {
50 this.pendingLanes = NoLanes;
51 this.suspendedLanes = NoLanes;
52 this.pingedLanes = NoLanes;
53 + this.expiredLanes = NoLanes;
54 this.mutableReadLanes = NoLanes;
55 this.finishedLanes = NoLanes;
56
packages/react-reconciler/src/ReactFiberRoot.old.js
+1
@@ -50,6 +50,7 @@ function FiberRootNode(containerInfo, tag, hydrate) {
50 this.pendingLanes = NoLanes;
51 this.suspendedLanes = NoLanes;
52 this.pingedLanes = NoLanes;
53 + this.expiredLanes = NoLanes;
54 this.mutableReadLanes = NoLanes;
55 this.finishedLanes = NoLanes;
56
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+11 -27
@@ -159,7 +159,7 @@ import {
159 markRootUpdated,
160 markRootSuspended as markRootSuspended_dontCallThisOneDirectly,
161 markRootPinged,
162 - markRootExpired,
162 + markRootEntangled,
163 markRootFinished,
164 getHighestPriorityLane,
165 addFiberToLanesMap,
@@ -787,22 +787,17 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
787 return null;
788 }
789
790 + // We disable time-slicing in some cases: if the work has been CPU-bound
791 + // for too long ("expired" work, to prevent starvation), or we're in
792 + // sync-updates-by-default mode.
793 // TODO: We only check `didTimeout` defensively, to account for a Scheduler
794 // bug we're still investigating. Once the bug in Scheduler is fixed,
795 // we can remove this, since we track expiration ourselves.
793 - if (!disableSchedulerTimeoutInWorkLoop && didTimeout) {
794 - // Something expired. Flush synchronously until there's no expired
795 - // work left.
796 - markRootExpired(root, lanes);
797 - // This will schedule a synchronous callback.
798 - ensureRootIsScheduled(root, now());
799 - return null;
800 - }
801 -
802 - let exitStatus = shouldTimeSlice(root, lanes)
803 - ? renderRootConcurrent(root, lanes)
804 - : // Time slicing is disabled for default updates in this root.
805 - renderRootSync(root, lanes);
796 + let exitStatus =
797 + shouldTimeSlice(root, lanes) &&
798 + (disableSchedulerTimeoutInWorkLoop || !didTimeout)
799 + ? renderRootConcurrent(root, lanes)
800 + : renderRootSync(root, lanes);
801 if (exitStatus !== RootIncomplete) {
802 if (exitStatus === RootErrored) {
803 executionContext |= RetryAfterError;
@@ -990,16 +985,7 @@ function performSyncWorkOnRoot(root) {
985 flushPassiveEffects();
986
987 let lanes = getNextLanes(root, NoLanes);
993 - if (includesSomeLane(lanes, SyncLane)) {
994 - if (
995 - root === workInProgressRoot &&
996 - includesSomeLane(lanes, workInProgressRootRenderLanes)
997 - ) {
998 - // There's a partial tree, and at least one of its lanes has expired. Finish
999 - // rendering it before rendering the rest of the expired work.
1000 - lanes = workInProgressRootRenderLanes;
1001 - }
1002 - } else {
988 + if (!includesSomeLane(lanes, SyncLane)) {
989 // There's no remaining sync work left.
990 ensureRootIsScheduled(root, now());
991 return null;
@@ -1052,11 +1038,9 @@ function performSyncWorkOnRoot(root) {
1038 return null;
1039 }
1040
1055 -// TODO: Do we still need this API? I think we can delete it. Was only used
1056 -// internally.
1041 export function flushRoot(root: FiberRoot, lanes: Lanes) {
1042 if (lanes !== NoLanes) {
1059 - markRootExpired(root, lanes);
1043 + markRootEntangled(root, mergeLanes(lanes, SyncLane));
1044 ensureRootIsScheduled(root, now());
1045 if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
1046 resetRenderTimer();
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+11 -27
@@ -159,7 +159,7 @@ import {
159 markRootUpdated,
160 markRootSuspended as markRootSuspended_dontCallThisOneDirectly,
161 markRootPinged,
162 - markRootExpired,
162 + markRootEntangled,
163 markRootFinished,
164 getHighestPriorityLane,
165 addFiberToLanesMap,
@@ -787,22 +787,17 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
787 return null;
788 }
789
790 + // We disable time-slicing in some cases: if the work has been CPU-bound
791 + // for too long ("expired" work, to prevent starvation), or we're in
792 + // sync-updates-by-default mode.
793 // TODO: We only check `didTimeout` defensively, to account for a Scheduler
794 // bug we're still investigating. Once the bug in Scheduler is fixed,
795 // we can remove this, since we track expiration ourselves.
793 - if (!disableSchedulerTimeoutInWorkLoop && didTimeout) {
794 - // Something expired. Flush synchronously until there's no expired
795 - // work left.
796 - markRootExpired(root, lanes);
797 - // This will schedule a synchronous callback.
798 - ensureRootIsScheduled(root, now());
799 - return null;
800 - }
801 -
802 - let exitStatus = shouldTimeSlice(root, lanes)
803 - ? renderRootConcurrent(root, lanes)
804 - : // Time slicing is disabled for default updates in this root.
805 - renderRootSync(root, lanes);
796 + let exitStatus =
797 + shouldTimeSlice(root, lanes) &&
798 + (disableSchedulerTimeoutInWorkLoop || !didTimeout)
799 + ? renderRootConcurrent(root, lanes)
800 + : renderRootSync(root, lanes);
801 if (exitStatus !== RootIncomplete) {
802 if (exitStatus === RootErrored) {
803 executionContext |= RetryAfterError;
@@ -990,16 +985,7 @@ function performSyncWorkOnRoot(root) {
985 flushPassiveEffects();
986
987 let lanes = getNextLanes(root, NoLanes);
993 - if (includesSomeLane(lanes, SyncLane)) {
994 - if (
995 - root === workInProgressRoot &&
996 - includesSomeLane(lanes, workInProgressRootRenderLanes)
997 - ) {
998 - // There's a partial tree, and at least one of its lanes has expired. Finish
999 - // rendering it before rendering the rest of the expired work.
1000 - lanes = workInProgressRootRenderLanes;
1001 - }
1002 - } else {
988 + if (!includesSomeLane(lanes, SyncLane)) {
989 // There's no remaining sync work left.
990 ensureRootIsScheduled(root, now());
991 return null;
@@ -1052,11 +1038,9 @@ function performSyncWorkOnRoot(root) {
1038 return null;
1039 }
1040
1055 -// TODO: Do we still need this API? I think we can delete it. Was only used
1056 -// internally.
1041 export function flushRoot(root: FiberRoot, lanes: Lanes) {
1042 if (lanes !== NoLanes) {
1059 - markRootExpired(root, lanes);
1043 + markRootEntangled(root, mergeLanes(lanes, SyncLane));
1044 ensureRootIsScheduled(root, now());
1045 if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
1046 resetRenderTimer();
packages/react-reconciler/src/ReactInternalTypes.js
+1
@@ -228,6 +228,7 @@ type BaseFiberRootProperties = {|
228 pendingLanes: Lanes,
229 suspendedLanes: Lanes,
230 pingedLanes: Lanes,
231 + expiredLanes: Lanes,
232 mutableReadLanes: Lanes,
233
234 finishedLanes: Lanes,
packages/react-reconciler/src/__tests__/ReactExpiration-test.js
+148 -168
@@ -15,6 +15,8 @@ let Scheduler;
15 let readText;
16 let resolveText;
17 let startTransition;
18 +let useState;
19 +let useEffect;
20
21 describe('ReactExpiration', () => {
22 beforeEach(() => {
@@ -24,6 +26,8 @@ describe('ReactExpiration', () => {
26 ReactNoop = require('react-noop-renderer');
27 Scheduler = require('scheduler');
28 startTransition = React.unstable_startTransition;
29 + useState = React.useState;
30 + useEffect = React.useEffect;
31
32 const textCache = new Map();
33
@@ -478,9 +482,7 @@ describe('ReactExpiration', () => {
482 });
483
484 // @gate experimental || !enableSyncDefaultUpdates
481 - it('prevents starvation by sync updates', async () => {
482 - const {useState} = React;
483 -
485 + it('prevents starvation by sync updates by disabling time slicing if too much time has elapsed', async () => {
486 let updateSyncPri;
487 let updateNormalPri;
488 function App() {
@@ -519,15 +521,17 @@ describe('ReactExpiration', () => {
521 }
522 expect(Scheduler).toFlushAndYieldThrough(['Sync pri: 0']);
523 updateSyncPri();
524 + expect(Scheduler).toHaveYielded(['Sync pri: 1', 'Normal pri: 0']);
525 +
526 + // The remaining work hasn't expired, so the render phase is time sliced.
527 + // In other words, we can flush just the first child without flushing
528 + // the rest.
529 + Scheduler.unstable_flushNumberOfYields(1);
530 + // Yield right after first child.
531 + expect(Scheduler).toHaveYielded(['Sync pri: 1']);
532 + // Now do the rest.
533 + expect(Scheduler).toFlushAndYield(['Normal pri: 1']);
534 });
523 - expect(Scheduler).toHaveYielded([
524 - // Interrupt high pri update to render sync update
525 - 'Sync pri: 1',
526 - 'Normal pri: 0',
527 - // Now render normal pri
528 - 'Sync pri: 1',
529 - 'Normal pri: 1',
530 - ]);
535 expect(root).toMatchRenderedOutput('Sync pri: 1, Normal pri: 1');
536
537 // Do the same thing, but starve the first update
@@ -547,22 +551,18 @@ describe('ReactExpiration', () => {
551 // starvation of normal priority updates.)
552 Scheduler.unstable_advanceTime(10000);
553
550 - // So when we get a high pri update, we shouldn't interrupt
554 updateSyncPri();
555 + expect(Scheduler).toHaveYielded(['Sync pri: 2', 'Normal pri: 1']);
556 +
557 + // The remaining work _has_ expired, so the render phase is _not_ time
558 + // sliced. Attempting to flush just the first child also flushes the rest.
559 + Scheduler.unstable_flushNumberOfYields(1);
560 + expect(Scheduler).toHaveYielded(['Sync pri: 2', 'Normal pri: 2']);
561 });
553 - expect(Scheduler).toHaveYielded([
554 - // Finish normal pri update
555 - 'Normal pri: 2',
556 - // Then do high pri update
557 - 'Sync pri: 2',
558 - 'Normal pri: 2',
559 - ]);
562 expect(root).toMatchRenderedOutput('Sync pri: 2, Normal pri: 2');
563 });
564
565 it('idle work never expires', async () => {
564 - const {useState} = React;
565 -
566 let updateSyncPri;
567 let updateIdlePri;
568 function App() {
@@ -629,23 +629,19 @@ describe('ReactExpiration', () => {
629 });
630
631 // @gate experimental
632 - it('a single update can expire without forcing all other updates to expire', async () => {
633 - const {useState} = React;
634 -
635 - let updateHighPri;
636 - let updateNormalPri;
632 + it('when multiple lanes expire, we can finish the in-progress one without including the others', async () => {
633 + let setA;
634 + let setB;
635 function App() {
638 - const [highPri, setHighPri] = useState(0);
639 - const [normalPri, setNormalPri] = useState(0);
640 - updateHighPri = () => ReactNoop.flushSync(() => setHighPri(n => n + 1));
641 - updateNormalPri = () => setNormalPri(n => n + 1);
636 + const [a, _setA] = useState(0);
637 + const [b, _setB] = useState(0);
638 + setA = _setA;
639 + setB = _setB;
640 return (
641 <>
644 - <Text text={'High pri: ' + highPri} />
645 - {', '}
646 - <Text text={'Normal pri: ' + normalPri} />
647 - {', '}
648 - <Text text="Sibling" />
642 + <Text text={'A' + a} />
643 + <Text text={'B' + b} />
644 + <Text text="C" />
645 </>
646 );
647 }
@@ -654,184 +650,168 @@ describe('ReactExpiration', () => {
650 await ReactNoop.act(async () => {
651 root.render(<App />);
652 });
657 - expect(Scheduler).toHaveYielded([
658 - 'High pri: 0',
659 - 'Normal pri: 0',
660 - 'Sibling',
661 - ]);
662 - expect(root).toMatchRenderedOutput('High pri: 0, Normal pri: 0, Sibling');
653 + expect(Scheduler).toHaveYielded(['A0', 'B0', 'C']);
654 + expect(root).toMatchRenderedOutput('A0B0C');
655
656 await ReactNoop.act(async () => {
665 - // Partially render an update
657 startTransition(() => {
667 - updateNormalPri();
658 + setA(1);
659 });
669 - expect(Scheduler).toFlushAndYieldThrough(['High pri: 0']);
670 -
671 - // Some time goes by. Schedule another update.
672 - // This will be placed into a separate batch.
673 - Scheduler.unstable_advanceTime(4000);
674 -
660 + expect(Scheduler).toFlushAndYieldThrough(['A1']);
661 startTransition(() => {
676 - updateNormalPri();
677 - });
678 - // Keep rendering the first update
679 - expect(Scheduler).toFlushAndYieldThrough(['Normal pri: 1']);
680 - // More time goes by. Enough to expire the first batch, but not the
681 - // second one.
682 - Scheduler.unstable_advanceTime(1000);
683 - // Attempt to interrupt with a high pri update.
684 - await ReactNoop.act(async () => {
685 - updateHighPri();
662 + setB(1);
663 });
687 -
688 - expect(Scheduler).toHaveYielded([
689 - // The first update expired
690 - 'Sibling',
691 - // Then render the high pri update
692 - 'High pri: 1',
693 - 'Normal pri: 1',
694 - 'Sibling',
695 - // Then the second normal pri update
696 - 'High pri: 1',
697 - 'Normal pri: 2',
698 - 'Sibling',
699 - ]);
664 + // Expire both the transitions
665 + Scheduler.unstable_advanceTime(10000);
666 + // Both transitions have expired, but since they aren't related
667 + // (entangled), we should be able to finish the in-progress transition
668 + // without also including the next one.
669 + Scheduler.unstable_flushNumberOfYields(1);
670 + expect(Scheduler).toHaveYielded(['B0', 'C']);
671 + expect(root).toMatchRenderedOutput('A1B0C');
672 +
673 + // The next transition also finishes without yielding.
674 + Scheduler.unstable_flushNumberOfYields(1);
675 + expect(Scheduler).toHaveYielded(['A1', 'B1', 'C']);
676 + expect(root).toMatchRenderedOutput('A1B1C');
677 });
678 });
679
680 // @gate experimental || !enableSyncDefaultUpdates
704 - it('detects starvation in multiple batches', async () => {
705 - const {useState} = React;
681 + it('updates do not expire while they are IO-bound', async () => {
682 + const {Suspense} = React;
683
707 - let updateHighPri;
708 - let updateNormalPri;
709 - function App() {
710 - const [highPri, setHighPri] = useState(0);
711 - const [normalPri, setNormalPri] = useState(0);
712 - updateHighPri = () => {
713 - ReactNoop.flushSync(() => {
714 - setHighPri(n => n + 1);
715 - });
716 - };
717 - updateNormalPri = () => setNormalPri(n => n + 1);
684 + function App({step}) {
685 return (
719 - <>
720 - <Text text={'High pri: ' + highPri} />
721 - {', '}
722 - <Text text={'Normal pri: ' + normalPri} />
723 - {', '}
724 - <Text text="Sibling" />
725 - </>
686 + <Suspense fallback={<Text text="Loading..." />}>
687 + <AsyncText text={'A' + step} />
688 + <Text text="B" />
689 + <Text text="C" />
690 + </Suspense>
691 );
692 }
693
694 const root = ReactNoop.createRoot();
695 await ReactNoop.act(async () => {
731 - root.render(<App />);
696 + await resolveText('A0');
697 + root.render(<App step={0} />);
698 });
733 - expect(Scheduler).toHaveYielded([
734 - 'High pri: 0',
735 - 'Normal pri: 0',
736 - 'Sibling',
737 - ]);
738 - expect(root).toMatchRenderedOutput('High pri: 0, Normal pri: 0, Sibling');
699 + expect(Scheduler).toHaveYielded(['A0', 'B', 'C']);
700 + expect(root).toMatchRenderedOutput('A0BC');
701
702 await ReactNoop.act(async () => {
741 - // Partially render an update
703 if (gate(flags => flags.enableSyncDefaultUpdates)) {
704 React.unstable_startTransition(() => {
744 - updateNormalPri();
705 + root.render(<App step={1} />);
706 });
707 } else {
747 - updateNormalPri();
708 + root.render(<App step={1} />);
709 }
749 - expect(Scheduler).toFlushAndYieldThrough(['High pri: 0']);
750 - // Some time goes by. In an interleaved event, schedule another update.
751 - // This will be placed into a separate batch.
752 - Scheduler.unstable_advanceTime(4000);
753 - updateNormalPri();
754 - // Keep rendering the first update
755 - expect(Scheduler).toFlushAndYieldThrough(['Normal pri: 1']);
756 - // More time goes by. This expires both of the updates just scheduled.
710 + expect(Scheduler).toFlushAndYield([
711 + 'Suspend! [A1]',
712 + 'B',
713 + 'C',
714 + 'Loading...',
715 + ]);
716 +
717 + // Lots of time elapses before the promise resolves
718 Scheduler.unstable_advanceTime(10000);
758 - expect(Scheduler).toHaveYielded([]);
719 + await resolveText('A1');
720 + expect(Scheduler).toHaveYielded(['Promise resolved [A1]']);
721
760 - // Attempt to interrupt with a high pri update.
761 - updateHighPri();
762 -
763 - // Both normal pri updates should have expired.
764 - // The sync update and the expired normal pri updates render in a
765 - // single batch.
766 - expect(Scheduler).toHaveYielded([
767 - 'Sibling',
768 - 'High pri: 1',
769 - 'Normal pri: 2',
770 - 'Sibling',
771 - ]);
722 + // But the update doesn't expire, because it was IO bound. So we can
723 + // partially rendering without finishing.
724 + expect(Scheduler).toFlushAndYieldThrough(['A1']);
725 + expect(root).toMatchRenderedOutput('A0BC');
726 +
727 + // Lots more time elapses. We're CPU-bound now, so we should treat this
728 + // as starvation.
729 + Scheduler.unstable_advanceTime(10000);
730 +
731 + // The rest of the update finishes without yielding.
732 + Scheduler.unstable_flushNumberOfYields(1);
733 + expect(Scheduler).toHaveYielded(['B', 'C']);
734 });
735 });
736
775 - // @gate experimental || !enableSyncDefaultUpdates
776 - it('updates do not expire while they are IO-bound', async () => {
777 - const {Suspense} = React;
778 -
779 - function App({text}) {
737 + // @gate experimental
738 + it('flushSync should not affect expired work', async () => {
739 + let setA;
740 + let setB;
741 + function App() {
742 + const [a, _setA] = useState(0);
743 + const [b, _setB] = useState(0);
744 + setA = _setA;
745 + setB = _setB;
746 return (
781 - <Suspense fallback={<Text text="Loading..." />}>
782 - <AsyncText text={text} />
783 - {', '}
784 - <Text text="Sibling" />
785 - </Suspense>
747 + <>
748 + <Text text={'A' + a} />
749 + <Text text={'B' + b} />
750 + </>
751 );
752 }
753
754 const root = ReactNoop.createRoot();
755 await ReactNoop.act(async () => {
791 - await resolveText('A');
792 - root.render(<App text="A" />);
756 + root.render(<App />);
757 });
794 - expect(Scheduler).toHaveYielded(['A', 'Sibling']);
795 - expect(root).toMatchRenderedOutput('A, Sibling');
758 + expect(Scheduler).toHaveYielded(['A0', 'B0']);
759
760 await ReactNoop.act(async () => {
798 - if (gate(flags => flags.enableSyncDefaultUpdates)) {
799 - React.unstable_startTransition(() => {
800 - root.render(<App text="B" />);
801 - });
802 - } else {
803 - root.render(<App text="B" />);
804 - }
805 - expect(Scheduler).toFlushAndYield([
806 - 'Suspend! [B]',
807 - 'Sibling',
808 - 'Loading...',
809 - ]);
761 + startTransition(() => {
762 + setA(1);
763 + });
764 + expect(Scheduler).toFlushAndYieldThrough(['A1']);
765
811 - // Lots of time elapses before the promise resolves
766 + // Expire the in-progress update
767 Scheduler.unstable_advanceTime(10000);
813 - await resolveText('B');
814 - expect(Scheduler).toHaveYielded(['Promise resolved [B]']);
768
816 - // But the update doesn't expire, because it was IO bound. So we can
817 - // partially rendering without finishing.
818 - expect(Scheduler).toFlushAndYieldThrough(['B']);
819 - expect(root).toMatchRenderedOutput('A, Sibling');
769 + ReactNoop.flushSync(() => {
770 + setB(1);
771 + });
772 + expect(Scheduler).toHaveYielded(['A0', 'B1']);
773
821 - // Lots more time elapses. We're CPU-bound now, so we should treat this
822 - // as starvation.
774 + // Now flush the original update. Because it expired, it should finish
775 + // without yielding.
776 + Scheduler.unstable_flushNumberOfYields(1);
777 + expect(Scheduler).toHaveYielded(['A1', 'B1']);
778 + });
779 + });
780 +
781 + // @gate experimental
782 + it('passive effects of expired update flush after paint', async () => {
783 + function App({step}) {
784 + useEffect(() => {
785 + Scheduler.unstable_yieldValue('Effect: ' + step);
786 + }, [step]);
787 + return (
788 + <>
789 + <Text text={'A' + step} />
790 + <Text text={'B' + step} />
791 + <Text text={'C' + step} />
792 + </>
793 + );
794 + }
795 +
796 + const root = ReactNoop.createRoot();
797 + await ReactNoop.act(async () => {
798 + root.render(<App step={0} />);
799 + });
800 + expect(Scheduler).toHaveYielded(['A0', 'B0', 'C0', 'Effect: 0']);
801 + expect(root).toMatchRenderedOutput('A0B0C0');
802 +
803 + await ReactNoop.act(async () => {
804 + startTransition(() => {
805 + root.render(<App step={1} />);
806 + });
807 + // Expire the update
808 Scheduler.unstable_advanceTime(10000);
809
825 - // Attempt to interrupt with a sync update.
826 - ReactNoop.flushSync(() => root.render(<App text="A" />));
827 - expect(Scheduler).toHaveYielded([
828 - // Because the previous update had already expired, we don't interrupt
829 - // it. Finish rendering it first.
830 - 'Sibling',
831 - // Then do the sync update.
832 - 'A',
833 - 'Sibling',
834 - ]);
810 + // The update finishes without yielding. But it does not flush the effect.
811 + Scheduler.unstable_flushNumberOfYields(1);
812 + expect(Scheduler).toHaveYielded(['A1', 'B1', 'C1']);
813 });
814 + // The effect flushes after paint.
815 + expect(Scheduler).toHaveYielded(['Effect: 1']);
816 });
817 });
packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js
+1 -2
@@ -300,10 +300,9 @@ describe(
300 ReactNoop.render(<App />);
301 });
302
303 - ReactNoop.flushSync();
304 -
303 // Because the render expired, React should finish the tree without
304 // consulting `shouldYield` again
305 + Scheduler.unstable_flushNumberOfYields(1);
306 expect(Scheduler).toHaveYielded(['B', 'C']);
307 });
308 });
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
+7 -26
@@ -1958,32 +1958,13 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1958 await advanceTimers(5000);
1959
1960 // Retry with the new content.
1961 - if (gate(flags => flags.disableSchedulerTimeoutInWorkLoop)) {
1962 - expect(Scheduler).toFlushAndYield([
1963 - 'A',
1964 - // B still suspends
1965 - 'Suspend! [B]',
1966 - 'Loading more...',
1967 - ]);
1968 - } else {
1969 - // In this branch, right as we start rendering, we detect that the work
1970 - // has expired (via Scheduler's didTimeout argument) and re-schedule the
1971 - // work as synchronous. Since sync work does not flow through Scheduler,
1972 - // we need to use `flushSync`.
1973 - //
1974 - // Usually we would use `act`, which fluses both sync work and Scheduler
1975 - // work, but that would also force the fallback to display, and this test
1976 - // is specifically about whether we delay or show the fallback.
1977 - expect(Scheduler).toFlushAndYield([]);
1978 - // This will flush the synchronous callback we just scheduled.
1979 - ReactNoop.flushSync();
1980 - expect(Scheduler).toHaveYielded([
1981 - 'A',
1982 - // B still suspends
1983 - 'Suspend! [B]',
1984 - 'Loading more...',
1985 - ]);
1986 - }
1961 + expect(Scheduler).toFlushAndYield([
1962 + 'A',
1963 + // B still suspends
1964 + 'Suspend! [B]',
1965 + 'Loading more...',
1966 + ]);
1967 +
1968 // Because we've already been waiting for so long we've exceeded
1969 // our threshold and we show the next level immediately.
1970 expect(ReactNoop.getChildren()).toEqual([