Record "actual" times for all Fibers within a Profiler tree (alt) (#12910)
* Moved actual time fields from Profiler stateNode to Fiber * Record actual time for all Fibers within a ProfileMode tree * Changed how profiler accumulates time This change gives up on accumulating time across renders of different priority, but in exchange- simplifies how the commit phase (reset) code works, and perhaps also makes the profiling code more compatible with future resuming behavior
Brian Vaughn committed
May 25, 2018 at 14:51 UTC
55787006710bd8ef89a01d60c4a9cfa58c71035d
8 files changed
+114
-51
packages/react-reconciler/src/ReactFiber.js
+30
-8
@@ -152,8 +152,24 @@ export type Fiber = {|
152
// memory if we need to.
153
alternate: Fiber | null,
154
155
- // Profiling metrics
155
+ // Time spent rendering this Fiber and its descendants for the current update.
156
+ // This tells us how well the tree makes use of sCU for memoization.
157
+ // This field is only set when the enableProfilerTimer flag is enabled.
158
+ actualDuration?: number,
159
+
160
+ // If the Fiber is currently active in the "render" phase,
161
+ // This marks the time at which the work began.
162
+ // This field is only set when the enableProfilerTimer flag is enabled.
163
+ actualStartTime?: number,
164
+
165
+ // Duration of the most recent render time for this Fiber.
166
+ // This value is not updated when we bailout for memoization purposes.
167
+ // This field is only set when the enableProfilerTimer flag is enabled.
168
selfBaseTime?: number,
169
+
170
+ // Sum of base times for all descedents of this Fiber.
171
+ // This value bubbles up during the "complete" phase.
172
+ // This field is only set when the enableProfilerTimer flag is enabled.
173
treeBaseTime?: number,
174
175
// Conceptual aliases
@@ -211,6 +227,8 @@ function FiberNode(
227
this.alternate = null;
228
229
if (enableProfilerTimer) {
230
+ this.actualDuration = 0;
231
+ this.actualStartTime = 0;
232
this.selfBaseTime = 0;
233
this.treeBaseTime = 0;
234
}
@@ -295,6 +313,15 @@ export function createWorkInProgress(
313
workInProgress.nextEffect = null;
314
workInProgress.firstEffect = null;
315
workInProgress.lastEffect = null;
316
+
317
+ if (enableProfilerTimer) {
318
+ // We intentionally reset, rather than copy, actualDuration & actualStartTime.
319
+ // This prevents time from endlessly accumulating in new commits.
320
+ // This has the downside of resetting values for different priority renders,
321
+ // But works for yielding (the common case) and should support resuming.
322
+ workInProgress.actualDuration = 0;
323
+ workInProgress.actualStartTime = 0;
324
+ }
325
}
326
327
workInProgress.expirationTime = expirationTime;
@@ -460,13 +487,6 @@ export function createFiberFromProfiler(
487
const fiber = createFiber(Profiler, pendingProps, key, mode | ProfileMode);
488
fiber.type = REACT_PROFILER_TYPE;
489
fiber.expirationTime = expirationTime;
463
- if (enableProfilerTimer) {
464
- fiber.stateNode = {
465
- elapsedPauseTimeAtStart: 0,
466
- duration: 0,
467
- startTime: 0,
468
- };
469
- }
490
491
return fiber;
492
}
@@ -541,6 +561,8 @@ export function assignFiberPropertiesInDEV(
561
target.expirationTime = source.expirationTime;
562
target.alternate = source.alternate;
563
if (enableProfilerTimer) {
564
+ target.actualDuration = source.actualDuration;
565
+ target.actualStartTime = source.actualStartTime;
566
target.selfBaseTime = source.selfBaseTime;
567
target.treeBaseTime = source.treeBaseTime;
568
}
packages/react-reconciler/src/ReactFiberBeginWork.js
+7
-11
@@ -61,7 +61,7 @@ import {
61
} from './ReactChildFiber';
62
import {processUpdateQueue} from './ReactUpdateQueue';
63
import {NoWork, Never} from './ReactFiberExpirationTime';
64
-import {AsyncMode, StrictMode} from './ReactTypeOfMode';
64
+import {AsyncMode, ProfileMode, StrictMode} from './ReactTypeOfMode';
65
import {
66
shouldSetTextContent,
67
shouldDeprioritizeSubtree,
@@ -214,11 +214,6 @@ function updateMode(current, workInProgress) {
214
function updateProfiler(current, workInProgress) {
215
const nextProps = workInProgress.pendingProps;
216
if (enableProfilerTimer) {
217
- // Start render timer here and push start time onto queue
218
- markActualRenderTimeStarted(workInProgress);
219
-
220
- // Let the "complete" phase know to stop the timer,
221
- // And the scheduler to record the measured time.
217
workInProgress.effectTag |= Update;
218
}
219
if (workInProgress.memoizedProps === nextProps) {
@@ -1111,11 +1106,6 @@ function bailoutOnLowPriority(current, workInProgress) {
1106
case ContextProvider:
1107
pushProvider(workInProgress);
1108
break;
1114
- case Profiler:
1115
- if (enableProfilerTimer) {
1116
- markActualRenderTimeStarted(workInProgress);
1117
- }
1118
- break;
1109
}
1110
// TODO: What if this is currently in progress?
1111
// How can that happen? How is this not being cloned?
@@ -1138,6 +1128,12 @@ function beginWork(
1128
workInProgress: Fiber,
1129
renderExpirationTime: ExpirationTime,
1130
): Fiber | null {
1131
+ if (enableProfilerTimer) {
1132
+ if (workInProgress.mode & ProfileMode) {
1133
+ markActualRenderTimeStarted(workInProgress);
1134
+ }
1135
+ }
1136
+
1137
if (
1138
workInProgress.expirationTime === NoWork ||
1139
workInProgress.expirationTime > renderExpirationTime
packages/react-reconciler/src/ReactFiberCommitWork.js
+2
-6
@@ -806,15 +806,11 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {
806
onRender(
807
finishedWork.memoizedProps.id,
808
current === null ? 'mount' : 'update',
809
- finishedWork.stateNode.duration,
809
+ finishedWork.actualDuration,
810
finishedWork.treeBaseTime,
811
- finishedWork.stateNode.startTime,
811
+ finishedWork.actualStartTime,
812
getCommitTime(),
813
);
814
-
815
- // Reset actualTime after successful commit.
816
- // By default, we append to this time to account for errors and pauses.
817
- finishedWork.stateNode.duration = 0;
814
}
815
return;
816
}
packages/react-reconciler/src/ReactFiberCompleteWork.js
+8
-3
@@ -38,6 +38,7 @@ import {
38
TimeoutComponent,
39
} from 'shared/ReactTypeOfWork';
40
import {Placement, Ref, Update} from 'shared/ReactTypeOfSideEffect';
41
+import {ProfileMode} from './ReactTypeOfMode';
42
import invariant from 'fbjs/lib/invariant';
43
44
import {
@@ -312,6 +313,13 @@ function completeWork(
313
renderExpirationTime: ExpirationTime,
314
): Fiber | null {
315
const newProps = workInProgress.pendingProps;
316
+
317
+ if (enableProfilerTimer) {
318
+ if (workInProgress.mode & ProfileMode) {
319
+ recordElapsedActualRenderTime(workInProgress);
320
+ }
321
+ }
322
+
323
switch (workInProgress.tag) {
324
case FunctionalComponent:
325
return null;
@@ -489,9 +497,6 @@ function completeWork(
497
case Mode:
498
return null;
499
case Profiler:
492
- if (enableProfilerTimer) {
493
- recordElapsedActualRenderTime(workInProgress);
494
- }
500
return null;
501
case HostPortal:
502
popHostContainer(workInProgress);
packages/react-reconciler/src/ReactFiberScheduler.js
+7
@@ -106,6 +106,7 @@ import {
106
checkActualRenderTimeStackEmpty,
107
pauseActualRenderTimerIfRunning,
108
recordCommitTime,
109
+ recordElapsedActualRenderTime,
110
recordElapsedBaseRenderTimeIfRunning,
111
resetActualRenderTimer,
112
resumeActualRenderTimerIfPaused,
@@ -311,6 +312,10 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
312
clearCaughtError();
313
314
if (enableProfilerTimer) {
315
+ if (failedUnitOfWork.mode & ProfileMode) {
316
+ recordElapsedActualRenderTime(failedUnitOfWork);
317
+ }
318
+
319
// Stop "base" render timer again (after the re-thrown error).
320
stopBaseRenderTimerIfRunning();
321
}
@@ -573,6 +578,8 @@ function commitRoot(finishedWork: Fiber): ExpirationTime {
578
stopCommitSnapshotEffectsTimer();
579
580
if (enableProfilerTimer) {
581
+ // Mark the current commit time to be shared by all Profilers in this batch.
582
+ // This enables them to be grouped later.
583
recordCommitTime();
584
}
585
packages/react-reconciler/src/ReactFiberUnwindWork.js
+15
-8
@@ -20,7 +20,6 @@ import {
20
HostComponent,
21
HostPortal,
22
ContextProvider,
23
- Profiler,
23
TimeoutComponent,
24
} from 'shared/ReactTypeOfWork';
25
import {
@@ -34,6 +33,7 @@ import {
33
enableProfilerTimer,
34
enableSuspense,
35
} from 'shared/ReactFeatureFlags';
36
+import {ProfileMode} from './ReactTypeOfMode';
37
38
import {createCapturedValue} from './ReactCapturedValue';
39
import {
@@ -301,6 +301,12 @@ function unwindWork(
301
renderIsExpired: boolean,
302
renderExpirationTime: ExpirationTime,
303
) {
304
+ if (enableProfilerTimer) {
305
+ if (workInProgress.mode & ProfileMode) {
306
+ recordElapsedActualRenderTime(workInProgress);
307
+ }
308
+ }
309
+
310
switch (workInProgress.tag) {
311
case ClassComponent: {
312
popLegacyContextProvider(workInProgress);
@@ -345,6 +351,14 @@ function unwindWork(
351
}
352
353
function unwindInterruptedWork(interruptedWork: Fiber) {
354
+ if (enableProfilerTimer) {
355
+ if (interruptedWork.mode & ProfileMode) {
356
+ // Resume in case we're picking up on work that was paused.
357
+ resumeActualRenderTimerIfPaused();
358
+ recordElapsedActualRenderTime(interruptedWork);
359
+ }
360
+ }
361
+
362
switch (interruptedWork.tag) {
363
case ClassComponent: {
364
popLegacyContextProvider(interruptedWork);
@@ -365,13 +379,6 @@ function unwindInterruptedWork(interruptedWork: Fiber) {
379
case ContextProvider:
380
popProvider(interruptedWork);
381
break;
368
- case Profiler:
369
- if (enableProfilerTimer) {
370
- // Resume in case we're picking up on work that was paused.
371
- resumeActualRenderTimerIfPaused();
372
- recordElapsedActualRenderTime(interruptedWork);
373
- }
374
- break;
382
default:
383
break;
384
}
packages/react-reconciler/src/ReactProfilerTimer.js
+13
-9
@@ -9,6 +9,7 @@
9
10
import type {Fiber} from './ReactFiber';
11
12
+import getComponentName from 'shared/getComponentName';
13
import {enableProfilerTimer} from 'shared/ReactFeatureFlags';
14
15
import warning from 'fbjs/lib/warning';
@@ -76,9 +77,10 @@ function markActualRenderTimeStarted(fiber: Fiber): void {
77
if (__DEV__) {
78
fiberStack.push(fiber);
79
}
79
- const stateNode = fiber.stateNode;
80
- stateNode.elapsedPauseTimeAtStart = totalElapsedPauseTime;
81
- stateNode.startTime = now();
80
+
81
+ fiber.actualDuration =
82
+ now() - ((fiber.actualDuration: any): number) - totalElapsedPauseTime;
83
+ fiber.actualStartTime = now();
84
}
85
86
function pauseActualRenderTimerIfRunning(): void {
@@ -95,13 +97,15 @@ function recordElapsedActualRenderTime(fiber: Fiber): void {
97
return;
98
}
99
if (__DEV__) {
98
- warning(fiber === fiberStack.pop(), 'Unexpected Fiber popped.');
100
+ warning(
101
+ fiber === fiberStack.pop(),
102
+ 'Unexpected Fiber (%s) popped.',
103
+ getComponentName(fiber),
104
+ );
105
}
100
- const stateNode = fiber.stateNode;
101
- stateNode.duration +=
102
- now() -
103
- (totalElapsedPauseTime - stateNode.elapsedPauseTimeAtStart) -
104
- stateNode.startTime;
106
+
107
+ fiber.actualDuration =
108
+ now() - totalElapsedPauseTime - ((fiber.actualDuration: any): number);
109
}
110
111
function resetActualRenderTimer(): void {
packages/react/src/__tests__/ReactProfiler-test.internal.js
+32
-6
@@ -228,6 +228,28 @@ describe('Profiler', () => {
228
expect(call[3]).toBe(10); // base time
229
expect(call[4]).toBe(35); // start time
230
expect(call[5]).toBe(45); // commit time
231
+
232
+ callback.mockReset();
233
+
234
+ advanceTimeBy(20); // 45 -> 65
235
+
236
+ renderer.update(
237
+ <React.unstable_Profiler id="test" onRender={callback}>
238
+ <AdvanceTime byAmount={4} />
239
+ </React.unstable_Profiler>,
240
+ );
241
+
242
+ expect(callback).toHaveBeenCalledTimes(1);
243
+
244
+ [call] = callback.mock.calls;
245
+
246
+ expect(call).toHaveLength(6);
247
+ expect(call[0]).toBe('test');
248
+ expect(call[1]).toBe('update');
249
+ expect(call[2]).toBe(4); // actual time
250
+ expect(call[3]).toBe(4); // base time
251
+ expect(call[4]).toBe(65); // start time
252
+ expect(call[5]).toBe(69); // commit time
253
});
254
255
it('includes render times of nested Profilers in their parent times', () => {
@@ -576,7 +598,9 @@ describe('Profiler', () => {
598
</React.unstable_Profiler>,
599
{unstable_isAsync: true},
600
);
579
- expect(renderer.unstable_flushThrough(['first'])).toEqual(['Yield:10']);
601
+ expect(renderer.unstable_flushThrough(['Yield:10'])).toEqual([
602
+ 'Yield:10',
603
+ ]);
604
expect(callback).toHaveBeenCalledTimes(0);
605
606
// Simulate time moving forward while frame is paused.
@@ -681,11 +705,12 @@ describe('Profiler', () => {
705
}),
706
).toEqual(['Yield:11']);
707
684
- // Verify that the actual time includes all three durations above.
685
- // And the base time includes only the final rendered tree times.
708
+ // The actual time should include only the most recent render,
709
+ // Because this lets us avoid a lot of commit phase reset complexity.
710
+ // The base time includes only the final rendered tree times.
711
expect(callback).toHaveBeenCalledTimes(1);
712
call = callback.mock.calls[0];
688
- expect(call[2]).toBe(19); // actual time
713
+ expect(call[2]).toBe(11); // actual time
714
expect(call[3]).toBe(11); // base time
715
expect(call[4]).toBe(264); // start time
716
expect(call[5]).toBe(275); // commit time
@@ -772,12 +797,13 @@ describe('Profiler', () => {
797
renderer.unstable_flushSync(() => second.setState({renderTime: 30})),
798
).toEqual(['SecondComponent:30', 'Yield:7']);
799
775
- // Verify that the actual time includes time spent in the both renders so far (10ms and 37ms).
800
+ // The actual time should include only the most recent render (37ms),
801
+ // Because this lets us avoid a lot of commit phase reset complexity.
802
// The base time should include the more recent times for the SecondComponent subtree,
803
// As well as the original times for the FirstComponent subtree.
804
expect(callback).toHaveBeenCalledTimes(1);
805
call = callback.mock.calls[0];
780
- expect(call[2]).toBe(47); // actual time
806
+ expect(call[2]).toBe(37); // actual time
807
expect(call[3]).toBe(42); // base time
808
expect(call[4]).toBe(229); // start time
809
expect(call[5]).toBe(266); // commit time