Skip to content

Commit 9b35dd2

Browse files
author
Brian Vaughn
authored
Permanently removed component stacks from scheduling profiler data (facebook#19615)
These stacks improve the profiler data but they're expensive to generate and generating them can also cause runtime errors in larger applications (although an exact repro has been hard to nail down). Removing them for now. We can revisit adding them after this profiler has been integrated into the DevTools extension and we can generate them lazily.
1 parent 3f8115c commit 9b35dd2

12 files changed

+33
-172
lines changed

‎packages/react-reconciler/src/SchedulingProfiler.js

+9-52
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,9 @@ import type {Lane, Lanes} from './ReactFiberLane';
1111
import type {Fiber} from './ReactInternalTypes';
1212
import type {Wakeable} from 'shared/ReactTypes';
1313

14-
import {
15-
enableSchedulingProfiler,
16-
enableSchedulingProfilerComponentStacks,
17-
} from 'shared/ReactFeatureFlags';
14+
import {enableSchedulingProfiler} from 'shared/ReactFeatureFlags';
1815
import ReactVersion from 'shared/ReactVersion';
1916
import getComponentName from 'shared/getComponentName';
20-
import {getStackByFiberInDevAndProd} from './ReactFiberComponentStack';
2117

2218
/**
2319
* If performance exists and supports the subset of the User Timing API that we
@@ -65,51 +61,16 @@ function getWakeableID(wakeable: Wakeable): number {
6561
return ((wakeableIDs.get(wakeable): any): number);
6662
}
6763

68-
let getComponentStackByFiber = function getComponentStackByFiberDisabled(
69-
fiber: Fiber,
70-
): string {
71-
return '';
72-
};
73-
74-
if (enableSchedulingProfilerComponentStacks) {
75-
// $FlowFixMe: Flow cannot handle polymorphic WeakMaps
76-
const cachedFiberStacks: WeakMap<Fiber, string> = new PossiblyWeakMap();
77-
getComponentStackByFiber = function cacheFirstGetComponentStackByFiber(
78-
fiber: Fiber,
79-
): string {
80-
if (cachedFiberStacks.has(fiber)) {
81-
return ((cachedFiberStacks.get(fiber): any): string);
82-
} else {
83-
const alternate = fiber.alternate;
84-
if (alternate !== null && cachedFiberStacks.has(alternate)) {
85-
return ((cachedFiberStacks.get(alternate): any): string);
86-
}
87-
}
88-
// TODO (brian) Generate and store temporary ID so DevTools can match up a component stack later.
89-
const componentStack = getStackByFiberInDevAndProd(fiber) || '';
90-
cachedFiberStacks.set(fiber, componentStack);
91-
return componentStack;
92-
};
93-
}
94-
9564
export function markComponentSuspended(fiber: Fiber, wakeable: Wakeable): void {
9665
if (enableSchedulingProfiler) {
9766
if (supportsUserTiming) {
9867
const id = getWakeableID(wakeable);
9968
const componentName = getComponentName(fiber.type) || 'Unknown';
100-
const componentStack = getComponentStackByFiber(fiber);
101-
performance.mark(
102-
`--suspense-suspend-${id}-${componentName}-${componentStack}`,
103-
);
69+
// TODO Add component stack id
70+
performance.mark(`--suspense-suspend-${id}-${componentName}`);
10471
wakeable.then(
105-
() =>
106-
performance.mark(
107-
`--suspense-resolved-${id}-${componentName}-${componentStack}`,
108-
),
109-
() =>
110-
performance.mark(
111-
`--suspense-rejected-${id}-${componentName}-${componentStack}`,
112-
),
72+
() => performance.mark(`--suspense-resolved-${id}-${componentName}`),
73+
() => performance.mark(`--suspense-rejected-${id}-${componentName}`),
11374
);
11475
}
11576
}
@@ -183,11 +144,9 @@ export function markForceUpdateScheduled(fiber: Fiber, lane: Lane): void {
183144
if (enableSchedulingProfiler) {
184145
if (supportsUserTiming) {
185146
const componentName = getComponentName(fiber.type) || 'Unknown';
186-
const componentStack = getComponentStackByFiber(fiber);
147+
// TODO Add component stack id
187148
performance.mark(
188-
`--schedule-forced-update-${formatLanes(
189-
lane,
190-
)}-${componentName}-${componentStack}`,
149+
`--schedule-forced-update-${formatLanes(lane)}-${componentName}`,
191150
);
192151
}
193152
}
@@ -197,11 +156,9 @@ export function markStateUpdateScheduled(fiber: Fiber, lane: Lane): void {
197156
if (enableSchedulingProfiler) {
198157
if (supportsUserTiming) {
199158
const componentName = getComponentName(fiber.type) || 'Unknown';
200-
const componentStack = getComponentStackByFiber(fiber);
159+
// TODO Add component stack id
201160
performance.mark(
202-
`--schedule-state-update-${formatLanes(
203-
lane,
204-
)}-${componentName}-${componentStack}`,
161+
`--schedule-state-update-${formatLanes(lane)}-${componentName}`,
205162
);
206163
}
207164
}

‎packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js

+24-105
Original file line numberDiff line numberDiff line change
@@ -12,29 +12,6 @@
1212

1313
import ReactVersion from 'shared/ReactVersion';
1414

15-
function normalizeCodeLocInfo(str) {
16-
return (
17-
str &&
18-
str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) {
19-
return '\n in ' + name + ' (at **)';
20-
})
21-
);
22-
}
23-
24-
// TODO (enableSchedulingProfilerComponentStacks) Clean this up once the feature flag has been removed.
25-
function toggleComponentStacks(mark) {
26-
let expectedMark = mark;
27-
gate(({enableSchedulingProfilerComponentStacks}) => {
28-
if (!enableSchedulingProfilerComponentStacks) {
29-
const index = mark.indexOf('\n ');
30-
if (index >= 0) {
31-
expectedMark = mark.substr(0, index);
32-
}
33-
}
34-
});
35-
return expectedMark;
36-
}
37-
3815
describe('SchedulingProfiler', () => {
3916
let React;
4017
let ReactTestRenderer;
@@ -162,9 +139,7 @@ describe('SchedulingProfiler', () => {
162139
`--react-init-${ReactVersion}`,
163140
'--schedule-render-1',
164141
'--render-start-1',
165-
toggleComponentStacks(
166-
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
167-
),
142+
'--suspense-suspend-0-Example',
168143
'--render-stop',
169144
'--commit-start-1',
170145
'--layout-effects-start-1',
@@ -175,11 +150,7 @@ describe('SchedulingProfiler', () => {
175150
marks.splice(0);
176151

177152
await fakeSuspensePromise;
178-
expect(marks).toEqual([
179-
toggleComponentStacks(
180-
'--suspense-resolved-0-Example-\n at Example\n at Suspense',
181-
),
182-
]);
153+
expect(marks).toEqual(['--suspense-resolved-0-Example']);
183154
});
184155

185156
// @gate enableSchedulingProfiler
@@ -199,9 +170,7 @@ describe('SchedulingProfiler', () => {
199170
`--react-init-${ReactVersion}`,
200171
'--schedule-render-1',
201172
'--render-start-1',
202-
toggleComponentStacks(
203-
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
204-
),
173+
'--suspense-suspend-0-Example',
205174
'--render-stop',
206175
'--commit-start-1',
207176
'--layout-effects-start-1',
@@ -212,11 +181,7 @@ describe('SchedulingProfiler', () => {
212181
marks.splice(0);
213182

214183
await expect(fakeSuspensePromise).rejects.toThrow();
215-
expect(marks).toEqual([
216-
toggleComponentStacks(
217-
'--suspense-rejected-0-Example-\n at Example\n at Suspense',
218-
),
219-
]);
184+
expect(marks).toEqual(['--suspense-rejected-0-Example']);
220185
});
221186

222187
// @gate enableSchedulingProfiler
@@ -244,9 +209,7 @@ describe('SchedulingProfiler', () => {
244209

245210
expect(marks).toEqual([
246211
'--render-start-512',
247-
toggleComponentStacks(
248-
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
249-
),
212+
'--suspense-suspend-0-Example',
250213
'--render-stop',
251214
'--commit-start-512',
252215
'--layout-effects-start-512',
@@ -257,11 +220,7 @@ describe('SchedulingProfiler', () => {
257220
marks.splice(0);
258221

259222
await fakeSuspensePromise;
260-
expect(marks).toEqual([
261-
toggleComponentStacks(
262-
'--suspense-resolved-0-Example-\n at Example\n at Suspense',
263-
),
264-
]);
223+
expect(marks).toEqual(['--suspense-resolved-0-Example']);
265224
});
266225

267226
// @gate enableSchedulingProfiler
@@ -289,9 +248,7 @@ describe('SchedulingProfiler', () => {
289248

290249
expect(marks).toEqual([
291250
'--render-start-512',
292-
toggleComponentStacks(
293-
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
294-
),
251+
'--suspense-suspend-0-Example',
295252
'--render-stop',
296253
'--commit-start-512',
297254
'--layout-effects-start-512',
@@ -302,11 +259,7 @@ describe('SchedulingProfiler', () => {
302259
marks.splice(0);
303260

304261
await expect(fakeSuspensePromise).rejects.toThrow();
305-
expect(marks).toEqual([
306-
toggleComponentStacks(
307-
'--suspense-rejected-0-Example-\n at Example\n at Suspense',
308-
),
309-
]);
262+
expect(marks).toEqual(['--suspense-rejected-0-Example']);
310263
});
311264

312265
// @gate enableSchedulingProfiler
@@ -332,14 +285,12 @@ describe('SchedulingProfiler', () => {
332285

333286
expect(Scheduler).toFlushUntilNextPaint([]);
334287

335-
expect(marks.map(normalizeCodeLocInfo)).toEqual([
288+
expect(marks).toEqual([
336289
'--render-start-512',
337290
'--render-stop',
338291
'--commit-start-512',
339292
'--layout-effects-start-512',
340-
toggleComponentStacks(
341-
'--schedule-state-update-1-Example-\n in Example (at **)',
342-
),
293+
'--schedule-state-update-1-Example',
343294
'--layout-effects-stop',
344295
'--render-start-1',
345296
'--render-stop',
@@ -371,14 +322,12 @@ describe('SchedulingProfiler', () => {
371322

372323
expect(Scheduler).toFlushUntilNextPaint([]);
373324

374-
expect(marks.map(normalizeCodeLocInfo)).toEqual([
325+
expect(marks).toEqual([
375326
'--render-start-512',
376327
'--render-stop',
377328
'--commit-start-512',
378329
'--layout-effects-start-512',
379-
toggleComponentStacks(
380-
'--schedule-forced-update-1-Example-\n in Example (at **)',
381-
),
330+
'--schedule-forced-update-1-Example',
382331
'--layout-effects-stop',
383332
'--render-start-1',
384333
'--render-stop',
@@ -415,16 +364,8 @@ describe('SchedulingProfiler', () => {
415364

416365
gate(({old}) =>
417366
old
418-
? expect(marks.map(normalizeCodeLocInfo)).toContain(
419-
toggleComponentStacks(
420-
'--schedule-state-update-1024-Example-\n in Example (at **)',
421-
),
422-
)
423-
: expect(marks.map(normalizeCodeLocInfo)).toContain(
424-
toggleComponentStacks(
425-
'--schedule-state-update-512-Example-\n in Example (at **)',
426-
),
427-
),
367+
? expect(marks).toContain('--schedule-state-update-1024-Example')
368+
: expect(marks).toContain('--schedule-state-update-512-Example'),
428369
);
429370
});
430371

@@ -455,16 +396,8 @@ describe('SchedulingProfiler', () => {
455396

456397
gate(({old}) =>
457398
old
458-
? expect(marks.map(normalizeCodeLocInfo)).toContain(
459-
toggleComponentStacks(
460-
'--schedule-forced-update-1024-Example-\n in Example (at **)',
461-
),
462-
)
463-
: expect(marks.map(normalizeCodeLocInfo)).toContain(
464-
toggleComponentStacks(
465-
'--schedule-forced-update-512-Example-\n in Example (at **)',
466-
),
467-
),
399+
? expect(marks).toContain('--schedule-forced-update-1024-Example')
400+
: expect(marks).toContain('--schedule-forced-update-512-Example'),
468401
);
469402
});
470403

@@ -489,14 +422,12 @@ describe('SchedulingProfiler', () => {
489422

490423
expect(Scheduler).toFlushUntilNextPaint([]);
491424

492-
expect(marks.map(normalizeCodeLocInfo)).toEqual([
425+
expect(marks).toEqual([
493426
'--render-start-512',
494427
'--render-stop',
495428
'--commit-start-512',
496429
'--layout-effects-start-512',
497-
toggleComponentStacks(
498-
'--schedule-state-update-1-Example-\n in Example (at **)',
499-
),
430+
'--schedule-state-update-1-Example',
500431
'--layout-effects-stop',
501432
'--render-start-1',
502433
'--render-stop',
@@ -522,7 +453,7 @@ describe('SchedulingProfiler', () => {
522453

523454
gate(({old}) => {
524455
if (old) {
525-
expect(marks.map(normalizeCodeLocInfo)).toEqual([
456+
expect(marks).toEqual([
526457
`--react-init-${ReactVersion}`,
527458
'--schedule-render-512',
528459
'--render-start-512',
@@ -532,17 +463,15 @@ describe('SchedulingProfiler', () => {
532463
'--layout-effects-stop',
533464
'--commit-stop',
534465
'--passive-effects-start-512',
535-
toggleComponentStacks(
536-
'--schedule-state-update-1024-Example-\n in Example (at **)',
537-
),
466+
'--schedule-state-update-1024-Example',
538467
'--passive-effects-stop',
539468
'--render-start-1024',
540469
'--render-stop',
541470
'--commit-start-1024',
542471
'--commit-stop',
543472
]);
544473
} else {
545-
expect(marks.map(normalizeCodeLocInfo)).toEqual([
474+
expect(marks).toEqual([
546475
`--react-init-${ReactVersion}`,
547476
'--schedule-render-512',
548477
'--render-start-512',
@@ -552,9 +481,7 @@ describe('SchedulingProfiler', () => {
552481
'--layout-effects-stop',
553482
'--commit-stop',
554483
'--passive-effects-start-512',
555-
toggleComponentStacks(
556-
'--schedule-state-update-1024-Example-\n in Example (at **)',
557-
),
484+
'--schedule-state-update-1024-Example',
558485
'--passive-effects-stop',
559486
'--render-start-1024',
560487
'--render-stop',
@@ -583,16 +510,8 @@ describe('SchedulingProfiler', () => {
583510

584511
gate(({old}) =>
585512
old
586-
? expect(marks.map(normalizeCodeLocInfo)).toContain(
587-
toggleComponentStacks(
588-
'--schedule-state-update-1024-Example-\n in Example (at **)',
589-
),
590-
)
591-
: expect(marks.map(normalizeCodeLocInfo)).toContain(
592-
toggleComponentStacks(
593-
'--schedule-state-update-512-Example-\n in Example (at **)',
594-
),
595-
),
513+
? expect(marks).toContain('--schedule-state-update-1024-Example')
514+
: expect(marks).toContain('--schedule-state-update-512-Example'),
596515
);
597516
});
598517
});

‎packages/shared/ReactFeatureFlags.js

-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ export const enableDebugTracing = false;
1818
// Adds user timing marks for e.g. state updates, suspense, and work loop stuff,
1919
// for an experimental scheduling profiler tool.
2020
export const enableSchedulingProfiler = __PROFILE__ && __EXPERIMENTAL__;
21-
export const enableSchedulingProfilerComponentStacks = false;
2221

2322
// Helps identify side effects in render-phase lifecycle hooks and setState
2423
// reducers by double invoking them in Strict Mode.

‎packages/shared/forks/ReactFeatureFlags.native-fb.js

-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import typeof * as ExportsType from './ReactFeatureFlags.native-fb';
1313
// The rest of the flags are static for better dead code elimination.
1414
export const enableDebugTracing = false;
1515
export const enableSchedulingProfiler = false;
16-
export const enableSchedulingProfilerComponentStacks = false;
1716
export const enableProfilerTimer = __PROFILE__;
1817
export const enableProfilerCommitHooks = false;
1918
export const enableSchedulerTracing = __PROFILE__;

‎packages/shared/forks/ReactFeatureFlags.native-oss.js

-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import typeof * as ExportsType from './ReactFeatureFlags.native-oss';
1313
export const debugRenderPhaseSideEffectsForStrictMode = false;
1414
export const enableDebugTracing = false;
1515
export const enableSchedulingProfiler = false;
16-
export const enableSchedulingProfilerComponentStacks = false;
1716
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
1817
export const warnAboutDeprecatedLifecycles = true;
1918
export const enableProfilerTimer = __PROFILE__;

0 commit comments

Comments
 (0)