From a5af6123fdea1143d71eac4907c2e2fd9ec07d6f Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sun, 21 Jun 2026 10:51:35 -0400 Subject: [PATCH 1/6] Add a function list panel. Add a new "Function List" tab which shows a flat, sortable list of functions rather than a tree. Selection is per-function instead of per-call-node, so this adds a per-thread selected function to the URL state, along with the URL handling, reducers and selectors that go with it. Selecting a function replaces the current history entry instead of pushing a new one. Otherwise, holding the down arrow key in the list would flood the back/forward history and get rate-limited by the browser. --- locales/en-US/app.ftl | 1 + res/css/style.css | 3 +- src/actions/profile-view.ts | 49 ++++ src/app-logic/tabs-handling.ts | 2 + src/app-logic/url-handling.ts | 92 +++++++ src/components/app/Details.tsx | 2 + src/components/calltree/CallTree.css | 6 +- src/components/calltree/CallTree.tsx | 4 - src/components/calltree/FunctionList.tsx | 250 ++++++++++++++++++ .../calltree/ProfileFunctionListView.tsx | 20 ++ src/components/calltree/columns.ts | 44 +++ src/components/sidebar/index.tsx | 1 + src/profile-logic/call-tree.ts | 38 ++- src/profile-logic/symbolication.ts | 15 ++ src/reducers/profile-view.ts | 49 ++++ src/reducers/url-state.ts | 70 ++++- src/selectors/per-thread/stack-sample.ts | 24 ++ src/selectors/url-state.ts | 10 + src/test/components/Details.test.tsx | 3 + src/test/components/DetailsContainer.test.tsx | 1 + src/test/store/bottom-box.test.ts | 27 ++ src/test/store/icons.test.ts | 1 + src/test/store/symbolication.test.ts | 23 ++ src/test/store/useful-tabs.test.ts | 4 + src/test/unit/profile-tree.test.ts | 2 + src/test/url-handling.test.ts | 52 ++++ src/types/actions.ts | 16 ++ src/types/profile-derived.ts | 1 + src/types/state.ts | 13 + src/utils/types.ts | 1 + 30 files changed, 813 insertions(+), 11 deletions(-) create mode 100644 src/components/calltree/FunctionList.tsx create mode 100644 src/components/calltree/ProfileFunctionListView.tsx diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 5a1811bd72..a2478d7ec7 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -940,6 +940,7 @@ StackSettings--panel-search = ## Tab Bar for the bottom half of the analysis UI. TabBar--calltree-tab = Call Tree +TabBar--function-list-tab = Function List TabBar--flame-graph-tab = Flame Graph TabBar--stack-chart-tab = Stack Chart TabBar--marker-chart-tab = Marker Chart diff --git a/res/css/style.css b/res/css/style.css index fac5320156..ca0c17e3c5 100644 --- a/res/css/style.css +++ b/res/css/style.css @@ -57,7 +57,8 @@ body { flex-shrink: 1; } -.treeAndSidebarWrapper { +.treeAndSidebarWrapper, +.functionTableAndSidebarWrapper { display: flex; flex: 1; flex-flow: column nowrap; diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts index 56cb1adc4b..9da70090ae 100644 --- a/src/actions/profile-view.ts +++ b/src/actions/profile-view.ts @@ -34,6 +34,7 @@ import { getHiddenLocalTracks, getInvertCallstack, getHash, + getUrlState, } from 'firefox-profiler/selectors/url-state'; import { assertExhaustiveCheck, @@ -75,12 +76,14 @@ import type { TableViewOptions, SelectionContext, BottomBoxInfo, + IndexIntoFuncTable, } from 'firefox-profiler/types'; import { funcHasDirectRecursiveCall, funcHasRecursiveCall, } from '../profile-logic/transforms'; import { changeStoredProfileNameInDb } from 'firefox-profiler/app-logic/uploaded-profiles-db'; +import { replaceHistoryWithUrlState } from 'firefox-profiler/app-logic/url-handling'; import type { TabSlug } from '../app-logic/tabs-handling'; import type { CallNodeInfo } from '../profile-logic/call-node-info'; import type { SingleColumnSortState } from '../components/shared/TreeView'; @@ -131,6 +134,32 @@ export function changeSelectedCallNode( }; } +/** + * Select a function for a given thread in the function list. + * + * Replaces the current history entry rather than pushing a new one, so that + * holding e.g. the down arrow key in the function list doesn't get rate-limited + * by the browser and doesn't flood the back/forward history. + */ +export function changeSelectedFunctionIndex( + threadsKey: ThreadsKey, + selectedFunctionIndex: IndexIntoFuncTable | null, + context: SelectionContext = { source: 'auto' } +): ThunkAction { + return (dispatch, getState) => { + dispatch({ + type: 'CHANGE_SELECTED_FUNCTION', + selectedFunctionIndex, + threadsKey, + context, + }); + // Update window.history synchronously instead of waiting for the + // UrlManager's componentDidUpdate, which is deferred by React's render + // scheduling and would otherwise pushState a new entry. + replaceHistoryWithUrlState(getUrlState(getState())); + }; +} + /** * This action is used when the user right clicks on a call node (in panels such * as the call tree, the flame chart, or the stack chart). It's especially used @@ -147,6 +176,17 @@ export function changeRightClickedCallNode( }; } +export function changeRightClickedFunctionIndex( + threadsKey: ThreadsKey, + functionIndex: IndexIntoFuncTable | null +) { + return { + type: 'CHANGE_RIGHT_CLICKED_FUNCTION', + threadsKey, + functionIndex, + }; +} + /** * 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. @@ -1624,6 +1664,15 @@ export function changeMarkerTableSort( }; } +export function changeFunctionListSort( + sort: SingleColumnSortState[] | null +): Action { + return { + type: 'CHANGE_FUNCTION_LIST_SORT', + sort, + }; +} + export function changeNetworkSearchString(searchString: string): Action { return { type: 'CHANGE_NETWORK_SEARCH_STRING', diff --git a/src/app-logic/tabs-handling.ts b/src/app-logic/tabs-handling.ts index 3e2f205c3b..20f3742b48 100644 --- a/src/app-logic/tabs-handling.ts +++ b/src/app-logic/tabs-handling.ts @@ -9,6 +9,7 @@ */ export const tabsWithTitleL10nId = { calltree: 'TabBar--calltree-tab', + 'function-list': 'TabBar--function-list-tab', 'flame-graph': 'TabBar--flame-graph-tab', 'stack-chart': 'TabBar--stack-chart-tab', 'marker-chart': 'TabBar--marker-chart-tab', @@ -41,6 +42,7 @@ export const tabsWithTitleL10nIdArray: readonly TabsWithTitleL10nId[] = export const tabsShowingSampleData: readonly TabSlug[] = [ 'calltree', + 'function-list', 'flame-graph', 'stack-chart', ]; diff --git a/src/app-logic/url-handling.ts b/src/app-logic/url-handling.ts index 60c6b32a4c..1d3a03ff43 100644 --- a/src/app-logic/url-handling.ts +++ b/src/app-logic/url-handling.ts @@ -42,6 +42,7 @@ import type { IndexIntoFrameTable, MarkerIndex, SelectedMarkersPerThread, + SelectedFunctionsPerThread, } from 'firefox-profiler/types'; import { FuncFlag } from 'firefox-profiler/types'; import { @@ -121,6 +122,19 @@ export function getIsHistoryReplaceState(): boolean { return _isReplaceState; } +/** + * Synchronously replace the current history entry to match the given UrlState. + * + * Use this from action thunks for high-frequency actions (e.g. arrow-key + * navigation) where deferring the URL update to UrlManager.componentDidUpdate + * would result in unwanted pushState calls and history-flooding. By updating + * window.history synchronously here, the URL already matches the new state by + * the time UrlManager's componentDidUpdate runs, so it becomes a no-op. + */ +export function replaceHistoryWithUrlState(urlState: UrlState): void { + window.history.replaceState(urlState, document.title, urlFromState(urlState)); +} + function getPathParts(urlState: UrlState): string[] { const { dataSource } = urlState; switch (dataSource) { @@ -188,6 +202,8 @@ type CallTreeQuery = BaseQuery & { invertFlameGraph: null | undefined; hideIdleSamples: null | undefined; ctSummary: string; + functionListSort?: string; // "total-desc~self-asc" — primary first + selectedFunc?: number; // Selected function index for the current thread, e.g. 42 }; type MarkersQuery = BaseQuery & { @@ -236,6 +252,10 @@ type Query = BaseQuery & { marker?: MarkerIndex; markerSort?: string; + // Function list specific + functionListSort?: string; + selectedFunc?: number; + // Network specific networkSearch?: string; @@ -342,6 +362,7 @@ export function getQueryStringFromUrlState(urlState: UrlState): string { : undefined; /* falls through */ case 'flame-graph': + case 'function-list': case 'calltree': { query = baseQuery as CallTreeQueryShape; @@ -392,6 +413,16 @@ export function getQueryStringFromUrlState(urlState: UrlState): string { query.bottomFullscreen = true; } } + if (selectedTab === 'function-list') { + query.functionListSort = convertFunctionListSortToString( + urlState.profileSpecific.functionListSort + ); + const selectedFunc = + selectedThreadsKey !== null + ? urlState.profileSpecific.selectedFunctions[selectedThreadsKey] + : undefined; + query.selectedFunc = selectedFunc ?? undefined; + } break; } case 'marker-table': @@ -556,6 +587,19 @@ export function stateFromLocation( } } + // Parse the selected function for the current thread + const selectedFunctions: SelectedFunctionsPerThread = {}; + if ( + selectedThreadsKey !== null && + query.selectedFunc !== undefined && + query.selectedFunc !== null + ) { + const funcIndex = Number(query.selectedFunc); + if (Number.isInteger(funcIndex) && funcIndex >= 0) { + selectedFunctions[selectedThreadsKey] = funcIndex; + } + } + // tabID is used for the tab selector that we have in our full view. let tabID = null; if (query.tabID && Number.isInteger(Number(query.tabID))) { @@ -647,7 +691,11 @@ export function stateFromLocation( ? query.hiddenThreads.split('-').map((index) => Number(index)) : null, selectedMarkers, + selectedFunctions, markerTableSort: convertMarkerTableSortFromString(query.markerSort), + functionListSort: convertFunctionListSortFromString( + query.functionListSort + ), }, }; } @@ -694,6 +742,50 @@ function convertMarkerTableSortFromString( return parsed; } +// FunctionList sort URL encoding. Same convention as the marker table: +// internal storage is primary-last, URL is primary-first. `null` means "at the +// function list's default", encoded as an absent query parameter. +const VALID_FUNCTION_LIST_SORT_COLUMNS = new Set(['total', 'self']); + +function convertFunctionListSortToString( + sort: SingleColumnSortState[] | null +): string | undefined { + if (sort === null) { + return undefined; + } + return sort + .slice() + .reverse() + .map((s) => `${s.column}-${s.ascending ? 'asc' : 'desc'}`) + .join('~'); +} + +function convertFunctionListSortFromString( + raw: string | null | void +): SingleColumnSortState[] | null { + if (!raw) { + return null; + } + const parsed: SingleColumnSortState[] = []; + for (const part of raw.split('~')) { + const dashIndex = part.lastIndexOf('-'); + if (dashIndex === -1) { + return null; + } + const column = part.slice(0, dashIndex); + const dir = part.slice(dashIndex + 1); + if ( + !VALID_FUNCTION_LIST_SORT_COLUMNS.has(column) || + (dir !== 'asc' && dir !== 'desc') + ) { + return null; + } + parsed.push({ column, ascending: dir === 'asc' }); + } + // URL is primary-first; internal storage is primary-last. + return parsed.reverse(); +} + function convertGlobalTrackOrderFromString( rawString: string | null | void ): TrackIndex[] { diff --git a/src/components/app/Details.tsx b/src/components/app/Details.tsx index 643e6c5929..c571fd6945 100644 --- a/src/components/app/Details.tsx +++ b/src/components/app/Details.tsx @@ -10,6 +10,7 @@ import explicitConnect from 'firefox-profiler/utils/connect'; import { TabBar } from './TabBar'; import { LocalizedErrorBoundary } from './ErrorBoundary'; import { ProfileCallTreeView } from 'firefox-profiler/components/calltree/ProfileCallTreeView'; +import { ProfileFunctionListView } from 'firefox-profiler/components/calltree/ProfileFunctionListView'; import { MarkerTable } from 'firefox-profiler/components/marker-table'; import { StackChart } from 'firefox-profiler/components/stack-chart/'; import { MarkerChart } from 'firefox-profiler/components/marker-chart/'; @@ -122,6 +123,7 @@ class ProfileViewerImpl extends PureComponent { { { calltree: , + 'function-list': , 'flame-graph': , 'stack-chart': , 'marker-chart': , diff --git a/src/components/calltree/CallTree.css b/src/components/calltree/CallTree.css index 73e7f3935c..2efc3e8b38 100644 --- a/src/components/calltree/CallTree.css +++ b/src/components/calltree/CallTree.css @@ -7,8 +7,9 @@ text-align: right; } -/* The header for the totalPercent column is not visible */ -.treeViewHeaderColumn.totalPercent { +/* The headers for the percent columns are not visible */ +.treeViewHeaderColumn.totalPercent, +.treeViewHeaderColumn.selfPercent { display: none; } @@ -26,6 +27,7 @@ .treeViewRowColumn.total, .treeViewRowColumn.totalPercent, .treeViewRowColumn.self, +.treeViewRowColumn.selfPercent, .treeViewRowColumn.timestamp { text-align: right; } diff --git a/src/components/calltree/CallTree.tsx b/src/components/calltree/CallTree.tsx index a86053f130..9ec2279d9d 100644 --- a/src/components/calltree/CallTree.tsx +++ b/src/components/calltree/CallTree.tsx @@ -7,7 +7,6 @@ import { TreeView } from 'firefox-profiler/components/shared/TreeView'; import { CallTreeEmptyReasons } from './CallTreeEmptyReasons'; import { getInvertCallstack, - getImplementationFilter, getSearchStringsAsRegExp, getSelectedThreadsKey, } from 'firefox-profiler/selectors/url-state'; @@ -31,7 +30,6 @@ import { import type { State, - ImplementationFilter, ThreadsKey, IndexIntoCategoryList, IndexIntoCallNodeTable, @@ -61,7 +59,6 @@ type StateProps = { readonly searchStringsRegExp: RegExp | null; readonly disableOverscan: boolean; readonly invertCallstack: boolean; - readonly implementationFilter: ImplementationFilter; readonly callNodeMaxDepthPlusOne: number; readonly weightType: WeightType; readonly tableViewOptions: TableViewOptions; @@ -291,7 +288,6 @@ export const CallTree = explicitConnect<{}, StateProps, DispatchProps>({ searchStringsRegExp: getSearchStringsAsRegExp(state), disableOverscan: getPreviewSelectionIsBeingModified(state), invertCallstack: getInvertCallstack(state), - implementationFilter: getImplementationFilter(state), // Use the filtered call node max depth, rather than the preview filtered call node // max depth so that the width of the TreeView component is stable across preview // selections. diff --git a/src/components/calltree/FunctionList.tsx b/src/components/calltree/FunctionList.tsx new file mode 100644 index 0000000000..6c4c505a65 --- /dev/null +++ b/src/components/calltree/FunctionList.tsx @@ -0,0 +1,250 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +// @flow + +import { PureComponent } from 'react'; +import memoize from 'memoize-immutable'; +import explicitConnect from 'firefox-profiler/utils/connect'; +import { + TreeView, + ColumnSortState, +} from 'firefox-profiler/components/shared/TreeView'; +import { CallTreeEmptyReasons } from './CallTreeEmptyReasons'; +import { + getSearchStringsAsRegExp, + getSelectedThreadsKey, + getFunctionListSort, +} from 'firefox-profiler/selectors/url-state'; +import { + getScrollToSelectionGeneration, + getFocusCallTreeGeneration, + getCurrentTableViewOptions, + getPreviewSelectionIsBeingModified, +} from 'firefox-profiler/selectors/profile'; +import { selectedThreadSelectors } from 'firefox-profiler/selectors/per-thread'; +import { + changeRightClickedFunctionIndex, + changeSelectedFunctionIndex, + addTransformToStack, + changeTableViewOptions, + updateBottomBoxContentsAndMaybeOpen, + changeFunctionListSort, +} from 'firefox-profiler/actions/profile-view'; +import { + nameColumn, + libColumn, + functionListColumnsForWeightType, +} from './columns'; + +import type { + State, + ThreadsKey, + IndexIntoFuncTable, + CallNodeDisplayData, + WeightType, + TableViewOptions, + SelectionContext, +} from 'firefox-profiler/types'; +import type { CallTree } from 'firefox-profiler/profile-logic/call-tree'; + +import type { SingleColumnSortState } from 'firefox-profiler/components/shared/TreeView'; +import type { ConnectedProps } from 'firefox-profiler/utils/connect'; + +import './CallTree.css'; + +const DEFAULT_FUNCTION_LIST_SORT: SingleColumnSortState[] = [ + { column: 'total', ascending: false }, +]; +const DEFAULT_FUNCTION_LIST_SORT_STATE = new ColumnSortState( + DEFAULT_FUNCTION_LIST_SORT +); + +type StateProps = { + readonly threadsKey: ThreadsKey; + readonly scrollToSelectionGeneration: number; + readonly focusCallTreeGeneration: number; + readonly tree: CallTree; + readonly selectedFunctionIndex: IndexIntoFuncTable | null; + readonly rightClickedFunctionIndex: IndexIntoFuncTable | null; + readonly searchStringsRegExp: RegExp | null; + readonly disableOverscan: boolean; + readonly weightType: WeightType; + readonly tableViewOptions: TableViewOptions; + readonly sort: SingleColumnSortState[] | null; +}; + +type DispatchProps = { + readonly changeSelectedFunctionIndex: typeof changeSelectedFunctionIndex; + readonly changeRightClickedFunctionIndex: typeof changeRightClickedFunctionIndex; + readonly addTransformToStack: typeof addTransformToStack; + readonly updateBottomBoxContentsAndMaybeOpen: typeof updateBottomBoxContentsAndMaybeOpen; + readonly onTableViewOptionsChange: (opts: TableViewOptions) => any; + readonly changeFunctionListSort: typeof changeFunctionListSort; +}; + +type Props = ConnectedProps<{}, StateProps, DispatchProps>; + +class FunctionListImpl extends PureComponent { + _treeView: TreeView | null = null; + _takeTreeViewRef = (treeView: TreeView | null) => { + this._treeView = treeView; + }; + + _expandedIndexes: Array = []; + + _getColumnSort = memoize((sort: SingleColumnSortState[] | null) => + sort === null ? DEFAULT_FUNCTION_LIST_SORT_STATE : new ColumnSortState(sort) + ); + + _onColumnSortChange = (columnSort: ColumnSortState) => { + // Normalize matches-default to null so the URL omits the param and every + // client can treat "user hasn't overridden the default" uniformly. + this.props.changeFunctionListSort( + columnSort.equals(DEFAULT_FUNCTION_LIST_SORT_STATE) + ? null + : columnSort.entries + ); + }; + + override componentDidMount() { + this.focus(); + this.maybeProcureInitialSelection(); + + if (this.props.selectedFunctionIndex !== null && this._treeView) { + this._treeView.scrollSelectionIntoView(); + } + } + + override componentDidUpdate(prevProps: Props) { + if ( + this.props.focusCallTreeGeneration > prevProps.focusCallTreeGeneration + ) { + this.focus(); + } + + if ( + this.props.selectedFunctionIndex !== null && + this.props.scrollToSelectionGeneration > + prevProps.scrollToSelectionGeneration && + this._treeView + ) { + this._treeView.scrollSelectionIntoView(); + } + } + + maybeProcureInitialSelection() { + if (this.props.selectedFunctionIndex !== null) { + return; + } + const { tree, threadsKey, changeSelectedFunctionIndex } = this.props; + const firstRoot = tree.getRoots()[0]; + if (firstRoot !== undefined) { + changeSelectedFunctionIndex(threadsKey, firstRoot, { source: 'auto' }); + } + } + + focus() { + if (this._treeView) { + this._treeView.focus(); + } + } + + _onSelectionChange = ( + newSelectedFunction: IndexIntoFuncTable, + context: SelectionContext + ) => { + const { threadsKey, changeSelectedFunctionIndex } = this.props; + changeSelectedFunctionIndex(threadsKey, newSelectedFunction, context); + }; + + _onRightClickSelection = (newSelectedFunction: IndexIntoFuncTable) => { + const { threadsKey, changeRightClickedFunctionIndex } = this.props; + changeRightClickedFunctionIndex(threadsKey, newSelectedFunction); + }; + + _onExpandedCallNodesChange = ( + _newExpandedCallNodeIndexes: Array + ) => {}; + + _onKeyDown = (_event: React.KeyboardEvent) => {}; + + _onEnterOrDoubleClick = (nodeId: IndexIntoFuncTable) => { + const { tree, updateBottomBoxContentsAndMaybeOpen } = this.props; + const bottomBoxInfo = tree.getBottomBoxInfoForFunction(nodeId); + updateBottomBoxContentsAndMaybeOpen('function-list', bottomBoxInfo); + }; + + override render() { + const { + tree, + selectedFunctionIndex, + rightClickedFunctionIndex, + searchStringsRegExp, + disableOverscan, + weightType, + tableViewOptions, + onTableViewOptionsChange, + sort, + } = this.props; + if (tree.getRoots().length === 0) { + return ; + } + return ( + + ); + } +} + +export const FunctionList = explicitConnect<{}, StateProps, DispatchProps>({ + mapStateToProps: (state: State) => ({ + threadsKey: getSelectedThreadsKey(state), + scrollToSelectionGeneration: getScrollToSelectionGeneration(state), + focusCallTreeGeneration: getFocusCallTreeGeneration(state), + tree: selectedThreadSelectors.getFunctionListTree(state), + selectedFunctionIndex: + selectedThreadSelectors.getSelectedFunctionIndex(state), + rightClickedFunctionIndex: + selectedThreadSelectors.getRightClickedFunctionIndex(state), + searchStringsRegExp: getSearchStringsAsRegExp(state), + disableOverscan: getPreviewSelectionIsBeingModified(state), + weightType: selectedThreadSelectors.getWeightTypeForCallTree(state), + tableViewOptions: getCurrentTableViewOptions(state), + sort: getFunctionListSort(state), + }), + mapDispatchToProps: { + changeSelectedFunctionIndex, + changeRightClickedFunctionIndex, + addTransformToStack, + updateBottomBoxContentsAndMaybeOpen, + onTableViewOptionsChange: (options: TableViewOptions) => + changeTableViewOptions('calltree', options), + changeFunctionListSort, + }, + component: FunctionListImpl, +}); diff --git a/src/components/calltree/ProfileFunctionListView.tsx b/src/components/calltree/ProfileFunctionListView.tsx new file mode 100644 index 0000000000..7695a6c4d2 --- /dev/null +++ b/src/components/calltree/ProfileFunctionListView.tsx @@ -0,0 +1,20 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { FunctionList } from './FunctionList'; +import { StackSettings } from 'firefox-profiler/components/shared/StackSettings'; +import { TransformNavigator } from 'firefox-profiler/components/shared/TransformNavigator'; + +export const ProfileFunctionListView = () => ( +
+ + + +
+); diff --git a/src/components/calltree/columns.ts b/src/components/calltree/columns.ts index 14f597410a..a3f92fa837 100644 --- a/src/components/calltree/columns.ts +++ b/src/components/calltree/columns.ts @@ -128,3 +128,47 @@ export function treeColumnsForWeightType( throw assertExhaustiveCheck(weightType, 'Unhandled WeightType.'); } } + +function withSelfPercentColumn( + columns: MaybeResizableColumn[] +): MaybeResizableColumn[] { + const selfPercentColumn = { + propName: 'selfPercent', + titleL10nId: '', + initialWidth: 55, + hideDividerAfter: true, + }; + const selfColumnIndex = columns.findIndex((c) => c.propName === 'self'); + const selfColumn = { + ...columns[selfColumnIndex], + headerWidthAdjustment: selfPercentColumn.initialWidth, + }; + const newColumns = columns.slice(); + newColumns[selfColumnIndex] = selfColumn; + newColumns.splice(selfColumnIndex, 0, selfPercentColumn); + return newColumns; +} + +export const functionListColumnsForTracingMs: MaybeResizableColumn[] = + withSelfPercentColumn(treeColumnsForTracingMs); +export const functionListColumnsForSamples: MaybeResizableColumn[] = + withSelfPercentColumn(treeColumnsForSamples); +export const functionListColumnsForBytes: MaybeResizableColumn[] = + withSelfPercentColumn(treeColumnsForBytes); + +// Like `treeColumnsForWeightType`, but for the FunctionList table (which has +// an extra selfPercent column compared to the call-tree variants). +export function functionListColumnsForWeightType( + weightType: WeightType +): MaybeResizableColumn[] { + switch (weightType) { + case 'tracing-ms': + return functionListColumnsForTracingMs; + case 'samples': + return functionListColumnsForSamples; + case 'bytes': + return functionListColumnsForBytes; + default: + throw assertExhaustiveCheck(weightType, 'Unhandled WeightType.'); + } +} diff --git a/src/components/sidebar/index.tsx b/src/components/sidebar/index.tsx index a8115372a3..e4a6901069 100644 --- a/src/components/sidebar/index.tsx +++ b/src/components/sidebar/index.tsx @@ -15,6 +15,7 @@ export function selectSidebar( ): React.ComponentType<{}> | null { return { calltree: CallTreeSidebar, + 'function-list': CallTreeSidebar, 'flame-graph': CallTreeSidebar, 'stack-chart': null, 'marker-chart': null, diff --git a/src/profile-logic/call-tree.ts b/src/profile-logic/call-tree.ts index f870e0f703..e970a3a2d5 100644 --- a/src/profile-logic/call-tree.ts +++ b/src/profile-logic/call-tree.ts @@ -38,7 +38,10 @@ import { checkBit } from '../utils/bitset'; import * as ProfileData from './profile-data'; import type { CallTreeSummaryStrategy } from '../types/actions'; import type { CallNodeInfo, CallNodeInfoInverted } from './call-node-info'; -import type { SortableColumn } from '../components/shared/TreeView'; +import type { + ColumnSortState, + SortableColumn, +} from '../components/shared/TreeView'; import { getBottomBoxInfoForCallNode, getBottomBoxInfoForFunction, @@ -407,6 +410,12 @@ export class CallTree { // The call tree's own order (by total sample count) is the intended order, // so no columns are user-sortable. getSortableColumns(): SortableColumn[] { + if (this._internal instanceof CallTreeInternalFunctionList) { + return [ + { column: 'total', prefersDescending: true }, + { column: 'self', prefersDescending: true }, + ]; + } return []; } @@ -414,7 +423,28 @@ export class CallTree { return this._rootTotalSummary; } - getRoots() { + getRoots(sort: ColumnSortState | null = null): IndexIntoCallNodeTable[] { + if ( + sort !== null && + sort.entries.length > 0 && + this._internal instanceof CallTreeInternalFunctionList + ) { + const internal = this._internal; + return sort.sortItems( + this._roots, + ( + a: IndexIntoCallNodeTable, + b: IndexIntoCallNodeTable, + column: string + ) => { + const aValue = + column === 'self' ? internal.getSelf(a) : internal.getTotal(a); + const bValue = + column === 'self' ? internal.getSelf(b) : internal.getTotal(b); + return aValue - bValue; + } + ); + } return this._roots; } @@ -518,7 +548,7 @@ export class CallTree { let displayData: CallNodeDisplayData | void = this._displayDataByIndex.get(callNodeIndex); if (displayData === undefined) { - const { funcName, total, totalRelative, self } = + const { funcName, total, totalRelative, self, selfRelative } = this.getNodeData(callNodeIndex); const funcIndex = this._callNodeInfo.funcForNode(callNodeIndex); const categoryIndex = this._callNodeInfo.categoryForNode(callNodeIndex); @@ -561,6 +591,7 @@ export class CallTree { self ); const totalPercent = `${formatPercent(totalRelative)}`; + const selfPercent = `${formatPercent(selfRelative)}`; let ariaLabel; let totalWithUnit; @@ -611,6 +642,7 @@ export class CallTree { self: self === 0 ? '—' : formattedSelf, selfWithUnit: self === 0 ? '—' : selfWithUnit, totalPercent, + selfPercent, name: funcName, lib: libName.slice(0, 1000), // Dim platform pseudo-stacks. diff --git a/src/profile-logic/symbolication.ts b/src/profile-logic/symbolication.ts index 91b7e075dd..7c5162b686 100644 --- a/src/profile-logic/symbolication.ts +++ b/src/profile-logic/symbolication.ts @@ -1094,6 +1094,21 @@ export async function symbolicateProfile( ); } +// Returns the func that replaces the given func after symbolication. If the +// func expanded into a path of new funcs (from inlined functions), this returns +// the innermost one. This matches applyFuncSubstitutionToCallPath, where the +// leaf of the new call path is the innermost func. +// A func that is not present as a key in the map stays unchanged. +export function applyFuncSubstitutionToFunc( + oldFuncToNewFuncsMap: FuncToFuncsMap, + func: IndexIntoFuncTable +): IndexIntoFuncTable { + const newFuncs = oldFuncToNewFuncsMap.get(func); + return newFuncs === undefined || newFuncs.length === 0 + ? func + : newFuncs[newFuncs.length - 1]; +} + // Create a new call path, where each func in the old call path is // replaced with one or more funcs from the FuncToFuncsMap. // This is used during symbolication, where some previously separate diff --git a/src/reducers/profile-view.ts b/src/reducers/profile-view.ts index b2a58f78c0..b595c81d20 100644 --- a/src/reducers/profile-view.ts +++ b/src/reducers/profile-view.ts @@ -30,8 +30,10 @@ import type { ThreadsKey, Milliseconds, TableViewOptions, + RightClickedFunction, } from 'firefox-profiler/types'; import { + applyFuncSubstitutionToFunc, applyFuncSubstitutionToCallPath, applyFuncSubstitutionToPathSetAndIncludeNewAncestors, } from '../profile-logic/symbolication'; @@ -636,6 +638,7 @@ const scrollToSelectionGeneration: Reducer = (state = 0, action) => { case 'CHANGE_NETWORK_SEARCH_STRING': return state + 1; case 'CHANGE_SELECTED_CALL_NODE': + case 'CHANGE_SELECTED_FUNCTION': case 'CHANGE_SELECTED_MARKER': case 'CHANGE_SELECTED_NETWORK_MARKER': if (action.context.source === 'pointer') { @@ -781,6 +784,51 @@ const rightClickedCallNode: Reducer = ( } }; +const rightClickedFunction: Reducer = ( + state = null, + action +) => { + switch (action.type) { + case 'BULK_SYMBOLICATION': { + if (state === null) { + return null; + } + + return { + ...state, + functionIndex: applyFuncSubstitutionToFunc( + action.oldFuncToNewFuncsMap, + state.functionIndex + ), + }; + } + case 'CHANGE_RIGHT_CLICKED_FUNCTION': + if (action.functionIndex !== null) { + return { + threadsKey: action.threadsKey, + functionIndex: action.functionIndex, + }; + } + + return null; + case 'SET_CONTEXT_MENU_VISIBILITY': + // We want to change the state only when the menu is hidden. + if (action.isVisible) { + return state; + } + + return null; + case 'PROFILE_LOADED': + case 'CHANGE_INVERT_CALLSTACK': + case 'ADD_TRANSFORM_TO_STACK': + case 'POP_TRANSFORMS_FROM_STACK': + case 'CHANGE_IMPLEMENTATION_FILTER': + return null; + default: + return state; + } +}; + const rightClickedMarker: Reducer = ( state = null, action @@ -882,6 +930,7 @@ const profileViewReducer: Reducer = wrapReducerInResetter( lastNonShiftClick, rightClickedTrack, rightClickedCallNode, + rightClickedFunction, rightClickedMarker, hoveredMarker, mouseTimePosition, diff --git a/src/reducers/url-state.ts b/src/reducers/url-state.ts index 5374e6440f..7ac1e7a3d3 100644 --- a/src/reducers/url-state.ts +++ b/src/reducers/url-state.ts @@ -4,7 +4,7 @@ import { combineReducers } from 'redux'; import { oneLine } from 'common-tags'; -import { objectEntries } from '../utils/types'; +import { objectEntries, objectMap } from '../utils/types'; import { tabSlugs, tabsShowingSampleData } from '../app-logic/tabs-handling'; import type { @@ -24,12 +24,15 @@ import type { IsOpenPerPanelState, TabID, SelectedMarkersPerThread, + SelectedFunctionsPerThread, } from 'firefox-profiler/types'; import type { TabSlug } from '../app-logic/tabs-handling'; import type { SingleColumnSortState } from '../components/shared/TreeView'; import { translateThreadsKey } from 'firefox-profiler/profile-logic/profile-data'; import { translateTransformStack } from 'firefox-profiler/profile-logic/transforms'; +import { translateFuncIndex } from 'firefox-profiler/profile-logic/index-translation'; +import { applyFuncSubstitutionToFunc } from 'firefox-profiler/profile-logic/symbolication'; /* * This state file governs the state that comes from, and alters, the window @@ -206,6 +209,18 @@ const markerTableSort: Reducer = ( } }; +const functionListSort: Reducer = ( + state = null, + action +) => { + switch (action.type) { + case 'CHANGE_FUNCTION_LIST_SORT': + return action.sort; + default: + return state; + } +}; + const networkSearchString: Reducer = (state = '', action) => { switch (action.type) { case 'CHANGE_NETWORK_SEARCH_STRING': @@ -786,6 +801,57 @@ const selectedMarkers: Reducer = ( } }; +const selectedFunctions: Reducer = ( + state = {}, + action +): SelectedFunctionsPerThread => { + switch (action.type) { + case 'CHANGE_SELECTED_FUNCTION': { + const { threadsKey, selectedFunctionIndex } = action; + if (state[threadsKey] === selectedFunctionIndex) { + return state; + } + return { + ...state, + [threadsKey]: selectedFunctionIndex, + }; + } + case 'BULK_SYMBOLICATION': { + const { oldFuncToNewFuncsMap } = action; + return objectMap(state, (funcIndex) => + funcIndex === null + ? null + : applyFuncSubstitutionToFunc(oldFuncToNewFuncsMap, funcIndex) + ); + } + case 'SANITIZED_PROFILE_PUBLISHED': { + const { translationMaps } = action; + if (!translationMaps) { + return state; + } + + const newState: SelectedFunctionsPerThread = {}; + for (const [threadsKey, funcIndex] of objectEntries(state)) { + const newThreadsKey = translationMaps.oldThreadIndexToNew + ? translateThreadsKey(threadsKey, translationMaps.oldThreadIndexToNew) + : threadsKey; + if (newThreadsKey === null) { + // Discard the selection for threads that were removed. + continue; + } + + newState[newThreadsKey] = + funcIndex === null + ? null + : translateFuncIndex(funcIndex, translationMaps); + } + return newState; + } + default: + return state; + } +}; + /** * These values are specific to an individual profile. */ @@ -816,7 +882,9 @@ const profileSpecific = combineReducers({ showJsTracerSummary, tabFilter, selectedMarkers, + selectedFunctions, markerTableSort, + functionListSort, // The timeline tracks used to be hidden and sorted by thread indexes, rather than // track indexes. The only way to migrate this information to tracks-based data is to // first retrieve the profile, so they can't be upgraded by the normal url upgrading diff --git a/src/selectors/per-thread/stack-sample.ts b/src/selectors/per-thread/stack-sample.ts index 63347c19c1..7b45a97beb 100644 --- a/src/selectors/per-thread/stack-sample.ts +++ b/src/selectors/per-thread/stack-sample.ts @@ -42,6 +42,7 @@ import type { CallNodeSelfAndSummary, State, CallNodeTableBitSet, + IndexIntoFuncTable, } from 'firefox-profiler/types'; import type { CallNodeInfo, @@ -212,6 +213,10 @@ export function getStackAndSampleSelectorsPerThread( } ); + const getSelectedFunctionIndex: Selector = ( + state + ) => UrlState.getSelectedFunction(state, threadsKey); + const getSelectedCallNodePath: Selector = createSelector( threadSelectors.getViewOptions, UrlState.getInvertCallstack, @@ -648,6 +653,23 @@ export function getStackAndSampleSelectorsPerThread( } ); + const getRightClickedFunctionIndex: Selector = + createSelector( + ProfileSelectors.getProfileViewOptions, + (profileViewOptions) => { + const rightClickedFunctionInfo = + profileViewOptions.rightClickedFunction; + if ( + rightClickedFunctionInfo !== null && + threadsKey === rightClickedFunctionInfo.threadsKey + ) { + return rightClickedFunctionInfo.functionIndex; + } + + return null; + } + ); + return { unfilteredSamplesRange, getWeightTypeForCallTree, @@ -658,6 +680,7 @@ export function getStackAndSampleSelectorsPerThread( getAssemblyViewStackAddressInfo, getSelectedCallNodePath, getSelectedCallNodeIndex, + getSelectedFunctionIndex, getExpandedCallNodePaths, getExpandedCallNodeIndexes, getSampleCallNodesForFilteredThread, @@ -681,5 +704,6 @@ export function getStackAndSampleSelectorsPerThread( getNonInvertedFilteredCallNodeMaxDepthPlusOne, getFlameGraphTiming, getRightClickedCallNodeIndex, + getRightClickedFunctionIndex, }; } diff --git a/src/selectors/url-state.ts b/src/selectors/url-state.ts index 599b59a748..413ed8e175 100644 --- a/src/selectors/url-state.ts +++ b/src/selectors/url-state.ts @@ -32,6 +32,7 @@ import type { TabID, IndexIntoSourceTable, MarkerIndex, + IndexIntoFuncTable, } from 'firefox-profiler/types'; import type { TabSlug } from '../app-logic/tabs-handling'; @@ -121,6 +122,9 @@ export const getMarkersSearchString: Selector = (state) => export const getMarkerTableSort: Selector = ( state ) => getProfileSpecificState(state).markerTableSort; +export const getFunctionListSort: Selector = ( + state +) => getProfileSpecificState(state).functionListSort; export const getNetworkSearchString: Selector = (state) => getProfileSpecificState(state).networkSearchString; export const getSelectedTab: Selector = (state) => @@ -257,6 +261,12 @@ export const getSelectedMarker: DangerousSelectorWithArguments< > = (state, threadsKey) => getProfileSpecificState(state).selectedMarkers[threadsKey] ?? null; +export const getSelectedFunction: DangerousSelectorWithArguments< + IndexIntoFuncTable | null, + ThreadsKey +> = (state, threadsKey) => + getProfileSpecificState(state).selectedFunctions[threadsKey] ?? null; + export const getIsBottomBoxOpen: Selector = (state) => { const tab = getSelectedTab(state); return getProfileSpecificState(state).isBottomBoxOpenPerPanel[tab]; diff --git a/src/test/components/Details.test.tsx b/src/test/components/Details.test.tsx index 73b05db864..0970c890d0 100644 --- a/src/test/components/Details.test.tsx +++ b/src/test/components/Details.test.tsx @@ -20,6 +20,9 @@ import type { TabSlug } from '../../app-logic/tabs-handling'; jest.mock('../../components/calltree/ProfileCallTreeView', () => ({ ProfileCallTreeView: 'call-tree', })); +jest.mock('../../components/calltree/ProfileFunctionListView', () => ({ + ProfileFunctionListView: 'function-list', +})); jest.mock('../../components/flame-graph', () => ({ FlameGraph: 'flame-graph', })); diff --git a/src/test/components/DetailsContainer.test.tsx b/src/test/components/DetailsContainer.test.tsx index 54c142dba0..59d449fccb 100644 --- a/src/test/components/DetailsContainer.test.tsx +++ b/src/test/components/DetailsContainer.test.tsx @@ -37,6 +37,7 @@ describe('app/DetailsContainer', function () { const expectedSidebar: { [slug in TabSlug]: boolean } = { calltree: true, + 'function-list': true, 'flame-graph': true, 'stack-chart': false, 'marker-chart': false, diff --git a/src/test/store/bottom-box.test.ts b/src/test/store/bottom-box.test.ts index 7d3ef96ae3..4c62a329ef 100644 --- a/src/test/store/bottom-box.test.ts +++ b/src/test/store/bottom-box.test.ts @@ -425,4 +425,31 @@ describe('getBottomBoxInfoForFunction', function () { // scrollToLineNumber should be 31 (the innermost B), not 30 (the outer B). expect(bottomBoxInfo.scrollToLineNumber).toBe(31); }); + + it('opens the source view when double-clicking a function in the function list', function () { + const { profile, derivedThreads, funcNamesDictPerThread } = + getProfileFromTextSamples(` + A[file:a.js][line:20] + B[file:b.js][line:30] + `); + const { dispatch, getState } = storeWithProfile(profile); + const [thread] = derivedThreads; + const [{ B }] = funcNamesDictPerThread; + + dispatch(changeSelectedTab('function-list')); + + const bottomBoxInfo = getBottomBoxInfoForFunction( + B, + thread, + thread.samples + ); + dispatch( + updateBottomBoxContentsAndMaybeOpen('function-list', bottomBoxInfo) + ); + + expect(UrlStateSelectors.getIsBottomBoxOpen(getState())).toBeTrue(); + expect(UrlStateSelectors.getSourceViewScrollToLineNumber(getState())).toBe( + 30 + ); + }); }); diff --git a/src/test/store/icons.test.ts b/src/test/store/icons.test.ts index 3a9d3df13f..c1b77810b9 100644 --- a/src/test/store/icons.test.ts +++ b/src/test/store/icons.test.ts @@ -37,6 +37,7 @@ describe('actions/icons', function () { totalPercent: '0', self: '0', selfWithUnit: '0 ms', + selfPercent: '0', name: 'icon', lib: 'icon', isFrameLabel: false, diff --git a/src/test/store/symbolication.test.ts b/src/test/store/symbolication.test.ts index 1c230bc968..2b40ff9700 100644 --- a/src/test/store/symbolication.test.ts +++ b/src/test/store/symbolication.test.ts @@ -35,6 +35,7 @@ import { changeSelectedCallNode, changeExpandedCallNodes, changeImplementationFilter, + changeSelectedFunctionIndex, } from '../../actions/profile-view'; import { formatTree, formatStack } from '../fixtures/utils'; import { assertSetContainsOnly } from '../fixtures/custom-assertions'; @@ -449,6 +450,28 @@ describe('doSymbolicateProfile', function () { [['first symbol']].map(funcNamesToFuncIndexes) ); }); + + it('symbolicates the selected function', async function () { + const { + store: { dispatch, getState }, + profile, + symbolStore, + funcNamesToFuncIndexes, + } = init(); + + const [unsymbolicatedFunc] = funcNamesToFuncIndexes(['0x0000']); + dispatch(changeSelectedFunctionIndex(0, unsymbolicatedFunc)); + expect(selectedThreadSelectors.getSelectedFunctionIndex(getState())).toBe( + unsymbolicatedFunc + ); + + await doSymbolicateProfile(dispatch, profile, symbolStore); + const [symbolicatedFunc] = funcNamesToFuncIndexes(['first symbol']); + expect(symbolicatedFunc).not.toBe(unsymbolicatedFunc); + expect(selectedThreadSelectors.getSelectedFunctionIndex(getState())).toBe( + symbolicatedFunc + ); + }); }); it('can symbolicate a profile with a partial symbol table and re-symbolicate it with a complete symbol table', async () => { diff --git a/src/test/store/useful-tabs.test.ts b/src/test/store/useful-tabs.test.ts index 0d63df2759..5c84982d43 100644 --- a/src/test/store/useful-tabs.test.ts +++ b/src/test/store/useful-tabs.test.ts @@ -24,6 +24,7 @@ describe('getUsefulTabs', function () { const { getState } = storeWithProfile(profile); expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ 'calltree', + 'function-list', 'flame-graph', 'stack-chart', 'marker-chart', @@ -62,6 +63,7 @@ describe('getUsefulTabs', function () { }); expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ 'calltree', + 'function-list', 'flame-graph', 'stack-chart', 'marker-chart', @@ -89,6 +91,7 @@ describe('getUsefulTabs', function () { const { getState } = storeWithProfile(profile); expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ 'calltree', + 'function-list', 'flame-graph', 'stack-chart', 'marker-chart', @@ -121,6 +124,7 @@ describe('getUsefulTabs', function () { const { getState } = storeWithProfile(profile); expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ 'calltree', + 'function-list', 'flame-graph', 'stack-chart', 'marker-chart', diff --git a/src/test/unit/profile-tree.test.ts b/src/test/unit/profile-tree.test.ts index 48c82e95e1..1b6afc1682 100644 --- a/src/test/unit/profile-tree.test.ts +++ b/src/test/unit/profile-tree.test.ts @@ -332,6 +332,7 @@ describe('unfiltered call tree', function () { name: 'A', self: '—', selfWithUnit: '—', + selfPercent: '0%', total: '3', totalWithUnit: '3 samples', totalPercent: '100%', @@ -348,6 +349,7 @@ describe('unfiltered call tree', function () { name: 'I', self: '1', selfWithUnit: '1 sample', + selfPercent: '33%', total: '1', totalWithUnit: '1 sample', totalPercent: '33%', diff --git a/src/test/url-handling.test.ts b/src/test/url-handling.test.ts index c5779cff8d..d12d624998 100644 --- a/src/test/url-handling.test.ts +++ b/src/test/url-handling.test.ts @@ -23,6 +23,7 @@ import { changeIncludeIdleSamples, changeSelectedMarker, changeMarkerTableSort, + changeFunctionListSort, } from '../actions/profile-view'; import { changeSelectedTab, changeProfilesToCompare } from '../actions/app'; import { @@ -556,6 +557,57 @@ describe('marker table sort', function () { }); }); +describe('function list sort', function () { + function _getStoreOnFunctionList() { + const store = _getStoreWithURL(); + store.dispatch(changeSelectedTab('function-list')); + return store; + } + + it('omits the default sort from the URL', function () { + const { getState } = _getStoreOnFunctionList(); + expect(getQueryStringFromState(getState())).not.toContain( + 'functionListSort' + ); + }); + + it('serializes a non-default sort with primary first', function () { + const { getState, dispatch } = _getStoreOnFunctionList(); + // self desc primary, total asc tiebreaker (internal: primary last) + dispatch( + changeFunctionListSort([ + { column: 'total', ascending: true }, + { column: 'self', ascending: false }, + ]) + ); + expect(getQueryStringFromState(getState())).toContain( + 'functionListSort=self-desc~total-asc' + ); + }); + + it('round-trips a non-default sort through the URL', function () { + const { getState, dispatch } = _getStoreOnFunctionList(); + dispatch(changeFunctionListSort([{ column: 'self', ascending: false }])); + const url = urlFromState(getState().urlState); + const restored = stateFromLocation({ + pathname: new URL(url, 'http://localhost').pathname, + search: new URL(url, 'http://localhost').search, + hash: '', + }); + expect(restored.profileSpecific.functionListSort).toEqual([ + { column: 'self', ascending: false }, + ]); + }); + + it('falls back to the default when the URL has an invalid column', function () { + const { getState } = _getStoreWithURL({ + pathname: '/public/abc/function-list/', + search: '?functionListSort=bogus-desc', + }); + expect(getState().urlState.profileSpecific.functionListSort).toBeNull(); + }); +}); + describe('profileName', function () { it('serializes the profileName in the URL', function () { const { getState, dispatch } = _getStoreWithURL(); diff --git a/src/types/actions.ts b/src/types/actions.ts index 01d57310b3..e59e601795 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -17,6 +17,7 @@ import type { RawFrameTable, SourceLocationTable, SourceTable, + IndexIntoFuncTable, } from './profile'; import type { Thread, @@ -195,6 +196,12 @@ type ProfileAction = readonly optionalExpandedToCallNodePath: CallNodePath | undefined; readonly context: SelectionContext; } + | { + readonly type: 'CHANGE_SELECTED_FUNCTION'; + readonly threadsKey: ThreadsKey; + readonly selectedFunctionIndex: IndexIntoFuncTable | null; + readonly context: SelectionContext; + } | { readonly type: 'UPDATE_TRACK_THREAD_HEIGHT'; readonly height: CssPixels; @@ -205,6 +212,11 @@ type ProfileAction = readonly threadsKey: ThreadsKey; readonly callNodePath: CallNodePath | null; } + | { + readonly type: 'CHANGE_RIGHT_CLICKED_FUNCTION'; + readonly threadsKey: ThreadsKey; + readonly functionIndex: IndexIntoFuncTable | null; + } | { readonly type: 'FOCUS_CALL_TREE'; } @@ -566,6 +578,10 @@ type UrlStateAction = readonly type: 'CHANGE_MARKER_TABLE_SORT'; readonly sort: SingleColumnSortState[] | null; } + | { + readonly type: 'CHANGE_FUNCTION_LIST_SORT'; + readonly sort: SingleColumnSortState[] | null; + } | { readonly type: 'CHANGE_NETWORK_SEARCH_STRING'; readonly searchString: string; diff --git a/src/types/profile-derived.ts b/src/types/profile-derived.ts index 10fbae4239..4a5a1ddfa1 100644 --- a/src/types/profile-derived.ts +++ b/src/types/profile-derived.ts @@ -623,6 +623,7 @@ export type CallNodeDisplayData = Readonly<{ totalPercent: string; self: string; selfWithUnit: string; + selfPercent: string; name: string; lib: string; isFrameLabel: boolean; diff --git a/src/types/state.ts b/src/types/state.ts index 5583e72181..0a4872e0cb 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -23,6 +23,7 @@ import type { Pid, TabID, IndexIntoSourceTable, + IndexIntoFuncTable, } from './profile'; import type { @@ -83,6 +84,11 @@ export type RightClickedCallNode = { readonly callNodePath: CallNodePath; }; +export type RightClickedFunction = { + readonly threadsKey: ThreadsKey; + readonly functionIndex: IndexIntoFuncTable; +}; + export type MarkerReference = { readonly threadsKey: ThreadsKey; readonly markerIndex: MarkerIndex; @@ -92,6 +98,10 @@ export type SelectedMarkersPerThread = { [key: ThreadsKey]: MarkerIndex | null; }; +export type SelectedFunctionsPerThread = { + [key: ThreadsKey]: IndexIntoFuncTable | null; +}; + /** * Profile view state */ @@ -108,6 +118,7 @@ export type ProfileViewState = { lastNonShiftClick: LastNonShiftClickInformation | null; rightClickedTrack: TrackReference | null; rightClickedCallNode: RightClickedCallNode | null; + rightClickedFunction: RightClickedFunction | null; rightClickedMarker: MarkerReference | null; hoveredMarker: MarkerReference | null; mouseTimePosition: Milliseconds | null; @@ -386,7 +397,9 @@ export type ProfileSpecificUrlState = { legacyThreadOrder: ThreadIndex[] | null; legacyHiddenThreads: ThreadIndex[] | null; selectedMarkers: SelectedMarkersPerThread; + selectedFunctions: SelectedFunctionsPerThread; markerTableSort: SingleColumnSortState[] | null; + functionListSort: SingleColumnSortState[] | null; }; export type UrlState = { diff --git a/src/utils/types.ts b/src/utils/types.ts index 1d6df05f5e..9b41c5b1d9 100644 --- a/src/utils/types.ts +++ b/src/utils/types.ts @@ -42,6 +42,7 @@ export function toValidTabSlug(tabSlug: any): TabSlug | null { const coercedTabSlug = tabSlug as TabSlug; switch (coercedTabSlug) { case 'calltree': + case 'function-list': case 'stack-chart': case 'marker-chart': case 'network-chart': From ea14f38a38c1b1cf3185b74dd477337aec4c74cd Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sun, 21 Jun 2026 10:52:56 -0400 Subject: [PATCH 2/6] Add a context menu for the function list. The call tree's context menu operates on a call node path, which the function list does not have. Add a sibling menu that operates on a function index instead. The menu items are described by a data table rather than repeated JSX, because the same set of transforms is about to be reused by more than one view. --- src/actions/profile-view.ts | 2 +- src/components/app/Details.tsx | 2 + .../shared/FunctionListContextMenu.tsx | 486 ++++++++++++++++++ .../FunctionListContextMenu.test.tsx | 119 +++++ .../FunctionListContextMenu.test.tsx.snap | 216 ++++++++ 5 files changed, 824 insertions(+), 1 deletion(-) create mode 100644 src/components/shared/FunctionListContextMenu.tsx create mode 100644 src/test/components/FunctionListContextMenu.test.tsx create mode 100644 src/test/components/__snapshots__/FunctionListContextMenu.test.tsx.snap diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts index 9da70090ae..074c76ab73 100644 --- a/src/actions/profile-view.ts +++ b/src/actions/profile-view.ts @@ -179,7 +179,7 @@ export function changeRightClickedCallNode( export function changeRightClickedFunctionIndex( threadsKey: ThreadsKey, functionIndex: IndexIntoFuncTable | null -) { +): Action { return { type: 'CHANGE_RIGHT_CLICKED_FUNCTION', threadsKey, diff --git a/src/components/app/Details.tsx b/src/components/app/Details.tsx index c571fd6945..85d92cc99a 100644 --- a/src/components/app/Details.tsx +++ b/src/components/app/Details.tsx @@ -27,6 +27,7 @@ import { getSelectedTab } from 'firefox-profiler/selectors/url-state'; import { getIsSidebarOpen } from 'firefox-profiler/selectors/app'; import { selectedThreadSelectors } from 'firefox-profiler/selectors/per-thread'; import { CallNodeContextMenu } from 'firefox-profiler/components/shared/CallNodeContextMenu'; +import { FunctionListContextMenu } from 'firefox-profiler/components/shared/FunctionListContextMenu'; import { MaybeMarkerContextMenu } from 'firefox-profiler/components/shared/MarkerContextMenu'; import { toValidTabSlug } from 'firefox-profiler/utils/types'; @@ -135,6 +136,7 @@ class ProfileViewerImpl extends PureComponent { + ); diff --git a/src/components/shared/FunctionListContextMenu.tsx b/src/components/shared/FunctionListContextMenu.tsx new file mode 100644 index 0000000000..8625c16999 --- /dev/null +++ b/src/components/shared/FunctionListContextMenu.tsx @@ -0,0 +1,486 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import * as React from 'react'; +import { PureComponent } from 'react'; +import { MenuItem } from '@firefox-devtools/react-contextmenu'; +import { Localized } from '@fluent/react'; + +import { ContextMenu } from './ContextMenu'; +import explicitConnect from 'firefox-profiler/utils/connect'; +import { + funcHasDirectRecursiveCall, + funcHasRecursiveCall, +} from 'firefox-profiler/profile-logic/transforms'; +import { getFunctionName } from 'firefox-profiler/profile-logic/function-info'; + +import copy from 'copy-to-clipboard'; +import { + addTransformToStack, + addCollapseResourceTransformToStack, + setContextMenuVisibility, +} from 'firefox-profiler/actions/profile-view'; +import { getImplementationFilter } from 'firefox-profiler/selectors/url-state'; +import { getThreadSelectorsFromThreadsKey } from 'firefox-profiler/selectors/per-thread'; +import { + getProfileViewOptions, + getShouldDisplaySearchfox, +} from 'firefox-profiler/selectors/profile'; +import { oneLine } from 'common-tags'; + +import { + convertToTransformType, + assertExhaustiveCheck, +} from 'firefox-profiler/utils/types'; + +import type { + TransformType, + ImplementationFilter, + IndexIntoFuncTable, + Thread, + ThreadsKey, + CallNodeTable, + State, +} from 'firefox-profiler/types'; + +import { FuncFlag } from 'firefox-profiler/types'; +import type { ConnectedProps } from 'firefox-profiler/utils/connect'; + +import './CallNodeContextMenu.css'; + +// Context provided to data-table menu items so they can compute visibility, +// label, and l10n vars from the current right-clicked function. +type MenuItemContext = { + readonly funcIndex: IndexIntoFuncTable; + readonly callNodeTable: CallNodeTable; + readonly nameForResource: string | null; +}; + +// A descriptor for a transform menu item. The data table below drives the +// rendering of the menu, instead of repeating identical JSX three times. +type TransformMenuItem = { + readonly transform: TransformType; + readonly shortcut: string; + readonly icon: string; + readonly l10nId: string; + readonly content: (ctx: MenuItemContext) => string; + readonly visible?: (ctx: MenuItemContext) => boolean; + readonly l10nVars?: (ctx: MenuItemContext) => Record; + readonly l10nElems?: Record; +}; + +const MENU_ITEMS: ReadonlyArray = [ + { + transform: 'merge-function', + shortcut: 'm', + icon: 'Merge', + l10nId: 'CallNodeContextMenu--transform-merge-function', + content: () => 'Merge function', + }, + { + transform: 'focus-function', + shortcut: 'f', + icon: 'Focus', + l10nId: 'CallNodeContextMenu--transform-focus-function', + content: () => 'Focus on function', + }, + { + transform: 'focus-self', + shortcut: 'S', + icon: 'FocusSelf', + l10nId: 'CallNodeContextMenu--transform-focus-self', + content: () => 'Focus on self only', + }, + { + transform: 'collapse-function-subtree', + shortcut: 'c', + icon: 'Collapse', + l10nId: 'CallNodeContextMenu--transform-collapse-function-subtree', + content: () => 'Collapse function', + }, + { + transform: 'collapse-resource', + shortcut: 'C', + icon: 'Collapse', + l10nId: 'CallNodeContextMenu--transform-collapse-resource', + content: ({ nameForResource }) => `Collapse ${nameForResource}`, + visible: ({ nameForResource }) => nameForResource !== null, + l10nVars: ({ nameForResource }) => ({ + nameForResource: nameForResource ?? '', + }), + l10nElems: { strong: }, + }, + { + transform: 'collapse-recursion', + shortcut: 'r', + icon: 'Collapse', + l10nId: 'CallNodeContextMenu--transform-collapse-recursion', + content: () => 'Collapse recursion', + visible: ({ funcIndex, callNodeTable }) => + funcHasRecursiveCall(callNodeTable, funcIndex), + }, + { + transform: 'collapse-direct-recursion', + shortcut: 'R', + icon: 'Collapse', + l10nId: 'CallNodeContextMenu--transform-collapse-direct-recursion-only', + content: () => 'Collapse direct recursion only', + visible: ({ funcIndex, callNodeTable }) => + funcHasDirectRecursiveCall(callNodeTable, funcIndex), + }, + { + transform: 'drop-function', + shortcut: 'd', + icon: 'Drop', + l10nId: 'CallNodeContextMenu--transform-drop-function', + content: () => 'Drop samples with this function', + }, +]; + +type StateProps = { + readonly thread: Thread | null; + readonly threadsKey: ThreadsKey | null; + readonly rightClickedFunctionIndex: IndexIntoFuncTable | null; + readonly callNodeTable: CallNodeTable | null; + readonly implementation: ImplementationFilter; + readonly displaySearchfox: boolean; +}; + +type DispatchProps = { + readonly addTransformToStack: typeof addTransformToStack; + readonly addCollapseResourceTransformToStack: typeof addCollapseResourceTransformToStack; + readonly setContextMenuVisibility: typeof setContextMenuVisibility; +}; + +type Props = ConnectedProps<{}, StateProps, DispatchProps>; + +class FunctionListContextMenuImpl extends PureComponent { + _hidingTimeout: NodeJS.Timeout | null = null; + + _onShow = () => { + if (this._hidingTimeout) { + clearTimeout(this._hidingTimeout); + } + this.props.setContextMenuVisibility(true); + }; + + _onHide = () => { + this._hidingTimeout = setTimeout(() => { + this._hidingTimeout = null; + this.props.setContextMenuVisibility(false); + }); + }; + + _getRightClickedInfo(): null | { + readonly thread: Thread; + readonly threadsKey: ThreadsKey; + readonly funcIndex: IndexIntoFuncTable; + readonly callNodeTable: CallNodeTable; + } { + const { thread, threadsKey, rightClickedFunctionIndex, callNodeTable } = + this.props; + if ( + thread !== null && + threadsKey !== null && + rightClickedFunctionIndex !== null && + callNodeTable !== null + ) { + return { + thread, + threadsKey, + funcIndex: rightClickedFunctionIndex, + callNodeTable, + }; + } + return null; + } + + _getFunctionName(): string { + const info = this._getRightClickedInfo(); + if (info === null) { + throw new Error( + "The context menu assumes there is a right-clicked function and there wasn't one." + ); + } + const { + thread: { stringTable, funcTable }, + funcIndex, + } = info; + const isJS = (funcTable.flags[funcIndex] & FuncFlag.IsJS) !== 0; + const functionCall = stringTable.getString(funcTable.name[funcIndex]); + return isJS ? functionCall : getFunctionName(functionCall); + } + + lookupFunctionOnSearchfox(): void { + window.open( + `https://searchfox.org/mozilla-central/search?q=${encodeURIComponent( + this._getFunctionName() + )}`, + '_blank' + ); + } + + copyFunctionName(): void { + copy(this._getFunctionName()); + } + + getNameForSelectedResource(): string | null { + const info = this._getRightClickedInfo(); + if (info === null) { + throw new Error( + "The context menu assumes there is a right-clicked function and there wasn't one." + ); + } + const { + thread: { funcTable, stringTable, resourceTable, sources }, + funcIndex, + } = info; + const funcFlags = funcTable.flags[funcIndex]; + if ((funcFlags & FuncFlag.IsJS) !== 0) { + if ((funcFlags & FuncFlag.HasSource) === 0) { + return null; + } + const sourceIndex = funcTable.source[funcIndex]; + return stringTable.getString(sources.filename[sourceIndex]); + } + if ((funcFlags & FuncFlag.HasResource) === 0) { + return null; + } + const resourceIndex = funcTable.resource[funcIndex]; + return stringTable.getString(resourceTable.name[resourceIndex]); + } + + addTransformToStack(type: TransformType): void { + const { + addTransformToStack, + addCollapseResourceTransformToStack, + implementation, + } = this.props; + const info = this._getRightClickedInfo(); + if (info === null) { + throw new Error( + "The context menu assumes there is a right-clicked function and there wasn't one." + ); + } + const { threadsKey, thread, funcIndex } = info; + + switch (type) { + case 'focus-function': + addTransformToStack(threadsKey, { + type: 'focus-function', + funcIndex, + }); + break; + case 'focus-self': + addTransformToStack(threadsKey, { + type: 'focus-self', + funcIndex, + implementation, + }); + break; + case 'merge-function': + addTransformToStack(threadsKey, { + type: 'merge-function', + funcIndex, + }); + break; + case 'drop-function': + addTransformToStack(threadsKey, { + type: 'drop-function', + funcIndex, + }); + break; + case 'collapse-resource': { + const resourceIndex = thread.funcTable.resource[funcIndex]; + addCollapseResourceTransformToStack( + threadsKey, + resourceIndex, + implementation + ); + break; + } + case 'collapse-direct-recursion': + addTransformToStack(threadsKey, { + type: 'collapse-direct-recursion', + funcIndex, + implementation, + }); + break; + case 'collapse-recursion': + addTransformToStack(threadsKey, { + type: 'collapse-recursion', + funcIndex, + }); + break; + case 'collapse-function-subtree': + addTransformToStack(threadsKey, { + type: 'collapse-function-subtree', + funcIndex, + }); + break; + case 'focus-subtree': + case 'merge-call-node': + case 'focus-category': + case 'filter-samples': + throw new Error( + `The transform "${type}" is not supported in the function list context menu.` + ); + default: + assertExhaustiveCheck(type); + } + } + + _handleClick = ( + _event: React.ChangeEvent, + data: { type: string } + ): void => { + const { type } = data; + + const transformType = convertToTransformType(type); + if (transformType) { + this.addTransformToStack(transformType); + return; + } + + switch (type) { + case 'searchfox': + this.lookupFunctionOnSearchfox(); + break; + case 'copy-function-name': + this.copyFunctionName(); + break; + default: + throw new Error(`Unknown type ${type}`); + } + }; + + renderTransformMenuItem(item: TransformMenuItem, ctx: MenuItemContext) { + return ( + + + +
+ {item.content(ctx)} +
+ + {item.shortcut} + + ); + } + + renderContextMenuContents() { + const { displaySearchfox } = this.props; + const info = this._getRightClickedInfo(); + + if (info === null) { + console.error( + "The context menu assumes there is a right-clicked function and there wasn't one." + ); + return
; + } + + const ctx: MenuItemContext = { + funcIndex: info.funcIndex, + callNodeTable: info.callNodeTable, + nameForResource: this.getNameForSelectedResource(), + }; + + const renderedItems = MENU_ITEMS.filter( + (item) => !item.visible || item.visible(ctx) + ).map((item) => this.renderTransformMenuItem(item, ctx)); + + return ( + <> + {renderedItems} + +
+ + {displaySearchfox ? ( + + + Look up the function name on Searchfox + + + ) : null} + + + Copy function name + + + + ); + } + + override render() { + if (this._getRightClickedInfo() === null) { + return null; + } + + return ( + + {this.renderContextMenuContents()} + + ); + } +} + +export const FunctionListContextMenu = explicitConnect< + {}, + StateProps, + DispatchProps +>({ + mapStateToProps: (state: State) => { + const rightClickedFunction = + getProfileViewOptions(state).rightClickedFunction; + + let thread = null; + let threadsKey = null; + let rightClickedFunctionIndex = null; + let callNodeTable = null; + + if (rightClickedFunction !== null) { + const selectors = getThreadSelectorsFromThreadsKey( + rightClickedFunction.threadsKey + ); + thread = selectors.getFilteredThread(state); + threadsKey = rightClickedFunction.threadsKey; + rightClickedFunctionIndex = rightClickedFunction.functionIndex; + // Use the non-inverted call node table for recursion detection. + callNodeTable = selectors.getCallNodeInfo(state).getCallNodeTable(); + } + + return { + thread, + threadsKey, + rightClickedFunctionIndex, + callNodeTable, + implementation: getImplementationFilter(state), + displaySearchfox: getShouldDisplaySearchfox(state), + }; + }, + mapDispatchToProps: { + addTransformToStack, + addCollapseResourceTransformToStack, + setContextMenuVisibility, + }, + component: FunctionListContextMenuImpl, +}); diff --git a/src/test/components/FunctionListContextMenu.test.tsx b/src/test/components/FunctionListContextMenu.test.tsx new file mode 100644 index 0000000000..6f85957870 --- /dev/null +++ b/src/test/components/FunctionListContextMenu.test.tsx @@ -0,0 +1,119 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { Provider } from 'react-redux'; +import copy from 'copy-to-clipboard'; + +import { render, screen } from 'firefox-profiler/test/fixtures/testing-library'; +import { FunctionListContextMenu } from '../../components/shared/FunctionListContextMenu'; +import { storeWithProfile } from '../fixtures/stores'; +import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; +import { fireFullClick } from '../fixtures/utils'; +import { + changeRightClickedFunctionIndex, + setContextMenuVisibility, +} from '../../actions/profile-view'; +import { selectedThreadSelectors } from '../../selectors/per-thread'; +import { ensureExists } from '../../utils/types'; + +describe('FunctionListContextMenu', function () { + // Create a profile that exercises all the conditional menu items: + // - B[lib:XUL] appears three times in a row (direct + indirect recursion) + // - B[lib:XUL] belongs to the XUL library (collapse-resource) + function createStore() { + const { + profile, + funcNamesDictPerThread: [{ B }], + } = getProfileFromTextSamples(` + A A A + B[lib:XUL] B[lib:XUL] B[lib:XUL] + B[lib:XUL] B[lib:XUL] B[lib:XUL] + B[lib:XUL] B[lib:XUL] B[lib:XUL] + C C H + D F I + E E + `); + const store = storeWithProfile(profile); + store.dispatch(changeRightClickedFunctionIndex(0, B)); + return store; + } + + function setup(store = createStore()) { + store.dispatch(setContextMenuVisibility(true)); + const renderResult = render( + + + + ); + return { ...renderResult, getState: store.getState }; + } + + describe('basic rendering', function () { + it('does not render when no function is right-clicked', () => { + const store = storeWithProfile(getProfileFromTextSamples('A').profile); + store.dispatch(setContextMenuVisibility(true)); + const { container } = render( + + + + ); + expect(container.querySelector('.react-contextmenu')).toBeNull(); + }); + + it('renders a full context menu when a function is right-clicked', () => { + const { container } = setup(); + expect( + ensureExists( + container.querySelector('.react-contextmenu'), + `Couldn't find the context menu root component .react-contextmenu` + ).children.length > 1 + ).toBeTruthy(); + expect(container.firstChild).toMatchSnapshot(); + }); + + it('does not include call-node-specific transforms', () => { + setup(); + expect(screen.queryByText(/Merge node only/)).not.toBeInTheDocument(); + expect( + screen.queryByText(/Focus on subtree only/) + ).not.toBeInTheDocument(); + expect(screen.queryByText(/Expand all/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Copy stack/)).not.toBeInTheDocument(); + }); + }); + + describe('clicking on transforms', function () { + const fixtures = [ + { matcher: /Merge function/, type: 'merge-function' }, + { matcher: /Focus on function/, type: 'focus-function' }, + { matcher: /Focus on self only/, type: 'focus-self' }, + { matcher: /Collapse function/, type: 'collapse-function-subtree' }, + { matcher: /XUL/, type: 'collapse-resource' }, + { matcher: /^Collapse recursion/, type: 'collapse-recursion' }, + { + matcher: /Collapse direct recursion/, + type: 'collapse-direct-recursion', + }, + { matcher: /Drop samples/, type: 'drop-function' }, + ]; + + fixtures.forEach(({ matcher, type }) => { + it(`adds a transform for "${type}"`, function () { + const { getState } = setup(); + fireFullClick(screen.getByText(matcher)); + expect( + selectedThreadSelectors.getTransformStack(getState())[0].type + ).toBe(type); + }); + }); + }); + + describe('clicking on utility items', function () { + it('can copy a function name', function () { + setup(); + fireFullClick(screen.getByText('Copy function name')); + expect(copy).toHaveBeenCalledWith('B'); + }); + }); +}); diff --git a/src/test/components/__snapshots__/FunctionListContextMenu.test.tsx.snap b/src/test/components/__snapshots__/FunctionListContextMenu.test.tsx.snap new file mode 100644 index 0000000000..ce0c6f9ce3 --- /dev/null +++ b/src/test/components/__snapshots__/FunctionListContextMenu.test.tsx.snap @@ -0,0 +1,216 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`FunctionListContextMenu basic rendering renders a full context menu when a function is right-clicked 1`] = ` +
+ +
+`; From 6a9026a6d46e0afc8a10249a26fbe58d069d10cf Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sun, 21 Jun 2026 10:53:02 -0400 Subject: [PATCH 3/6] Add transform shortcut keys to the function list. These are the same single-key shortcuts the call tree context menu offers (m, f, S, c, C, r, R, d), applied to the selected function. --- src/actions/profile-view.ts | 91 ++++++--- src/components/calltree/FunctionList.tsx | 24 ++- .../components/TransformShortcuts.test.tsx | 176 ++++++++++++++++++ 3 files changed, 260 insertions(+), 31 deletions(-) diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts index 074c76ab73..d185f425be 100644 --- a/src/actions/profile-view.ts +++ b/src/actions/profile-view.ts @@ -2081,49 +2081,89 @@ export function handleCallNodeTransformShortcut( if (event.metaKey || event.ctrlKey || event.altKey) { return; } - const threadSelectors = getThreadSelectorsFromThreadsKey(threadsKey); - const unfilteredThread = threadSelectors.getThread(getState()); - const implementation = getImplementationFilter(getState()); - const inverted = getInvertCallstack(getState()); - const callNodePath = callNodeInfo.getCallNodePathFromIndex(callNodeIndex); const funcIndex = callNodeInfo.funcForNode(callNodeIndex); - const category = callNodeInfo.categoryForNode(callNodeIndex); - - const callNodeTable = callNodeInfo.getCallNodeTable(); switch (event.key) { - case 'F': + case 'F': { + const callNodePath = + callNodeInfo.getCallNodePathFromIndex(callNodeIndex); + const implementation = getImplementationFilter(getState()); + const inverted = getInvertCallstack(getState()); dispatch( addTransformToStack(threadsKey, { type: 'focus-subtree', - callNodePath: callNodePath, + callNodePath, implementation, inverted, }) ); break; - case 'f': + } + case 'M': { + const callNodePath = + callNodeInfo.getCallNodePathFromIndex(callNodeIndex); + const implementation = getImplementationFilter(getState()); dispatch( addTransformToStack(threadsKey, { - type: 'focus-function', - funcIndex, + type: 'merge-call-node', + callNodePath, + implementation, }) ); break; - case 'S': + } + case 'g': { + const category = callNodeInfo.categoryForNode(callNodeIndex); dispatch( addTransformToStack(threadsKey, { - type: 'focus-self', + type: 'focus-category', + category, + }) + ); + break; + } + default: + dispatch( + handleFunctionTransformShortcut( + event, + threadsKey, + callNodeInfo, + funcIndex + ) + ); + } + }; +} + +export function handleFunctionTransformShortcut( + event: React.KeyboardEvent, + threadsKey: ThreadsKey, + callNodeInfo: CallNodeInfo, + funcIndex: IndexIntoFuncTable +): ThunkAction { + return (dispatch, getState) => { + if (event.metaKey || event.ctrlKey || event.altKey) { + return; + } + const threadSelectors = getThreadSelectorsFromThreadsKey(threadsKey); + const implementation = getImplementationFilter(getState()); + const callNodeTable = callNodeInfo.getCallNodeTable(); + const unfilteredThread = threadSelectors.getThread(getState()); + + switch (event.key) { + case 'f': + dispatch( + addTransformToStack(threadsKey, { + type: 'focus-function', funcIndex, - implementation, }) ); break; - case 'M': + case 'S': dispatch( addTransformToStack(threadsKey, { - type: 'merge-call-node', - callNodePath: callNodePath, + type: 'focus-self', + funcIndex, implementation, }) ); @@ -2182,7 +2222,7 @@ export function handleCallNodeTransformShortcut( } break; } - case 'c': { + case 'c': dispatch( addTransformToStack(threadsKey, { type: 'collapse-function-subtree', @@ -2190,17 +2230,8 @@ export function handleCallNodeTransformShortcut( }) ); break; - } - case 'g': - dispatch( - addTransformToStack(threadsKey, { - type: 'focus-category', - category, - }) - ); - break; default: - // This did not match a call tree transform. + // This did not match a function transform. } }; } diff --git a/src/components/calltree/FunctionList.tsx b/src/components/calltree/FunctionList.tsx index 6c4c505a65..09d8b2e371 100644 --- a/src/components/calltree/FunctionList.tsx +++ b/src/components/calltree/FunctionList.tsx @@ -30,6 +30,7 @@ import { changeTableViewOptions, updateBottomBoxContentsAndMaybeOpen, changeFunctionListSort, + handleFunctionTransformShortcut, } from 'firefox-profiler/actions/profile-view'; import { nameColumn, @@ -47,6 +48,7 @@ import type { SelectionContext, } from 'firefox-profiler/types'; import type { CallTree } from 'firefox-profiler/profile-logic/call-tree'; +import type { CallNodeInfo } from 'firefox-profiler/profile-logic/call-node-info'; import type { SingleColumnSortState } from 'firefox-profiler/components/shared/TreeView'; import type { ConnectedProps } from 'firefox-profiler/utils/connect'; @@ -62,6 +64,7 @@ const DEFAULT_FUNCTION_LIST_SORT_STATE = new ColumnSortState( type StateProps = { readonly threadsKey: ThreadsKey; + readonly callNodeInfo: CallNodeInfo; readonly scrollToSelectionGeneration: number; readonly focusCallTreeGeneration: number; readonly tree: CallTree; @@ -79,6 +82,7 @@ type DispatchProps = { readonly changeRightClickedFunctionIndex: typeof changeRightClickedFunctionIndex; readonly addTransformToStack: typeof addTransformToStack; readonly updateBottomBoxContentsAndMaybeOpen: typeof updateBottomBoxContentsAndMaybeOpen; + readonly handleFunctionTransformShortcut: typeof handleFunctionTransformShortcut; readonly onTableViewOptionsChange: (opts: TableViewOptions) => any; readonly changeFunctionListSort: typeof changeFunctionListSort; }; @@ -167,7 +171,23 @@ class FunctionListImpl extends PureComponent { _newExpandedCallNodeIndexes: Array ) => {}; - _onKeyDown = (_event: React.KeyboardEvent) => {}; + _onKeyDown = (event: React.KeyboardEvent) => { + const { + selectedFunctionIndex, + rightClickedFunctionIndex, + threadsKey, + callNodeInfo, + handleFunctionTransformShortcut, + } = this.props; + const funcIndex = + rightClickedFunctionIndex !== null + ? rightClickedFunctionIndex + : selectedFunctionIndex; + if (funcIndex === null) { + return; + } + handleFunctionTransformShortcut(event, threadsKey, callNodeInfo, funcIndex); + }; _onEnterOrDoubleClick = (nodeId: IndexIntoFuncTable) => { const { tree, updateBottomBoxContentsAndMaybeOpen } = this.props; @@ -224,6 +244,7 @@ class FunctionListImpl extends PureComponent { export const FunctionList = explicitConnect<{}, StateProps, DispatchProps>({ mapStateToProps: (state: State) => ({ threadsKey: getSelectedThreadsKey(state), + callNodeInfo: selectedThreadSelectors.getCallNodeInfo(state), scrollToSelectionGeneration: getScrollToSelectionGeneration(state), focusCallTreeGeneration: getFocusCallTreeGeneration(state), tree: selectedThreadSelectors.getFunctionListTree(state), @@ -242,6 +263,7 @@ export const FunctionList = explicitConnect<{}, StateProps, DispatchProps>({ changeRightClickedFunctionIndex, addTransformToStack, updateBottomBoxContentsAndMaybeOpen, + handleFunctionTransformShortcut, onTableViewOptionsChange: (options: TableViewOptions) => changeTableViewOptions('calltree', options), changeFunctionListSort, diff --git a/src/test/components/TransformShortcuts.test.tsx b/src/test/components/TransformShortcuts.test.tsx index aefbcfbd4a..dd17a20645 100644 --- a/src/test/components/TransformShortcuts.test.tsx +++ b/src/test/components/TransformShortcuts.test.tsx @@ -11,6 +11,8 @@ import { storeWithProfile } from '../fixtures/stores'; import { changeSelectedCallNode, changeRightClickedCallNode, + changeSelectedFunctionIndex, + changeRightClickedFunctionIndex, } from '../../actions/profile-view'; import { FlameGraph } from '../../components/flame-graph'; import { selectedThreadSelectors } from 'firefox-profiler/selectors'; @@ -18,6 +20,7 @@ import { ensureExists, objectEntries } from '../../utils/types'; import { fireFullKeyPress } from '../fixtures/utils'; import { autoMockCanvasContext } from '../fixtures/mocks/canvas-context'; import { ProfileCallTreeView } from '../../components/calltree/ProfileCallTreeView'; +import { ProfileFunctionListView } from '../../components/calltree/ProfileFunctionListView'; import { StackChart } from 'firefox-profiler/components/stack-chart'; import type { Transform, @@ -186,6 +189,104 @@ const pressKeyBuilder = (className: string) => (options: KeyPressOptions) => { fireFullKeyPress(div, options); }; +function testFunctionTransformKeyboardShortcuts( + setup: () => { + getTransform: () => null | Transform; + pressKey: (options: KeyPressOptions) => void; + expectedFuncIndex: IndexIntoFuncTable; + expectedResourceIndex: IndexIntoResourceTable; + } +) { + describe('function shortcuts', () => { + it('handles focus-function', () => { + const { pressKey, getTransform, expectedFuncIndex } = setup(); + pressKey({ key: 'f' }); + expect(getTransform()).toEqual({ + type: 'focus-function', + funcIndex: expectedFuncIndex, + }); + }); + + it('handles focus-self', () => { + const { pressKey, getTransform, expectedFuncIndex } = setup(); + pressKey({ key: 'S' }); + expect(getTransform()).toMatchObject({ + type: 'focus-self', + funcIndex: expectedFuncIndex, + }); + }); + + it('handles merge function', () => { + const { pressKey, getTransform, expectedFuncIndex } = setup(); + pressKey({ key: 'm' }); + expect(getTransform()).toEqual({ + type: 'merge-function', + funcIndex: expectedFuncIndex, + }); + }); + + it('handles drop function', () => { + const { pressKey, getTransform, expectedFuncIndex } = setup(); + pressKey({ key: 'd' }); + expect(getTransform()).toEqual({ + type: 'drop-function', + funcIndex: expectedFuncIndex, + }); + }); + + it('handles collapse resource', () => { + const { pressKey, getTransform, expectedResourceIndex } = setup(); + pressKey({ key: 'C' }); + expect(getTransform()).toMatchObject({ + type: 'collapse-resource', + resourceIndex: expectedResourceIndex, + }); + }); + + it('handles collapse recursion', () => { + const { pressKey, getTransform, expectedFuncIndex } = setup(); + pressKey({ key: 'r' }); + expect(getTransform()).toMatchObject({ + type: 'collapse-recursion', + funcIndex: expectedFuncIndex, + }); + }); + + it('handles collapse direct recursion', () => { + const { pressKey, getTransform, expectedFuncIndex } = setup(); + pressKey({ key: 'R' }); + expect(getTransform()).toMatchObject({ + type: 'collapse-direct-recursion', + funcIndex: expectedFuncIndex, + }); + }); + + it('handles collapse function subtree', () => { + const { pressKey, getTransform, expectedFuncIndex } = setup(); + pressKey({ key: 'c' }); + expect(getTransform()).toEqual({ + type: 'collapse-function-subtree', + funcIndex: expectedFuncIndex, + }); + }); + + it('does not handle call-node-specific shortcuts', () => { + const { pressKey, getTransform } = setup(); + pressKey({ key: 'F' }); // focus-subtree + pressKey({ key: 'M' }); // merge-call-node + pressKey({ key: 'g' }); // focus-category + expect(getTransform()).toBeNull(); + }); + + it('ignores shortcuts with modifiers', () => { + const { pressKey, getTransform } = setup(); + pressKey({ key: 'c', ctrlKey: true }); + pressKey({ key: 'c', metaKey: true }); + expect(getTransform()).toBeNull(); + }); + }); // end describe('function shortcuts') +} + /* eslint-disable jest/no-standalone-expect */ // Disable the jest/no-standalone-expect rule because eslint doesn't know that // these expectations will run in a test block later. @@ -326,3 +427,78 @@ describe('stack chart transform shortcuts', () => { }); } }); + +/* eslint-disable jest/no-standalone-expect */ +const functionListActions = { + 'a selected function': ( + { dispatch, getState }: Store, + { B }: FuncNamesDict + ) => { + act(() => { + dispatch(changeSelectedFunctionIndex(0, B)); + }); + expect( + selectedThreadSelectors.getSelectedFunctionIndex(getState()) + ).not.toBeNull(); + expect( + selectedThreadSelectors.getRightClickedFunctionIndex(getState()) + ).toBeNull(); + }, + 'a right-clicked function': ( + { dispatch, getState }: Store, + { B }: FuncNamesDict + ) => { + act(() => { + dispatch(changeSelectedFunctionIndex(0, null)); + }); + act(() => { + dispatch(changeRightClickedFunctionIndex(0, B)); + }); + expect( + selectedThreadSelectors.getSelectedFunctionIndex(getState()) + ).toBeNull(); + expect( + selectedThreadSelectors.getRightClickedFunctionIndex(getState()) + ).not.toBeNull(); + }, + 'both a selected and a right-clicked function': ( + { dispatch, getState }: Store, + { A, B }: FuncNamesDict + ) => { + act(() => { + dispatch(changeSelectedFunctionIndex(0, A)); + }); + act(() => { + dispatch(changeRightClickedFunctionIndex(0, B)); + }); + expect( + selectedThreadSelectors.getSelectedFunctionIndex(getState()) + ).not.toBeNull(); + expect( + selectedThreadSelectors.getRightClickedFunctionIndex(getState()) + ).not.toBeNull(); + }, +}; +/* eslint-enable jest/no-standalone-expect */ + +describe('function list transform shortcuts', () => { + for (const [name, action] of objectEntries(functionListActions)) { + describe(`with ${name}`, () => { + testFunctionTransformKeyboardShortcuts(() => { + const { store, funcNames, getTransform } = setupStore( + + ); + + const { B } = funcNames; + action(store, funcNames); + + return { + getTransform, + pressKey: pressKeyBuilder('treeViewBody'), + expectedFuncIndex: B, + expectedResourceIndex: 0, + }; + }); + }); + } +}); From a2c04ea054642d7245f80b03b30c13bda24c6f03 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 3 Sep 2026 15:39:47 -0400 Subject: [PATCH 4/6] Highlight the selected function's samples in the activity graph. When the function list tab is active, the activity graph should highlight every sample whose stack contains the selected function, rather than the samples belonging to a selected call node. --- src/components/timeline/TrackThread.tsx | 5 +- src/profile-logic/profile-data.ts | 53 +++++++ src/selectors/per-thread/stack-sample.ts | 14 ++ src/test/unit/profile-data.test.ts | 176 +++++++++++++++++++++++ 4 files changed, 247 insertions(+), 1 deletion(-) diff --git a/src/components/timeline/TrackThread.tsx b/src/components/timeline/TrackThread.tsx index 39909bd0e5..50562db42d 100644 --- a/src/components/timeline/TrackThread.tsx +++ b/src/components/timeline/TrackThread.tsx @@ -25,6 +25,7 @@ import { getImplementationFilter, getZeroAt, getProfileTimelineUnit, + getSelectedTab, } from 'firefox-profiler/selectors'; import { TimelineMarkersJank, @@ -348,7 +349,9 @@ export const TimelineTrackThread = explicitConnect< hasFileIoMarkers: selectors.getTimelineFileIoMarkerIndexes(state).length !== 0, sampleRelations: - selectors.getSampleRelationsToSelectedNodeInFilteredThread(state), + getSelectedTab(state) === 'function-list' + ? selectors.getSampleRelationsForFunctionListTab(state) + : selectors.getSampleRelationsToSelectedNodeInFilteredThread(state), treeOrderSampleComparator: selectors.getTreeOrderComparatorInFilteredThread(state), selectedThreadIndexes, diff --git a/src/profile-logic/profile-data.ts b/src/profile-logic/profile-data.ts index 01ebf66d3c..32994fb031 100644 --- a/src/profile-logic/profile-data.ts +++ b/src/profile-logic/profile-data.ts @@ -1280,6 +1280,59 @@ export function getSampleRelationsToNode( ); } +/** + * Go through the samples, and determine how each of them relates to the given + * function. A sample counts towards the function's total if the function is + * anywhere on its stack, and towards its self time if the stack ends in it. + * Unrelated samples are all marked as Before, since the function list has no + * tree order to follow. + */ +export function getSampleRelationsToFunction( + sampleCallNodes: Array, + selectedFunctionIndex: IndexIntoFuncTable | null, + callNodeTable: CallNodeTable +): SampleRelations { + if (selectedFunctionIndex === null) { + return _getSampleRelationsForNoNode(sampleCallNodes); + } + + const sampleCount = sampleCallNodes.length; + + // Go through each call node, and label it as containing the function or not. + // callNodeContainsFunc is a callNodeIndex => bool map, implemented as a U8 typed + // array for better performance. 0 means false, 1 means true. + const callNodeCount = callNodeTable.length; + const callNodeContainsFunc = new Uint8Array(callNodeCount); + for (let callNodeIndex = 0; callNodeIndex < callNodeCount; callNodeIndex++) { + const prefix = callNodeTable.prefix[callNodeIndex]; + const funcIndex = callNodeTable.func[callNodeIndex]; + if ( + funcIndex === selectedFunctionIndex || + // The parent of this stack contained the function. + (prefix !== -1 && callNodeContainsFunc[prefix] === 1) + ) { + callNodeContainsFunc[callNodeIndex] = 1; + } + } + + const relations = new Uint8Array(sampleCount); + for (let sampleIndex = 0; sampleIndex < sampleCount; sampleIndex++) { + let sampleRelation: SampleRelationToNode = SampleRelationToNode.FilteredOut; + const callNodeIndex = sampleCallNodes[sampleIndex]; + if (callNodeIndex !== null) { + if (callNodeTable.func[callNodeIndex] === selectedFunctionIndex) { + sampleRelation = SampleRelationToNode.TotalAndSelf; + } else if (callNodeContainsFunc[callNodeIndex] === 1) { + sampleRelation = SampleRelationToNode.TotalButNotSelf; + } else { + sampleRelation = SampleRelationToNode.Before; + } + } + relations[sampleIndex] = sampleRelation; + } + return new SampleRelations(relations); +} + /** * This function returns the function index for a specific call node path. This * is the last element of this path, or the leaf element of the path. diff --git a/src/selectors/per-thread/stack-sample.ts b/src/selectors/per-thread/stack-sample.ts index 7b45a97beb..47a8fde025 100644 --- a/src/selectors/per-thread/stack-sample.ts +++ b/src/selectors/per-thread/stack-sample.ts @@ -352,6 +352,19 @@ export function getStackAndSampleSelectorsPerThread( } ); + const getSampleRelationsForFunctionListTab: Selector = + createSelector( + getSampleCallNodesForFilteredThread, + _getCallNodeTable, + getSelectedFunctionIndex, + (sampleCallNodes, callNodeTable, selectedFunctionIndex) => + ProfileData.getSampleRelationsToFunction( + sampleCallNodes, + selectedFunctionIndex, + callNodeTable + ) + ); + const getTreeOrderComparatorInFilteredThread: Selector< ( sampleIndexA: IndexIntoSamplesTable, @@ -685,6 +698,7 @@ export function getStackAndSampleSelectorsPerThread( getExpandedCallNodeIndexes, getSampleCallNodesForFilteredThread, getSampleRelationsToSelectedNodeInFilteredThread, + getSampleRelationsForFunctionListTab, getPreviewFilteredCtssSampleCallNodes, getPreviewFilteredCtssSampleRelations, getSelectedCallNodeIsInvertedRoot, diff --git a/src/test/unit/profile-data.test.ts b/src/test/unit/profile-data.test.ts index d8256f3af6..de6baccb6a 100644 --- a/src/test/unit/profile-data.test.ts +++ b/src/test/unit/profile-data.test.ts @@ -19,6 +19,7 @@ import { getSampleIndexToCallNodeIndex, getTreeOrderComparator, getSampleRelationsToNode, + getSampleRelationsToFunction, SampleRelations, extractProfileFilterPageData, findAddressProofForFile, @@ -1319,6 +1320,181 @@ describe('getSampleRelationsToNode', function () { }); }); +describe('getSampleRelationsToFunction', function () { + function setup(textSamples: string) { + const { + derivedThreads, + funcNamesDictPerThread: [funcNamesDict], + } = getProfileFromTextSamples(textSamples); + const [thread] = derivedThreads; + const callNodeInfo = getCallNodeInfo( + thread.stackTable, + thread.frameTable, + 0 + ); + const sampleCallNodes = getSampleIndexToCallNodeIndex( + thread.samples.stack, + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex() + ); + return { + callNodeTable: callNodeInfo.getCallNodeTable(), + sampleCallNodes, + funcNamesDict, + }; + } + + it('marks all non-filtered samples as contributing when nothing is selected', function () { + const { callNodeTable, sampleCallNodes } = setup(` + A A A + B C + `); + const relations = getSampleRelationsToFunction( + sampleCallNodes, + null, + callNodeTable + ); + expect([0, 1, 2].map((i) => relations.contributesToTotal(i))).toEqual([ + true, + true, + true, + ]); + }); + + it('marks samples whose stack contains the function as total, and those ending in it as self', function () { + // 0 1 2 3 4 + // A A A A A + // B D B D D + // C E F G + const { + callNodeTable, + sampleCallNodes, + funcNamesDict: { B, D }, + } = setup(` + A A A A A + B D B D D + C E F G + `); + + // Selecting function B: samples 0, 2 have B in their stack. + expect( + getSampleRelationsToFunction( + sampleCallNodes, + B, + callNodeTable + ).toArrayForTesting() + ).toEqual([ + SampleRelationToNode.TotalButNotSelf, + SampleRelationToNode.Before, + SampleRelationToNode.TotalButNotSelf, + SampleRelationToNode.Before, + SampleRelationToNode.Before, + ]); + + // Selecting function D: samples 1, 3, 4 have D in their stack, and + // sample 4 ends in D. + expect( + getSampleRelationsToFunction( + sampleCallNodes, + D, + callNodeTable + ).toArrayForTesting() + ).toEqual([ + SampleRelationToNode.Before, + SampleRelationToNode.TotalButNotSelf, + SampleRelationToNode.Before, + SampleRelationToNode.TotalButNotSelf, + SampleRelationToNode.TotalAndSelf, + ]); + }); + + it('marks filtered-out samples as FilteredOut', function () { + const { + callNodeTable, + sampleCallNodes, + funcNamesDict: { B }, + } = setup(` + A A A + B C + `); + // Manually null out sample 2's call node, as if it was filtered out. + sampleCallNodes[2] = null; + + expect( + getSampleRelationsToFunction( + sampleCallNodes, + B, + callNodeTable + ).toArrayForTesting() + ).toEqual([ + SampleRelationToNode.TotalAndSelf, + SampleRelationToNode.Before, + SampleRelationToNode.FilteredOut, + ]); + }); + + it('matches samples whose ancestor call node contains the function, not just the leaf', function () { + // 0 1 + // A A + // B C + // D + const { + callNodeTable, + sampleCallNodes, + funcNamesDict: { A, B }, + } = setup(` + A A + B C + D + `); + + // A is an ancestor of everything. + expect( + getSampleRelationsToFunction( + sampleCallNodes, + A, + callNodeTable + ).toArrayForTesting() + ).toEqual([ + SampleRelationToNode.TotalButNotSelf, + SampleRelationToNode.TotalButNotSelf, + ]); + + // B is only in the path A->B->D. + expect( + getSampleRelationsToFunction( + sampleCallNodes, + B, + callNodeTable + ).toArrayForTesting() + ).toEqual([ + SampleRelationToNode.TotalButNotSelf, + SampleRelationToNode.Before, + ]); + }); + + it('counts recursive samples as self when the leaf is the function', function () { + const { + callNodeTable, + sampleCallNodes, + funcNamesDict: { A }, + } = setup(` + A A + B B + A + `); + expect( + getSampleRelationsToFunction( + sampleCallNodes, + A, + callNodeTable + ).toArrayForTesting() + ).toEqual([ + SampleRelationToNode.TotalAndSelf, + SampleRelationToNode.TotalButNotSelf, + ]); + }); +}); + describe('extractProfileFilterPageData', function () { const pages = { mozilla: { From 3a477cbb3184cebad07574704adfde0c2da87488 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sun, 21 Jun 2026 10:53:03 -0400 Subject: [PATCH 5/6] Select the self function when clicking in the activity graph. Clicking a sample already selects the corresponding self call node. When the function list tab is active, also select that call node's function so the click has an effect there too. --- src/actions/profile-view.ts | 32 +++++++++++++++++++++++++ src/components/timeline/TrackThread.tsx | 5 ++++ 2 files changed, 37 insertions(+) diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts index d185f425be..6cf7597242 100644 --- a/src/actions/profile-view.ts +++ b/src/actions/profile-view.ts @@ -247,6 +247,38 @@ export function selectSelfCallNode( }; } +/** + * Like selectSelfCallNode, but selects the function of the self call node + * instead. Used when the function list tab is active. + */ +export function selectSelfFunction( + threadsKey: ThreadsKey, + sampleIndex: IndexIntoSamplesTable | null +): ThunkAction { + return (dispatch, getState) => { + if (sampleIndex === null || sampleIndex < 0) { + dispatch(changeSelectedFunctionIndex(threadsKey, null)); + return; + } + const threadSelectors = getThreadSelectorsFromThreadsKey(threadsKey); + const sampleCallNodes = + threadSelectors.getSampleCallNodesForFilteredThread(getState()); + if (sampleIndex >= sampleCallNodes.length) { + dispatch(changeSelectedFunctionIndex(threadsKey, null)); + return; + } + const nonInvertedSelfCallNode = sampleCallNodes[sampleIndex]; + if (nonInvertedSelfCallNode === null) { + dispatch(changeSelectedFunctionIndex(threadsKey, null)); + return; + } + const callNodeInfo = threadSelectors.getCallNodeInfo(getState()); + const funcIndex = + callNodeInfo.getCallNodeTable().func[nonInvertedSelfCallNode]; + dispatch(changeSelectedFunctionIndex(threadsKey, funcIndex)); + }; +} + /** * This selects a set of thread from thread indexes. * Please use it in tests only. diff --git a/src/components/timeline/TrackThread.tsx b/src/components/timeline/TrackThread.tsx index 50562db42d..2e0eb8142d 100644 --- a/src/components/timeline/TrackThread.tsx +++ b/src/components/timeline/TrackThread.tsx @@ -38,6 +38,7 @@ import { changeSelectedCallNode, focusCallTree, selectSelfCallNode, + selectSelfFunction, } from 'firefox-profiler/actions/profile-view'; import { reportTrackThreadHeight } from 'firefox-profiler/actions/app'; import { EmptyThreadIndicator } from './EmptyThreadIndicator'; @@ -101,6 +102,7 @@ type DispatchProps = { readonly changeSelectedCallNode: typeof changeSelectedCallNode; readonly focusCallTree: typeof focusCallTree; readonly selectSelfCallNode: typeof selectSelfCallNode; + readonly selectSelfFunction: typeof selectSelfFunction; readonly reportTrackThreadHeight: typeof reportTrackThreadHeight; }; @@ -124,6 +126,7 @@ class TimelineTrackThreadImpl extends PureComponent { const { threadsKey, selectSelfCallNode, + selectSelfFunction, focusCallTree, selectedThreadIndexes, callTreeVisible, @@ -131,6 +134,7 @@ class TimelineTrackThreadImpl extends PureComponent { // Sample clicking only works for one thread. See issue #2709 if (selectedThreadIndexes.size === 1) { + selectSelfFunction(threadsKey, sampleIndex); selectSelfCallNode(threadsKey, sampleIndex); if (sampleIndex !== null && callTreeVisible) { @@ -367,6 +371,7 @@ export const TimelineTrackThread = explicitConnect< changeSelectedCallNode, focusCallTree, selectSelfCallNode, + selectSelfFunction, reportTrackThreadHeight, }, component: withSize(TimelineTrackThreadImpl), From 1ccc6ef13cd19d400b18a100d382845565b983e2 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Tue, 22 Sep 2026 16:08:42 -0400 Subject: [PATCH 6/6] Implement sidebar timings for the function list. --- locales/en-US/app.ftl | 5 +- src/components/sidebar/CallTreeSidebar.tsx | 41 ++++- src/components/sidebar/index.tsx | 4 +- src/selectors/per-thread/index.ts | 75 ++++++++ src/selectors/per-thread/stack-sample.ts | 14 ++ src/test/components/CallTreeSidebar.test.tsx | 113 ++++++++++++- .../CallTreeSidebar.test.tsx.snap | 160 ++++++++++++++++++ 7 files changed, 403 insertions(+), 9 deletions(-) diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index a2478d7ec7..ecc8089fbc 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -227,10 +227,13 @@ CallTree--inlining-badge = (inlined) .title = Calls to { $calledFunction } were inlined into { $outerFunction } by the compiler. ## CallTreeSidebar -## This is the sidebar component that is used in Call Tree and Flame Graph panels. +## This is the sidebar component that is used in Call Tree, Function List and +## Flame Graph panels. CallTreeSidebar--select-a-node = Select a node to display information about it. CallTreeSidebar--call-node-details = Call node details +CallTreeSidebar--select-a-function = Select a function to display information about it. +CallTreeSidebar--function-details = Function details ## CallTreeSidebar timing information ## diff --git a/src/components/sidebar/CallTreeSidebar.tsx b/src/components/sidebar/CallTreeSidebar.tsx index fe03d9dc5a..d4e42670d7 100644 --- a/src/components/sidebar/CallTreeSidebar.tsx +++ b/src/components/sidebar/CallTreeSidebar.tsx @@ -11,6 +11,7 @@ import { assertExhaustiveCheck } from 'firefox-profiler/utils/types'; import { selectedThreadSelectors, selectedNodeSelectors, + selectedFunctionSelectors, } from 'firefox-profiler/selectors/per-thread'; import { getSelectedThreadsKey } from 'firefox-profiler/selectors/url-state'; import { toggleOpenCategoryInSidebar } from 'firefox-profiler/actions/app'; @@ -193,6 +194,9 @@ export const CategoryBreakdown = explicitConnect< }); type StateProps = { + // Whether the sidebar describes a call node (call tree, flame graph) or a + // function (function list). + readonly selectionKind: 'call-node' | 'function'; readonly selectedNodeIndex: IndexIntoCallNodeTable | null; readonly selectedThreadsKey: ThreadsKey; readonly name: string; @@ -268,6 +272,7 @@ class CallTreeSidebarImpl extends React.PureComponent { override render() { const { + selectionKind, selectedNodeIndex, name, lib, @@ -284,7 +289,13 @@ class CallTreeSidebarImpl extends React.PureComponent { if (selectedNodeIndex === null) { return (
- +
Select a node to display some information about it.
@@ -323,9 +334,15 @@ class CallTreeSidebarImpl extends React.PureComponent { ) : null}

- -
Call node details
- + {selectionKind === 'function' ? ( + +
Function details
+ + ) : ( + +
Call node details
+ + )}

{selectedNodeTracedSelfAndTotal ? ( { export const CallTreeSidebar = explicitConnect<{}, StateProps, {}>({ mapStateToProps: (state) => ({ + selectionKind: 'call-node', selectedNodeIndex: selectedThreadSelectors.getSelectedCallNodeIndex(state), selectedThreadsKey: getSelectedThreadsKey(state), name: getFunctionName(selectedNodeSelectors.getName(state)), @@ -433,3 +451,18 @@ export const CallTreeSidebar = explicitConnect<{}, StateProps, {}>({ }), component: CallTreeSidebarImpl, }); + +export const FunctionListSidebar = explicitConnect<{}, StateProps, {}>({ + mapStateToProps: (state) => ({ + selectionKind: 'function', + selectedNodeIndex: selectedThreadSelectors.getSelectedFunctionIndex(state), + selectedThreadsKey: getSelectedThreadsKey(state), + name: getFunctionName(selectedFunctionSelectors.getName(state)), + lib: selectedFunctionSelectors.getLib(state), + timings: selectedFunctionSelectors.getTimingsForSidebar(state), + categoryList: getCategories(state), + weightType: selectedThreadSelectors.getWeightTypeForCallTree(state), + selectedNodeTracedSelfAndTotal: null, + }), + component: CallTreeSidebarImpl, +}); diff --git a/src/components/sidebar/index.tsx b/src/components/sidebar/index.tsx index e4a6901069..6b601d72fa 100644 --- a/src/components/sidebar/index.tsx +++ b/src/components/sidebar/index.tsx @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import type * as React from 'react'; -import { CallTreeSidebar } from './CallTreeSidebar'; +import { CallTreeSidebar, FunctionListSidebar } from './CallTreeSidebar'; import { MarkerSidebar } from './MarkerSidebar'; import type { TabSlug } from 'firefox-profiler/app-logic/tabs-handling'; @@ -15,7 +15,7 @@ export function selectSidebar( ): React.ComponentType<{}> | null { return { calltree: CallTreeSidebar, - 'function-list': CallTreeSidebar, + 'function-list': FunctionListSidebar, 'flame-graph': CallTreeSidebar, 'stack-chart': null, 'marker-chart': null, diff --git a/src/selectors/per-thread/index.ts b/src/selectors/per-thread/index.ts index dc0d4aa863..2ba294fcc6 100644 --- a/src/selectors/per-thread/index.ts +++ b/src/selectors/per-thread/index.ts @@ -270,3 +270,78 @@ export const selectedNodeSelectors: NodeSelectors = (() => { getTimingsForSidebar, }; })(); + +export type FunctionSelectors = { + readonly getName: Selector; + readonly getLib: Selector; + readonly getTimingsForSidebar: Selector; +}; + +/** + * The equivalent of selectedNodeSelectors for the function selected in the + * function list. + */ +export const selectedFunctionSelectors: FunctionSelectors = (() => { + const getName: Selector = createSelector( + selectedThreadSelectors.getSelectedFunctionIndex, + selectedThreadSelectors.getFilteredThread, + (funcIndex, { stringTable, funcTable }) => { + if (funcIndex === null) { + return ''; + } + return stringTable.getString(funcTable.name[funcIndex]); + } + ); + + const getLib: Selector = createSelector( + selectedThreadSelectors.getSelectedFunctionIndex, + selectedThreadSelectors.getFilteredThread, + ProfileSelectors.getSourceTable, + ( + funcIndex, + { + stringTable, + frameTable, + funcTable, + resourceTable, + sourceLocationTable, + }, + sources + ) => { + if (funcIndex === null) { + return ''; + } + return ProfileData.getOriginAnnotationForFunc( + funcIndex, + null, + frameTable, + funcTable, + resourceTable, + stringTable, + sources, + sourceLocationTable + ); + } + ); + + const getTimingsForSidebar: Selector = createSelector( + ProfileSelectors.getCategories, + selectedThreadSelectors.getPreviewFilteredCtssSamples, + selectedThreadSelectors.getPreviewFilteredCtssSampleCategoriesAndSubcategories, + selectedThreadSelectors.getPreviewFilteredCtssSampleRelationsToSelectedFunction, + (categories, samples, sampleCategoriesAndSubcategories, sampleRelations) => + ProfileData.getCallNodeTimings( + categories, + samples, + sampleCategoriesAndSubcategories, + sampleRelations, + false + ) + ); + + return { + getName, + getLib, + getTimingsForSidebar, + }; +})(); diff --git a/src/selectors/per-thread/stack-sample.ts b/src/selectors/per-thread/stack-sample.ts index 47a8fde025..ebbf68a882 100644 --- a/src/selectors/per-thread/stack-sample.ts +++ b/src/selectors/per-thread/stack-sample.ts @@ -365,6 +365,19 @@ export function getStackAndSampleSelectorsPerThread( ) ); + const getPreviewFilteredCtssSampleRelationsToSelectedFunction: Selector = + createSelector( + getPreviewFilteredCtssSampleCallNodes, + _getCallNodeTable, + getSelectedFunctionIndex, + (sampleCallNodes, callNodeTable, selectedFunctionIndex) => + ProfileData.getSampleRelationsToFunction( + sampleCallNodes, + selectedFunctionIndex, + callNodeTable + ) + ); + const getTreeOrderComparatorInFilteredThread: Selector< ( sampleIndexA: IndexIntoSamplesTable, @@ -701,6 +714,7 @@ export function getStackAndSampleSelectorsPerThread( getSampleRelationsForFunctionListTab, getPreviewFilteredCtssSampleCallNodes, getPreviewFilteredCtssSampleRelations, + getPreviewFilteredCtssSampleRelationsToSelectedFunction, getSelectedCallNodeIsInvertedRoot, getTreeOrderComparatorInFilteredThread, getCallTree, diff --git a/src/test/components/CallTreeSidebar.test.tsx b/src/test/components/CallTreeSidebar.test.tsx index 49cbc7190c..232e2a2dae 100644 --- a/src/test/components/CallTreeSidebar.test.tsx +++ b/src/test/components/CallTreeSidebar.test.tsx @@ -4,9 +4,13 @@ import { Provider } from 'react-redux'; import { render, act } from 'firefox-profiler/test/fixtures/testing-library'; -import { CallTreeSidebar } from '../../components/sidebar/CallTreeSidebar'; +import { + CallTreeSidebar, + FunctionListSidebar, +} from '../../components/sidebar/CallTreeSidebar'; import { changeSelectedCallNode, + changeSelectedFunctionIndex, changeInvertCallstack, } from '../../actions/profile-view'; @@ -14,7 +18,11 @@ import { storeWithProfile } from '../fixtures/stores'; import type { FuncNamesDict } from '../fixtures/profiles/processed-profile'; import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; -import type { CallNodePath, Profile } from 'firefox-profiler/types'; +import type { + CallNodePath, + IndexIntoFuncTable, + Profile, +} from 'firefox-profiler/types'; import { ensureExists } from '../../utils/types'; import { fireFullClick } from '../fixtures/utils'; @@ -204,3 +212,104 @@ describe('CallTreeSidebar', function () { expect(getAllByText('FakeSubCategoryC')[1]).toBeInTheDocument(); }); }); + +describe('FunctionListSidebar', function () { + function setup(textSamples: string) { + const { + profile, + funcNamesDictPerThread: [funcNamesDict], + } = getProfileFromTextSamples(textSamples); + const store = storeWithProfile(profile); + + const selectFunction = (funcIndex: IndexIntoFuncTable) => { + act(() => { + store.dispatch(changeSelectedFunctionIndex(0, funcIndex)); + }); + }; + + const renderResult = render( + + + + ); + + return { ...renderResult, funcNamesDict, selectFunction }; + } + + // Returns the [label, percentage, value] triples of the sidebar details. + function getDetails(container: HTMLElement): string[][] { + return Array.from(container.querySelectorAll('.sidebar-label')).map( + (label) => [ + label.textContent ?? '', + label.nextElementSibling?.textContent ?? '', + label.nextElementSibling?.nextElementSibling?.textContent ?? '', + ] + ); + } + + const textSamples = ` + A A A A + B B B B + Cjs Cjs H[cat:Layout] H[cat:Layout] + D F I[cat:Idle] + Ejs Ejs + `; + + it('asks to select a function when none is selected', () => { + const { getByText } = setup(textSamples); + expect( + getByText('Select a function to display information about it.') + ).toBeInTheDocument(); + }); + + it('displays the timings of the selected function', () => { + const { + container, + getByText, + selectFunction, + funcNamesDict: { B, Ejs, H }, + } = setup(textSamples); + + selectFunction(B); + expect(getByText('Function details')).toBeInTheDocument(); + expect(container.querySelector('h2.sidebar-title')).toHaveAttribute( + 'title', + 'B\n(click to select)' + ); + expect(getDetails(container).slice(0, 2)).toEqual([ + ['Running samples', '100%', '4'], + ['Self samples', '—', '—'], + ]); + + selectFunction(Ejs); + expect(getDetails(container).slice(0, 2)).toEqual([ + ['Running samples', '50%', '2'], + ['Self samples', '50%', '2'], + ]); + + selectFunction(H); + expect(getDetails(container).slice(0, 2)).toEqual([ + ['Running samples', '50%', '2'], + ['Self samples', '25%', '1'], + ]); + expect(container.firstChild).toMatchSnapshot(); + }); + + it('counts recursive functions only once per sample', () => { + const { + container, + selectFunction, + funcNamesDict: { A }, + } = setup(` + A A B + B A + A + `); + + selectFunction(A); + expect(getDetails(container).slice(0, 2)).toEqual([ + ['Running samples', '67%', '2'], + ['Self samples', '67%', '2'], + ]); + }); +}); diff --git a/src/test/components/__snapshots__/CallTreeSidebar.test.tsx.snap b/src/test/components/__snapshots__/CallTreeSidebar.test.tsx.snap index e313cdf49b..a8a535442c 100644 --- a/src/test/components/__snapshots__/CallTreeSidebar.test.tsx.snap +++ b/src/test/components/__snapshots__/CallTreeSidebar.test.tsx.snap @@ -1620,3 +1620,163 @@ exports[`CallTreeSidebar matches the snapshots when displaying data about the cu
`; + +exports[`FunctionListSidebar displays the timings of the selected function 1`] = ` + +`;