Skip to content

Commit

Permalink
Scheduling Profiler: De-emphasize React internal frames (facebook#22588)
Browse files Browse the repository at this point in the history
This commit adds code to all React bundles to explicitly register the beginning and ending of the module. This is done by creating Error objects (which capture the file name, line number, and column number) and passing them explicitly to a DevTools hook (when present).

Next, as the Scheduling Profiler logs metadata to the User Timing API, it prints these module ranges along with other metadata (like Lane values and profiler version number).

Lastly, the Scheduling Profiler UI compares stack frames to these ranges when drawing the flame graph and dims or de-emphasizes frames that fall within an internal module.

The net effect of this is that user code (and 3rd party code) stands out clearly in the flame graph while React internal modules are dimmed.

Internal module ranges are completely optional. Older profiling samples, or ones recorded without the React DevTools extension installed, will simply not dim the internal frames.
  • Loading branch information
Brian Vaughn authored and zhengjitf committed Apr 15, 2022
1 parent 749b608 commit 42a754b
Show file tree
Hide file tree
Showing 21 changed files with 543 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@

declare var chrome: any;

import {__DEBUG__} from 'react-devtools-shared/src/constants';
import {
INTERNAL_EXTENSION_ID,
LOCAL_EXTENSION_ID,
__DEBUG__,
} from 'react-devtools-shared/src/constants';
import {getBrowserName} from './utils';
import {
EXTENSION_INSTALL_CHECK,
EXTENSION_INSTALLATION_TYPE,
INTERNAL_EXTENSION_ID,
LOCAL_EXTENSION_ID,
} from './constants';

const IS_CHROME = getBrowserName() === 'Chrome';
Expand Down
10 changes: 6 additions & 4 deletions packages/react-devtools-extensions/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
* @flow strict-local
*/

import {
CHROME_WEBSTORE_EXTENSION_ID,
INTERNAL_EXTENSION_ID,
LOCAL_EXTENSION_ID,
} from 'react-devtools-shared/src/constants';

declare var chrome: any;

export const CURRENT_EXTENSION_ID = chrome.runtime.id;
Expand All @@ -15,10 +21,6 @@ export const EXTENSION_INSTALL_CHECK = 'extension-install-check';
export const SHOW_DUPLICATE_EXTENSION_WARNING =
'show-duplicate-extension-warning';

export const CHROME_WEBSTORE_EXTENSION_ID = 'fmkadmapgofadopljbjfkapdkoienihi';
export const INTERNAL_EXTENSION_ID = 'dnjnjgbfilfphmojnmhliehogmojhclc';
export const LOCAL_EXTENSION_ID = 'ikiahnapldjmdmpkmfhjdjilojjhgcbf';

export const EXTENSION_INSTALLATION_TYPE:
| 'public'
| 'internal'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ function AutoSizedCanvas({
surface,
defaultFrame,
data.flamechart,
data.internalModuleSourceToRanges,
data.duration,
);
flamechartViewRef.current = flamechartView;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
Flamechart,
FlamechartStackFrame,
FlamechartStackLayer,
InternalModuleSourceToRanges,
} from '../types';
import type {
Interaction,
Expand All @@ -30,6 +31,7 @@ import {
rectIntersectsRect,
verticallyStackedLayout,
} from '../view-base';
import {isInternalModule} from './utils/moduleFilters';
import {
durationToWidth,
positioningScaleFactor,
Expand Down Expand Up @@ -76,6 +78,8 @@ class FlamechartStackLayerView extends View {
/** A set of `stackLayer`'s frames, for efficient lookup. */
_stackFrameSet: Set<FlamechartStackFrame>;

_internalModuleSourceToRanges: InternalModuleSourceToRanges;

_intrinsicSize: Size;

_hoveredStackFrame: FlamechartStackFrame | null = null;
Expand All @@ -85,11 +89,13 @@ class FlamechartStackLayerView extends View {
surface: Surface,
frame: Rect,
stackLayer: FlamechartStackLayer,
internalModuleSourceToRanges: InternalModuleSourceToRanges,
duration: number,
) {
super(surface, frame);
this._stackLayer = stackLayer;
this._stackFrameSet = new Set(stackLayer);
this._internalModuleSourceToRanges = internalModuleSourceToRanges;
this._intrinsicSize = {
width: duration,
height: FLAMECHART_FRAME_HEIGHT,
Expand Down Expand Up @@ -160,9 +166,19 @@ class FlamechartStackLayerView extends View {
}

const showHoverHighlight = _hoveredStackFrame === _stackLayer[i];
context.fillStyle = showHoverHighlight
? hoverColorForStackFrame(stackFrame)
: defaultColorForStackFrame(stackFrame);

let textFillStyle;
if (isInternalModule(this._internalModuleSourceToRanges, stackFrame)) {
context.fillStyle = showHoverHighlight
? COLORS.INTERNAL_MODULE_FRAME_HOVER
: COLORS.INTERNAL_MODULE_FRAME;
textFillStyle = COLORS.INTERNAL_MODULE_FRAME_TEXT;
} else {
context.fillStyle = showHoverHighlight
? hoverColorForStackFrame(stackFrame)
: defaultColorForStackFrame(stackFrame);
textFillStyle = COLORS.TEXT_COLOR;
}

const drawableRect = intersectionOfRects(nodeRect, visibleArea);
context.fillRect(
Expand All @@ -172,7 +188,9 @@ class FlamechartStackLayerView extends View {
drawableRect.size.height,
);

drawText(name, context, nodeRect, drawableRect);
drawText(name, context, nodeRect, drawableRect, {
fillStyle: textFillStyle,
});
}

// Render bottom border.
Expand Down Expand Up @@ -264,13 +282,22 @@ export class FlamechartView extends View {
surface: Surface,
frame: Rect,
flamechart: Flamechart,
internalModuleSourceToRanges: InternalModuleSourceToRanges,
duration: number,
) {
super(surface, frame, layeredLayout);
this.setDataAndUpdateSubviews(flamechart, duration);
this.setDataAndUpdateSubviews(
flamechart,
internalModuleSourceToRanges,
duration,
);
}

setDataAndUpdateSubviews(flamechart: Flamechart, duration: number) {
setDataAndUpdateSubviews(
flamechart: Flamechart,
internalModuleSourceToRanges: InternalModuleSourceToRanges,
duration: number,
) {
const {surface, frame, _onHover, _hoveredStackFrame} = this;

// Clear existing rows on data update
Expand All @@ -285,6 +312,7 @@ export class FlamechartView extends View {
surface,
frame,
stackLayer,
internalModuleSourceToRanges,
duration,
);
this._verticalStackView.addSubview(rowView);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ export const MIN_INTERVAL_SIZE_PX = 70;
// TODO Replace this with "export let" vars
export let COLORS = {
BACKGROUND: '',
INTERNAL_MODULE_FRAME: '',
INTERNAL_MODULE_FRAME_HOVER: '',
INTERNAL_MODULE_FRAME_TEXT: '',
NATIVE_EVENT: '',
NATIVE_EVENT_HOVER: '',
NETWORK_PRIMARY: '',
Expand Down Expand Up @@ -107,6 +110,15 @@ export function updateColorsToMatchTheme(element: Element): boolean {

COLORS = {
BACKGROUND: computedStyle.getPropertyValue('--color-background'),
INTERNAL_MODULE_FRAME: computedStyle.getPropertyValue(
'--color-scheduling-profiler-internal-module',
),
INTERNAL_MODULE_FRAME_HOVER: computedStyle.getPropertyValue(
'--color-scheduling-profiler-internal-module-hover',
),
INTERNAL_MODULE_FRAME_TEXT: computedStyle.getPropertyValue(
'--color-scheduling-profiler-internal-module-text',
),
NATIVE_EVENT: computedStyle.getPropertyValue(
'--color-scheduling-profiler-native-event',
),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

export const outerErrorA = new Error();

export const moduleStartError = new Error();
export const innerError = new Error();
export const moduleStopError = new Error();

export const outerErrorB = new Error();
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

export const moduleAStartError = new Error();
export const innerErrorA = new Error();
export const moduleAStopError = new Error();

export const outerError = new Error();

export const moduleBStartError = new Error();
export const innerErrorB = new Error();
export const moduleBStopError = new Error();
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import {isInternalModule} from '../moduleFilters';

describe('isInternalModule', () => {
let map;

function createFlamechartStackFrame(scriptUrl, locationLine, locationColumn) {
return {
name: 'test',
timestamp: 0,
duration: 1,
scriptUrl,
locationLine,
locationColumn,
};
}

function createStackFrame(fileName, lineNumber, columnNumber) {
return {
columnNumber: columnNumber,
lineNumber: lineNumber,
fileName: fileName,
functionName: 'test',
source: ` at test (${fileName}:${lineNumber}:${columnNumber})`,
};
}

beforeEach(() => {
map = new Map();
map.set('foo', [
[createStackFrame('foo', 10, 0), createStackFrame('foo', 15, 100)],
]);
map.set('bar', [
[createStackFrame('bar', 10, 0), createStackFrame('bar', 15, 100)],
[createStackFrame('bar', 20, 0), createStackFrame('bar', 25, 100)],
]);
});

it('should properly identify stack frames within the provided module ranges', () => {
expect(
isInternalModule(map, createFlamechartStackFrame('foo', 10, 0)),
).toBe(true);
expect(
isInternalModule(map, createFlamechartStackFrame('foo', 12, 35)),
).toBe(true);
expect(
isInternalModule(map, createFlamechartStackFrame('foo', 15, 100)),
).toBe(true);
expect(
isInternalModule(map, createFlamechartStackFrame('bar', 12, 0)),
).toBe(true);
expect(
isInternalModule(map, createFlamechartStackFrame('bar', 22, 125)),
).toBe(true);
});

it('should properly identify stack frames outside of the provided module ranges', () => {
expect(isInternalModule(map, createFlamechartStackFrame('foo', 9, 0))).toBe(
false,
);
expect(
isInternalModule(map, createFlamechartStackFrame('foo', 15, 101)),
).toBe(false);
expect(
isInternalModule(map, createFlamechartStackFrame('bar', 17, 0)),
).toBe(false);
expect(
isInternalModule(map, createFlamechartStackFrame('baz', 12, 0)),
).toBe(false);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import type {
FlamechartStackFrame,
InternalModuleSourceToRanges,
} from '../../types';

import {
CHROME_WEBSTORE_EXTENSION_ID,
INTERNAL_EXTENSION_ID,
LOCAL_EXTENSION_ID,
} from 'react-devtools-shared/src/constants';

export function isInternalModule(
internalModuleSourceToRanges: InternalModuleSourceToRanges,
flamechartStackFrame: FlamechartStackFrame,
): boolean {
const {locationColumn, locationLine, scriptUrl} = flamechartStackFrame;

if (scriptUrl == null || locationColumn == null || locationLine == null) {
// This could indicate a browser-internal API like performance.mark().
return false;
}

// Internal modules are only registered if DevTools was running when the profile was captured,
// but DevTools should also hide its own frames to avoid over-emphasizing them.
if (
// Handle webpack-internal:// sources
scriptUrl.includes('/react-devtools') ||
scriptUrl.includes('/react_devtools') ||
// Filter out known extension IDs
scriptUrl.includes(CHROME_WEBSTORE_EXTENSION_ID) ||
scriptUrl.includes(INTERNAL_EXTENSION_ID) ||
scriptUrl.includes(LOCAL_EXTENSION_ID)
// Unfortunately this won't get everything, like relatively loaded chunks or Web Worker files.
) {
return true;
}

// Filter out React internal packages.
const ranges = internalModuleSourceToRanges.get(scriptUrl);
if (ranges != null) {
for (let i = 0; i < ranges.length; i++) {
const [startStackFrame, stopStackFrame] = ranges[i];

const isAfterStart =
locationLine > startStackFrame.lineNumber ||
(locationLine === startStackFrame.lineNumber &&
locationColumn >= startStackFrame.columnNumber);
const isBeforeStop =
locationLine < stopStackFrame.lineNumber ||
(locationLine === stopStackFrame.lineNumber &&
locationColumn <= stopStackFrame.columnNumber);

if (isAfterStart && isBeforeStop) {
return true;
}
}
}

return false;
}
Loading

0 comments on commit 42a754b

Please sign in to comment.