Skip to content
89 changes: 38 additions & 51 deletions src/actions/profile-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,77 +148,64 @@ export function changeRightClickedCallNode(
}

/**
* Given a threadIndex and a sampleIndex, select the call node at the top ("leaf")
* of that sample's stack.
* Given a threadIndex and a sampleIndex, select the call node which carries the
* sample's self time. In the inverted tree, this will be a root node.
*/
export function selectLeafCallNode(
export function selectSelfCallNode(
threadsKey: ThreadsKey,
sampleIndex: IndexIntoSamplesTable | null
): ThunkAction<void> {
return (dispatch, getState) => {
const threadSelectors = getThreadSelectorsFromThreadsKey(threadsKey);
const sampleCallNodes =
threadSelectors.getSampleIndexToCallNodeIndexForFilteredThread(
threadSelectors.getSampleIndexToNonInvertedCallNodeIndexForFilteredThread(
getState()
);
const callNodeInfo = threadSelectors.getCallNodeInfo(getState());

let newSelectedCallNode = -1;
if (
sampleIndex !== null &&
sampleIndex >= 0 &&
sampleIndex < sampleCallNodes.length
sampleIndex === null ||
sampleIndex < 0 ||
sampleIndex >= sampleCallNodes.length
) {
newSelectedCallNode = sampleCallNodes[sampleIndex] ?? -1;
}

dispatch(
changeSelectedCallNode(
threadsKey,
callNodeInfo.getCallNodePathFromIndex(newSelectedCallNode)
)
);
};
}

/**
* Given a threadIndex and a sampleIndex, select the call node at the bottom ("root")
* of that sample's stack.
*/
export function selectRootCallNode(
threadsKey: ThreadsKey,
sampleIndex: IndexIntoSamplesTable | null
): ThunkAction<void> {
return (dispatch, getState) => {
const threadSelectors = getThreadSelectorsFromThreadsKey(threadsKey);
const filteredThread = threadSelectors.getFilteredThread(getState());
const callNodeInfo = threadSelectors.getCallNodeInfo(getState());

if (sampleIndex === null) {
dispatch(changeSelectedCallNode(threadsKey, []));
return;
}
const newSelectedStack = filteredThread.samples.stack[sampleIndex];
if (newSelectedStack === null || newSelectedStack === undefined) {

const nonInvertedSelfCallNode = sampleCallNodes[sampleIndex];
if (nonInvertedSelfCallNode === null) {
dispatch(changeSelectedCallNode(threadsKey, []));
return;
}
const stackIndexToCallNodeIndex =
callNodeInfo.getStackIndexToCallNodeIndex();
const newSelectedCallNode = stackIndexToCallNodeIndex[newSelectedStack];

const selectedCallNodePath =
callNodeInfo.getCallNodePathFromIndex(newSelectedCallNode);
const rootCallNodePath = [selectedCallNodePath[0]];
const callNodeInfo = threadSelectors.getCallNodeInfo(getState());

dispatch(
changeSelectedCallNode(
threadsKey,
rootCallNodePath,
{ source: 'auto' },
selectedCallNodePath
)
);
// Compute the call path based on the non-inverted call node table.
// We're not calling callNodeInfo.getCallNodePathFromIndex here because we
// only have a non-inverted call node index, which wouldn't be accepted by
// the inverted call node info.
const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable();
const callNodePath = [];
let cni = nonInvertedSelfCallNode;
while (cni !== -1) {
callNodePath.push(callNodeTable.func[cni]);
cni = callNodeTable.prefix[cni];
}

if (callNodeInfo.isInverted()) {
// In the inverted tree, we want to select the inverted tree root node
// with the "self" function, and also expand the path to the non-inverted root.
dispatch(
changeSelectedCallNode(
threadsKey,
callNodePath.slice(0, 1), // Select a root node
{ source: 'auto' },
callNodePath // Expand the full path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, this makes me think we might want to do that as well when switching from non-inverted to inverted.
(looking at the code, it looks like we're trying to do it but it's buggy)

)
);
} else {
// In the non-inverted tree, we want to select the self node.
dispatch(changeSelectedCallNode(threadsKey, callNodePath.reverse()));
}
};
}

Expand Down
6 changes: 2 additions & 4 deletions src/components/flame-graph/Canvas.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class FlameGraphCanvasImpl extends React.PureComponent<Props> {
return;
}

const callNodeTable = callNodeInfo.getCallNodeTable();
const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable();
const depth = callNodeTable.depth[selectedCallNodeIndex];
const y = (maxStackDepthPlusOne - depth - 1) * ROW_HEIGHT;

Expand Down Expand Up @@ -237,7 +237,7 @@ class FlameGraphCanvasImpl extends React.PureComponent<Props> {
fastFillStyle.set('#ffffff');
ctx.fillRect(0, 0, deviceContainerWidth, deviceContainerHeight);

const callNodeTable = callNodeInfo.getCallNodeTable();
const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable();

const startDepth = Math.floor(
maxStackDepthPlusOne - viewportBottom / stackFrameHeight
Expand Down Expand Up @@ -367,7 +367,6 @@ class FlameGraphCanvasImpl extends React.PureComponent<Props> {
shouldDisplayTooltips,
categories,
interval,
isInverted,
callTreeSummaryStrategy,
innerWindowIDToPageMap,
weightType,
Expand Down Expand Up @@ -426,7 +425,6 @@ class FlameGraphCanvasImpl extends React.PureComponent<Props> {
callNodeIndex,
callNodeInfo,
interval,
isInverted,
thread,
unfilteredThread,
sampleIndexOffset,
Expand Down
8 changes: 4 additions & 4 deletions src/components/flame-graph/FlameGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class FlameGraphImpl extends React.PureComponent<Props> {
_wideEnough = (callNodeIndex: IndexIntoCallNodeTable): boolean => {
const { flameGraphTiming, callNodeInfo } = this.props;

const callNodeTable = callNodeInfo.getCallNodeTable();
const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable();
const depth = callNodeTable.depth[callNodeIndex];
const row = flameGraphTiming[depth];
const columnIndex = row.callNode.indexOf(callNodeIndex);
Expand All @@ -188,7 +188,7 @@ class FlameGraphImpl extends React.PureComponent<Props> {

let callNodeIndex = startingCallNodeIndex;

const callNodeTable = callNodeInfo.getCallNodeTable();
const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable();
const depth = callNodeTable.depth[callNodeIndex];
const row = flameGraphTiming[depth];
let columnIndex = row.callNode.indexOf(callNodeIndex);
Expand Down Expand Up @@ -219,7 +219,7 @@ class FlameGraphImpl extends React.PureComponent<Props> {
changeSelectedCallNode,
handleCallNodeTransformShortcut,
} = this.props;
const callNodeTable = callNodeInfo.getCallNodeTable();
const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable();

if (
// Please do not forget to update the switch/case below if changing the array to allow more keys.
Expand Down Expand Up @@ -305,7 +305,7 @@ class FlameGraphImpl extends React.PureComponent<Props> {
if (document.activeElement === this._viewport) {
event.preventDefault();
const { callNodeInfo, selectedCallNodeIndex, thread } = this.props;
const callNodeTable = callNodeInfo.getCallNodeTable();
const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable();
if (selectedCallNodeIndex !== null) {
const funcIndex = callNodeTable.func[selectedCallNodeIndex];
const funcName = thread.stringTable.getString(
Expand Down
21 changes: 11 additions & 10 deletions src/components/shared/thread/StackGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type Props = {|
+className: string,
+thread: Thread,
+samplesSelectedStates: null | SelectedState[],
+sampleCallNodes: Array<IndexIntoCallNodeTable | null>,
+sampleNonInvertedCallNodes: Array<IndexIntoCallNodeTable | null>,
+interval: Milliseconds,
+rangeStart: Milliseconds,
+rangeEnd: Milliseconds,
Expand All @@ -38,13 +38,14 @@ type Props = {|

export class ThreadStackGraph extends PureComponent<Props> {
_heightFunction = (sampleIndex: IndexIntoSamplesTable): number | null => {
const { callNodeInfo, sampleCallNodes } = this.props;
const callNodeIndex = sampleCallNodes[sampleIndex];
if (callNodeIndex === null) {
const { callNodeInfo, sampleNonInvertedCallNodes } = this.props;
const nonInvertedCallNodeIndex = sampleNonInvertedCallNodes[sampleIndex];
if (nonInvertedCallNodeIndex === null) {
return null;
}
const callNodeTable = callNodeInfo.getCallNodeTable();
return callNodeTable.depth[callNodeIndex];

const nonInvertedCallNodeTable = callNodeInfo.getNonInvertedCallNodeTable();
return nonInvertedCallNodeTable.depth[nonInvertedCallNodeIndex];
};

render() {
Expand All @@ -60,12 +61,12 @@ export class ThreadStackGraph extends PureComponent<Props> {
trackName,
onSampleClick,
} = this.props;
const callNodeTable = callNodeInfo.getCallNodeTable();
const nonInvertedCallNodeTable = callNodeInfo.getNonInvertedCallNodeTable();

let maxDepth = 0;
for (let i = 0; i < callNodeTable.depth.length; i++) {
if (callNodeTable.depth[i] > maxDepth) {
maxDepth = callNodeTable.depth[i];
for (let i = 0; i < nonInvertedCallNodeTable.depth.length; i++) {
if (nonInvertedCallNodeTable.depth[i] > maxDepth) {
maxDepth = nonInvertedCallNodeTable.depth[i];
}
}
Comment on lines +67 to 71

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This makes me think there's a maxDepth property in the call node table, could we use it directly?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(not especially in this PR though)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh I forgot about that property, but you're right!


Expand Down
36 changes: 12 additions & 24 deletions src/components/timeline/TrackThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
getCategories,
getSelectedThreadIndexes,
getTimelineType,
getInvertCallstack,
getThreadSelectorsFromThreadsKey,
getMaxThreadCPUDeltaPerMs,
getIsExperimentalCPUGraphsEnabled,
Expand All @@ -38,8 +37,7 @@ import {
updatePreviewSelection,
changeSelectedCallNode,
focusCallTree,
selectLeafCallNode,
selectRootCallNode,
selectSelfCallNode,
} from 'firefox-profiler/actions/profile-view';
import { reportTrackThreadHeight } from 'firefox-profiler/actions/app';
import { EmptyThreadIndicator } from './EmptyThreadIndicator';
Expand Down Expand Up @@ -85,8 +83,7 @@ type StateProps = {|
+timelineType: TimelineType,
+hasFileIoMarkers: boolean,
+samplesSelectedStates: null | SelectedState[],
+sampleCallNodes: Array<IndexIntoCallNodeTable | null>,
+invertCallstack: boolean,
+sampleNonInvertedCallNodes: Array<IndexIntoCallNodeTable | null>,
+treeOrderSampleComparator: (
IndexIntoSamplesTable,
IndexIntoSamplesTable
Expand All @@ -103,8 +100,7 @@ type DispatchProps = {|
+updatePreviewSelection: typeof updatePreviewSelection,
+changeSelectedCallNode: typeof changeSelectedCallNode,
+focusCallTree: typeof focusCallTree,
+selectLeafCallNode: typeof selectLeafCallNode,
+selectRootCallNode: typeof selectRootCallNode,
+selectSelfCallNode: typeof selectSelfCallNode,
+reportTrackThreadHeight: typeof reportTrackThreadHeight,
|};

Expand All @@ -130,23 +126,15 @@ class TimelineTrackThreadImpl extends PureComponent<Props> {

const {
threadsKey,
selectLeafCallNode,
selectRootCallNode,
selectSelfCallNode,
focusCallTree,
invertCallstack,
selectedThreadIndexes,
callTreeVisible,
} = this.props;

// Sample clicking only works for one thread. See issue #2709
if (selectedThreadIndexes.size === 1) {
if (invertCallstack) {
// When we're displaying the inverted call stack, the "leaf" call node we're
// interested in is actually displayed as the "root" of the tree.
selectRootCallNode(threadsKey, sampleIndex);
} else {
selectLeafCallNode(threadsKey, sampleIndex);
}
selectSelfCallNode(threadsKey, sampleIndex);

if (sampleIndex !== null && callTreeVisible) {
// If the user clicked outside of the activity graph (sampleIndex === null),
Expand Down Expand Up @@ -198,7 +186,7 @@ class TimelineTrackThreadImpl extends PureComponent<Props> {
timelineType,
hasFileIoMarkers,
showMemoryMarkers,
sampleCallNodes,
sampleNonInvertedCallNodes,
samplesSelectedStates,
treeOrderSampleComparator,
trackType,
Expand Down Expand Up @@ -317,7 +305,7 @@ class TimelineTrackThreadImpl extends PureComponent<Props> {
rangeStart={rangeStart}
rangeEnd={rangeEnd}
callNodeInfo={callNodeInfo}
sampleCallNodes={sampleCallNodes}
sampleNonInvertedCallNodes={sampleNonInvertedCallNodes}
samplesSelectedStates={samplesSelectedStates}
categories={categories}
onSampleClick={this._onSampleClick}
Expand Down Expand Up @@ -352,13 +340,14 @@ export const TimelineTrackThread = explicitConnect<
fullThread.samples.threadCPUDelta !== undefined;

return {
invertCallstack: getInvertCallstack(state),
fullThread,
filteredThread: selectors.getFilteredThread(state),
rangeFilteredThread: selectors.getRangeFilteredThread(state),
callNodeInfo: selectors.getCallNodeInfo(state),
sampleCallNodes:
selectors.getSampleIndexToCallNodeIndexForFilteredThread(state),
sampleNonInvertedCallNodes:
selectors.getSampleIndexToNonInvertedCallNodeIndexForFilteredThread(
state
),
unfilteredSamplesRange: selectors.unfilteredSamplesRange(state),
interval: getProfileInterval(state),
rangeStart: committedRange.start,
Expand All @@ -385,8 +374,7 @@ export const TimelineTrackThread = explicitConnect<
updatePreviewSelection,
changeSelectedCallNode,
focusCallTree,
selectLeafCallNode,
selectRootCallNode,
selectSelfCallNode,
reportTrackThreadHeight,
},
component: withSize<Props>(TimelineTrackThreadImpl),
Expand Down
3 changes: 2 additions & 1 deletion src/profile-logic/address-timings.js
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,8 @@ export function getStackAddressInfoForCallNodeNonInverted(
callNodeInfo: CallNodeInfo,
nativeSymbol: IndexIntoNativeSymbolTable
): StackAddressInfo {
const stackIndexToCallNodeIndex = callNodeInfo.getStackIndexToCallNodeIndex();
const stackIndexToCallNodeIndex =
callNodeInfo.getStackIndexToNonInvertedCallNodeIndex();

// "self address" == "the address which a stack's self time is contributed to"
const callNodeSelfAddressForAllStacks = [];
Expand Down
Loading