Skip to content

Commit c580082

Browse files
authored
Measure time between scheduling an async callback and flushing it (#11506)
* Measure time between scheduling an async callback and flushing it Helps detect starvation issues. * Debug comments should print directly above the next measure * Better warning message Most users won't know what "expires" means
1 parent ae7639c commit c580082

File tree

4 files changed

+129
-22
lines changed

4 files changed

+129
-22
lines changed

packages/react-reconciler/src/ReactDebugFiberPerf.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ let hasScheduledUpdateInCurrentCommit: boolean = false;
5858
let hasScheduledUpdateInCurrentPhase: boolean = false;
5959
let commitCountInCurrentWorkLoop: number = 0;
6060
let effectCountInCurrentCommit: number = 0;
61+
let isWaitingForCallback: boolean = false;
6162
// During commits, we only show a measurement once per method name
6263
// to avoid stretch the commit phase with measurement overhead.
6364
const labelsInCurrentCommit: Set<string> = new Set();
@@ -231,6 +232,29 @@ export function recordScheduleUpdate(): void {
231232
}
232233
}
233234

235+
export function startRequestCallbackTimer(): void {
236+
if (enableUserTimingAPI) {
237+
if (supportsUserTiming && !isWaitingForCallback) {
238+
isWaitingForCallback = true;
239+
beginMark('(Waiting for async callback...)');
240+
}
241+
}
242+
}
243+
244+
export function stopRequestCallbackTimer(didExpire: boolean): void {
245+
if (enableUserTimingAPI) {
246+
if (supportsUserTiming) {
247+
isWaitingForCallback = false;
248+
const warning = didExpire ? 'React was blocked by main thread' : null;
249+
endMark(
250+
'(Waiting for async callback...)',
251+
'(Waiting for async callback...)',
252+
warning,
253+
);
254+
}
255+
}
256+
}
257+
234258
export function startWorkTimer(fiber: Fiber): void {
235259
if (enableUserTimingAPI) {
236260
if (!supportsUserTiming || shouldIgnoreFiber(fiber)) {

packages/react-reconciler/src/ReactFiberScheduler.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
HostPortal,
3434
ClassComponent,
3535
} from 'shared/ReactTypeOfWork';
36+
import {enableUserTimingAPI} from 'shared/ReactFeatureFlags';
3637
import getComponentName from 'shared/getComponentName';
3738
import invariant from 'fbjs/lib/invariant';
3839
import warning from 'fbjs/lib/warning';
@@ -47,6 +48,8 @@ import ReactDebugCurrentFiber from './ReactDebugCurrentFiber';
4748
import {
4849
recordEffect,
4950
recordScheduleUpdate,
51+
startRequestCallbackTimer,
52+
stopRequestCallbackTimer,
5053
startWorkTimer,
5154
stopWorkTimer,
5255
stopFailedWorkTimer,
@@ -1337,6 +1340,7 @@ export default function<T, P, I, TI, PI, C, CC, CX, PL>(
13371340
performWork(Sync, null);
13381341
} else if (!isCallbackScheduled) {
13391342
isCallbackScheduled = true;
1343+
startRequestCallbackTimer();
13401344
scheduleDeferredCallback(performAsyncWork);
13411345
}
13421346
}
@@ -1425,8 +1429,14 @@ export default function<T, P, I, TI, PI, C, CC, CX, PL>(
14251429
deadline = dl;
14261430

14271431
// Keep working on roots until there's no more work, or until the we reach
1428-
// the deadlne.
1432+
// the deadline.
14291433
findHighestPriorityRoot();
1434+
1435+
if (enableUserTimingAPI && deadline !== null) {
1436+
const didExpire = nextFlushedExpirationTime < recalculateCurrentTime();
1437+
stopRequestCallbackTimer(didExpire);
1438+
}
1439+
14301440
while (
14311441
nextFlushedRoot !== null &&
14321442
nextFlushedExpirationTime !== NoWork &&
@@ -1449,6 +1459,7 @@ export default function<T, P, I, TI, PI, C, CC, CX, PL>(
14491459
// If there's work left over, schedule a new callback.
14501460
if (nextFlushedRoot !== null && !isCallbackScheduled) {
14511461
isCallbackScheduled = true;
1462+
startRequestCallbackTimer();
14521463
scheduleDeferredCallback(performAsyncWork);
14531464
}
14541465

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ describe('ReactDebugFiberPerf', () => {
2020
let activeMeasure;
2121
let knownMarks;
2222
let knownMeasures;
23+
let comments;
2324

2425
function resetFlamechart() {
2526
root = {
@@ -35,12 +36,11 @@ describe('ReactDebugFiberPerf', () => {
3536
activeMeasure = root;
3637
knownMarks = new Set();
3738
knownMeasures = new Set();
39+
comments = [];
3840
}
3941

4042
function addComment(comment) {
41-
activeMeasure.children.push(
42-
`${' '.repeat(activeMeasure.indent + 1)}// ${comment}`,
43-
);
43+
comments.push(comment);
4444
}
4545

4646
function getFlameChart() {
@@ -67,9 +67,11 @@ describe('ReactDebugFiberPerf', () => {
6767
// Will be assigned on measure() call:
6868
label: null,
6969
parent: activeMeasure,
70+
comments,
7071
toString() {
7172
return (
7273
[
74+
...this.comments.map(c => ' '.repeat(this.indent) + '// ' + c),
7375
' '.repeat(this.indent) + this.label,
7476
...this.children.map(c => c.toString()),
7577
].join('\n') +
@@ -78,6 +80,7 @@ describe('ReactDebugFiberPerf', () => {
7880
);
7981
},
8082
};
83+
comments = [];
8184
// Step one level deeper
8285
activeMeasure.children.push(measure);
8386
activeMeasure = measure;
@@ -546,4 +549,15 @@ describe('ReactDebugFiberPerf', () => {
546549
ReactNoop.flush();
547550
expect(getFlameChart()).toMatchSnapshot();
548551
});
552+
553+
it('warns if async work expires (starvation)', () => {
554+
function Foo() {
555+
return <span />;
556+
}
557+
558+
ReactNoop.render(<Foo />);
559+
ReactNoop.expire(5000);
560+
ReactNoop.flush();
561+
expect(getFlameChart()).toMatchSnapshot();
562+
});
549563
});

0 commit comments

Comments
 (0)