diff --git a/src/components/calltree/CallTree.js b/src/components/calltree/CallTree.js index cdfbda64eb..eb9f67e51d 100644 --- a/src/components/calltree/CallTree.js +++ b/src/components/calltree/CallTree.js @@ -20,6 +20,7 @@ import { getScrollToSelectionGeneration, getFocusCallTreeGeneration, getPreviewSelection, + getCategories, } from 'firefox-profiler/selectors/profile'; import { selectedThreadSelectors } from 'firefox-profiler/selectors/per-thread'; import { @@ -37,6 +38,7 @@ import type { ImplementationFilter, ThreadsKey, CallNodeInfo, + CategoryList, IndexIntoCallNodeTable, CallNodeDisplayData, WeightType, @@ -54,6 +56,7 @@ type StateProps = {| +focusCallTreeGeneration: number, +tree: CallTreeType, +callNodeInfo: CallNodeInfo, + +categories: CategoryList, +selectedCallNodeIndex: IndexIntoCallNodeTable | null, +rightClickedCallNodeIndex: IndexIntoCallNodeTable | null, +expandedCallNodeIndexes: Array, @@ -144,27 +147,29 @@ class CallTreeImpl extends PureComponent { componentDidMount() { this.focus(); - if (this.props.selectedCallNodeIndex === null) { - this.procureInterestingInitialSelection(); - } else if (this._treeView) { + this.maybeProcureInterestingInitialSelection(); + + if (this.props.selectedCallNodeIndex === null && this._treeView) { this._treeView.scrollSelectionIntoView(); } } componentDidUpdate(prevProps) { if ( - this.props.scrollToSelectionGeneration > - prevProps.scrollToSelectionGeneration + this.props.focusCallTreeGeneration > prevProps.focusCallTreeGeneration ) { - if (this._treeView) { - this._treeView.scrollSelectionIntoView(); - } + this.focus(); } + this.maybeProcureInterestingInitialSelection(); + if ( - this.props.focusCallTreeGeneration > prevProps.focusCallTreeGeneration + this.props.selectedCallNodeIndex !== null && + this.props.scrollToSelectionGeneration > + prevProps.scrollToSelectionGeneration && + this._treeView ) { - this.focus(); + this._treeView.scrollSelectionIntoView(); } } @@ -228,35 +233,109 @@ class CallTreeImpl extends PureComponent { openSourceView(file, 'calltree'); }; - procureInterestingInitialSelection() { + maybeProcureInterestingInitialSelection() { // Expand the heaviest callstack up to a certain depth and select the frame // at that depth. - const { tree, expandedCallNodeIndexes } = this.props; - const newExpandedCallNodeIndexes = expandedCallNodeIndexes.slice(); - const maxInterestingDepth = 17; // scientifically determined - let currentCallNodeIndex = tree.getRoots()[0]; - if (currentCallNodeIndex === undefined) { - // This tree is empty. + const { + tree, + expandedCallNodeIndexes, + selectedCallNodeIndex, + callNodeInfo: { callNodeTable }, + categories, + } = this.props; + + if (selectedCallNodeIndex !== null || expandedCallNodeIndexes.length > 0) { + // Let's not change some existing state. return; } - newExpandedCallNodeIndexes.push(currentCallNodeIndex); - for (let i = 0; i < maxInterestingDepth; i++) { - const children = tree.getChildren(currentCallNodeIndex); - if (children.length === 0) { - break; + + const newExpandedCallNodeIndexes = []; + let nodeToSelect = null; + + // This value is completely arbitrary and looked good on Julien's machine + // when this was implemented. In the future we may want to look at the + // available space instead. + const maxVisibleLines = 70; + const minimumDepth = 10; + + const idleCategoryIndex = categories.findIndex( + (category) => category.name === 'Idle' + ); + + let nodesToVisit = tree.getRoots(); + let visibleLinesCount = nodesToVisit.length; + + while (nodesToVisit.length) { + const newNodesToVisit = []; + const nonIdleNodes = nodesToVisit.filter( + (nodeIndex) => callNodeTable.category[nodeIndex] !== idleCategoryIndex + ); + + // Look at the sum of the running times for all non-idle nodes + const sumOfNonIdleRunningTime = nonIdleNodes.reduce( + (sum, nodeIndex) => sum + tree.getNodeData(nodeIndex).total, + 0 + ); + const runningTimeThreshold = sumOfNonIdleRunningTime / 20; + + for (let i = 0; i < nonIdleNodes.length; i++) { + const nodeIndex = nonIdleNodes[i]; + const nodeData = tree.getNodeData(nodeIndex); + + if (i === 0) { + nodeToSelect = nodeIndex; + } + + if (nodeData.self > nodeData.total * 0.95) { + // This node doesn't have interesting children, let's move on. + continue; + } + + if (i !== 0) { + // The first node is always expanded, but let's check whether we + // should open more nodes at this level. + const thisRunningTime = nodeData.total; + if (thisRunningTime < runningTimeThreshold) { + // This node doesn't have a lot of running time. + // The remaining nodes will have even less, let's break out of the + // inner loop. + break; + } + } + + const children = tree.getChildren(nodeIndex); + const nodeDepth = tree.getDepth(nodeIndex); + + if ( + i !== 0 && + nodeDepth > minimumDepth && + visibleLinesCount + children.length > maxVisibleLines + ) { + // Expanding this node would exceed our budget. + // Let's look at the other nodes in case they have a smaller amount of children. + continue; + } + + newExpandedCallNodeIndexes.push(nodeIndex); + visibleLinesCount += children.length; + + if (i === 0) { + // Look at the children of the heaviest node only. + newNodesToVisit.push(...children); + } } - currentCallNodeIndex = children[0]; - newExpandedCallNodeIndexes.push(currentCallNodeIndex); + + nodesToVisit = newNodesToVisit; + } + + if (newExpandedCallNodeIndexes.length > 0) { + // Take care to not trigger a state change if there's nothing to change, + // to avoid infinite render loop. + this._onExpandedCallNodesChange(newExpandedCallNodeIndexes); } - this._onExpandedCallNodesChange(newExpandedCallNodeIndexes); - - const category = tree.getDisplayData(currentCallNodeIndex).categoryName; - if (category !== 'Idle') { - // If we selected the call node with a "idle" category, we'd have a - // completely dimmed activity graph because idle stacks are not drawn in - // this graph. Because this isn't probably what the average user wants we - // do it only when the category is something different. - this._onSelectedCallNodeChange(currentCallNodeIndex); + + if (nodeToSelect !== null) { + this._onSelectedCallNodeChange(nodeToSelect); } } @@ -308,6 +387,7 @@ export const CallTree = explicitConnect<{||}, StateProps, DispatchProps>({ focusCallTreeGeneration: getFocusCallTreeGeneration(state), tree: selectedThreadSelectors.getCallTree(state), callNodeInfo: selectedThreadSelectors.getCallNodeInfo(state), + categories: getCategories(state), selectedCallNodeIndex: selectedThreadSelectors.getSelectedCallNodeIndex(state), rightClickedCallNodeIndex: diff --git a/src/test/components/ProfileCallTreeView.test.js b/src/test/components/ProfileCallTreeView.test.js index 9e9ed6cf3a..1c547c0fc8 100644 --- a/src/test/components/ProfileCallTreeView.test.js +++ b/src/test/components/ProfileCallTreeView.test.js @@ -283,14 +283,14 @@ describe('calltree/ProfileCallTreeView', function () { expect(getRowElement('C')).toHaveClass('isSelected'); }); - it('does not select the heaviest stack if it is idle', () => { + it('does select a non-idle stack if the heaviest stack is idle', () => { const { profile } = getProfileFromTextSamples(` - A A A A A - B C[cat:Idle] C[cat:Idle] C[cat:Idle] D - E E + A A A A A A + B C[cat:Idle] C[cat:Idle] C[cat:Idle] B D + E E F `); - const { container } = setup(profile); - expect(container.querySelector('.treeViewRow.isSelected')).toBeFalsy(); + const { getRowElement } = setup(profile); + expect(getRowElement('E')).toHaveClass('isSelected'); }); }); diff --git a/src/test/components/__snapshots__/ProfileCallTreeView.test.js.snap b/src/test/components/__snapshots__/ProfileCallTreeView.test.js.snap index 53f3829e06..874036df32 100644 --- a/src/test/components/__snapshots__/ProfileCallTreeView.test.js.snap +++ b/src/test/components/__snapshots__/ProfileCallTreeView.test.js.snap @@ -192,7 +192,7 @@ allocated or deallocated in the program." >
+
+ + 20% + + + 3 + + + — + + +
+ +
+
@@ -863,7 +924,7 @@ allocated or deallocated in the program." >
+
+ + -20% + + + -3 + + + — + + +
+ +
+
@@ -1534,7 +1656,7 @@ allocated or deallocated in the program." >
+
+ + 27% + + + 11 + + + — + + +
+ +
+
@@ -2193,7 +2376,7 @@ allocated or deallocated in the program." >
+
+ + 20% + + + 3 + + + — + + +
+ +
-
- - - - - - -`; - -exports[`ProfileCallTreeView with unbalanced native allocations matches the snapshot for native deallocations 1`] = ` -
-
-
+
+ + + + + + +`; + +exports[`ProfileCallTreeView with unbalanced native allocations matches the snapshot for native deallocations 1`] = ` +
+
+
  • + + -59% + + + -24 + + + -11 + + +
    + +
    +
    +
    - 67% + 33% - 2 + 1 - — + 1
    +
    +
    +
    +
    +