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.

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

Add page information for JS frames and display it inside JS tooltips #2300

Merged
merged 4 commits into from Nov 15, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 19 additions & 6 deletions docs-developer/gecko-profile-format.md
Expand Up @@ -72,6 +72,17 @@ The source data format is de-duplicated to make it quicker to transfer in the JS
}
},

// Array of visited pages during the profiling session.

pages: [
canova marked this conversation as resolved.
Show resolved Hide resolved
{
browsingContextID: 10,
innerWindowID: 2,
url: "https://profiler.firefox.com",
embedderInnerWindowID: 0,
}
],

// Array of shared library data.

libs: [
Expand Down Expand Up @@ -189,19 +200,21 @@ The source data format is de-duplicated to make it quicker to transfer in the JS
schema: {
location: 0,
relevantForJS: 1,
implementation: 2,
optimizations: 3,
line: 4,
column: 5,
category: 6,
subcategory: 7
innerWindowID: 2
gregtatum marked this conversation as resolved.
Show resolved Hide resolved
implementation: 3,
optimizations: 4,
line: 5,
column: 6,
category: 7,
subcategory: 8,
},
data: [
[
18, // index into stringTable, points to strings like:
// JS: "Startup::XRE_Main"
// C++: "0x7fff7d962da1"
false, // for label frames, whether this label should be shown in "JS only" stacks
2 // for JS frames, an innerWindowID that corresponds to a page object in the profile.pages array.
40, // for JS frames, an index into the string table, usually "Baseline" or "Ion"
null, // JSON info about JIT optimizations.
1536, // The line of code
Expand Down
4 changes: 2 additions & 2 deletions src/app-logic/constants.js
Expand Up @@ -5,10 +5,10 @@
// @flow

// The current version of the Gecko profile format.
export const GECKO_PROFILE_VERSION = 18;
export const GECKO_PROFILE_VERSION = 19;

// The current version of the "processed" profile format.
export const PROCESSED_PROFILE_VERSION = 26;
export const PROCESSED_PROFILE_VERSION = 27;

// The following are the margin sizes for the left and right of the timeline. Independent
// components need to share these values.
Expand Down
5 changes: 4 additions & 1 deletion src/components/flame-graph/Canvas.js
Expand Up @@ -16,7 +16,7 @@ import { TooltipCallNode } from '../tooltip/CallNode';
import { getTimingsForCallNodeIndex } from '../../profile-logic/profile-data';
import MixedTupleMap from 'mixedtuplemap';

import type { Thread, CategoryList } from '../../types/profile';
import type { Thread, CategoryList, PageList } from '../../types/profile';
import type { CssPixels, Milliseconds } from '../../types/units';
import type {
FlameGraphTiming,
Expand All @@ -34,6 +34,7 @@ import type { CallTreeSummaryStrategy } from '../../types/actions';

export type OwnProps = {|
+thread: Thread,
+pages: PageList | null,
+unfilteredThread: Thread,
+sampleIndexOffset: number,
+maxStackDepth: number,
Expand Down Expand Up @@ -260,6 +261,7 @@ class FlameGraphCanvas extends React.PureComponent<Props> {
interval,
isInverted,
callTreeSummaryStrategy,
pages,
} = this.props;

if (!shouldDisplayTooltips()) {
Expand All @@ -285,6 +287,7 @@ class FlameGraphCanvas extends React.PureComponent<Props> {
// doesn't over-render.
<TooltipCallNode
thread={thread}
pages={pages}
interval={interval}
callNodeIndex={callNodeIndex}
callNodeInfo={callNodeInfo}
Expand Down
7 changes: 6 additions & 1 deletion src/components/flame-graph/FlameGraph.js
Expand Up @@ -12,6 +12,7 @@ import {
getPreviewSelection,
getScrollToSelectionGeneration,
getProfileInterval,
getPageList,
} from '../../selectors/profile';
import { selectedThreadSelectors } from '../../selectors/per-thread';
import {
Expand All @@ -28,7 +29,7 @@ import {
import { getIconsWithClassNames } from '../../selectors/icons';
import { BackgroundImageStyleDef } from '../shared/StyleDef';

import type { Thread, CategoryList } from '../../types/profile';
import type { Thread, CategoryList, PageList } from '../../types/profile';
import type { Milliseconds } from '../../types/units';
import type { FlameGraphTiming } from '../../profile-logic/flame-graph';
import type {
Expand Down Expand Up @@ -57,6 +58,7 @@ const SELECTABLE_THRESHOLD = 0.001;

type StateProps = {|
+thread: Thread,
+pages: PageList | null,
+unfilteredThread: Thread,
+sampleIndexOffset: number,
+maxStackDepth: number,
Expand Down Expand Up @@ -268,6 +270,7 @@ class FlameGraph extends React.PureComponent<Props> {
categories,
interval,
isInverted,
pages,
} = this.props;

const maxViewportHeight = maxStackDepth * STACK_FRAME_HEIGHT;
Expand Down Expand Up @@ -305,6 +308,7 @@ class FlameGraph extends React.PureComponent<Props> {
// FlameGraphCanvas props
chartProps={{
thread,
pages,
unfilteredThread,
sampleIndexOffset,
maxStackDepth,
Expand Down Expand Up @@ -366,6 +370,7 @@ export default explicitConnect<{||}, StateProps, DispatchProps>({
interval: getProfileInterval(state),
isInverted: getInvertCallstack(state),
callTreeSummaryStrategy: getCallTreeSummaryStrategy(state),
pages: getPageList(state),
};
},
mapDispatchToProps: {
Expand Down
5 changes: 4 additions & 1 deletion src/components/stack-chart/Canvas.js
Expand Up @@ -22,7 +22,7 @@ import { updatePreviewSelection } from '../../actions/profile-view';
import { mapCategoryColorNameToStackChartStyles } from '../../utils/colors';
import { TooltipCallNode } from '../tooltip/CallNode';

import type { Thread, CategoryList } from '../../types/profile';
import type { Thread, CategoryList, PageList } from '../../types/profile';
import type {
CallNodeInfo,
IndexIntoCallNodeTable,
Expand All @@ -43,6 +43,7 @@ import type { WrapFunctionInDispatch } from '../../utils/connect';

type OwnProps = {|
+thread: Thread,
+pages: PageList | null,
+interval: Milliseconds,
+rangeStart: Milliseconds,
+rangeEnd: Milliseconds,
Expand Down Expand Up @@ -392,6 +393,7 @@ class StackChartCanvas extends React.PureComponent<Props> {
callNodeInfo,
shouldDisplayTooltips,
interval,
pages,
} = this.props;

if (!shouldDisplayTooltips()) {
Expand All @@ -406,6 +408,7 @@ class StackChartCanvas extends React.PureComponent<Props> {
return (
<TooltipCallNode
thread={thread}
pages={pages}
interval={interval}
callNodeIndex={callNodeIndex}
callNodeInfo={callNodeInfo}
Expand Down
7 changes: 6 additions & 1 deletion src/components/stack-chart/index.js
Expand Up @@ -17,6 +17,7 @@ import {
getPreviewSelection,
getScrollToSelectionGeneration,
getCategories,
getPageList,
} from '../../selectors/profile';
import { selectedThreadSelectors } from '../../selectors/per-thread';
import { getSelectedThreadIndex } from '../../selectors/url-state';
Expand All @@ -31,7 +32,7 @@ import {
} from '../../actions/profile-view';

import { getCallNodePathFromIndex } from '../../profile-logic/profile-data';
import type { Thread, CategoryList } from '../../types/profile';
import type { Thread, CategoryList, PageList } from '../../types/profile';
import type {
CallNodeInfo,
IndexIntoCallNodeTable,
Expand All @@ -50,6 +51,7 @@ const STACK_FRAME_HEIGHT = 16;

type StateProps = {|
+thread: Thread,
+pages: PageList | null,
+maxStackDepth: number,
+stackTimingByDepth: StackTimingByDepth,
+timeRange: { start: Milliseconds, end: Milliseconds },
Expand Down Expand Up @@ -140,6 +142,7 @@ class StackChartGraph extends React.PureComponent<Props> {
categories,
selectedCallNodeIndex,
scrollToSelectionGeneration,
pages,
} = this.props;

const maxViewportHeight = maxStackDepth * STACK_FRAME_HEIGHT;
Expand Down Expand Up @@ -177,6 +180,7 @@ class StackChartGraph extends React.PureComponent<Props> {
chartProps={{
interval,
thread,
pages,
stackTimingByDepth,
// $FlowFixMe Error introduced by upgrading to v0.96.0. See issue #1936.
updatePreviewSelection,
Expand Down Expand Up @@ -223,6 +227,7 @@ export default explicitConnect<{||}, StateProps, DispatchProps>({
state
),
scrollToSelectionGeneration: getScrollToSelectionGeneration(state),
pages: getPageList(state),
};
},
mapDispatchToProps: {
Expand Down
48 changes: 46 additions & 2 deletions src/components/tooltip/CallNode.js
Expand Up @@ -14,7 +14,7 @@ import {
} from '../../profile-logic/profile-data';

import type { CallTree } from '../../profile-logic/call-tree';
import type { Thread, CategoryList } from '../../types/profile';
import type { Thread, CategoryList, PageList } from '../../types/profile';
import type {
IndexIntoCallNodeTable,
CallNodeDisplayData,
Expand All @@ -31,6 +31,7 @@ const GRAPH_HEIGHT = 10;

type Props = {|
+thread: Thread,
+pages: PageList | null,
+callNodeIndex: IndexIntoCallNodeTable,
+callNodeInfo: CallNodeInfo,
+categories: CategoryList,
Expand Down Expand Up @@ -157,12 +158,14 @@ export class TooltipCallNode extends React.PureComponent<Props> {
callTree,
timings,
callTreeSummaryStrategy,
pages,
callNodeInfo: { callNodeTable },
} = this.props;
const categoryIndex = callNodeTable.category[callNodeIndex];
const categoryColor = categories[categoryIndex].color;
const subcategoryIndex = callNodeTable.subcategory[callNodeIndex];
const funcIndex = callNodeTable.func[callNodeIndex];
const innerWindowID = callNodeTable.innerWindowID[callNodeIndex];
const funcStringIndex = thread.funcTable.name[funcIndex];
const funcName = thread.stringTable.getString(funcStringIndex);

Expand Down Expand Up @@ -190,7 +193,7 @@ export class TooltipCallNode extends React.PureComponent<Props> {
// the elements as direct children.
resourceOrFileName = [
<div className="tooltipLabel" key="file">
File:
Script URL:
</div>,
fileName,
];
Expand All @@ -212,6 +215,45 @@ export class TooltipCallNode extends React.PureComponent<Props> {
}
}

// Finding current frame and parent frame URL(if there is).
let pageAndParentPageURL;
if (pages) {
const page = pages.find(p => p.innerWindowID === innerWindowID);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional, or follow-up: I noticed in the codecov report that this area wasn't covered by a test. It'd be good to snapshot test, and maybe use getByText to find the URLs in the component.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding tests for this, but probably it would be good to review those tests. Will open a follow-up PR for that.

if (page) {
if (page.embedderInnerWindowID !== 0) {
// This is an iframe since it has an embedder.
pageAndParentPageURL = [
<div className="tooltipLabel" key="iframe">
iframe URL:
</div>,
<div key="iframeVal">{page.url}</div>,
];

// Getting the embedder URL now.
const parentPage = pages.find(
p => p.innerWindowID === page.embedderInnerWindowID
);
// Ideally it should find a page.
if (parentPage) {
pageAndParentPageURL.push(
<div className="tooltipLabel" key="page">
Page URL:
</div>,
<div key="pageVal">{parentPage.url}</div>
);
}
} else {
// This is a regular page without an embedder.
pageAndParentPageURL = [
<div className="tooltipLabel" key="page">
Page URL:
</div>,
<div key="pageVal">{page.url}</div>,
];
}
}
}

const stackType = getStackType(thread, funcIndex);
let stackTypeLabel;
switch (stackType) {
Expand Down Expand Up @@ -279,6 +321,8 @@ export class TooltipCallNode extends React.PureComponent<Props> {
)}
</div>
{/* --------------------------------------------------------------- */}
{pageAndParentPageURL}
{/* --------------------------------------------------------------- */}
{resourceOrFileName}
</div>
</div>
Expand Down
1 change: 1 addition & 0 deletions src/profile-logic/comparison.js
Expand Up @@ -567,6 +567,7 @@ function combineFrameTables(
// really implement one.
newFrameTable.subcategory.push(frameTable.subcategory[i]);
newFrameTable.func.push(newFunc);
newFrameTable.innerWindowID.push(frameTable.innerWindowID[i]);
newFrameTable.implementation.push(
implementation === null
? null
Expand Down
2 changes: 2 additions & 0 deletions src/profile-logic/data-structures.js
Expand Up @@ -89,6 +89,7 @@ export function getEmptyFrameTable(): FrameTable {
category: [],
subcategory: [],
func: [],
innerWindowID: [],
implementation: [],
line: [],
column: [],
Expand All @@ -107,6 +108,7 @@ export function shallowCloneFrameTable(frameTable: FrameTable): FrameTable {
category: frameTable.category.slice(),
subcategory: frameTable.subcategory.slice(),
func: frameTable.func.slice(),
innerWindowID: frameTable.innerWindowID.slice(),
implementation: frameTable.implementation.slice(),
line: frameTable.line.slice(),
column: frameTable.column.slice(),
Expand Down
33 changes: 33 additions & 0 deletions src/profile-logic/gecko-profile-versioning.js
Expand Up @@ -826,5 +826,38 @@ const _upgraders = {
}
convertToVersion18Recursive(profile);
},
[19]: profile => {
// Profiles now have an innerWindowID property in the frameTable.
// We are filling this array with 0 values because we have no idea what that value might be.
canova marked this conversation as resolved.
Show resolved Hide resolved
function convertToVersion19Recursive(p) {
for (const thread of p.threads) {
const { frameTable } = thread;
frameTable.schema = {
location: 0,
relevantForJS: 1,
innerWindowID: 2,
implementation: 3,
optimizations: 4,
line: 5,
column: 6,
category: 7,
subcategory: 8,
};
for (
let frameIndex = 0;
frameIndex < frameTable.data.length;
frameIndex++
) {
// Adding 0 for every frame.
const innerWindowIDIndex = frameTable.schema.innerWindowID;
frameTable.data[frameIndex].splice(innerWindowIDIndex, 0, 0);
}
}
for (const subprocessProfile of p.processes) {
convertToVersion19Recursive(subprocessProfile);
}
}
convertToVersion19Recursive(profile);
},
};
/* eslint-enable no-useless-computed-key */
1 change: 1 addition & 0 deletions src/profile-logic/import/chrome.js
Expand Up @@ -242,6 +242,7 @@ async function processTracingEvents(
frameTable.category[frameIndex] = javascriptCategoryIndex;
frameTable.subcategory[frameIndex] = 0;
frameTable.func[frameIndex] = funcId;
frameTable.innerWindowID[frameIndex] = 0;
frameTable.implementation[frameIndex] = null;
frameTable.line[frameIndex] =
lineNumber === undefined ? null : lineNumber;
Expand Down
1 change: 1 addition & 0 deletions src/profile-logic/js-tracer.js
Expand Up @@ -633,6 +633,7 @@ export function convertJsTracerToThreadWithoutSamples(
frameTable.address.push(blankStringIndex);
frameTable.category.push(otherCategory);
frameTable.func.push(funcIndex);
frameTable.innerWindowID.push(0);
frameTable.implementation.push(implementation);
frameTable.line.push(line);
frameTable.column.push(column);
Expand Down