Skip to content

Commit

Permalink
Permanently removed component stacks from scheduling profiler data (f…
Browse files Browse the repository at this point in the history
…acebook#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.
  • Loading branch information
Brian Vaughn authored and koto committed Jun 15, 2021
1 parent 9abceb5 commit be1c872
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 172 deletions.
61 changes: 9 additions & 52 deletions packages/react-reconciler/src/SchedulingProfiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,9 @@ import type {Lane, Lanes} from './ReactFiberLane';
import type {Fiber} from './ReactInternalTypes';
import type {Wakeable} from 'shared/ReactTypes';

import {
enableSchedulingProfiler,
enableSchedulingProfilerComponentStacks,
} from 'shared/ReactFeatureFlags';
import {enableSchedulingProfiler} from 'shared/ReactFeatureFlags';
import ReactVersion from 'shared/ReactVersion';
import getComponentName from 'shared/getComponentName';
import {getStackByFiberInDevAndProd} from './ReactFiberComponentStack';

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

let getComponentStackByFiber = function getComponentStackByFiberDisabled(
fiber: Fiber,
): string {
return '';
};

if (enableSchedulingProfilerComponentStacks) {
// $FlowFixMe: Flow cannot handle polymorphic WeakMaps
const cachedFiberStacks: WeakMap<Fiber, string> = new PossiblyWeakMap();
getComponentStackByFiber = function cacheFirstGetComponentStackByFiber(
fiber: Fiber,
): string {
if (cachedFiberStacks.has(fiber)) {
return ((cachedFiberStacks.get(fiber): any): string);
} else {
const alternate = fiber.alternate;
if (alternate !== null && cachedFiberStacks.has(alternate)) {
return ((cachedFiberStacks.get(alternate): any): string);
}
}
// TODO (brian) Generate and store temporary ID so DevTools can match up a component stack later.
const componentStack = getStackByFiberInDevAndProd(fiber) || '';
cachedFiberStacks.set(fiber, componentStack);
return componentStack;
};
}

export function markComponentSuspended(fiber: Fiber, wakeable: Wakeable): void {
if (enableSchedulingProfiler) {
if (supportsUserTiming) {
const id = getWakeableID(wakeable);
const componentName = getComponentName(fiber.type) || 'Unknown';
const componentStack = getComponentStackByFiber(fiber);
performance.mark(
`--suspense-suspend-${id}-${componentName}-${componentStack}`,
);
// TODO Add component stack id
performance.mark(`--suspense-suspend-${id}-${componentName}`);
wakeable.then(
() =>
performance.mark(
`--suspense-resolved-${id}-${componentName}-${componentStack}`,
),
() =>
performance.mark(
`--suspense-rejected-${id}-${componentName}-${componentStack}`,
),
() => performance.mark(`--suspense-resolved-${id}-${componentName}`),
() => performance.mark(`--suspense-rejected-${id}-${componentName}`),
);
}
}
Expand Down Expand Up @@ -183,11 +144,9 @@ export function markForceUpdateScheduled(fiber: Fiber, lane: Lane): void {
if (enableSchedulingProfiler) {
if (supportsUserTiming) {
const componentName = getComponentName(fiber.type) || 'Unknown';
const componentStack = getComponentStackByFiber(fiber);
// TODO Add component stack id
performance.mark(
`--schedule-forced-update-${formatLanes(
lane,
)}-${componentName}-${componentStack}`,
`--schedule-forced-update-${formatLanes(lane)}-${componentName}`,
);
}
}
Expand All @@ -197,11 +156,9 @@ export function markStateUpdateScheduled(fiber: Fiber, lane: Lane): void {
if (enableSchedulingProfiler) {
if (supportsUserTiming) {
const componentName = getComponentName(fiber.type) || 'Unknown';
const componentStack = getComponentStackByFiber(fiber);
// TODO Add component stack id
performance.mark(
`--schedule-state-update-${formatLanes(
lane,
)}-${componentName}-${componentStack}`,
`--schedule-state-update-${formatLanes(lane)}-${componentName}`,
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,6 @@

import ReactVersion from 'shared/ReactVersion';

function normalizeCodeLocInfo(str) {
return (
str &&
str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) {
return '\n in ' + name + ' (at **)';
})
);
}

// TODO (enableSchedulingProfilerComponentStacks) Clean this up once the feature flag has been removed.
function toggleComponentStacks(mark) {
let expectedMark = mark;
gate(({enableSchedulingProfilerComponentStacks}) => {
if (!enableSchedulingProfilerComponentStacks) {
const index = mark.indexOf('\n ');
if (index >= 0) {
expectedMark = mark.substr(0, index);
}
}
});
return expectedMark;
}

describe('SchedulingProfiler', () => {
let React;
let ReactTestRenderer;
Expand Down Expand Up @@ -162,9 +139,7 @@ describe('SchedulingProfiler', () => {
`--react-init-${ReactVersion}`,
'--schedule-render-1',
'--render-start-1',
toggleComponentStacks(
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
),
'--suspense-suspend-0-Example',
'--render-stop',
'--commit-start-1',
'--layout-effects-start-1',
Expand All @@ -175,11 +150,7 @@ describe('SchedulingProfiler', () => {
marks.splice(0);

await fakeSuspensePromise;
expect(marks).toEqual([
toggleComponentStacks(
'--suspense-resolved-0-Example-\n at Example\n at Suspense',
),
]);
expect(marks).toEqual(['--suspense-resolved-0-Example']);
});

// @gate enableSchedulingProfiler
Expand All @@ -199,9 +170,7 @@ describe('SchedulingProfiler', () => {
`--react-init-${ReactVersion}`,
'--schedule-render-1',
'--render-start-1',
toggleComponentStacks(
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
),
'--suspense-suspend-0-Example',
'--render-stop',
'--commit-start-1',
'--layout-effects-start-1',
Expand All @@ -212,11 +181,7 @@ describe('SchedulingProfiler', () => {
marks.splice(0);

await expect(fakeSuspensePromise).rejects.toThrow();
expect(marks).toEqual([
toggleComponentStacks(
'--suspense-rejected-0-Example-\n at Example\n at Suspense',
),
]);
expect(marks).toEqual(['--suspense-rejected-0-Example']);
});

// @gate enableSchedulingProfiler
Expand Down Expand Up @@ -244,9 +209,7 @@ describe('SchedulingProfiler', () => {

expect(marks).toEqual([
'--render-start-512',
toggleComponentStacks(
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
),
'--suspense-suspend-0-Example',
'--render-stop',
'--commit-start-512',
'--layout-effects-start-512',
Expand All @@ -257,11 +220,7 @@ describe('SchedulingProfiler', () => {
marks.splice(0);

await fakeSuspensePromise;
expect(marks).toEqual([
toggleComponentStacks(
'--suspense-resolved-0-Example-\n at Example\n at Suspense',
),
]);
expect(marks).toEqual(['--suspense-resolved-0-Example']);
});

// @gate enableSchedulingProfiler
Expand Down Expand Up @@ -289,9 +248,7 @@ describe('SchedulingProfiler', () => {

expect(marks).toEqual([
'--render-start-512',
toggleComponentStacks(
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
),
'--suspense-suspend-0-Example',
'--render-stop',
'--commit-start-512',
'--layout-effects-start-512',
Expand All @@ -302,11 +259,7 @@ describe('SchedulingProfiler', () => {
marks.splice(0);

await expect(fakeSuspensePromise).rejects.toThrow();
expect(marks).toEqual([
toggleComponentStacks(
'--suspense-rejected-0-Example-\n at Example\n at Suspense',
),
]);
expect(marks).toEqual(['--suspense-rejected-0-Example']);
});

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

expect(Scheduler).toFlushUntilNextPaint([]);

expect(marks.map(normalizeCodeLocInfo)).toEqual([
expect(marks).toEqual([
'--render-start-512',
'--render-stop',
'--commit-start-512',
'--layout-effects-start-512',
toggleComponentStacks(
'--schedule-state-update-1-Example-\n in Example (at **)',
),
'--schedule-state-update-1-Example',
'--layout-effects-stop',
'--render-start-1',
'--render-stop',
Expand Down Expand Up @@ -371,14 +322,12 @@ describe('SchedulingProfiler', () => {

expect(Scheduler).toFlushUntilNextPaint([]);

expect(marks.map(normalizeCodeLocInfo)).toEqual([
expect(marks).toEqual([
'--render-start-512',
'--render-stop',
'--commit-start-512',
'--layout-effects-start-512',
toggleComponentStacks(
'--schedule-forced-update-1-Example-\n in Example (at **)',
),
'--schedule-forced-update-1-Example',
'--layout-effects-stop',
'--render-start-1',
'--render-stop',
Expand Down Expand Up @@ -415,16 +364,8 @@ describe('SchedulingProfiler', () => {

gate(({old}) =>
old
? expect(marks.map(normalizeCodeLocInfo)).toContain(
toggleComponentStacks(
'--schedule-state-update-1024-Example-\n in Example (at **)',
),
)
: expect(marks.map(normalizeCodeLocInfo)).toContain(
toggleComponentStacks(
'--schedule-state-update-512-Example-\n in Example (at **)',
),
),
? expect(marks).toContain('--schedule-state-update-1024-Example')
: expect(marks).toContain('--schedule-state-update-512-Example'),
);
});

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

gate(({old}) =>
old
? expect(marks.map(normalizeCodeLocInfo)).toContain(
toggleComponentStacks(
'--schedule-forced-update-1024-Example-\n in Example (at **)',
),
)
: expect(marks.map(normalizeCodeLocInfo)).toContain(
toggleComponentStacks(
'--schedule-forced-update-512-Example-\n in Example (at **)',
),
),
? expect(marks).toContain('--schedule-forced-update-1024-Example')
: expect(marks).toContain('--schedule-forced-update-512-Example'),
);
});

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

expect(Scheduler).toFlushUntilNextPaint([]);

expect(marks.map(normalizeCodeLocInfo)).toEqual([
expect(marks).toEqual([
'--render-start-512',
'--render-stop',
'--commit-start-512',
'--layout-effects-start-512',
toggleComponentStacks(
'--schedule-state-update-1-Example-\n in Example (at **)',
),
'--schedule-state-update-1-Example',
'--layout-effects-stop',
'--render-start-1',
'--render-stop',
Expand All @@ -522,7 +453,7 @@ describe('SchedulingProfiler', () => {

gate(({old}) => {
if (old) {
expect(marks.map(normalizeCodeLocInfo)).toEqual([
expect(marks).toEqual([
`--react-init-${ReactVersion}`,
'--schedule-render-512',
'--render-start-512',
Expand All @@ -532,17 +463,15 @@ describe('SchedulingProfiler', () => {
'--layout-effects-stop',
'--commit-stop',
'--passive-effects-start-512',
toggleComponentStacks(
'--schedule-state-update-1024-Example-\n in Example (at **)',
),
'--schedule-state-update-1024-Example',
'--passive-effects-stop',
'--render-start-1024',
'--render-stop',
'--commit-start-1024',
'--commit-stop',
]);
} else {
expect(marks.map(normalizeCodeLocInfo)).toEqual([
expect(marks).toEqual([
`--react-init-${ReactVersion}`,
'--schedule-render-512',
'--render-start-512',
Expand All @@ -552,9 +481,7 @@ describe('SchedulingProfiler', () => {
'--layout-effects-stop',
'--commit-stop',
'--passive-effects-start-512',
toggleComponentStacks(
'--schedule-state-update-1024-Example-\n in Example (at **)',
),
'--schedule-state-update-1024-Example',
'--passive-effects-stop',
'--render-start-1024',
'--render-stop',
Expand Down Expand Up @@ -583,16 +510,8 @@ describe('SchedulingProfiler', () => {

gate(({old}) =>
old
? expect(marks.map(normalizeCodeLocInfo)).toContain(
toggleComponentStacks(
'--schedule-state-update-1024-Example-\n in Example (at **)',
),
)
: expect(marks.map(normalizeCodeLocInfo)).toContain(
toggleComponentStacks(
'--schedule-state-update-512-Example-\n in Example (at **)',
),
),
? expect(marks).toContain('--schedule-state-update-1024-Example')
: expect(marks).toContain('--schedule-state-update-512-Example'),
);
});
});
1 change: 0 additions & 1 deletion packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export const enableDebugTracing = false;
// Adds user timing marks for e.g. state updates, suspense, and work loop stuff,
// for an experimental scheduling profiler tool.
export const enableSchedulingProfiler = __PROFILE__ && __EXPERIMENTAL__;
export const enableSchedulingProfilerComponentStacks = false;

// Helps identify side effects in render-phase lifecycle hooks and setState
// reducers by double invoking them in Strict Mode.
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import typeof * as ExportsType from './ReactFeatureFlags.native-fb';
// The rest of the flags are static for better dead code elimination.
export const enableDebugTracing = false;
export const enableSchedulingProfiler = false;
export const enableSchedulingProfilerComponentStacks = false;
export const enableProfilerTimer = __PROFILE__;
export const enableProfilerCommitHooks = false;
export const enableSchedulerTracing = __PROFILE__;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import typeof * as ExportsType from './ReactFeatureFlags.native-oss';
export const debugRenderPhaseSideEffectsForStrictMode = false;
export const enableDebugTracing = false;
export const enableSchedulingProfiler = false;
export const enableSchedulingProfilerComponentStacks = false;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
export const warnAboutDeprecatedLifecycles = true;
export const enableProfilerTimer = __PROFILE__;
Expand Down
Loading

0 comments on commit be1c872

Please sign in to comment.