Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Permanently removed component stacks from scheduling profiler data #19615

Merged
merged 1 commit into from
Aug 14, 2020
Merged

Permanently removed component stacks from scheduling profiler data #19615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 9 additions & 52 deletions packages/react-reconciler/src/SchedulingProfiler.js
View file
Open in desktop
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
129 changes: 24 additions & 105 deletions packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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