diff --git a/src/components/calltree/CallTree.js b/src/components/calltree/CallTree.js index cdfbda64eb..dd111d2f10 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,77 @@ 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) { + + const newExpandedCallNodeIndexes = []; + // 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 children = tree.getRoots(); + let nodeToSelect = null; + let visibleLinesCount = children.length; + + while (true) { + const firstNonIdleNode = children.find( + (nodeIndex) => callNodeTable.category[nodeIndex] !== idleCategoryIndex + ); + + if (firstNonIdleNode === undefined) { + break; + } + + nodeToSelect = firstNonIdleNode; + + const nodeData = tree.getNodeData(firstNonIdleNode); + if (nodeData.self > nodeData.total * 0.95) { + // This node doesn't have interesting children, let's stop here. break; } - currentCallNodeIndex = children[0]; - newExpandedCallNodeIndexes.push(currentCallNodeIndex); + + const depth = tree.getDepth(firstNonIdleNode); + children = tree.getChildren(firstNonIdleNode); + + if ( + depth > minimumDepth && + visibleLinesCount + children.length > maxVisibleLines + ) { + // Expanding this node would exceed our budget. + break; + } + + newExpandedCallNodeIndexes.push(firstNonIdleNode); + visibleLinesCount += children.length; + } + + 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 +355,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..151520cc41 100644 --- a/src/test/components/__snapshots__/ProfileCallTreeView.test.js.snap +++ b/src/test/components/__snapshots__/ProfileCallTreeView.test.js.snap @@ -3795,7 +3795,7 @@ for understanding where time was actually spent in a program." class="react-contextmenu-wrapper treeViewContextMenu" >
- - 67% - - - 2 - - - 2 - - -
- -
-
- — - - -
- -
-
- - 67% - - 2 - - — - - - 33% - - - 1 - - - — - - -
- -
-
- - 33% - - - 1 - - - — - - -
- -
-
- - 33% - - - 1 - - - — - - -
- -
-
- - - - - Z - - -
-
- - - - - Y - - -
-
- - - - - X - - - libX.so - -
-
- - - - - B - - -
-
- -
-