From 2e38000c3ce2ea84d2f3fab3e2bbfc633503fdb7 Mon Sep 17 00:00:00 2001 From: "loongtao.zhang" Date: Fri, 5 Jun 2026 10:31:03 +0800 Subject: [PATCH 1/2] feat: invert call stack in flame graph to render as icicle graph --- src/components/flame-graph/Canvas.tsx | 56 ++++++---- src/components/flame-graph/FlameGraph.tsx | 23 ++-- .../flame-graph/MaybeFlameGraph.tsx | 38 +------ src/components/flame-graph/index.tsx | 2 +- src/profile-logic/flame-graph.ts | 74 ++++++++++++- src/profile-logic/profile-data.ts | 3 +- src/selectors/per-thread/stack-sample.ts | 104 +++++++++++++++--- src/selectors/url-state.ts | 3 +- src/test/components/FlameGraph.test.tsx | 16 ++- .../__snapshots__/FlameGraph.test.tsx.snap | 13 +++ 10 files changed, 240 insertions(+), 92 deletions(-) diff --git a/src/components/flame-graph/Canvas.tsx b/src/components/flame-graph/Canvas.tsx index 1c163512b2..fa0ce71a9b 100644 --- a/src/components/flame-graph/Canvas.tsx +++ b/src/components/flame-graph/Canvas.tsx @@ -125,7 +125,10 @@ class FlameGraphCanvasImpl extends React.PureComponent { // selection or applying a transform), move the viewport // vertically so that its offset from the base of the flame graph // is maintained. - if (prevProps.maxStackDepthPlusOne !== this.props.maxStackDepthPlusOne) { + if ( + !this.props.isInverted && + prevProps.maxStackDepthPlusOne !== this.props.maxStackDepthPlusOne + ) { this.props.viewport.moveViewport( 0, (prevProps.maxStackDepthPlusOne - this.props.maxStackDepthPlusOne) * @@ -151,16 +154,21 @@ class FlameGraphCanvasImpl extends React.PureComponent { } _scrollSelectionIntoView = () => { - const { selectedCallNodeIndex, maxStackDepthPlusOne, callNodeInfo } = - this.props; + const { + selectedCallNodeIndex, + maxStackDepthPlusOne, + callNodeInfo, + isInverted, + } = this.props; if (selectedCallNodeIndex === null) { return; } - const callNodeTable = callNodeInfo.getCallNodeTable(); - const depth = callNodeTable.depth[selectedCallNodeIndex]; - const y = (maxStackDepthPlusOne - depth - 1) * ROW_HEIGHT; + const depth = callNodeInfo.depthForNode(selectedCallNodeIndex); + const y = isInverted + ? depth * ROW_HEIGHT + : (maxStackDepthPlusOne - depth - 1) * ROW_HEIGHT; if (y < this.props.viewport.viewportTop) { this.props.viewport.moveViewport(0, this.props.viewport.viewportTop - y); @@ -192,6 +200,7 @@ class FlameGraphCanvasImpl extends React.PureComponent { viewportTop, viewportBottom, }, + isInverted, } = this.props; const { hoveredItem } = hoverInfo; @@ -232,14 +241,12 @@ class FlameGraphCanvasImpl extends React.PureComponent { fastFillStyle.set(getBackgroundColor()); ctx.fillRect(0, 0, deviceContainerWidth, deviceContainerHeight); - const callNodeTable = callNodeInfo.getCallNodeTable(); - - const startDepth = Math.floor( - maxStackDepthPlusOne - viewportBottom / stackFrameHeight - ); - const endDepth = Math.ceil( - maxStackDepthPlusOne - viewportTop / stackFrameHeight - ); + const startDepth = isInverted + ? Math.floor(viewportTop / stackFrameHeight) + : Math.floor(maxStackDepthPlusOne - viewportBottom / stackFrameHeight); + const endDepth = isInverted + ? Math.ceil(viewportBottom / stackFrameHeight) + : Math.ceil(maxStackDepthPlusOne - viewportTop / stackFrameHeight); // Only draw the stack frames that are vertically within view. // The graph is drawn from bottom to top, in order of increasing depth. @@ -251,10 +258,12 @@ class FlameGraphCanvasImpl extends React.PureComponent { continue; } - const cssRowTop: CssPixels = - (maxStackDepthPlusOne - depth - 1) * ROW_HEIGHT - viewportTop; - const cssRowBottom: CssPixels = - (maxStackDepthPlusOne - depth) * ROW_HEIGHT - viewportTop; + const cssRowTop: CssPixels = isInverted + ? depth * ROW_HEIGHT - viewportTop + : (maxStackDepthPlusOne - depth - 1) * ROW_HEIGHT - viewportTop; + const cssRowBottom: CssPixels = isInverted + ? (depth + 1) * ROW_HEIGHT - viewportTop + : (maxStackDepthPlusOne - depth) * ROW_HEIGHT - viewportTop; const deviceRowTop: DevicePixels = snap(cssRowTop * cssToDeviceScale); const deviceRowBottom: DevicePixels = snap(cssRowBottom * cssToDeviceScale) - 1; @@ -300,7 +309,7 @@ class FlameGraphCanvasImpl extends React.PureComponent { i === hoveredItem.flameGraphTimingIndex; const isHighlighted = isSelected || isRightClicked || isHovered; - const categoryIndex = callNodeTable.category[callNodeIndex]; + const categoryIndex = callNodeInfo.categoryForNode(callNodeIndex); const category = categories[categoryIndex]; const colorStyles = mapCategoryColorNameToStackChartStyles( category.color @@ -322,7 +331,7 @@ class FlameGraphCanvasImpl extends React.PureComponent { deviceBoxLeft + deviceHorizontalPadding; const deviceTextWidth: DevicePixels = deviceBoxRight - deviceTextLeft; if (deviceTextWidth > textMeasurement.minWidth) { - const funcIndex = callNodeTable.func[callNodeIndex]; + const funcIndex = callNodeInfo.funcForNode(callNodeIndex); const funcName = thread.stringTable.getString( thread.funcTable.name[funcIndex] ); @@ -472,11 +481,12 @@ class FlameGraphCanvasImpl extends React.PureComponent { flameGraphTiming, maxStackDepthPlusOne, viewport: { viewportTop, containerWidth }, + isInverted, } = this.props; const pos = x / containerWidth; - const depth = Math.floor( - maxStackDepthPlusOne - (y + viewportTop) / ROW_HEIGHT - ); + const depth = isInverted + ? Math.floor((y + viewportTop) / ROW_HEIGHT) + : Math.floor(maxStackDepthPlusOne - (y + viewportTop) / ROW_HEIGHT); const stackTiming = flameGraphTiming[depth]; if (!stackTiming) { diff --git a/src/components/flame-graph/FlameGraph.tsx b/src/components/flame-graph/FlameGraph.tsx index 376d0f3025..008caa52cc 100644 --- a/src/components/flame-graph/FlameGraph.tsx +++ b/src/components/flame-graph/FlameGraph.tsx @@ -4,7 +4,10 @@ import * as React from 'react'; import { explicitConnectWithForwardRef } from '../../utils/connect'; -import { FlameGraphCanvas } from './Canvas'; +import { + FlameGraphCanvas, + type OwnProps as FlameGraphCanvasProps, +} from './Canvas'; import { getCategories, @@ -28,7 +31,6 @@ import { updateBottomBoxContentsAndMaybeOpen, } from 'firefox-profiler/actions/profile-view'; import { extractNonInvertedCallTreeTimings } from 'firefox-profiler/profile-logic/call-tree'; -import { ensureExists } from 'firefox-profiler/utils/types'; import type { Thread, @@ -352,10 +354,7 @@ class FlameGraphImpl // different, and the flame graph is only used with non-inverted timings.) const tracedTimingNonInverted = tracedTiming !== null - ? ensureExists( - extractNonInvertedCallTreeTimings(tracedTiming), - 'The flame graph should only ever see non-inverted timings, see UrlState.getInvertCallstack' - ) + ? extractNonInvertedCallTreeTimings(tracedTiming) : null; const maxViewportHeight = maxStackDepthPlusOne * STACK_FRAME_HEIGHT; @@ -376,7 +375,7 @@ class FlameGraphImpl maxViewportHeight, maximumZoom: 1, previewSelection, - startsAtBottom: true, + startsAtBottom: !isInverted, disableHorizontalMovement: true, viewportNeedsUpdate, marginLeft: 0, @@ -416,12 +415,16 @@ class FlameGraphImpl } } -function viewportNeedsUpdate() { - // By always returning false we prevent the viewport from being +function viewportNeedsUpdate( + prevProps: FlameGraphCanvasProps, + nextProps: FlameGraphCanvasProps +) { + // By returning false we prevent the viewport from being // reset and scrolled all the way to the bottom when doing // operations like changing the time selection or applying a // transform. - return false; + // We only reset the viewport if the invertCallstack setting changes. + return prevProps.isInverted !== nextProps.isInverted; } export const FlameGraph = explicitConnectWithForwardRef< diff --git a/src/components/flame-graph/MaybeFlameGraph.tsx b/src/components/flame-graph/MaybeFlameGraph.tsx index 5dd0f0f227..205b228305 100644 --- a/src/components/flame-graph/MaybeFlameGraph.tsx +++ b/src/components/flame-graph/MaybeFlameGraph.tsx @@ -4,9 +4,7 @@ import * as React from 'react'; import { explicitConnectWithForwardRef } from 'firefox-profiler/utils/connect'; -import { getInvertCallstack } from '../../selectors/url-state'; import { selectedThreadSelectors } from '../../selectors/per-thread'; -import { changeInvertCallstack } from '../../actions/profile-view'; import { FlameGraphEmptyReasons } from './FlameGraphEmptyReasons'; import { FlameGraph, type FlameGraphHandle } from './FlameGraph'; @@ -14,26 +12,15 @@ import type { ConnectedProps } from 'firefox-profiler/utils/connect'; import './MaybeFlameGraph.css'; -// TODO: This component isn't needed any more. Whenever the selected tab -// is "flame-graph", `invertCallstack` will be `false`. is -// only used in the "flame-graph" tab. - type StateProps = { readonly isPreviewSelectionEmpty: boolean; - readonly invertCallstack: boolean; -}; -type DispatchProps = { - readonly changeInvertCallstack: typeof changeInvertCallstack; }; +type DispatchProps = {}; type Props = ConnectedProps<{}, StateProps, DispatchProps>; class MaybeFlameGraphImpl extends React.PureComponent { _flameGraph: React.RefObject = React.createRef(); - _onSwitchToNormalCallstackClick = () => { - this.props.changeInvertCallstack(false); - }; - override componentDidMount() { const flameGraph = this._flameGraph.current; if (flameGraph) { @@ -42,28 +29,12 @@ class MaybeFlameGraphImpl extends React.PureComponent { } override render() { - const { isPreviewSelectionEmpty, invertCallstack } = this.props; + const { isPreviewSelectionEmpty } = this.props; if (isPreviewSelectionEmpty) { return ; } - if (invertCallstack) { - return ( -
-

The Flame Graph is not available for inverted call stacks

-

- {' '} - to show the Flame Graph. -

-
- ); - } return ; } } @@ -76,13 +47,10 @@ export const MaybeFlameGraph = explicitConnectWithForwardRef< >({ mapStateToProps: (state) => { return { - invertCallstack: getInvertCallstack(state), isPreviewSelectionEmpty: !selectedThreadSelectors.getHasPreviewFilteredCtssSamples(state), }; }, - mapDispatchToProps: { - changeInvertCallstack, - }, + mapDispatchToProps: {}, component: MaybeFlameGraphImpl, }); diff --git a/src/components/flame-graph/index.tsx b/src/components/flame-graph/index.tsx index 141147302c..854fe49763 100644 --- a/src/components/flame-graph/index.tsx +++ b/src/components/flame-graph/index.tsx @@ -13,7 +13,7 @@ const FlameGraphView = () => ( role="tabpanel" aria-labelledby="flame-graph-tab-button" > - + diff --git a/src/profile-logic/flame-graph.ts b/src/profile-logic/flame-graph.ts index 2cf8e3701d..af24acd96a 100644 --- a/src/profile-logic/flame-graph.ts +++ b/src/profile-logic/flame-graph.ts @@ -8,7 +8,7 @@ import type { IndexIntoCallNodeTable, } from 'firefox-profiler/types'; import type { StringTable } from 'firefox-profiler/utils/string-table'; -import type { CallTreeTimingsNonInverted } from './call-tree'; +import type { CallTreeTimingsNonInverted, CallTree } from './call-tree'; import { bisectionRightByStrKey } from 'firefox-profiler/utils/bisect'; @@ -307,3 +307,75 @@ export function getFlameGraphTiming( return timing; } + +export function computeFlameGraphTimingFromCallTree( + callTree: CallTree +): FlameGraphTiming { + const rootTotalSummary = callTree.getTotal(); + if (rootTotalSummary === 0) { + return []; + } + + const timing: FlameGraphTiming = []; + + function traverse( + nodeIndex: IndexIntoCallNodeTable, + depth: number, + startX: number + ): number { + const { self, total } = callTree.getNodeData(nodeIndex); + if (total === 0) { + return startX; + } + + const totalRelative = Math.abs(total / rootTotalSummary); + const endX = startX + totalRelative; + + if (!timing[depth]) { + timing[depth] = { + start: [], + end: [], + selfRelative: [], + callNode: [], + length: 0, + }; + } + + timing[depth].start.push(startX); + timing[depth].end.push(endX); + timing[depth].selfRelative.push(Math.abs(self / rootTotalSummary)); + timing[depth].callNode.push(nodeIndex); + timing[depth].length++; + + const children = [...callTree.getChildren(nodeIndex)]; + if (children.length > 0) { + // Sort children alphabetically by function name. + children.sort((a, b) => { + const nameA = callTree.getNodeData(a).funcName; + const nameB = callTree.getNodeData(b).funcName; + return nameA.localeCompare(nameB); + }); + + let currentChildStart = startX; + for (const child of children) { + currentChildStart = traverse(child, depth + 1, currentChildStart); + } + } + + return endX; + } + + const roots = [...callTree.getRoots()]; + roots.sort((a, b) => { + const nameA = callTree.getNodeData(a).funcName; + const nameB = callTree.getNodeData(b).funcName; + return nameA.localeCompare(nameB); + }); + + let currentStart = 0; + for (const root of roots) { + currentStart = traverse(root, 0, currentStart); + } + + return timing; +} diff --git a/src/profile-logic/profile-data.ts b/src/profile-logic/profile-data.ts index 787fdc7fda..29a61bed8f 100644 --- a/src/profile-logic/profile-data.ts +++ b/src/profile-logic/profile-data.ts @@ -2686,7 +2686,6 @@ export function computeCallNodeMaxDepthPlusOne( // computed for the filtered thread, but a samples-like table can use the preview // filtered thread, which involves a subset of the total call nodes. let maxDepth = -1; - const callNodeTable = callNodeInfo.getCallNodeTable(); // TODO: Use sampleCallNodes instead const stackIndexToCallNodeIndex = callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); @@ -2696,7 +2695,7 @@ export function computeCallNodeMaxDepthPlusOne( continue; } const callNodeIndex = stackIndexToCallNodeIndex[stackIndex]; - const depth = callNodeTable.depth[callNodeIndex]; + const depth = callNodeInfo.depthForNode(callNodeIndex); if (depth > maxDepth) { maxDepth = depth; } diff --git a/src/selectors/per-thread/stack-sample.ts b/src/selectors/per-thread/stack-sample.ts index 21bbecb964..c3a7a6ae1b 100644 --- a/src/selectors/per-thread/stack-sample.ts +++ b/src/selectors/per-thread/stack-sample.ts @@ -102,7 +102,10 @@ export function getStackAndSampleSelectorsPerThread( if (time.length === 0) { return null; } - return { start: time[0], end: time[time.length - 1] + interval }; + return { + start: time[0], + end: time[time.length - 1] + interval, + }; } ); @@ -141,6 +144,9 @@ export function getStackAndSampleSelectorsPerThread( return _getNonInvertedCallNodeInfo(state); }; + const getNonInvertedCallNodeInfo: Selector = + _getNonInvertedCallNodeInfo; + const _getCallNodeTable: Selector = (state) => _getNonInvertedCallNodeInfo(state).getCallNodeTable(); @@ -309,6 +315,13 @@ export function getStackAndSampleSelectorsPerThread( ProfileData.computeCallNodeMaxDepthPlusOne ); + const getNonInvertedFilteredCallNodeMaxDepthPlusOne: Selector = + createSelector( + threadSelectors.getFilteredCtssSamples, + _getNonInvertedCallNodeInfo, + ProfileData.computeCallNodeMaxDepthPlusOne + ); + /** * When computing the call tree, a "samples" table is used, which * can represent a variety of formats with different weight types. @@ -321,6 +334,32 @@ export function getStackAndSampleSelectorsPerThread( (samples) => samples.weightType || 'samples' ); + const _getSampleIndexToNonInvertedCallNodeIndexForPreviewFilteredCtssThreadNonInverted: Selector< + Array + > = createSelector( + (state: State) => + threadSelectors.getPreviewFilteredCtssSamples(state).stack, + (state: State) => + _getNonInvertedCallNodeInfo( + state + ).getStackIndexToNonInvertedCallNodeIndex(), + ProfileData.getSampleIndexToCallNodeIndex + ); + + const getCallNodeSelfAndSummaryNonInverted: Selector = + createSelector( + threadSelectors.getPreviewFilteredCtssSamples, + _getSampleIndexToNonInvertedCallNodeIndexForPreviewFilteredCtssThreadNonInverted, + _getNonInvertedCallNodeInfo, + (samples, sampleIndexToCallNodeIndex, callNodeInfo) => { + return CallTree.computeCallNodeSelfAndSummary( + samples, + sampleIndexToCallNodeIndex, + callNodeInfo.getCallNodeTable().length + ); + } + ); + const getCallNodeSelfAndSummary: Selector = createSelector( threadSelectors.getPreviewFilteredCtssSamples, @@ -343,8 +382,8 @@ export function getStackAndSampleSelectorsPerThread( const getCallTreeTimingsNonInverted: Selector = createSelector( - getCallNodeInfo, - getCallNodeSelfAndSummary, + _getNonInvertedCallNodeInfo, + getCallNodeSelfAndSummaryNonInverted, CallTree.computeCallTreeTimingsNonInverted ); @@ -369,6 +408,23 @@ export function getStackAndSampleSelectorsPerThread( CallTree.getCallTree ); + const getNonInvertedCallTreeTimingsWrapper: Selector = + createSelector(getCallTreeTimingsNonInverted, (timings) => ({ + type: 'NON_INVERTED' as const, + timings, + })); + + const getNonInvertedCallTree: Selector = createSelector( + threadSelectors.getFilteredThread, + _getNonInvertedCallNodeInfo, + ProfileSelectors.getCategories, + threadSelectors.getPreviewFilteredCtssSamples, + getNonInvertedCallTreeTimingsWrapper, + getWeightTypeForCallTree, + ProfileSelectors.getSourceTable, + CallTree.getCallTree + ); + const getFunctionListTree: Selector = createSelector( threadSelectors.getFilteredThread, _getInvertedCallNodeInfo, @@ -431,6 +487,30 @@ export function getStackAndSampleSelectorsPerThread( } ); + const getTracedTimingNonInverted: Selector = + createSelector( + threadSelectors.getPreviewFilteredCtssSamples, + _getSampleIndexToNonInvertedCallNodeIndexForPreviewFilteredCtssThreadNonInverted, + _getNonInvertedCallNodeInfo, + ProfileSelectors.getProfileInterval, + (samples, sampleIndexToCallNodeIndex, callNodeInfo, interval) => { + const CallNodeSelfAndSummary = + CallTree.computeCallNodeTracedSelfAndSummary( + samples, + sampleIndexToCallNodeIndex, + callNodeInfo.getCallNodeTable().length, + interval + ); + if (CallNodeSelfAndSummary === null) { + return null; + } + return CallTree.computeCallTreeTimings( + callNodeInfo, + CallNodeSelfAndSummary + ); + } + ); + const getTracedSelfAndTotalForSelectedCallNode: Selector = createSelector( getSelectedCallNodeIndex, @@ -465,20 +545,8 @@ export function getStackAndSampleSelectorsPerThread( > = (state) => _getStackTimingByDepthWithMap(state).sameWidthsIndexToTimestampMap; - const getFlameGraphRows: Selector = createSelector( - (state: State) => getCallNodeInfo(state).getCallNodeTable(), - (state: State) => threadSelectors.getFilteredThread(state).funcTable, - (state: State) => threadSelectors.getFilteredThread(state).stringTable, - FlameGraph.computeFlameGraphRows - ); - const getFlameGraphTiming: Selector = - createSelector( - getFlameGraphRows, - (state: State) => getCallNodeInfo(state).getCallNodeTable(), - getCallTreeTimingsNonInverted, - FlameGraph.getFlameGraphTiming - ); + createSelector(getCallTree, FlameGraph.computeFlameGraphTimingFromCallTree); const getRightClickedCallNodeIndex: Selector = createSelector( @@ -502,6 +570,7 @@ export function getStackAndSampleSelectorsPerThread( unfilteredSamplesRange, getWeightTypeForCallTree, getCallNodeInfo, + getNonInvertedCallNodeInfo, getSourceViewStackLineInfo, getAssemblyViewNativeSymbolIndex, getAssemblyViewStackAddressInfo, @@ -513,15 +582,18 @@ export function getStackAndSampleSelectorsPerThread( getSampleSelectedStatesInFilteredThread, getTreeOrderComparatorInFilteredThread, getCallTree, + getNonInvertedCallTree, getFunctionListTree, getFunctionListTimings, getSourceViewLineTimings, getAssemblyViewAddressTimings, getTracedTiming, + getTracedTimingNonInverted, getTracedSelfAndTotalForSelectedCallNode, getStackTimingByDepth, getSameWidthsIndexToTimestampMap, getFilteredCallNodeMaxDepthPlusOne, + getNonInvertedFilteredCallNodeMaxDepthPlusOne, getFlameGraphTiming, getRightClickedCallNodeIndex, }; diff --git a/src/selectors/url-state.ts b/src/selectors/url-state.ts index bf2bc8fe65..31406f8302 100644 --- a/src/selectors/url-state.ts +++ b/src/selectors/url-state.ts @@ -122,7 +122,8 @@ export const getNetworkSearchString: Selector = (state) => export const getSelectedTab: Selector = (state) => getUrlState(state).selectedTab; export const getInvertCallstack: Selector = (state) => - getSelectedTab(state) === 'calltree' && + (getSelectedTab(state) === 'calltree' || + getSelectedTab(state) === 'flame-graph') && getProfileSpecificState(state).invertCallstack; export const getIncludeIdleSamples: Selector = (state) => getProfileSpecificState(state).includeIdleSamples; diff --git a/src/test/components/FlameGraph.test.tsx b/src/test/components/FlameGraph.test.tsx index 18baab4dd2..08288fcfa0 100644 --- a/src/test/components/FlameGraph.test.tsx +++ b/src/test/components/FlameGraph.test.tsx @@ -80,13 +80,22 @@ describe('FlameGraph', function () { expect(drawCalls.length).toBeGreaterThan(0); }); - it('ignores invertCallstack and always displays non-inverted', () => { - const { getState, dispatch } = setupFlameGraph(); + it('respects invertCallstack and toggles icicle graph mode', () => { + const { getState, dispatch, flushRafCalls } = setupFlameGraph(); expect(getInvertCallstack(getState())).toBe(false); + + const initialDrawCalls = flushDrawLog(); + act(() => { dispatch(changeInvertCallstack(true)); }); - expect(getInvertCallstack(getState())).toBe(false); + expect(getInvertCallstack(getState())).toBe(true); + flushRafCalls(); + + const icicleDrawCalls = flushDrawLog(); + expect(icicleDrawCalls.length).toBeGreaterThan(0); + expect(icicleDrawCalls).not.toEqual(initialDrawCalls); + act(() => { dispatch(changeInvertCallstack(false)); }); @@ -443,5 +452,6 @@ function setupFlameGraph() { getContextMenu, clickMenuItem, findFillTextPosition, + flushRafCalls, }; } diff --git a/src/test/components/__snapshots__/FlameGraph.test.tsx.snap b/src/test/components/__snapshots__/FlameGraph.test.tsx.snap index 9af19bd667..f5a0a6bafd 100644 --- a/src/test/components/__snapshots__/FlameGraph.test.tsx.snap +++ b/src/test/components/__snapshots__/FlameGraph.test.tsx.snap @@ -471,6 +471,19 @@ exports[`FlameGraph matches the snapshot 1`] = `
  • +