Skip to content

Commit

Permalink
Move profiler component stacks behind a feature flag
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Jul 17, 2020
1 parent d466d95 commit 2d15507
Show file tree
Hide file tree
Showing 11 changed files with 114 additions and 36 deletions.
49 changes: 31 additions & 18 deletions packages/react-reconciler/src/SchedulingProfiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import type {Lane, Lanes} from './ReactFiberLane';
import type {Fiber} from './ReactInternalTypes';
import type {Wakeable} from 'shared/ReactTypes';

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

Expand Down Expand Up @@ -54,29 +57,39 @@ function getWakeableID(wakeable: Wakeable): number {
return ((wakeableIDs.get(wakeable): any): number);
}

// $FlowFixMe: Flow cannot handle polymorphic WeakMaps
const cachedFiberStacks: WeakMap<Fiber, string> = new PossiblyWeakMap();
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);
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;
// 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 = cacheFirstGetComponentStackByFiber(fiber);
const componentStack = getComponentStackByFiber(fiber);
performance.mark(
`--suspense-suspend-${id}-${componentName}-${componentStack}`,
);
Expand Down Expand Up @@ -162,7 +175,7 @@ export function markForceUpdateScheduled(fiber: Fiber, lane: Lane): void {
if (enableSchedulingProfiler) {
if (supportsUserTiming) {
const componentName = getComponentName(fiber.type) || 'Unknown';
const componentStack = cacheFirstGetComponentStackByFiber(fiber);
const componentStack = getComponentStackByFiber(fiber);
performance.mark(
`--schedule-forced-update-${formatLanes(
lane,
Expand All @@ -176,7 +189,7 @@ export function markStateUpdateScheduled(fiber: Fiber, lane: Lane): void {
if (enableSchedulingProfiler) {
if (supportsUserTiming) {
const componentName = getComponentName(fiber.type) || 'Unknown';
const componentStack = cacheFirstGetComponentStackByFiber(fiber);
const componentStack = getComponentStackByFiber(fiber);
performance.mark(
`--schedule-state-update-${formatLanes(
lane,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,20 @@ function normalizeCodeLocInfo(str) {
);
}

// 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 @@ -136,7 +150,9 @@ describe('SchedulingProfiler', () => {
expect(marks).toEqual([
'--schedule-render-1',
'--render-start-1',
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
toggleComponentStacks(
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
),
'--render-stop',
'--commit-start-1',
'--layout-effects-start-1',
Expand All @@ -148,7 +164,9 @@ describe('SchedulingProfiler', () => {

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

Expand All @@ -168,7 +186,9 @@ describe('SchedulingProfiler', () => {
expect(marks).toEqual([
'--schedule-render-1',
'--render-start-1',
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
toggleComponentStacks(
'--suspense-suspend-0-Example-\n at Example\n at Suspense',
),
'--render-stop',
'--commit-start-1',
'--layout-effects-start-1',
Expand All @@ -180,7 +200,9 @@ describe('SchedulingProfiler', () => {

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

Expand All @@ -206,7 +228,9 @@ describe('SchedulingProfiler', () => {

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

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

Expand All @@ -244,7 +270,9 @@ describe('SchedulingProfiler', () => {

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

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

Expand Down Expand Up @@ -285,7 +315,9 @@ describe('SchedulingProfiler', () => {
'--render-stop',
'--commit-start-512',
'--layout-effects-start-512',
'--schedule-state-update-1-Example-\n in Example (at **)',
toggleComponentStacks(
'--schedule-state-update-1-Example-\n in Example (at **)',
),
'--layout-effects-stop',
'--render-start-1',
'--render-stop',
Expand Down Expand Up @@ -319,7 +351,9 @@ describe('SchedulingProfiler', () => {
'--render-stop',
'--commit-start-512',
'--layout-effects-start-512',
'--schedule-forced-update-1-Example-\n in Example (at **)',
toggleComponentStacks(
'--schedule-forced-update-1-Example-\n in Example (at **)',
),
'--layout-effects-stop',
'--render-start-1',
'--render-stop',
Expand Down Expand Up @@ -354,10 +388,14 @@ describe('SchedulingProfiler', () => {
gate(({old}) =>
old
? expect(marks.map(normalizeCodeLocInfo)).toContain(
'--schedule-state-update-1024-Example-\n in Example (at **)',
toggleComponentStacks(
'--schedule-state-update-1024-Example-\n in Example (at **)',
),
)
: expect(marks.map(normalizeCodeLocInfo)).toContain(
'--schedule-state-update-512-Example-\n in Example (at **)',
toggleComponentStacks(
'--schedule-state-update-512-Example-\n in Example (at **)',
),
),
);
});
Expand Down Expand Up @@ -387,10 +425,14 @@ describe('SchedulingProfiler', () => {
gate(({old}) =>
old
? expect(marks.map(normalizeCodeLocInfo)).toContain(
'--schedule-forced-update-1024-Example-\n in Example (at **)',
toggleComponentStacks(
'--schedule-forced-update-1024-Example-\n in Example (at **)',
),
)
: expect(marks.map(normalizeCodeLocInfo)).toContain(
'--schedule-forced-update-512-Example-\n in Example (at **)',
toggleComponentStacks(
'--schedule-forced-update-512-Example-\n in Example (at **)',
),
),
);
});
Expand Down Expand Up @@ -418,7 +460,9 @@ describe('SchedulingProfiler', () => {
'--render-stop',
'--commit-start-512',
'--layout-effects-start-512',
'--schedule-state-update-1-Example-\n in Example (at **)',
toggleComponentStacks(
'--schedule-state-update-1-Example-\n in Example (at **)',
),
'--layout-effects-stop',
'--render-start-1',
'--render-stop',
Expand All @@ -441,6 +485,7 @@ describe('SchedulingProfiler', () => {
ReactTestRenderer.act(() => {
ReactTestRenderer.create(<Example />, {unstable_isConcurrent: true});
});

expect(marks.map(normalizeCodeLocInfo)).toEqual([
'--schedule-render-512',
'--render-start-512',
Expand All @@ -450,7 +495,9 @@ describe('SchedulingProfiler', () => {
'--layout-effects-stop',
'--commit-stop',
'--passive-effects-start-512',
'--schedule-state-update-1024-Example-\n in Example (at **)',
toggleComponentStacks(
'--schedule-state-update-1024-Example-\n in Example (at **)',
),
'--passive-effects-stop',
'--render-start-1024',
'--render-stop',
Expand All @@ -476,10 +523,14 @@ describe('SchedulingProfiler', () => {
gate(({old}) =>
old
? expect(marks.map(normalizeCodeLocInfo)).toContain(
'--schedule-state-update-1024-Example-\n in Example (at **)',
toggleComponentStacks(
'--schedule-state-update-1024-Example-\n in Example (at **)',
),
)
: expect(marks.map(normalizeCodeLocInfo)).toContain(
'--schedule-state-update-512-Example-\n in Example (at **)',
toggleComponentStacks(
'--schedule-state-update-512-Example-\n in Example (at **)',
),
),
);
});
Expand Down
1 change: 1 addition & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ 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: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import typeof * as ExportsType from './ReactFeatureFlags.test-renderer';
export const debugRenderPhaseSideEffectsForStrictMode = false;
export const enableDebugTracing = false;
export const enableSchedulingProfiler = false;
export const enableSchedulingProfilerComponentStacks = false;
export const warnAboutDeprecatedLifecycles = true;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
export const enableProfilerTimer = __PROFILE__;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import typeof * as ExportsType from './ReactFeatureFlags.test-renderer.www';
export const debugRenderPhaseSideEffectsForStrictMode = false;
export const enableDebugTracing = false;
export const enableSchedulingProfiler = false;
export const enableSchedulingProfilerComponentStacks = false;
export const warnAboutDeprecatedLifecycles = true;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
export const enableProfilerTimer = __PROFILE__;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import typeof * as ExportsType from './ReactFeatureFlags.testing';
export const debugRenderPhaseSideEffectsForStrictMode = false;
export const enableDebugTracing = false;
export const enableSchedulingProfiler = false;
export const enableSchedulingProfilerComponentStacks = false;
export const warnAboutDeprecatedLifecycles = true;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
export const enableProfilerTimer = __PROFILE__;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import typeof * as ExportsType from './ReactFeatureFlags.testing.www';
export const debugRenderPhaseSideEffectsForStrictMode = false;
export const enableDebugTracing = false;
export const enableSchedulingProfiler = false;
export const enableSchedulingProfilerComponentStacks = false;
export const warnAboutDeprecatedLifecycles = true;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
export const enableProfilerTimer = false;
Expand Down
6 changes: 6 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ export const decoupleUpdatePriorityFromScheduler = __VARIANT__;
// NOTE: This feature will only work in DEV mode; all callsights are wrapped with __DEV__.
export const enableDebugTracing = false;

// TODO: getStackByFiberInDevAndProd() causes errors when synced to www.
// This flag can be used to disable component stacks for the profiler marks,
// so that the feature can be synced for others,
// while still enabling investigation into the underlying source of the errors.
export const enableSchedulingProfilerComponentStacks = false;

// This only has an effect in the new reconciler. But also, the new reconciler
// is only enabled when __VARIANT__ is true. So this is set to the opposite of
// __VARIANT__ so that it's `false` when running against the new reconciler.
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export const {
deferRenderPhaseUpdateToNextBatch,
decoupleUpdatePriorityFromScheduler,
enableDebugTracing,
enableSchedulingProfilerComponentStacks,
enableFormEventDelegation,
} = dynamicFeatureFlags;

Expand Down

0 comments on commit 2d15507

Please sign in to comment.