Apply the Just Noticeable Difference to suspense timeouts (#15367)
* Apply the Just Noticeable Difference boundary * Clamp suspense timeout to expiration time
Sebastian Markbåge committed
Apr 10, 2019 at 17:16 UTC
c25c59c808d90bfa78c40d0a19527d384a490cc8
4 files changed
+239
-13
packages/react-reconciler/src/ReactFiberCompleteWork.js
+2
-2
@@ -692,9 +692,9 @@ function completeWork(
692
// Mark the event time of the switching from fallback to normal children,
693
// based on the start of when we first showed the fallback. This time
694
// was given a normal pri expiration time at the time it was shown.
695
- const fallbackExpirationTimeExpTime: ExpirationTime =
695
+ const fallbackExpirationTime: ExpirationTime =
696
prevState.fallbackExpirationTime;
697
- markRenderEventTime(fallbackExpirationTimeExpTime);
697
+ markRenderEventTime(fallbackExpirationTime);
698
699
// Delete the fallback.
700
// TODO: Would it be better to store the fallback fragment on
packages/react-reconciler/src/ReactFiberScheduler.new.js
+49
-7
@@ -160,6 +160,8 @@ import {
160
} from 'shared/ReactErrorUtils';
161
import {onCommitRoot} from './ReactFiberDevToolsHook';
162
163
+const ceil = Math.ceil;
164
+
165
const {
166
ReactCurrentDispatcher,
167
ReactCurrentOwner,
@@ -893,10 +895,12 @@ function renderRoot(
895
// track any event times. That can happen if we retried but nothing switched
896
// from fallback to content. There's no reason to delay doing no work.
897
if (workInProgressRootMostRecentEventTime !== Sync) {
896
- const msUntilTimeout = computeMsUntilTimeout(
898
+ let msUntilTimeout = computeMsUntilTimeout(
899
workInProgressRootMostRecentEventTime,
900
+ expirationTime,
901
);
899
- if (msUntilTimeout > 0) {
902
+ // Don't bother with a very short suspense time.
903
+ if (msUntilTimeout > 10) {
904
// The render is suspended, it hasn't timed out, and there's no lower
905
// priority work to do. Instead of committing the fallback
906
// immediately, wait for more data to arrive.
@@ -1815,7 +1819,35 @@ export function resolveRetryThenable(boundaryFiber: Fiber, thenable: Thenable) {
1819
retryTimedOutBoundary(boundaryFiber);
1820
}
1821
1818
-function computeMsUntilTimeout(mostRecentEventTime: ExpirationTime) {
1822
+// Computes the next Just Noticeable Difference (JND) boundary.
1823
+// The theory is that a person can't tell the difference between small differences in time.
1824
+// Therefore, if we wait a bit longer than necessary that won't translate to a noticeable
1825
+// difference in the experience. However, waiting for longer might mean that we can avoid
1826
+// showing an intermediate loading state. The longer we have already waited, the harder it
1827
+// is to tell small differences in time. Therefore, the longer we've already waited,
1828
+// the longer we can wait additionally. At some point we have to give up though.
1829
+// We pick a train model where the next boundary commits at a consistent schedule.
1830
+// These particular numbers are vague estimates. We expect to adjust them based on research.
1831
+function jnd(timeElapsed: number) {
1832
+ return timeElapsed < 120
1833
+ ? 120
1834
+ : timeElapsed < 480
1835
+ ? 480
1836
+ : timeElapsed < 1080
1837
+ ? 1080
1838
+ : timeElapsed < 1920
1839
+ ? 1920
1840
+ : timeElapsed < 3000
1841
+ ? 3000
1842
+ : timeElapsed < 4320
1843
+ ? 4320
1844
+ : ceil(timeElapsed / 1960) * 1960;
1845
+}
1846
+
1847
+function computeMsUntilTimeout(
1848
+ mostRecentEventTime: ExpirationTime,
1849
+ committedExpirationTime: ExpirationTime,
1850
+) {
1851
if (disableYielding) {
1852
// Timeout immediately when yielding is disabled.
1853
return 0;
@@ -1825,11 +1857,21 @@ function computeMsUntilTimeout(mostRecentEventTime: ExpirationTime) {
1857
const currentTimeMs: number = now();
1858
const timeElapsed = currentTimeMs - eventTimeMs;
1859
1828
- // TODO: Account for the Just Noticeable Difference
1829
- const timeoutMs = 150;
1830
- const msUntilTimeout = timeoutMs - timeElapsed;
1860
+ let msUntilTimeout = jnd(timeElapsed) - timeElapsed;
1861
+
1862
+ // Compute the time until this render pass would expire.
1863
+ const timeUntilExpirationMs =
1864
+ expirationTimeToMs(committedExpirationTime) + initialTimeMs - currentTimeMs;
1865
+
1866
+ // Clamp the timeout to the expiration time.
1867
+ // TODO: Once the event time is exact instead of inferred from expiration time
1868
+ // we don't need this.
1869
+ if (timeUntilExpirationMs < msUntilTimeout) {
1870
+ msUntilTimeout = timeUntilExpirationMs;
1871
+ }
1872
+
1873
// This is the value that is passed to `setTimeout`.
1832
- return msUntilTimeout < 0 ? 0 : msUntilTimeout;
1874
+ return msUntilTimeout;
1875
}
1876
1877
function checkForNestedUpdates() {
packages/react-reconciler/src/ReactFiberScheduler.old.js
+32
-4
@@ -1241,6 +1241,22 @@ function workLoop(isYieldy) {
1241
}
1242
}
1243
1244
+function jnd(timeElapsed: number) {
1245
+ return timeElapsed < 120
1246
+ ? 120
1247
+ : timeElapsed < 480
1248
+ ? 480
1249
+ : timeElapsed < 1080
1250
+ ? 1080
1251
+ : timeElapsed < 1920
1252
+ ? 1920
1253
+ : timeElapsed < 3000
1254
+ ? 3000
1255
+ : timeElapsed < 4320
1256
+ ? 4320
1257
+ : Math.ceil(timeElapsed / 1960) * 1960;
1258
+}
1259
+
1260
function renderRoot(root: FiberRoot, isYieldy: boolean): void {
1261
invariant(
1262
!isWorking,
@@ -1518,10 +1534,22 @@ function renderRoot(root: FiberRoot, isYieldy: boolean): void {
1534
const currentTimeMs: number = now();
1535
const timeElapsed = currentTimeMs - eventTimeMs;
1536
1521
- // TODO: Account for the Just Noticeable Difference
1522
- const timeoutMs = 150;
1523
- let msUntilTimeout = timeoutMs - timeElapsed;
1524
- msUntilTimeout = msUntilTimeout < 0 ? 0 : msUntilTimeout;
1537
+ let msUntilTimeout = jnd(timeElapsed) - timeElapsed;
1538
+
1539
+ if (msUntilTimeout < 10) {
1540
+ // Don't bother with a very short suspense time.
1541
+ msUntilTimeout = 0;
1542
+ } else {
1543
+ // Compute the time until this render pass would expire.
1544
+ const timeUntilExpirationMs =
1545
+ expirationTimeToMs(suspendedExpirationTime) +
1546
+ originalStartTimeMs -
1547
+ currentTimeMs;
1548
+ // Clamp the timeout to the expiration time.
1549
+ if (timeUntilExpirationMs < msUntilTimeout) {
1550
+ msUntilTimeout = timeUntilExpirationMs;
1551
+ }
1552
+ }
1553
1554
const rootExpirationTime = root.expirationTime;
1555
onSuspend(
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
+156
@@ -1668,6 +1668,162 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1668
</React.Fragment>,
1669
);
1670
});
1671
+
1672
+ it('suspends for longer if something took a long (CPU bound) time to render', async () => {
1673
+ function Foo() {
1674
+ Scheduler.yieldValue('Foo');
1675
+ return (
1676
+ <Suspense fallback={<Text text="Loading..." />}>
1677
+ <AsyncText text="A" ms={5000} />
1678
+ </Suspense>
1679
+ );
1680
+ }
1681
+
1682
+ ReactNoop.render(<Foo />);
1683
+ Scheduler.advanceTime(100);
1684
+ await advanceTimers(100);
1685
+ // Start rendering
1686
+ expect(Scheduler).toFlushAndYieldThrough(['Foo']);
1687
+ // For some reason it took a long time to render Foo.
1688
+ Scheduler.advanceTime(1250);
1689
+ await advanceTimers(1250);
1690
+ expect(Scheduler).toFlushAndYield([
1691
+ // A suspends
1692
+ 'Suspend! [A]',
1693
+ 'Loading...',
1694
+ ]);
1695
+ // We're now suspended and we haven't shown anything yet.
1696
+ expect(ReactNoop.getChildren()).toEqual([]);
1697
+
1698
+ // Flush some of the time
1699
+ Scheduler.advanceTime(450);
1700
+ await advanceTimers(450);
1701
+ // Because we've already been waiting for so long we can
1702
+ // wait a bit longer. Still nothing...
1703
+ expect(Scheduler).toFlushWithoutYielding();
1704
+ expect(ReactNoop.getChildren()).toEqual([]);
1705
+
1706
+ // Eventually we'll show the fallback.
1707
+ Scheduler.advanceTime(500);
1708
+ await advanceTimers(500);
1709
+ // No need to rerender.
1710
+ expect(Scheduler).toFlushWithoutYielding();
1711
+ expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
1712
+
1713
+ // Flush the promise completely
1714
+ Scheduler.advanceTime(4500);
1715
+ await advanceTimers(4500);
1716
+ // Renders successfully
1717
+ expect(Scheduler).toHaveYielded(['Promise resolved [A]']);
1718
+ expect(Scheduler).toFlushAndYield(['A']);
1719
+ expect(ReactNoop.getChildren()).toEqual([span('A')]);
1720
+ });
1721
+
1722
+ it('suspends for longer if a fallback has been shown for a long time', async () => {
1723
+ function Foo() {
1724
+ Scheduler.yieldValue('Foo');
1725
+ return (
1726
+ <Suspense fallback={<Text text="Loading..." />}>
1727
+ <AsyncText text="A" ms={5000} />
1728
+ <Suspense fallback={<Text text="Loading more..." />}>
1729
+ <AsyncText text="B" ms={10000} />
1730
+ </Suspense>
1731
+ </Suspense>
1732
+ );
1733
+ }
1734
+
1735
+ ReactNoop.render(<Foo />);
1736
+ // Start rendering
1737
+ expect(Scheduler).toFlushAndYield([
1738
+ 'Foo',
1739
+ // A suspends
1740
+ 'Suspend! [A]',
1741
+ // B suspends
1742
+ 'Suspend! [B]',
1743
+ 'Loading more...',
1744
+ 'Loading...',
1745
+ ]);
1746
+ // We're now suspended and we haven't shown anything yet.
1747
+ expect(ReactNoop.getChildren()).toEqual([]);
1748
+
1749
+ // Show the fallback.
1750
+ Scheduler.advanceTime(400);
1751
+ await advanceTimers(400);
1752
+ expect(Scheduler).toFlushWithoutYielding();
1753
+ expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
1754
+
1755
+ // Wait a long time.
1756
+ Scheduler.advanceTime(5000);
1757
+ await advanceTimers(5000);
1758
+ expect(Scheduler).toHaveYielded(['Promise resolved [A]']);
1759
+
1760
+ // Retry with the new content.
1761
+ expect(Scheduler).toFlushAndYield([
1762
+ 'A',
1763
+ // B still suspends
1764
+ 'Suspend! [B]',
1765
+ 'Loading more...',
1766
+ ]);
1767
+ // Because we've already been waiting for so long we can
1768
+ // wait a bit longer. Still nothing...
1769
+ Scheduler.advanceTime(600);
1770
+ await advanceTimers(600);
1771
+ expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
1772
+
1773
+ // Eventually we'll show more content with inner fallback.
1774
+ Scheduler.advanceTime(3000);
1775
+ await advanceTimers(3000);
1776
+ // No need to rerender.
1777
+ expect(Scheduler).toFlushWithoutYielding();
1778
+ expect(ReactNoop.getChildren()).toEqual([
1779
+ span('A'),
1780
+ span('Loading more...'),
1781
+ ]);
1782
+
1783
+ // Flush the last promise completely
1784
+ Scheduler.advanceTime(4500);
1785
+ await advanceTimers(4500);
1786
+ // Renders successfully
1787
+ expect(Scheduler).toHaveYielded(['Promise resolved [B]']);
1788
+ expect(Scheduler).toFlushAndYield(['B']);
1789
+ expect(ReactNoop.getChildren()).toEqual([span('A'), span('B')]);
1790
+ });
1791
+
1792
+ it('does not suspend for very long after a higher priority update', async () => {
1793
+ function Foo() {
1794
+ Scheduler.yieldValue('Foo');
1795
+ return (
1796
+ <Suspense fallback={<Text text="Loading..." />}>
1797
+ <AsyncText text="A" ms={5000} />
1798
+ </Suspense>
1799
+ );
1800
+ }
1801
+
1802
+ ReactNoop.interactiveUpdates(() => ReactNoop.render(<Foo />));
1803
+ expect(Scheduler).toFlushAndYieldThrough(['Foo']);
1804
+
1805
+ // Advance some time.
1806
+ Scheduler.advanceTime(100);
1807
+ await advanceTimers(100);
1808
+
1809
+ expect(Scheduler).toFlushAndYield([
1810
+ // A suspends
1811
+ 'Suspend! [A]',
1812
+ 'Loading...',
1813
+ ]);
1814
+ // We're now suspended and we haven't shown anything yet.
1815
+ expect(ReactNoop.getChildren()).toEqual([]);
1816
+
1817
+ // Flush some of the time
1818
+ Scheduler.advanceTime(500);
1819
+ await advanceTimers(500);
1820
+ // We should have already shown the fallback.
1821
+ // When we wrote this test, we inferred the start time of high priority
1822
+ // updates as way earlier in the past. This test ensures that we don't
1823
+ // use this assumption to add a very long JND.
1824
+ expect(Scheduler).toFlushWithoutYielding();
1825
+ expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
1826
+ });
1827
});
1828
1829
// TODO: