Transfer actualDuration only once for SuspenseList (#18959)
Sebastian Markbåge committed
May 20, 2020 at 15:12 UTC
a30a1c6ef3a621a57e9372c8249439dbfd9a7375
9 files changed
+271
-26
packages/react-reconciler/src/ReactFiberCompleteWork.new.js
+9
-1
@@ -58,7 +58,7 @@ import {
58
OffscreenComponent,
59
LegacyHiddenComponent,
60
} from './ReactWorkTags';
61
-import {NoMode, BlockingMode} from './ReactTypeOfMode';
61
+import {NoMode, BlockingMode, ProfileMode} from './ReactTypeOfMode';
62
import {
63
Ref,
64
Update,
@@ -128,6 +128,7 @@ import {
128
enableFundamentalAPI,
129
enableScopeAPI,
130
enableBlocksAPI,
131
+ enableProfilerTimer,
132
} from 'shared/ReactFeatureFlags';
133
import {
134
markSpawnedWork,
@@ -141,6 +142,7 @@ import {OffscreenLane} from './ReactFiberLane';
142
import {resetChildFibers} from './ReactChildFiber.new';
143
import {updateDeprecatedEventListeners} from './ReactFiberDeprecatedEvents.new';
144
import {createScopeInstance} from './ReactFiberScope.new';
145
+import {transferActualDuration} from './ReactProfilerTimer.new';
146
147
function markUpdate(workInProgress: Fiber) {
148
// Tag the fiber with an update effect. This turns a Placement into
@@ -890,6 +892,12 @@ function completeWork(
892
// Something suspended. Re-render with the fallback children.
893
workInProgress.lanes = renderLanes;
894
// Do not reset the effect list.
895
+ if (
896
+ enableProfilerTimer &&
897
+ (workInProgress.mode & ProfileMode) !== NoMode
898
+ ) {
899
+ transferActualDuration(workInProgress);
900
+ }
901
return workInProgress;
902
}
903
packages/react-reconciler/src/ReactFiberCompleteWork.old.js
+10
-1
@@ -54,7 +54,7 @@ import {
54
ScopeComponent,
55
Block,
56
} from './ReactWorkTags';
57
-import {NoMode, BlockingMode} from './ReactTypeOfMode';
57
+import {NoMode, BlockingMode, ProfileMode} from './ReactTypeOfMode';
58
import {
59
Ref,
60
Update,
@@ -125,6 +125,7 @@ import {
125
enableFundamentalAPI,
126
enableScopeAPI,
127
enableBlocksAPI,
128
+ enableProfilerTimer,
129
} from 'shared/ReactFeatureFlags';
130
import {
131
markSpawnedWork,
@@ -137,6 +138,7 @@ import {Never} from './ReactFiberExpirationTime.old';
138
import {resetChildFibers} from './ReactChildFiber.old';
139
import {updateDeprecatedEventListeners} from './ReactFiberDeprecatedEvents.old';
140
import {createScopeInstance} from './ReactFiberScope.old';
141
+import {transferActualDuration} from './ReactProfilerTimer.old';
142
143
function markUpdate(workInProgress: Fiber) {
144
// Tag the fiber with an update effect. This turns a Placement into
@@ -885,6 +887,12 @@ function completeWork(
887
if ((workInProgress.effectTag & DidCapture) !== NoEffect) {
888
// Something suspended. Re-render with the fallback children.
889
workInProgress.expirationTime = renderExpirationTime;
890
+ if (
891
+ enableProfilerTimer &&
892
+ (workInProgress.mode & ProfileMode) !== NoMode
893
+ ) {
894
+ transferActualDuration(workInProgress);
895
+ }
896
// Do not reset the effect list.
897
return workInProgress;
898
}
@@ -1084,6 +1092,7 @@ function completeWork(
1092
ForceSuspenseFallback,
1093
),
1094
);
1095
+
1096
return workInProgress.child;
1097
}
1098
row = row.sibling;
packages/react-reconciler/src/ReactFiberUnwindWork.new.js
+18
-1
@@ -24,7 +24,11 @@ import {
24
LegacyHiddenComponent,
25
} from './ReactWorkTags';
26
import {DidCapture, NoEffect, ShouldCapture} from './ReactSideEffectTags';
27
-import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags';
27
+import {NoMode, ProfileMode} from './ReactTypeOfMode';
28
+import {
29
+ enableSuspenseServerRenderer,
30
+ enableProfilerTimer,
31
+} from 'shared/ReactFeatureFlags';
32
33
import {popHostContainer, popHostContext} from './ReactFiberHostContext.new';
34
import {popSuspenseContext} from './ReactFiberSuspenseContext.new';
@@ -36,6 +40,7 @@ import {
40
} from './ReactFiberContext.new';
41
import {popProvider} from './ReactFiberNewContext.new';
42
import {popRenderLanes} from './ReactFiberWorkLoop.new';
43
+import {transferActualDuration} from './ReactProfilerTimer.new';
44
45
import invariant from 'shared/invariant';
46
@@ -49,6 +54,12 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
54
const effectTag = workInProgress.effectTag;
55
if (effectTag & ShouldCapture) {
56
workInProgress.effectTag = (effectTag & ~ShouldCapture) | DidCapture;
57
+ if (
58
+ enableProfilerTimer &&
59
+ (workInProgress.mode & ProfileMode) !== NoMode
60
+ ) {
61
+ transferActualDuration(workInProgress);
62
+ }
63
return workInProgress;
64
}
65
return null;
@@ -89,6 +100,12 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
100
if (effectTag & ShouldCapture) {
101
workInProgress.effectTag = (effectTag & ~ShouldCapture) | DidCapture;
102
// Captured a suspense effect. Re-render the boundary.
103
+ if (
104
+ enableProfilerTimer &&
105
+ (workInProgress.mode & ProfileMode) !== NoMode
106
+ ) {
107
+ transferActualDuration(workInProgress);
108
+ }
109
return workInProgress;
110
}
111
return null;
packages/react-reconciler/src/ReactFiberUnwindWork.old.js
+18
-1
@@ -22,7 +22,11 @@ import {
22
SuspenseListComponent,
23
} from './ReactWorkTags';
24
import {DidCapture, NoEffect, ShouldCapture} from './ReactSideEffectTags';
25
-import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags';
25
+import {NoMode, ProfileMode} from './ReactTypeOfMode';
26
+import {
27
+ enableSuspenseServerRenderer,
28
+ enableProfilerTimer,
29
+} from 'shared/ReactFeatureFlags';
30
31
import {popHostContainer, popHostContext} from './ReactFiberHostContext.old';
32
import {popSuspenseContext} from './ReactFiberSuspenseContext.old';
@@ -33,6 +37,7 @@ import {
37
popTopLevelContextObject as popTopLevelLegacyContextObject,
38
} from './ReactFiberContext.old';
39
import {popProvider} from './ReactFiberNewContext.old';
40
+import {transferActualDuration} from './ReactProfilerTimer.old';
41
42
import invariant from 'shared/invariant';
43
@@ -49,6 +54,12 @@ function unwindWork(
54
const effectTag = workInProgress.effectTag;
55
if (effectTag & ShouldCapture) {
56
workInProgress.effectTag = (effectTag & ~ShouldCapture) | DidCapture;
57
+ if (
58
+ enableProfilerTimer &&
59
+ (workInProgress.mode & ProfileMode) !== NoMode
60
+ ) {
61
+ transferActualDuration(workInProgress);
62
+ }
63
return workInProgress;
64
}
65
return null;
@@ -89,6 +100,12 @@ function unwindWork(
100
if (effectTag & ShouldCapture) {
101
workInProgress.effectTag = (effectTag & ~ShouldCapture) | DidCapture;
102
// Captured a suspense effect. Re-render the boundary.
103
+ if (
104
+ enableProfilerTimer &&
105
+ (workInProgress.mode & ProfileMode) !== NoMode
106
+ ) {
107
+ transferActualDuration(workInProgress);
108
+ }
109
return workInProgress;
110
}
111
return null;
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+12
-11
@@ -1571,7 +1571,6 @@ function completeUnitOfWork(unitOfWork: Fiber): void {
1571
stopProfilerTimerIfRunningAndRecordDelta(completedWork, false);
1572
}
1573
resetCurrentDebugFiberInDEV();
1574
- resetChildLanes(completedWork);
1574
1575
if (next !== null) {
1576
// Completing this fiber spawned new work. Work on that next.
@@ -1579,6 +1578,8 @@ function completeUnitOfWork(unitOfWork: Fiber): void {
1578
return;
1579
}
1580
1581
+ resetChildLanes(completedWork);
1582
+
1583
if (
1584
returnFiber !== null &&
1585
// Do not append effects to parents if a sibling failed to complete
@@ -1625,6 +1626,16 @@ function completeUnitOfWork(unitOfWork: Fiber): void {
1626
1627
// Because this fiber did not complete, don't reset its expiration time.
1628
1629
+ if (next !== null) {
1630
+ // If completing this work spawned new work, do that next. We'll come
1631
+ // back here again.
1632
+ // Since we're restarting, remove anything that is not a host effect
1633
+ // from the effect tag.
1634
+ next.effectTag &= HostEffectMask;
1635
+ workInProgress = next;
1636
+ return;
1637
+ }
1638
+
1639
if (
1640
enableProfilerTimer &&
1641
(completedWork.mode & ProfileMode) !== NoMode
@@ -1642,16 +1653,6 @@ function completeUnitOfWork(unitOfWork: Fiber): void {
1653
completedWork.actualDuration = actualDuration;
1654
}
1655
1645
- if (next !== null) {
1646
- // If completing this work spawned new work, do that next. We'll come
1647
- // back here again.
1648
- // Since we're restarting, remove anything that is not a host effect
1649
- // from the effect tag.
1650
- next.effectTag &= HostEffectMask;
1651
- workInProgress = next;
1652
- return;
1653
- }
1654
-
1656
if (returnFiber !== null) {
1657
// Mark the parent fiber as incomplete and clear its effect list.
1658
returnFiber.firstEffect = returnFiber.lastEffect = null;
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+12
-11
@@ -1643,7 +1643,6 @@ function completeUnitOfWork(unitOfWork: Fiber): void {
1643
stopProfilerTimerIfRunningAndRecordDelta(completedWork, false);
1644
}
1645
resetCurrentDebugFiberInDEV();
1646
- resetChildExpirationTime(completedWork);
1646
1647
if (next !== null) {
1648
// Completing this fiber spawned new work. Work on that next.
@@ -1651,6 +1650,8 @@ function completeUnitOfWork(unitOfWork: Fiber): void {
1650
return;
1651
}
1652
1653
+ resetChildExpirationTime(completedWork);
1654
+
1655
if (
1656
returnFiber !== null &&
1657
// Do not append effects to parents if a sibling failed to complete
@@ -1697,6 +1698,16 @@ function completeUnitOfWork(unitOfWork: Fiber): void {
1698
1699
// Because this fiber did not complete, don't reset its expiration time.
1700
1701
+ if (next !== null) {
1702
+ // If completing this work spawned new work, do that next. We'll come
1703
+ // back here again.
1704
+ // Since we're restarting, remove anything that is not a host effect
1705
+ // from the effect tag.
1706
+ next.effectTag &= HostEffectMask;
1707
+ workInProgress = next;
1708
+ return;
1709
+ }
1710
+
1711
if (
1712
enableProfilerTimer &&
1713
(completedWork.mode & ProfileMode) !== NoMode
@@ -1714,16 +1725,6 @@ function completeUnitOfWork(unitOfWork: Fiber): void {
1725
completedWork.actualDuration = actualDuration;
1726
}
1727
1717
- if (next !== null) {
1718
- // If completing this work spawned new work, do that next. We'll come
1719
- // back here again.
1720
- // Since we're restarting, remove anything that is not a host effect
1721
- // from the effect tag.
1722
- next.effectTag &= HostEffectMask;
1723
- workInProgress = next;
1724
- return;
1725
- }
1726
-
1728
if (returnFiber !== null) {
1729
// Mark the parent fiber as incomplete and clear its effect list.
1730
returnFiber.firstEffect = returnFiber.lastEffect = null;
packages/react-reconciler/src/ReactProfilerTimer.new.js
+12
@@ -148,6 +148,17 @@ function startPassiveEffectTimer(): void {
148
passiveEffectStartTime = now();
149
}
150
151
+function transferActualDuration(fiber: Fiber): void {
152
+ // Transfer time spent rendering these children so we don't lose it
153
+ // after we rerender. This is used as a helper in special cases
154
+ // where we should count the work of multiple passes.
155
+ let child = fiber.child;
156
+ while (child) {
157
+ fiber.actualDuration += child.actualDuration;
158
+ child = child.sibling;
159
+ }
160
+}
161
+
162
export {
163
getCommitTime,
164
recordCommitTime,
@@ -158,4 +169,5 @@ export {
169
startProfilerTimer,
170
stopProfilerTimerIfRunning,
171
stopProfilerTimerIfRunningAndRecordDelta,
172
+ transferActualDuration,
173
};
packages/react-reconciler/src/ReactProfilerTimer.old.js
+12
@@ -148,6 +148,17 @@ function startPassiveEffectTimer(): void {
148
passiveEffectStartTime = now();
149
}
150
151
+function transferActualDuration(fiber: Fiber): void {
152
+ // Transfer time spent rendering these children so we don't lose it
153
+ // after we rerender. This is used as a helper in special cases
154
+ // where we should count the work of multiple passes.
155
+ let child = fiber.child;
156
+ while (child) {
157
+ fiber.actualDuration += child.actualDuration;
158
+ child = child.sibling;
159
+ }
160
+}
161
+
162
export {
163
getCommitTime,
164
recordCommitTime,
@@ -158,4 +169,5 @@ export {
169
startProfilerTimer,
170
stopProfilerTimerIfRunning,
171
stopProfilerTimerIfRunningAndRecordDelta,
172
+ transferActualDuration,
173
};
packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js
+168
@@ -1,6 +1,7 @@
1
let React;
2
let ReactNoop;
3
let Scheduler;
4
+let Profiler;
5
let Suspense;
6
let SuspenseList;
7
@@ -11,6 +12,7 @@ describe('ReactSuspenseList', () => {
12
React = require('react');
13
ReactNoop = require('react-noop-renderer');
14
Scheduler = require('scheduler');
15
+ Profiler = React.Profiler;
16
Suspense = React.Suspense;
17
SuspenseList = React.unstable_SuspenseList;
18
});
@@ -2567,4 +2569,170 @@ describe('ReactSuspenseList', () => {
2569
</>,
2570
);
2571
});
2572
+
2573
+ // @gate experimental && enableProfilerTimer
2574
+ it('counts the actual duration when profiling a SuspenseList', async () => {
2575
+ // Order of parameters: id, phase, actualDuration, treeBaseDuration
2576
+ const onRender = jest.fn();
2577
+
2578
+ const Fallback = () => {
2579
+ Scheduler.unstable_yieldValue('Fallback');
2580
+ Scheduler.unstable_advanceTime(3);
2581
+ return <span>Loading...</span>;
2582
+ };
2583
+
2584
+ const A = createAsyncText('A');
2585
+ const B = createAsyncText('B');
2586
+ const C = createAsyncText('C');
2587
+ const D = createAsyncText('D');
2588
+ await A.resolve();
2589
+ await B.resolve();
2590
+
2591
+ function Sleep({time, children}) {
2592
+ Scheduler.unstable_advanceTime(time);
2593
+ return children;
2594
+ }
2595
+
2596
+ function App({addRow, suspendTail}) {
2597
+ Scheduler.unstable_yieldValue('App');
2598
+ return (
2599
+ <Profiler id="root" onRender={onRender}>
2600
+ <SuspenseList revealOrder="forwards">
2601
+ <Suspense fallback={<Fallback />}>
2602
+ <Sleep time={1}>
2603
+ <A />
2604
+ </Sleep>
2605
+ </Suspense>
2606
+ <Suspense fallback={<Fallback />}>
2607
+ <Sleep time={4}>
2608
+ <B />
2609
+ </Sleep>
2610
+ </Suspense>
2611
+ <Suspense fallback={<Fallback />}>
2612
+ <Sleep time={5}>{suspendTail ? <C /> : <Text text="C" />}</Sleep>
2613
+ </Suspense>
2614
+ {addRow ? (
2615
+ <Suspense fallback={<Fallback />}>
2616
+ <Sleep time={12}>
2617
+ <D />
2618
+ </Sleep>
2619
+ </Suspense>
2620
+ ) : null}
2621
+ </SuspenseList>
2622
+ </Profiler>
2623
+ );
2624
+ }
2625
+
2626
+ ReactNoop.render(<App suspendTail={true} />);
2627
+
2628
+ expect(Scheduler).toFlushAndYield([
2629
+ 'App',
2630
+ 'A',
2631
+ 'B',
2632
+ 'Suspend! [C]',
2633
+ 'Fallback',
2634
+ ]);
2635
+ expect(ReactNoop).toMatchRenderedOutput(
2636
+ <>
2637
+ <span>A</span>
2638
+ <span>B</span>
2639
+ <span>Loading...</span>
2640
+ </>,
2641
+ );
2642
+ expect(onRender).toHaveBeenCalledTimes(1);
2643
+
2644
+ // The treeBaseDuration should be the time to render each child. The last
2645
+ // one counts the fallback time.
2646
+ // The actualDuration should also include the 5ms spent rendering the
2647
+ // last suspended row.
2648
+
2649
+ // actualDuration
2650
+ expect(onRender.mock.calls[0][2]).toBe(1 + 4 + 5 + 3);
2651
+ // treeBaseDuration
2652
+ expect(onRender.mock.calls[0][3]).toBe(1 + 4 + 3);
2653
+
2654
+ ReactNoop.render(<App suspendTail={false} />);
2655
+
2656
+ expect(Scheduler).toFlushAndYield(['App', 'A', 'B', 'C']);
2657
+
2658
+ expect(ReactNoop).toMatchRenderedOutput(
2659
+ <>
2660
+ <span>A</span>
2661
+ <span>B</span>
2662
+ <span>C</span>
2663
+ </>,
2664
+ );
2665
+ expect(onRender).toHaveBeenCalledTimes(2);
2666
+
2667
+ // actualDuration
2668
+ expect(onRender.mock.calls[1][2]).toBe(1 + 4 + 5);
2669
+ // treeBaseDuration
2670
+ expect(onRender.mock.calls[1][3]).toBe(1 + 4 + 5);
2671
+
2672
+ ReactNoop.render(<App addRow={true} suspendTail={true} />);
2673
+
2674
+ expect(Scheduler).toFlushAndYield([
2675
+ 'App',
2676
+ 'A',
2677
+ 'B',
2678
+ 'Suspend! [C]',
2679
+ 'Fallback',
2680
+ // We rendered in together mode for the head, now we re-render with forced suspense.
2681
+ 'A',
2682
+ 'B',
2683
+ 'Suspend! [C]',
2684
+ 'Fallback',
2685
+ // Lastly we render the tail.
2686
+ 'Fallback',
2687
+ ]);
2688
+
2689
+ // Flush suspended time.
2690
+ jest.advanceTimersByTime(1000);
2691
+
2692
+ expect(ReactNoop).toMatchRenderedOutput(
2693
+ <>
2694
+ <span>A</span>
2695
+ <span>B</span>
2696
+ <span hidden={true}>C</span>
2697
+ <span>Loading...</span>
2698
+ <span>Loading...</span>
2699
+ </>,
2700
+ );
2701
+ expect(onRender).toHaveBeenCalledTimes(3);
2702
+
2703
+ // The treeBaseDuration should be the time to render the first two
2704
+ // children and then two fallbacks.
2705
+ // The actualDuration should also include rendering the content of
2706
+ // the first fallback, as well as the second pass to render the head
2707
+ // with force fallback mode.
2708
+
2709
+ // actualDuration
2710
+ expect(onRender.mock.calls[2][2]).toBe((1 + 4 + 5 + 3) * 2 + 3);
2711
+ // treeBaseDuration
2712
+ expect(onRender.mock.calls[2][3]).toBe(
2713
+ 1 +
2714
+ 4 +
2715
+ 3 +
2716
+ 3 +
2717
+ /* Resuspending a boundary also includes the content in base duration but it shouldn't */ 5,
2718
+ );
2719
+
2720
+ await C.resolve();
2721
+
2722
+ expect(Scheduler).toFlushAndYield(['C', 'Suspend! [D]']);
2723
+ expect(ReactNoop).toMatchRenderedOutput(
2724
+ <>
2725
+ <span>A</span>
2726
+ <span>B</span>
2727
+ <span>C</span>
2728
+ <span>Loading...</span>
2729
+ </>,
2730
+ );
2731
+ expect(onRender).toHaveBeenCalledTimes(4);
2732
+
2733
+ // actualDuration
2734
+ expect(onRender.mock.calls[3][2]).toBe(5 + 12);
2735
+ // treeBaseDuration
2736
+ expect(onRender.mock.calls[3][3]).toBe(1 + 4 + 5 + 3);
2737
+ });
2738
});