From 46b51e07c665ca77e686a4d24dd72ae01b892943 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sun, 11 Jan 2026 20:34:30 -0500 Subject: [PATCH 1/8] Remove unused prop disableOverscan. --- src/components/app/BottomBox.tsx | 10 +--------- src/components/shared/AssemblyView.tsx | 1 - src/components/shared/SourceView.tsx | 1 - 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/components/app/BottomBox.tsx b/src/components/app/BottomBox.tsx index c2631c42fb..dc18867078 100644 --- a/src/components/app/BottomBox.tsx +++ b/src/components/app/BottomBox.tsx @@ -29,10 +29,7 @@ import { getSourceViewCode, getAssemblyViewCode, } from 'firefox-profiler/selectors/code'; -import { - getPreviewSelectionIsBeingModified, - getSourceViewFile, -} from 'firefox-profiler/selectors/profile'; +import { getSourceViewFile } from 'firefox-profiler/selectors/profile'; import explicitConnect from 'firefox-profiler/utils/connect'; import type { ConnectedProps } from 'firefox-profiler/utils/connect'; @@ -64,7 +61,6 @@ type StateProps = { readonly assemblyViewScrollGeneration: number; readonly globalAddressTimings: AddressTimings; readonly selectedCallNodeAddressTimings: AddressTimings; - readonly disableOverscan: boolean; }; type DispatchProps = { @@ -160,7 +156,6 @@ class BottomBoxImpl extends React.PureComponent { sourceViewFile, sourceViewCode, globalLineTimings, - disableOverscan, sourceViewScrollGeneration, sourceViewLineNumber, selectedCallNodeLineTimings, @@ -229,7 +224,6 @@ class BottomBoxImpl extends React.PureComponent { ) : null} {displaySourceView ? ( {
{assemblyViewNativeSymbol !== null ? ( ({ selectedNodeSelectors.getAssemblyViewAddressTimings(state), assemblyViewScrollGeneration: getAssemblyViewScrollGeneration(state), assemblyViewIsOpen: getAssemblyViewIsOpen(state), - disableOverscan: getPreviewSelectionIsBeingModified(state), }), mapDispatchToProps: { closeBottomBox, diff --git a/src/components/shared/AssemblyView.tsx b/src/components/shared/AssemblyView.tsx index 729699d172..2d443ea4b5 100644 --- a/src/components/shared/AssemblyView.tsx +++ b/src/components/shared/AssemblyView.tsx @@ -44,7 +44,6 @@ for understanding where time was actually spent in a program." type AssemblyViewProps = { readonly timings: AddressTimings; readonly assemblyCode: DecodedInstruction[]; - readonly disableOverscan: boolean; readonly nativeSymbol: NativeSymbolInfo | null; readonly scrollToHotSpotGeneration: number; readonly hotSpotTimings: AddressTimings; diff --git a/src/components/shared/SourceView.tsx b/src/components/shared/SourceView.tsx index 7563e42d79..969d097729 100644 --- a/src/components/shared/SourceView.tsx +++ b/src/components/shared/SourceView.tsx @@ -40,7 +40,6 @@ for understanding where time was actually spent in a program." type SourceViewProps = { readonly timings: LineTimings; readonly sourceCode: string; - readonly disableOverscan: boolean; readonly filePath: string | null; readonly scrollToHotSpotGeneration: number; readonly scrollToLineNumber?: number; From 583ec0f87089912941720d0e0245ee42939a34e4 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 12 Jan 2026 18:13:19 -0500 Subject: [PATCH 2/8] Slightly simplify CallNodeContextMenu props. --- src/components/shared/CallNodeContextMenu.tsx | 29 ++++++------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/components/shared/CallNodeContextMenu.tsx b/src/components/shared/CallNodeContextMenu.tsx index 7e2633f6cf..c90008ef23 100644 --- a/src/components/shared/CallNodeContextMenu.tsx +++ b/src/components/shared/CallNodeContextMenu.tsx @@ -52,7 +52,6 @@ import type { TransformType, ImplementationFilter, IndexIntoCallNodeTable, - CallNodePath, Thread, ThreadsKey, CategoryList, @@ -70,7 +69,6 @@ type StateProps = { readonly threadsKey: ThreadsKey | null; readonly categories: CategoryList; readonly callNodeInfo: CallNodeInfo | null; - readonly rightClickedCallNodePath: CallNodePath | null; readonly rightClickedCallNodeIndex: IndexIntoCallNodeTable | null; readonly implementation: ImplementationFilter; readonly inverted: boolean; @@ -337,9 +335,10 @@ class CallNodeContextMenuImpl extends React.PureComponent { ); } - const { threadsKey, callNodePath, thread, callNodeIndex, callNodeInfo } = + const { threadsKey, thread, callNodeIndex, callNodeInfo } = rightClickedCallNodeInfo; - const selectedFunc = callNodePath[callNodePath.length - 1]; + const selectedFunc = callNodeInfo.funcForNode(callNodeIndex); + const callNodePath = callNodeInfo.getCallNodePathFromIndex(callNodeIndex); const category = callNodeInfo.categoryForNode(callNodeIndex); switch (type) { case 'focus-subtree': @@ -503,11 +502,12 @@ class CallNodeContextMenuImpl extends React.PureComponent { } const { - callNodePath, + callNodeInfo, + callNodeIndex, thread: { funcTable, stringTable, resourceTable, sources }, } = rightClickedCallNodeInfo; - const funcIndex = callNodePath[callNodePath.length - 1]; + const funcIndex = callNodeInfo.funcForNode(callNodeIndex); if (funcIndex === undefined) { return null; } @@ -534,29 +534,21 @@ class CallNodeContextMenuImpl extends React.PureComponent { readonly thread: Thread; readonly threadsKey: ThreadsKey; readonly callNodeInfo: CallNodeInfo; - readonly callNodePath: CallNodePath; readonly callNodeIndex: IndexIntoCallNodeTable; } { - const { - thread, - threadsKey, - callNodeInfo, - rightClickedCallNodePath, - rightClickedCallNodeIndex, - } = this.props; + const { thread, threadsKey, callNodeInfo, rightClickedCallNodeIndex } = + this.props; if ( thread && threadsKey !== null && callNodeInfo && - rightClickedCallNodePath && - typeof rightClickedCallNodeIndex === 'number' + rightClickedCallNodeIndex !== null ) { return { thread, threadsKey, callNodeInfo, - callNodePath: rightClickedCallNodePath, callNodeIndex: rightClickedCallNodeIndex, }; } @@ -898,7 +890,6 @@ export const CallNodeContextMenu = explicitConnect< let thread = null; let threadsKey = null; let callNodeInfo = null; - let rightClickedCallNodePath = null; let rightClickedCallNodeIndex = null; if (rightClickedCallNodeInfo !== null) { @@ -909,7 +900,6 @@ export const CallNodeContextMenu = explicitConnect< thread = selectors.getFilteredThread(state); threadsKey = rightClickedCallNodeInfo.threadsKey; callNodeInfo = selectors.getCallNodeInfo(state); - rightClickedCallNodePath = rightClickedCallNodeInfo.callNodePath; rightClickedCallNodeIndex = selectors.getRightClickedCallNodeIndex(state); } @@ -918,7 +908,6 @@ export const CallNodeContextMenu = explicitConnect< threadsKey, categories: getCategories(state), callNodeInfo, - rightClickedCallNodePath, rightClickedCallNodeIndex, implementation: getImplementationFilter(state), inverted: getInvertCallstack(state), From ce4a5f7b18d09ef5a62c1cba921396149b02cb4e Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 12 Jan 2026 18:43:55 -0500 Subject: [PATCH 3/8] Split lineNumber into scrollToLineNumber and highlightedLineNumber --- src/actions/profile-view.ts | 13 +++++++++++-- src/components/app/BottomBox.tsx | 16 ++++++++++------ src/profile-logic/profile-data.ts | 3 ++- src/reducers/url-state.ts | 8 ++++++-- src/selectors/url-state.ts | 8 ++++++-- src/types/actions.ts | 3 ++- src/types/profile-derived.ts | 3 ++- src/types/state.ts | 4 +++- 8 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts index 0885bcc50f..eb0e5c08e7 100644 --- a/src/actions/profile-view.ts +++ b/src/actions/profile-view.ts @@ -1918,8 +1918,16 @@ export function changeTableViewOptions( export function updateBottomBoxContentsAndMaybeOpen( currentTab: TabSlug, - { libIndex, sourceIndex, nativeSymbols, lineNumber }: BottomBoxInfo + bottomBoxInfo: BottomBoxInfo ): Action { + const { + libIndex, + sourceIndex, + scrollToLineNumber, + highlightLineNumber, + nativeSymbols, + } = bottomBoxInfo; + const haveSource = sourceIndex !== null; const haveAssembly = nativeSymbols.length !== 0; @@ -1944,7 +1952,8 @@ export function updateBottomBoxContentsAndMaybeOpen( currentTab, shouldOpenBottomBox, shouldOpenAssemblyView, - lineNumber, + scrollToLineNumber, + highlightLineNumber, }; } diff --git a/src/components/app/BottomBox.tsx b/src/components/app/BottomBox.tsx index dc18867078..0b5b8b27b1 100644 --- a/src/components/app/BottomBox.tsx +++ b/src/components/app/BottomBox.tsx @@ -14,7 +14,8 @@ import { CodeLoadingOverlay } from './CodeLoadingOverlay'; import { CodeErrorOverlay } from './CodeErrorOverlay'; import { getSourceViewScrollGeneration, - getSourceViewLineNumber, + getSourceViewScrollToLineNumber, + getSourceViewHighlightedLine, getAssemblyViewIsOpen, getAssemblyViewNativeSymbol, getAssemblyViewScrollGeneration, @@ -52,7 +53,8 @@ type StateProps = { readonly sourceViewFile: string | null; readonly sourceViewCode: SourceCodeStatus | void; readonly sourceViewScrollGeneration: number; - readonly sourceViewLineNumber?: number; + readonly sourceViewScrollToLineNumber?: number; + readonly sourceViewHighlightedLine?: number; readonly globalLineTimings: LineTimings; readonly selectedCallNodeLineTimings: LineTimings; readonly assemblyViewIsOpen: boolean; @@ -157,8 +159,9 @@ class BottomBoxImpl extends React.PureComponent { sourceViewCode, globalLineTimings, sourceViewScrollGeneration, - sourceViewLineNumber, selectedCallNodeLineTimings, + sourceViewScrollToLineNumber, + sourceViewHighlightedLine, assemblyViewIsOpen, assemblyViewScrollGeneration, assemblyViewNativeSymbol, @@ -228,9 +231,9 @@ class BottomBoxImpl extends React.PureComponent { sourceCode={sourceCode} filePath={path} scrollToHotSpotGeneration={sourceViewScrollGeneration} - scrollToLineNumber={sourceViewLineNumber} hotSpotTimings={selectedCallNodeLineTimings} - highlightedLine={sourceViewLineNumber} + scrollToLineNumber={sourceViewScrollToLineNumber} + highlightedLine={sourceViewHighlightedLine} ref={this._sourceView} /> ) : null} @@ -298,7 +301,8 @@ export const BottomBox = explicitConnect<{}, StateProps, DispatchProps>({ selectedCallNodeLineTimings: selectedNodeSelectors.getSourceViewLineTimings(state), sourceViewScrollGeneration: getSourceViewScrollGeneration(state), - sourceViewLineNumber: getSourceViewLineNumber(state), + sourceViewScrollToLineNumber: getSourceViewScrollToLineNumber(state), + sourceViewHighlightedLine: getSourceViewHighlightedLine(state), assemblyViewNativeSymbol: getAssemblyViewNativeSymbol(state), assemblyViewCode: getAssemblyViewCode(state), globalAddressTimings: diff --git a/src/profile-logic/profile-data.ts b/src/profile-logic/profile-data.ts index 97525c5502..165c84d2ea 100644 --- a/src/profile-logic/profile-data.ts +++ b/src/profile-logic/profile-data.ts @@ -4058,7 +4058,8 @@ export function getBottomBoxInfoForStackFrame( libIndex, sourceIndex, nativeSymbols: nativeSymbolInfos, - lineNumber, + scrollToLineNumber: lineNumber, + highlightLineNumber: lineNumber, }; } diff --git a/src/reducers/url-state.ts b/src/reducers/url-state.ts index 8823e926ce..be3d4329a9 100644 --- a/src/reducers/url-state.ts +++ b/src/reducers/url-state.ts @@ -570,11 +570,15 @@ const sourceView: Reducer = ( ) => { switch (action.type) { case 'UPDATE_BOTTOM_BOX': { + const shouldScroll = action.scrollToLineNumber !== undefined; return { - scrollGeneration: state.scrollGeneration + 1, + scrollGeneration: shouldScroll + ? state.scrollGeneration + 1 + : state.scrollGeneration, libIndex: action.libIndex, sourceIndex: action.sourceIndex, - lineNumber: action.lineNumber, + scrollToLineNumber: action.scrollToLineNumber, + highlightedLine: action.highlightLineNumber, }; } default: diff --git a/src/selectors/url-state.ts b/src/selectors/url-state.ts index 2587b5462c..4cf2c27ff2 100644 --- a/src/selectors/url-state.ts +++ b/src/selectors/url-state.ts @@ -75,8 +75,12 @@ export const getSourceViewSourceIndex: Selector = ( ) => getProfileSpecificState(state).sourceView.sourceIndex; export const getSourceViewScrollGeneration: Selector = (state) => getProfileSpecificState(state).sourceView.scrollGeneration; -export const getSourceViewLineNumber: Selector = (state) => - getProfileSpecificState(state).sourceView.lineNumber; +export const getSourceViewScrollToLineNumber: Selector = ( + state +) => getProfileSpecificState(state).sourceView.scrollToLineNumber; +export const getSourceViewHighlightedLine: Selector = ( + state +) => getProfileSpecificState(state).sourceView.highlightedLine; export const getAssemblyViewIsOpen: Selector = (state) => getProfileSpecificState(state).assemblyView.isOpen; export const getAssemblyViewNativeSymbol: Selector = ( diff --git a/src/types/actions.ts b/src/types/actions.ts index 3804235664..1709a40394 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -343,7 +343,8 @@ type ProfileAction = readonly currentTab: TabSlug; readonly shouldOpenBottomBox: boolean; readonly shouldOpenAssemblyView: boolean; - readonly lineNumber?: number; + readonly scrollToLineNumber?: number; + readonly highlightLineNumber?: number; } | { readonly type: 'OPEN_ASSEMBLY_VIEW'; diff --git a/src/types/profile-derived.ts b/src/types/profile-derived.ts index c37e45319e..c52bf10a71 100644 --- a/src/types/profile-derived.ts +++ b/src/types/profile-derived.ts @@ -777,7 +777,8 @@ export type BottomBoxInfo = { nativeSymbols: NativeSymbolInfo[]; // Optional line number to scroll to in the source view. // If not specified, the source view will scroll to the hottest line. - lineNumber?: number; + scrollToLineNumber?: number; + highlightLineNumber?: number; }; /** diff --git a/src/types/state.ts b/src/types/state.ts index 0989fd0472..0ffcf4c4c5 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -253,7 +253,9 @@ export type SourceViewState = { sourceIndex: IndexIntoSourceTable | null; // Optional line number to scroll to in the source view. // If not specified, the source view will scroll to the hottest line. - lineNumber?: number; + scrollToLineNumber?: number; + // Optional line number of the highlighted in the source view. + highlightedLine?: number; }; export type AssemblyViewState = { From ca1de43dd2ad6ac22ff36b4ea3f67363e72780c3 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 12 Jan 2026 18:43:55 -0500 Subject: [PATCH 4/8] Move two functions out of profile-data.ts into a new file bottom-box.ts. I want to add a call to a function from line-timings.ts but line-timings.ts already depends on profile-data.ts, and I can't create a circular dependency between profile-data.ts and line-timings.ts. --- src/components/marker-chart/Canvas.tsx | 2 +- src/components/shared/CallNodeContextMenu.tsx | 6 +- src/components/sidebar/MarkerSidebar.tsx | 2 +- src/components/stack-chart/index.tsx | 2 +- src/profile-logic/bottom-box.ts | 121 ++++++++++++++++++ src/profile-logic/call-tree.ts | 2 +- src/profile-logic/profile-data.ts | 105 --------------- src/test/store/bottom-box.test.ts | 2 +- 8 files changed, 128 insertions(+), 114 deletions(-) create mode 100644 src/profile-logic/bottom-box.ts diff --git a/src/components/marker-chart/Canvas.tsx b/src/components/marker-chart/Canvas.tsx index ec6544ad32..431109c133 100644 --- a/src/components/marker-chart/Canvas.tsx +++ b/src/components/marker-chart/Canvas.tsx @@ -49,7 +49,7 @@ import { isValidGraphColor, } from 'firefox-profiler/profile-logic/graph-color'; import { getSchemaFromMarker } from 'firefox-profiler/profile-logic/marker-schema'; -import { getBottomBoxInfoForStackFrame } from 'firefox-profiler/profile-logic/profile-data'; +import { getBottomBoxInfoForStackFrame } from 'firefox-profiler/profile-logic/bottom-box'; import type { ChartCanvasScale, diff --git a/src/components/shared/CallNodeContextMenu.tsx b/src/components/shared/CallNodeContextMenu.tsx index c90008ef23..d976e3985d 100644 --- a/src/components/shared/CallNodeContextMenu.tsx +++ b/src/components/shared/CallNodeContextMenu.tsx @@ -14,10 +14,7 @@ import { funcHasRecursiveCall, } from 'firefox-profiler/profile-logic/transforms'; import { getFunctionName } from 'firefox-profiler/profile-logic/function-info'; -import { - getBottomBoxInfoForCallNode, - getOriginAnnotationForFunc, -} from 'firefox-profiler/profile-logic/profile-data'; +import { getOriginAnnotationForFunc } from 'firefox-profiler/profile-logic/profile-data'; import { getCategories } from 'firefox-profiler/selectors'; import copy from 'copy-to-clipboard'; @@ -89,6 +86,7 @@ type DispatchProps = { type Props = ConnectedProps<{}, StateProps, DispatchProps>; import './CallNodeContextMenu.css'; +import { getBottomBoxInfoForCallNode } from 'firefox-profiler/profile-logic/bottom-box'; class CallNodeContextMenuImpl extends React.PureComponent { _hidingTimeout: NodeJS.Timeout | null = null; diff --git a/src/components/sidebar/MarkerSidebar.tsx b/src/components/sidebar/MarkerSidebar.tsx index 0ffbd9db69..91b3a8f278 100644 --- a/src/components/sidebar/MarkerSidebar.tsx +++ b/src/components/sidebar/MarkerSidebar.tsx @@ -13,7 +13,7 @@ import { } from 'firefox-profiler/selectors/url-state'; import { TooltipMarker } from 'firefox-profiler/components/tooltip/Marker'; import { updateBottomBoxContentsAndMaybeOpen } from 'firefox-profiler/actions/profile-view'; -import { getBottomBoxInfoForStackFrame } from 'firefox-profiler/profile-logic/profile-data'; +import { getBottomBoxInfoForStackFrame } from 'firefox-profiler/profile-logic/bottom-box'; import type { ConnectedProps } from 'firefox-profiler/utils/connect'; import type { diff --git a/src/components/stack-chart/index.tsx b/src/components/stack-chart/index.tsx index ad706dd84c..c0d31c9f3f 100644 --- a/src/components/stack-chart/index.tsx +++ b/src/components/stack-chart/index.tsx @@ -39,7 +39,7 @@ import { changeMouseTimePosition, } from '../../actions/profile-view'; -import { getBottomBoxInfoForCallNode } from '../../profile-logic/profile-data'; +import { getBottomBoxInfoForCallNode } from '../../profile-logic/bottom-box'; import type { Thread, diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts new file mode 100644 index 0000000000..3a6baa76d9 --- /dev/null +++ b/src/profile-logic/bottom-box.ts @@ -0,0 +1,121 @@ +/* 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 { resourceTypes } from './data-structures'; + +import type { + Thread, + IndexIntoStackTable, + IndexIntoCallNodeTable, + BottomBoxInfo, +} from 'firefox-profiler/types'; +import type { CallNodeInfo } from './call-node-info'; +import { + getNativeSymbolInfo, + getNativeSymbolsForCallNode, +} from './profile-data'; + +/** + * Calculate the BottomBoxInfo for a call node, i.e. information about which + * things should be shown in the profiler UI's "bottom box" when this call node + * is double-clicked. + * + * We always want to update all panes in the bottom box when a new call node is + * double-clicked, so that we don't show inconsistent information side-by-side. + */ +export function getBottomBoxInfoForCallNode( + callNodeIndex: IndexIntoCallNodeTable, + callNodeInfo: CallNodeInfo, + thread: Thread +): BottomBoxInfo { + const { + stackTable, + frameTable, + funcTable, + stringTable, + resourceTable, + nativeSymbols, + } = thread; + + const funcIndex = callNodeInfo.funcForNode(callNodeIndex); + const sourceIndex = funcTable.source[funcIndex]; + const resource = funcTable.resource[funcIndex]; + const libIndex = + resource !== -1 && resourceTable.type[resource] === resourceTypes.library + ? resourceTable.lib[resource] + : null; + const nativeSymbolsForCallNode = getNativeSymbolsForCallNode( + callNodeIndex, + callNodeInfo, + stackTable, + frameTable + ); + const nativeSymbolInfosForCallNode = nativeSymbolsForCallNode.map( + (nativeSymbolIndex) => + getNativeSymbolInfo( + nativeSymbolIndex, + nativeSymbols, + frameTable, + stringTable + ) + ); + + return { + libIndex, + sourceIndex, + nativeSymbols: nativeSymbolInfosForCallNode, + }; +} + +/** + * Get bottom box info for a stack frame. This is similar to + * getBottomBoxInfoForCallNode but works directly with stack indexes. + */ +export function getBottomBoxInfoForStackFrame( + stackIndex: IndexIntoStackTable, + thread: Thread +): BottomBoxInfo { + const { + stackTable, + frameTable, + funcTable, + resourceTable, + nativeSymbols, + stringTable, + } = thread; + + const frameIndex = stackTable.frame[stackIndex]; + const funcIndex = frameTable.func[frameIndex]; + const sourceIndex = funcTable.source[funcIndex]; + const resource = funcTable.resource[funcIndex]; + const libIndex = + resource !== -1 && resourceTable.type[resource] === resourceTypes.library + ? resourceTable.lib[resource] + : null; + + // Get native symbol for this frame + const nativeSymbol = frameTable.nativeSymbol[frameIndex]; + const nativeSymbolInfos = + nativeSymbol !== null + ? [ + getNativeSymbolInfo( + nativeSymbol, + nativeSymbols, + frameTable, + stringTable + ), + ] + : []; + + // Extract line number from the frame + const lineNumber = frameTable.line[frameIndex] ?? undefined; + + return { + libIndex, + sourceIndex, + nativeSymbols: nativeSymbolInfos, + scrollToLineNumber: lineNumber, + highlightLineNumber: lineNumber, + }; +} diff --git a/src/profile-logic/call-tree.ts b/src/profile-logic/call-tree.ts index 5802354430..93864c7eda 100644 --- a/src/profile-logic/call-tree.ts +++ b/src/profile-logic/call-tree.ts @@ -6,7 +6,6 @@ import { timeCode } from '../utils/time-code'; import { getOriginAnnotationForFunc, getCategoryPairLabel, - getBottomBoxInfoForCallNode, } from './profile-data'; import { resourceTypes } from './data-structures'; import { getFunctionName } from './function-info'; @@ -37,6 +36,7 @@ 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 { getBottomBoxInfoForCallNode } from './bottom-box'; type CallNodeChildren = IndexIntoCallNodeTable[]; diff --git a/src/profile-logic/profile-data.ts b/src/profile-logic/profile-data.ts index 165c84d2ea..39efe49ae0 100644 --- a/src/profile-logic/profile-data.ts +++ b/src/profile-logic/profile-data.ts @@ -89,7 +89,6 @@ import type { AddressProof, TimelineType, NativeSymbolInfo, - BottomBoxInfo, Bytes, ThreadWithReservedFunctions, TabID, @@ -3959,110 +3958,6 @@ export function getNativeSymbolInfo( }; } -/** - * Calculate the BottomBoxInfo for a call node, i.e. information about which - * things should be shown in the profiler UI's "bottom box" when this call node - * is double-clicked. - * - * We always want to update all panes in the bottom box when a new call node is - * double-clicked, so that we don't show inconsistent information side-by-side. - */ -export function getBottomBoxInfoForCallNode( - callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfo, - thread: Thread -): BottomBoxInfo { - const { - stackTable, - frameTable, - funcTable, - stringTable, - resourceTable, - nativeSymbols, - } = thread; - - const funcIndex = callNodeInfo.funcForNode(callNodeIndex); - const sourceIndex = funcTable.source[funcIndex]; - const resource = funcTable.resource[funcIndex]; - const libIndex = - resource !== -1 && resourceTable.type[resource] === resourceTypes.library - ? resourceTable.lib[resource] - : null; - const nativeSymbolsForCallNode = getNativeSymbolsForCallNode( - callNodeIndex, - callNodeInfo, - stackTable, - frameTable - ); - const nativeSymbolInfosForCallNode = nativeSymbolsForCallNode.map( - (nativeSymbolIndex) => - getNativeSymbolInfo( - nativeSymbolIndex, - nativeSymbols, - frameTable, - stringTable - ) - ); - - return { - libIndex, - sourceIndex, - nativeSymbols: nativeSymbolInfosForCallNode, - }; -} - -/** - * Get bottom box info for a stack frame. This is similar to - * getBottomBoxInfoForCallNode but works directly with stack indexes. - */ -export function getBottomBoxInfoForStackFrame( - stackIndex: IndexIntoStackTable, - thread: Thread -): BottomBoxInfo { - const { - stackTable, - frameTable, - funcTable, - resourceTable, - nativeSymbols, - stringTable, - } = thread; - - const frameIndex = stackTable.frame[stackIndex]; - const funcIndex = frameTable.func[frameIndex]; - const sourceIndex = funcTable.source[funcIndex]; - const resource = funcTable.resource[funcIndex]; - const libIndex = - resource !== -1 && resourceTable.type[resource] === resourceTypes.library - ? resourceTable.lib[resource] - : null; - - // Get native symbol for this frame - const nativeSymbol = frameTable.nativeSymbol[frameIndex]; - const nativeSymbolInfos = - nativeSymbol !== null - ? [ - getNativeSymbolInfo( - nativeSymbol, - nativeSymbols, - frameTable, - stringTable - ), - ] - : []; - - // Extract line number from the frame - const lineNumber = frameTable.line[frameIndex] ?? undefined; - - return { - libIndex, - sourceIndex, - nativeSymbols: nativeSymbolInfos, - scrollToLineNumber: lineNumber, - highlightLineNumber: lineNumber, - }; -} - /** * Determines the timeline type by looking at the profile data. * diff --git a/src/test/store/bottom-box.test.ts b/src/test/store/bottom-box.test.ts index 2007e02f84..89bc43b99f 100644 --- a/src/test/store/bottom-box.test.ts +++ b/src/test/store/bottom-box.test.ts @@ -11,7 +11,7 @@ import { selectedNodeSelectors, } from '../../selectors/per-thread'; import { emptyAddressTimings } from '../../profile-logic/address-timings'; -import { getBottomBoxInfoForCallNode } from '../../profile-logic/profile-data'; +import { getBottomBoxInfoForCallNode } from '../../profile-logic/bottom-box'; import { changeSelectedCallNode, updateBottomBoxContentsAndMaybeOpen, From a69621a8bc1175c266269ffdf79f3e3c4edeffa9 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 29 Jan 2025 17:32:38 -0500 Subject: [PATCH 5/8] Compute the "scroll to hot spot" when making the bottom box info. This means we can compute the right line number for the "hot spot" for the call node that was passed to getBottomBoxInfoForCallNode, and not just for the selected call node. For example, getBottomBoxInfoForCallNode can be called with the right-clicked call node which can be different from the selected call node, and we were scrolling to the wrong spot in that case before this patch. Fixes #5758. --- src/components/app/BottomBox.tsx | 8 +-- src/components/shared/CallNodeContextMenu.tsx | 24 +++++++-- src/components/shared/IonGraphView.tsx | 1 - src/components/shared/SourceView.tsx | 42 ++-------------- src/components/stack-chart/index.tsx | 3 +- src/profile-logic/bottom-box.ts | 18 ++++++- src/profile-logic/call-tree.ts | 10 +++- src/selectors/per-thread/index.ts | 49 ------------------- src/selectors/per-thread/stack-sample.ts | 4 ++ src/test/fixtures/utils.ts | 2 + .../__snapshots__/profile-view.test.ts.snap | 17 +++++++ src/test/store/bottom-box.test.ts | 18 +++++-- src/test/unit/profile-tree.test.ts | 2 + src/types/profile-derived.ts | 1 - src/types/state.ts | 3 +- 15 files changed, 91 insertions(+), 111 deletions(-) diff --git a/src/components/app/BottomBox.tsx b/src/components/app/BottomBox.tsx index 0b5b8b27b1..cd73def7cc 100644 --- a/src/components/app/BottomBox.tsx +++ b/src/components/app/BottomBox.tsx @@ -56,7 +56,6 @@ type StateProps = { readonly sourceViewScrollToLineNumber?: number; readonly sourceViewHighlightedLine?: number; readonly globalLineTimings: LineTimings; - readonly selectedCallNodeLineTimings: LineTimings; readonly assemblyViewIsOpen: boolean; readonly assemblyViewNativeSymbol: NativeSymbolInfo | null; readonly assemblyViewCode: AssemblyCodeStatus | void; @@ -159,7 +158,6 @@ class BottomBoxImpl extends React.PureComponent { sourceViewCode, globalLineTimings, sourceViewScrollGeneration, - selectedCallNodeLineTimings, sourceViewScrollToLineNumber, sourceViewHighlightedLine, assemblyViewIsOpen, @@ -221,7 +219,6 @@ class BottomBoxImpl extends React.PureComponent { {displayIonGraph ? ( ) : null} @@ -230,8 +227,7 @@ class BottomBoxImpl extends React.PureComponent { timings={globalLineTimings} sourceCode={sourceCode} filePath={path} - scrollToHotSpotGeneration={sourceViewScrollGeneration} - hotSpotTimings={selectedCallNodeLineTimings} + scrollGeneration={sourceViewScrollGeneration} scrollToLineNumber={sourceViewScrollToLineNumber} highlightedLine={sourceViewHighlightedLine} ref={this._sourceView} @@ -298,8 +294,6 @@ export const BottomBox = explicitConnect<{}, StateProps, DispatchProps>({ sourceViewFile: getSourceViewFile(state), sourceViewCode: getSourceViewCode(state), globalLineTimings: selectedThreadSelectors.getSourceViewLineTimings(state), - selectedCallNodeLineTimings: - selectedNodeSelectors.getSourceViewLineTimings(state), sourceViewScrollGeneration: getSourceViewScrollGeneration(state), sourceViewScrollToLineNumber: getSourceViewScrollToLineNumber(state), sourceViewHighlightedLine: getSourceViewHighlightedLine(state), diff --git a/src/components/shared/CallNodeContextMenu.tsx b/src/components/shared/CallNodeContextMenu.tsx index d976e3985d..ebbe889971 100644 --- a/src/components/shared/CallNodeContextMenu.tsx +++ b/src/components/shared/CallNodeContextMenu.tsx @@ -54,6 +54,7 @@ import type { CategoryList, InnerWindowID, Page, + SamplesLikeTable, } from 'firefox-profiler/types'; import type { TabSlug } from 'firefox-profiler/app-logic/tabs-handling'; @@ -64,6 +65,7 @@ import type { BrowserConnectionStatus } from 'firefox-profiler/app-logic/browser type StateProps = { readonly thread: Thread | null; readonly threadsKey: ThreadsKey | null; + readonly previewFilteredCtssSamples: SamplesLikeTable | null; readonly categories: CategoryList; readonly callNodeInfo: CallNodeInfo | null; readonly rightClickedCallNodeIndex: IndexIntoCallNodeTable | null; @@ -222,11 +224,13 @@ class CallNodeContextMenuImpl extends React.PureComponent { ); } - const { callNodeIndex, thread, callNodeInfo } = rightClickedCallNodeInfo; + const { callNodeIndex, thread, callNodeInfo, previewFilteredCtssSamples } = + rightClickedCallNodeInfo; const bottomBoxInfo = getBottomBoxInfoForCallNode( callNodeIndex, callNodeInfo, - thread + thread, + previewFilteredCtssSamples ); updateBottomBoxContentsAndMaybeOpen(selectedTab, bottomBoxInfo); } @@ -531,21 +535,29 @@ class CallNodeContextMenuImpl extends React.PureComponent { getRightClickedCallNodeInfo(): null | { readonly thread: Thread; readonly threadsKey: ThreadsKey; + readonly previewFilteredCtssSamples: SamplesLikeTable; readonly callNodeInfo: CallNodeInfo; readonly callNodeIndex: IndexIntoCallNodeTable; } { - const { thread, threadsKey, callNodeInfo, rightClickedCallNodeIndex } = - this.props; + const { + thread, + threadsKey, + previewFilteredCtssSamples, + callNodeInfo, + rightClickedCallNodeIndex, + } = this.props; if ( thread && threadsKey !== null && + previewFilteredCtssSamples !== null && callNodeInfo && rightClickedCallNodeIndex !== null ) { return { thread, threadsKey, + previewFilteredCtssSamples, callNodeInfo, callNodeIndex: rightClickedCallNodeIndex, }; @@ -887,6 +899,7 @@ export const CallNodeContextMenu = explicitConnect< let thread = null; let threadsKey = null; + let previewFilteredCtssSamples = null; let callNodeInfo = null; let rightClickedCallNodeIndex = null; @@ -897,6 +910,8 @@ export const CallNodeContextMenu = explicitConnect< thread = selectors.getFilteredThread(state); threadsKey = rightClickedCallNodeInfo.threadsKey; + previewFilteredCtssSamples = + selectors.getPreviewFilteredCtssSamples(state); callNodeInfo = selectors.getCallNodeInfo(state); rightClickedCallNodeIndex = selectors.getRightClickedCallNodeIndex(state); } @@ -904,6 +919,7 @@ export const CallNodeContextMenu = explicitConnect< return { thread, threadsKey, + previewFilteredCtssSamples, categories: getCategories(state), callNodeInfo, rightClickedCallNodeIndex, diff --git a/src/components/shared/IonGraphView.tsx b/src/components/shared/IonGraphView.tsx index e14c07a22f..017f4390f7 100644 --- a/src/components/shared/IonGraphView.tsx +++ b/src/components/shared/IonGraphView.tsx @@ -11,7 +11,6 @@ import { useMemo } from 'react'; type IonGraphViewProps = { readonly sourceCode: string; readonly timings: LineTimings; - readonly hotSpotTimings: LineTimings; }; export function IonGraphView(props: IonGraphViewProps) { diff --git a/src/components/shared/SourceView.tsx b/src/components/shared/SourceView.tsx index 969d097729..02c2b8c848 100644 --- a/src/components/shared/SourceView.tsx +++ b/src/components/shared/SourceView.tsx @@ -4,7 +4,6 @@ import * as React from 'react'; import { ensureExists } from 'firefox-profiler/utils/types'; -import { mapGetKeyWithMaxValue } from 'firefox-profiler/utils'; import type { LineTimings } from 'firefox-profiler/types'; import type { SourceViewEditor } from './SourceView-codemirror'; @@ -41,9 +40,8 @@ type SourceViewProps = { readonly timings: LineTimings; readonly sourceCode: string; readonly filePath: string | null; - readonly scrollToHotSpotGeneration: number; + readonly scrollGeneration: number; readonly scrollToLineNumber?: number; - readonly hotSpotTimings: LineTimings; readonly highlightedLine?: number; }; @@ -53,36 +51,6 @@ export class SourceView extends React.PureComponent { _ref = React.createRef(); _editor: SourceViewEditor | null = null; - /** - * Scroll to the line with the most hits, based on the timings in - * timingsForScrolling. - * - * How is timingsForScrolling different from this.props.timings? - * In the current implementation, this.props.timings are always the "global" - * timings, i.e. they show the line hits for all samples in the current view, - * regardless of the selected call node. However, when opening the source - * view from a specific call node, you really want to see the code that's - * relevant to that specific call node, or at least that specific function. - * So timingsForScrolling are the timings that indicate just the line hits - * in the selected call node. This means that the "hotspot" will be somewhere - * in the selected function, and it will even be in the line that's most - * relevant to that specific call node. - * - * Sometimes, timingsForScrolling can be completely empty. This happens, for - * example, when the source view is showing a different file than the - * selected call node's function's file, for example because we just loaded - * from a URL and ended up with an arbitrary selected call node. - * In that case, pick the hotspot from the global line timings. - */ - _scrollToHotSpot(timingsForScrolling: LineTimings) { - const heaviestLine = - mapGetKeyWithMaxValue(timingsForScrolling.totalLineHits) ?? - mapGetKeyWithMaxValue(this.props.timings.totalLineHits); - if (heaviestLine !== undefined) { - this._scrollToLine(heaviestLine - 5); - } - } - _scrollToLine(lineNumber: number) { if (this._editor) { this._editor.scrollToLine(lineNumber); @@ -148,8 +116,6 @@ export class SourceView extends React.PureComponent { // If an explicit line number is provided, scroll to it. Otherwise, scroll to the hotspot. if (this.props.scrollToLineNumber !== undefined) { this._scrollToLine(Math.max(1, this.props.scrollToLineNumber - 5)); - } else { - this._scrollToHotSpot(this.props.hotSpotTimings); } })(); } @@ -178,14 +144,12 @@ export class SourceView extends React.PureComponent { if ( contentsChanged || - this.props.scrollToHotSpotGeneration !== - prevProps.scrollToHotSpotGeneration + this.props.scrollGeneration !== + prevProps.scrollGeneration ) { // If an explicit line number is provided, scroll to it. Otherwise, scroll to the hotspot. if (this.props.scrollToLineNumber !== undefined) { this._scrollToLine(Math.max(1, this.props.scrollToLineNumber - 5)); - } else { - this._scrollToHotSpot(this.props.hotSpotTimings); } } diff --git a/src/components/stack-chart/index.tsx b/src/components/stack-chart/index.tsx index c0d31c9f3f..d027f0b83c 100644 --- a/src/components/stack-chart/index.tsx +++ b/src/components/stack-chart/index.tsx @@ -170,7 +170,8 @@ class StackChartImpl extends React.PureComponent { const bottomBoxInfo = getBottomBoxInfoForCallNode( nodeIndex, callNodeInfo, - thread + thread, + thread.samples ); updateBottomBoxContentsAndMaybeOpen('stack-chart', bottomBoxInfo); return; diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts index 3a6baa76d9..dd8ef453c5 100644 --- a/src/profile-logic/bottom-box.ts +++ b/src/profile-logic/bottom-box.ts @@ -9,12 +9,15 @@ import type { IndexIntoStackTable, IndexIntoCallNodeTable, BottomBoxInfo, + SamplesLikeTable, } from 'firefox-profiler/types'; import type { CallNodeInfo } from './call-node-info'; import { getNativeSymbolInfo, getNativeSymbolsForCallNode, } from './profile-data'; +import { getLineTimings, getStackLineInfoForCallNode } from './line-timings'; +import { mapGetKeyWithMaxValue } from 'firefox-profiler/utils'; /** * Calculate the BottomBoxInfo for a call node, i.e. information about which @@ -27,7 +30,8 @@ import { export function getBottomBoxInfoForCallNode( callNodeIndex: IndexIntoCallNodeTable, callNodeInfo: CallNodeInfo, - thread: Thread + thread: Thread, + samples: SamplesLikeTable ): BottomBoxInfo { const { stackTable, @@ -61,10 +65,22 @@ export function getBottomBoxInfoForCallNode( ) ); + // Compute the hottest line, and ask the source view to scroll to it. + const stackLineInfo = getStackLineInfoForCallNode( + stackTable, + frameTable, + funcTable, + callNodeIndex, + callNodeInfo + ); + const callNodeLineTimings = getLineTimings(stackLineInfo, samples); + const hottestLine = mapGetKeyWithMaxValue(callNodeLineTimings.totalLineHits); + return { libIndex, sourceIndex, nativeSymbols: nativeSymbolInfosForCallNode, + scrollToLineNumber: hottestLine, }; } diff --git a/src/profile-logic/call-tree.ts b/src/profile-logic/call-tree.ts index 93864c7eda..71ab7b3f7a 100644 --- a/src/profile-logic/call-tree.ts +++ b/src/profile-logic/call-tree.ts @@ -359,6 +359,7 @@ export class CallTree { _internal: CallTreeInternal; _callNodeInfo: CallNodeInfo; _thread: Thread; + _previewFilteredCtssSamples: SamplesLikeTable; _rootTotalSummary: number; _displayDataByIndex: Map; // _children is indexed by IndexIntoCallNodeTable. Since they are @@ -372,6 +373,7 @@ export class CallTree { thread: Thread, categories: CategoryList, callNodeInfo: CallNodeInfo, + previewFilteredCtssSamples: SamplesLikeTable, internal: CallTreeInternal, rootTotalSummary: number, isHighPrecision: boolean, @@ -381,6 +383,7 @@ export class CallTree { this._internal = internal; this._callNodeInfo = callNodeInfo; this._thread = thread; + this._previewFilteredCtssSamples = previewFilteredCtssSamples; this._rootTotalSummary = rootTotalSummary; this._displayDataByIndex = new Map(); this._children = []; @@ -616,7 +619,8 @@ export class CallTree { return getBottomBoxInfoForCallNode( callNodeIndex, this._callNodeInfo, - this._thread + this._thread, + this._previewFilteredCtssSamples ); } @@ -1050,6 +1054,7 @@ export function getCallTree( thread: Thread, callNodeInfo: CallNodeInfo, categories: CategoryList, + previewFilteredCtssSamples: SamplesLikeTable, callTreeTimings: CallTreeTimings, weightType: WeightType ): CallTree { @@ -1061,6 +1066,7 @@ export function getCallTree( thread, categories, callNodeInfo, + previewFilteredCtssSamples, new CallTreeInternalNonInverted(callNodeInfo, timings), timings.rootTotalSummary, Boolean(thread.isJsTracer), @@ -1073,6 +1079,7 @@ export function getCallTree( thread, categories, callNodeInfo, + previewFilteredCtssSamples, new CallTreeInternalFunctionList(timings), timings.rootTotalSummary, Boolean(thread.isJsTracer), @@ -1085,6 +1092,7 @@ export function getCallTree( thread, categories, callNodeInfo, + previewFilteredCtssSamples, new CallTreeInternalInverted( ensureExists(callNodeInfo.asInverted()), timings diff --git a/src/selectors/per-thread/index.ts b/src/selectors/per-thread/index.ts index df364eccb4..aa3f7b4a65 100644 --- a/src/selectors/per-thread/index.ts +++ b/src/selectors/per-thread/index.ts @@ -23,10 +23,6 @@ import { getComposedSelectorsPerThread, type ComposedSelectorsPerThread, } from './composed'; -import { - getStackLineInfoForCallNode, - getLineTimings, -} from '../../profile-logic/line-timings'; import { getStackAddressInfoForCallNode, getAddressTimings, @@ -39,8 +35,6 @@ import type { ThreadIndex, Selector, ThreadsKey, - StackLineInfo, - LineTimings, StackAddressInfo, AddressTimings, State, @@ -205,8 +199,6 @@ export type NodeSelectors = { readonly getIsJS: Selector; readonly getLib: Selector; readonly getTimingsForSidebar: Selector; - readonly getSourceViewStackLineInfo: Selector; - readonly getSourceViewLineTimings: Selector; readonly getAssemblyViewStackAddressInfo: Selector; readonly getAssemblyViewAddressTimings: Selector; }; @@ -268,45 +260,6 @@ export const selectedNodeSelectors: NodeSelectors = (() => { ProfileData.getTimingsForPath ); - const getSourceViewStackLineInfo: Selector = - createSelector( - selectedThreadSelectors.getFilteredThread, - UrlState.getSourceViewSourceIndex, - selectedThreadSelectors.getCallNodeInfo, - selectedThreadSelectors.getSelectedCallNodeIndex, - ( - { stackTable, frameTable, funcTable }: Thread, - sourceViewSourceIndex, - callNodeInfo, - selectedCallNodeIndex - ): StackLineInfo | null => { - if (sourceViewSourceIndex === null || selectedCallNodeIndex === null) { - return null; - } - const selectedFunc = callNodeInfo.funcForNode(selectedCallNodeIndex); - const selectedSourceIndex = funcTable.source[selectedFunc]; - if ( - selectedSourceIndex === null || - selectedSourceIndex !== sourceViewSourceIndex - ) { - return null; - } - return getStackLineInfoForCallNode( - stackTable, - frameTable, - funcTable, - selectedCallNodeIndex, - callNodeInfo - ); - } - ); - - const getSourceViewLineTimings: Selector = createSelector( - getSourceViewStackLineInfo, - selectedThreadSelectors.getPreviewFilteredCtssSamples, - getLineTimings - ); - const getAssemblyViewStackAddressInfo: Selector = createSelector( selectedThreadSelectors.getFilteredThread, @@ -344,8 +297,6 @@ export const selectedNodeSelectors: NodeSelectors = (() => { getIsJS, getLib, getTimingsForSidebar, - getSourceViewStackLineInfo, - getSourceViewLineTimings, getAssemblyViewStackAddressInfo, getAssemblyViewAddressTimings, }; diff --git a/src/selectors/per-thread/stack-sample.ts b/src/selectors/per-thread/stack-sample.ts index 5b8b44b079..fa9944b9e3 100644 --- a/src/selectors/per-thread/stack-sample.ts +++ b/src/selectors/per-thread/stack-sample.ts @@ -340,6 +340,7 @@ export function getStackAndSampleSelectorsPerThread( threadSelectors.getFilteredThread, getCallNodeInfo, ProfileSelectors.getCategories, + threadSelectors.getPreviewFilteredCtssSamples, getCallTreeTimings, getWeightTypeForCallTree, ProfileSelectors.getSourceTable, @@ -350,12 +351,14 @@ export function getStackAndSampleSelectorsPerThread( threadSelectors.getFilteredThread, _getInvertedCallNodeInfo, ProfileSelectors.getCategories, + threadSelectors.getPreviewFilteredCtssSamples, getFunctionListTimings, getWeightTypeForCallTree, ( thread, callNodeInfoInverted, categories, + previewFilteredCtssSamples, functionListTimings, weightType ) => @@ -363,6 +366,7 @@ export function getStackAndSampleSelectorsPerThread( thread, callNodeInfoInverted, categories, + previewFilteredCtssSamples, { type: 'FUNCTION_LIST', timings: functionListTimings }, weightType ) diff --git a/src/test/fixtures/utils.ts b/src/test/fixtures/utils.ts index 0d5301b2c4..11b5f9955a 100644 --- a/src/test/fixtures/utils.ts +++ b/src/test/fixtures/utils.ts @@ -195,6 +195,7 @@ export function callTreeFromProfile( thread, callNodeInfo, ensureExists(profile.meta.categories), + thread.samples, callTreeTimings, 'samples' ); @@ -243,6 +244,7 @@ export function functionListTreeFromProfile( thread, invertedCallNodeInfo, ensureExists(profile.meta.categories), + thread.samples, { type: 'FUNCTION_LIST', timings: functionListTimings }, 'samples' ); diff --git a/src/test/store/__snapshots__/profile-view.test.ts.snap b/src/test/store/__snapshots__/profile-view.test.ts.snap index a0d8a23ea5..913438be86 100644 --- a/src/test/store/__snapshots__/profile-view.test.ts.snap +++ b/src/test/store/__snapshots__/profile-view.test.ts.snap @@ -2632,6 +2632,23 @@ CallTree { }, }, "_isHighPrecision": false, + "_previewFilteredCtssSamples": Object { + "eventDelay": Array [ + 0, + 0, + ], + "length": 2, + "stack": Array [ + 6, + 6, + ], + "time": Array [ + 4, + 5, + ], + "weight": null, + "weightType": "samples", + }, "_rootTotalSummary": 2, "_roots": Array [ 0, diff --git a/src/test/store/bottom-box.test.ts b/src/test/store/bottom-box.test.ts index 89bc43b99f..b8130ab41a 100644 --- a/src/test/store/bottom-box.test.ts +++ b/src/test/store/bottom-box.test.ts @@ -33,7 +33,7 @@ function getProfileWithNiceAddresses(): { return getProfileFromTextSamples(` A[lib:one][address:20][sym:Asym:20:][file:ab.cpp][line:20] A[lib:one][address:30][sym:Asym:20:][file:ab.cpp][line:22] A[lib:one][address:20][sym:Asym:20:][file:ab.cpp][line:20] A[lib:one][address:20][sym:Asym:20:][file:ab.cpp][line:20] B[lib:one][address:40][sym:Bsym:30:][file:ab.cpp][line:40] B[lib:one][address:30][sym:Asym:20:][file:ab.cpp][line:40][inl:1] B[lib:one][address:45][sym:Bsym:30:][file:ab.cpp][line:43] E[lib:one][address:31][sym:Esym:30:][file:cde.cpp][line:90] - C[lib:one][address:40][sym:Bsym:30:][file:cde.cpp][line:60][inl:1] C[lib:one][address:30][sym:Asym:20:][file:cde.cpp][line:62][inl:2] C[lib:one][address:45][sym:Bsym:30:][file:cde.cpp][line:63] F[lib:two][address:15][sym:Fsym:12:] + C[lib:one][address:40][sym:Bsym:30:][file:cde.cpp][line:60][inl:1] C[lib:one][address:30][sym:Asym:20:][file:cde.cpp][line:62][inl:2] C[lib:one][address:45][sym:Bsym:30:][file:cde.cpp][line:62] F[lib:two][address:15][sym:Fsym:12:] D[lib:one][address:51][sym:Dsym:40:][file:cde.cpp][line:80] `); } @@ -95,7 +95,8 @@ describe('bottom box', function () { const bottomBoxInfoD = getBottomBoxInfoForCallNode( abcd, callNodeInfo, - thread + thread, + thread.samples ); expect(bottomBoxInfoD.nativeSymbols).toEqual([nativeSymbolInfoD]); dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfoD)); @@ -182,7 +183,8 @@ describe('bottom box', function () { const bottomBoxInfoF = getBottomBoxInfoForCallNode( aef, callNodeInfo, - thread + thread, + thread.samples ); expect(bottomBoxInfoF.nativeSymbols).toEqual([nativeSymbolInfoF]); dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfoF)); @@ -232,7 +234,8 @@ describe('bottom box', function () { const bottomBoxInfoC = getBottomBoxInfoForCallNode( abc, callNodeInfo, - thread + thread, + thread.samples ); expect(new Set(bottomBoxInfoC.nativeSymbols)).toEqual( new Set([nativeSymbolInfoA, nativeSymbolInfoB]) @@ -242,8 +245,12 @@ describe('bottom box', function () { // Now the source view should be displayed and the assembly view should be // initialized but closed. The assembly view should show one of the two // native symbols. + // The source view should scroll to line 62 because the call node [A, B, C] + // has 2 samples in line 62 and only 1 sample in line 60. expect(UrlStateSelectors.getIsBottomBoxOpen(getState())).toBeTrue(); expect(ProfileSelectors.getSourceViewFile(getState())).toBe('cde.cpp'); + expect(UrlStateSelectors.getSourceViewScrollToLineNumber(getState())).toBe(62); + expect(UrlStateSelectors.getSourceViewHighlightedLine(getState())).toBe(undefined); expect(UrlStateSelectors.getAssemblyViewIsOpen(getState())).toBeFalse(); expect( ensureExists(UrlStateSelectors.getAssemblyViewNativeSymbol(getState())) @@ -266,7 +273,8 @@ describe('bottom box', function () { const bottomBoxInfoABC = getBottomBoxInfoForCallNode( ensureExists(callNodeInfo.getCallNodeIndexFromPath([A, B, C, D])), callNodeInfo, - thread + thread, + thread.samples ); dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfoABC)); diff --git a/src/test/unit/profile-tree.test.ts b/src/test/unit/profile-tree.test.ts index 4b4c70d513..33a7b5dce7 100644 --- a/src/test/unit/profile-tree.test.ts +++ b/src/test/unit/profile-tree.test.ts @@ -456,6 +456,7 @@ describe('inverted call tree', function () { thread, callNodeInfo, ensureExists(profile.meta.categories), + thread.samples, callTreeTimings, 'samples' ); @@ -497,6 +498,7 @@ describe('inverted call tree', function () { thread, invertedCallNodeInfo, ensureExists(profile.meta.categories), + thread.samples, invertedCallTreeTimings, 'samples' ); diff --git a/src/types/profile-derived.ts b/src/types/profile-derived.ts index c52bf10a71..8a3a37946a 100644 --- a/src/types/profile-derived.ts +++ b/src/types/profile-derived.ts @@ -776,7 +776,6 @@ export type BottomBoxInfo = { sourceIndex: IndexIntoSourceTable | null; nativeSymbols: NativeSymbolInfo[]; // Optional line number to scroll to in the source view. - // If not specified, the source view will scroll to the hottest line. scrollToLineNumber?: number; highlightLineNumber?: number; }; diff --git a/src/types/state.ts b/src/types/state.ts index 0ffcf4c4c5..302a6f0e59 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -252,9 +252,8 @@ export type SourceViewState = { // source. Null if a function without a file path was double clicked. sourceIndex: IndexIntoSourceTable | null; // Optional line number to scroll to in the source view. - // If not specified, the source view will scroll to the hottest line. scrollToLineNumber?: number; - // Optional line number of the highlighted in the source view. + // Optional line number of the highlighted line in the source view. highlightedLine?: number; }; From 0d7cde8626f4708e41642aa21c49c15d46fe11ac Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 14 Jan 2026 12:47:38 -0500 Subject: [PATCH 6/8] Make assembly view scrolling work like source view scrolling. --- src/actions/profile-view.ts | 13 ++--- src/components/app/BottomBox.tsx | 26 +++++---- src/components/shared/AssemblyView.tsx | 53 ++++++----------- src/components/shared/SourceView.tsx | 3 +- src/profile-logic/bottom-box.ts | 37 +++++++++++- src/reducers/url-state.ts | 7 ++- src/selectors/per-thread/index.ts | 43 -------------- src/selectors/url-state.ts | 8 +++ src/test/store/bottom-box.test.ts | 80 +++++++++++++++++++------- src/test/url-handling.test.ts | 4 ++ src/types/actions.ts | 2 + src/types/profile-derived.ts | 4 +- src/types/state.ts | 4 ++ 13 files changed, 160 insertions(+), 124 deletions(-) diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts index eb0e5c08e7..12b9cdf299 100644 --- a/src/actions/profile-view.ts +++ b/src/actions/profile-view.ts @@ -1925,6 +1925,9 @@ export function updateBottomBoxContentsAndMaybeOpen( sourceIndex, scrollToLineNumber, highlightLineNumber, + initialNativeSymbol, + scrollToInstructionAddress, + highlightInstructionAddress, nativeSymbols, } = bottomBoxInfo; @@ -1937,23 +1940,19 @@ export function updateBottomBoxContentsAndMaybeOpen( // view closed - unless the only thing we have is assembly. const shouldOpenAssemblyView = !haveSource && haveAssembly; - // If we have at least one native symbol to show assembly for, pick - // the first one arbitrarily. - // TODO: If we have more than one native symbol, pick the one - // with the highest total sample count. - const currentNativeSymbol = nativeSymbols.length !== 0 ? 0 : null; - return { type: 'UPDATE_BOTTOM_BOX', libIndex, sourceIndex, nativeSymbols, - currentNativeSymbol, + currentNativeSymbol: initialNativeSymbol, currentTab, shouldOpenBottomBox, shouldOpenAssemblyView, scrollToLineNumber, highlightLineNumber, + scrollToInstructionAddress, + highlightInstructionAddress, }; } diff --git a/src/components/app/BottomBox.tsx b/src/components/app/BottomBox.tsx index cd73def7cc..88c36c0c03 100644 --- a/src/components/app/BottomBox.tsx +++ b/src/components/app/BottomBox.tsx @@ -19,11 +19,10 @@ import { getAssemblyViewIsOpen, getAssemblyViewNativeSymbol, getAssemblyViewScrollGeneration, + getAssemblyViewScrollToInstructionAddress, + getAssemblyViewHighlightedInstruction, } from 'firefox-profiler/selectors/url-state'; -import { - selectedThreadSelectors, - selectedNodeSelectors, -} from 'firefox-profiler/selectors/per-thread'; +import { selectedThreadSelectors } from 'firefox-profiler/selectors/per-thread'; import { closeBottomBox } from 'firefox-profiler/actions/profile-view'; import { parseFileNameFromSymbolication } from 'firefox-profiler/utils/special-paths'; import { @@ -60,8 +59,9 @@ type StateProps = { readonly assemblyViewNativeSymbol: NativeSymbolInfo | null; readonly assemblyViewCode: AssemblyCodeStatus | void; readonly assemblyViewScrollGeneration: number; + readonly assemblyViewScrollToInstructionAddress?: number; + readonly assemblyViewHighlightedInstruction?: number; readonly globalAddressTimings: AddressTimings; - readonly selectedCallNodeAddressTimings: AddressTimings; }; type DispatchProps = { @@ -162,10 +162,11 @@ class BottomBoxImpl extends React.PureComponent { sourceViewHighlightedLine, assemblyViewIsOpen, assemblyViewScrollGeneration, + assemblyViewScrollToInstructionAddress, + assemblyViewHighlightedInstruction, assemblyViewNativeSymbol, assemblyViewCode, globalAddressTimings, - selectedCallNodeAddressTimings, } = this.props; const sourceCode = sourceViewCode && sourceViewCode.type === 'AVAILABLE' @@ -260,8 +261,11 @@ class BottomBoxImpl extends React.PureComponent { timings={globalAddressTimings} assemblyCode={assemblyCode} nativeSymbol={assemblyViewNativeSymbol} - scrollToHotSpotGeneration={assemblyViewScrollGeneration} - hotSpotTimings={selectedCallNodeAddressTimings} + scrollGeneration={assemblyViewScrollGeneration} + scrollToInstructionAddress={ + assemblyViewScrollToInstructionAddress + } + highlightedInstruction={assemblyViewHighlightedInstruction} ref={this._assemblyView} /> ) : null} @@ -301,9 +305,11 @@ export const BottomBox = explicitConnect<{}, StateProps, DispatchProps>({ assemblyViewCode: getAssemblyViewCode(state), globalAddressTimings: selectedThreadSelectors.getAssemblyViewAddressTimings(state), - selectedCallNodeAddressTimings: - selectedNodeSelectors.getAssemblyViewAddressTimings(state), assemblyViewScrollGeneration: getAssemblyViewScrollGeneration(state), + assemblyViewScrollToInstructionAddress: + getAssemblyViewScrollToInstructionAddress(state), + assemblyViewHighlightedInstruction: + getAssemblyViewHighlightedInstruction(state), assemblyViewIsOpen: getAssemblyViewIsOpen(state), }), mapDispatchToProps: { diff --git a/src/components/shared/AssemblyView.tsx b/src/components/shared/AssemblyView.tsx index 2d443ea4b5..8d5958d6ce 100644 --- a/src/components/shared/AssemblyView.tsx +++ b/src/components/shared/AssemblyView.tsx @@ -9,7 +9,6 @@ import type { NativeSymbolInfo, DecodedInstruction, } from 'firefox-profiler/types'; -import { mapGetKeyWithMaxValue } from 'firefox-profiler/utils'; import type { AssemblyViewEditor } from './AssemblyView-codemirror'; @@ -45,8 +44,9 @@ type AssemblyViewProps = { readonly timings: AddressTimings; readonly assemblyCode: DecodedInstruction[]; readonly nativeSymbol: NativeSymbolInfo | null; - readonly scrollToHotSpotGeneration: number; - readonly hotSpotTimings: AddressTimings; + readonly scrollGeneration: number; + readonly scrollToInstructionAddress?: number; + readonly highlightedInstruction?: number; }; let editorModulePromise: Promise | null = null; @@ -55,36 +55,6 @@ export class AssemblyView extends React.PureComponent { _ref = React.createRef(); _editor: AssemblyViewEditor | null = null; - /** - * Scroll to the line with the most hits, based on the timings in - * timingsForScrolling. - * - * How is timingsForScrolling different from this.props.timings? - * In the current implementation, this.props.timings are always the "global" - * timings, i.e. they show the line hits for all samples in the current view, - * regardless of the selected call node. However, when opening the assembly - * view from a specific call node, you really want to see the code that's - * relevant to that specific call node, or at least that specific function. - * So timingsForScrolling are the timings that indicate just the line hits - * in the selected call node. This means that the "hotspot" will be somewhere - * in the selected function, and it will even be in the line that's most - * relevant to that specific call node. - * - * Sometimes, timingsForScrolling can be completely empty. This happens, for - * example, when the assembly view is showing a different file than the - * selected call node's function's file, for example because we just loaded - * from a URL and ended up with an arbitrary selected call node. - * In that case, pick the hotspot from the global line timings. - */ - _scrollToHotSpot(timingsForScrolling: AddressTimings) { - const heaviestAddress = - mapGetKeyWithMaxValue(timingsForScrolling.totalAddressHits) ?? - mapGetKeyWithMaxValue(this.props.timings.totalAddressHits); - if (heaviestAddress !== undefined) { - this._scrollToAddressWithSpaceOnTop(heaviestAddress, 5); - } - } - _scrollToAddressWithSpaceOnTop(address: number, topSpaceLines: number) { if (this._editor) { this._editor.scrollToAddressWithSpaceOnTop(address, topSpaceLines); @@ -152,7 +122,12 @@ export class AssemblyView extends React.PureComponent { domParent ); this._editor = editor; - this._scrollToHotSpot(this.props.hotSpotTimings); + if (this.props.scrollToInstructionAddress !== undefined) { + this._scrollToAddressWithSpaceOnTop( + this.props.scrollToInstructionAddress, + 5 + ); + } })(); } @@ -176,10 +151,14 @@ export class AssemblyView extends React.PureComponent { if ( contentsChanged || - this.props.scrollToHotSpotGeneration !== - prevProps.scrollToHotSpotGeneration + this.props.scrollGeneration !== prevProps.scrollGeneration ) { - this._scrollToHotSpot(this.props.hotSpotTimings); + if (this.props.scrollToInstructionAddress !== undefined) { + this._scrollToAddressWithSpaceOnTop( + this.props.scrollToInstructionAddress, + 5 + ); + } } if (this.props.timings !== prevProps.timings) { diff --git a/src/components/shared/SourceView.tsx b/src/components/shared/SourceView.tsx index 02c2b8c848..7464c6a8c0 100644 --- a/src/components/shared/SourceView.tsx +++ b/src/components/shared/SourceView.tsx @@ -144,8 +144,7 @@ export class SourceView extends React.PureComponent { if ( contentsChanged || - this.props.scrollGeneration !== - prevProps.scrollGeneration + this.props.scrollGeneration !== prevProps.scrollGeneration ) { // If an explicit line number is provided, scroll to it. Otherwise, scroll to the hotspot. if (this.props.scrollToLineNumber !== undefined) { diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts index dd8ef453c5..a953b09bf0 100644 --- a/src/profile-logic/bottom-box.ts +++ b/src/profile-logic/bottom-box.ts @@ -18,6 +18,10 @@ import { } from './profile-data'; import { getLineTimings, getStackLineInfoForCallNode } from './line-timings'; import { mapGetKeyWithMaxValue } from 'firefox-profiler/utils'; +import { + getAddressTimings, + getStackAddressInfoForCallNode, +} from './address-timings'; /** * Calculate the BottomBoxInfo for a call node, i.e. information about which @@ -55,6 +59,13 @@ export function getBottomBoxInfoForCallNode( stackTable, frameTable ); + + // If we have at least one native symbol to show assembly for, pick + // the first one arbitrarily. + // TODO: If the we have more than one native symbol, pick the one + // with the highest total sample count. + const initialNativeSymbol = nativeSymbolsForCallNode.length !== 0 ? 0 : null; + const nativeSymbolInfosForCallNode = nativeSymbolsForCallNode.map( (nativeSymbolIndex) => getNativeSymbolInfo( @@ -65,7 +76,7 @@ export function getBottomBoxInfoForCallNode( ) ); - // Compute the hottest line, and ask the source view to scroll to it. + // Compute the hottest line, so we can ask the source view to scroll to it. const stackLineInfo = getStackLineInfoForCallNode( stackTable, frameTable, @@ -76,11 +87,29 @@ export function getBottomBoxInfoForCallNode( const callNodeLineTimings = getLineTimings(stackLineInfo, samples); const hottestLine = mapGetKeyWithMaxValue(callNodeLineTimings.totalLineHits); + // Compute the hottest instruction, so we can ask the assembly view to scroll to it. + let hottestInstructionAddress; + if (initialNativeSymbol !== null) { + const stackAddressInfo = getStackAddressInfoForCallNode( + stackTable, + frameTable, + callNodeIndex, + callNodeInfo, + nativeSymbolsForCallNode[initialNativeSymbol] + ); + const callNodeAddressTimings = getAddressTimings(stackAddressInfo, samples); + hottestInstructionAddress = mapGetKeyWithMaxValue( + callNodeAddressTimings.totalAddressHits + ); + } + return { libIndex, sourceIndex, nativeSymbols: nativeSymbolInfosForCallNode, + initialNativeSymbol, scrollToLineNumber: hottestLine, + scrollToInstructionAddress: hottestInstructionAddress, }; } @@ -124,6 +153,9 @@ export function getBottomBoxInfoForStackFrame( ] : []; + const instructionAddress = + nativeSymbol !== null ? frameTable.address[frameIndex] : undefined; + // Extract line number from the frame const lineNumber = frameTable.line[frameIndex] ?? undefined; @@ -131,7 +163,10 @@ export function getBottomBoxInfoForStackFrame( libIndex, sourceIndex, nativeSymbols: nativeSymbolInfos, + initialNativeSymbol: 0, scrollToLineNumber: lineNumber, highlightLineNumber: lineNumber, + scrollToInstructionAddress: instructionAddress, + highlightInstructionAddress: instructionAddress, }; } diff --git a/src/reducers/url-state.ts b/src/reducers/url-state.ts index be3d4329a9..463f5a93e9 100644 --- a/src/reducers/url-state.ts +++ b/src/reducers/url-state.ts @@ -599,11 +599,16 @@ const assemblyView: Reducer = ( case 'UPDATE_BOTTOM_BOX': { const { nativeSymbols, currentNativeSymbol, shouldOpenAssemblyView } = action; + const shouldScroll = action.scrollToInstructionAddress !== undefined; return { - scrollGeneration: state.scrollGeneration + 1, + scrollGeneration: shouldScroll + ? state.scrollGeneration + 1 + : state.scrollGeneration, nativeSymbols, currentNativeSymbol, isOpen: state.isOpen || shouldOpenAssemblyView, + scrollToInstructionAddress: action.scrollToInstructionAddress, + highlightedInstruction: action.highlightInstructionAddress, }; } case 'OPEN_ASSEMBLY_VIEW': { diff --git a/src/selectors/per-thread/index.ts b/src/selectors/per-thread/index.ts index aa3f7b4a65..130c55a0a0 100644 --- a/src/selectors/per-thread/index.ts +++ b/src/selectors/per-thread/index.ts @@ -23,20 +23,13 @@ import { getComposedSelectorsPerThread, type ComposedSelectorsPerThread, } from './composed'; -import { - getStackAddressInfoForCallNode, - getAddressTimings, -} from '../../profile-logic/address-timings'; import * as ProfileSelectors from '../profile'; import { ensureExists, getFirstItemFromSet } from '../../utils/types'; import type { - Thread, ThreadIndex, Selector, ThreadsKey, - StackAddressInfo, - AddressTimings, State, } from 'firefox-profiler/types'; @@ -199,8 +192,6 @@ export type NodeSelectors = { readonly getIsJS: Selector; readonly getLib: Selector; readonly getTimingsForSidebar: Selector; - readonly getAssemblyViewStackAddressInfo: Selector; - readonly getAssemblyViewAddressTimings: Selector; }; export const selectedNodeSelectors: NodeSelectors = (() => { @@ -260,44 +251,10 @@ export const selectedNodeSelectors: NodeSelectors = (() => { ProfileData.getTimingsForPath ); - const getAssemblyViewStackAddressInfo: Selector = - createSelector( - selectedThreadSelectors.getFilteredThread, - selectedThreadSelectors.getAssemblyViewNativeSymbolIndex, - selectedThreadSelectors.getCallNodeInfo, - selectedThreadSelectors.getSelectedCallNodeIndex, - ( - { stackTable, frameTable }: Thread, - nativeSymbolIndex, - callNodeInfo, - selectedCallNodeIndex - ): StackAddressInfo | null => { - if (nativeSymbolIndex === null || selectedCallNodeIndex === null) { - return null; - } - return getStackAddressInfoForCallNode( - stackTable, - frameTable, - selectedCallNodeIndex, - callNodeInfo, - nativeSymbolIndex - ); - } - ); - - const getAssemblyViewAddressTimings: Selector = - createSelector( - getAssemblyViewStackAddressInfo, - selectedThreadSelectors.getPreviewFilteredCtssSamples, - getAddressTimings - ); - return { getName, getIsJS, getLib, getTimingsForSidebar, - getAssemblyViewStackAddressInfo, - getAssemblyViewAddressTimings, }; })(); diff --git a/src/selectors/url-state.ts b/src/selectors/url-state.ts index 4cf2c27ff2..0c5dd66d39 100644 --- a/src/selectors/url-state.ts +++ b/src/selectors/url-state.ts @@ -94,6 +94,14 @@ export const getAssemblyViewNativeSymbol: Selector = ( }; export const getAssemblyViewScrollGeneration: Selector = (state) => getProfileSpecificState(state).assemblyView.scrollGeneration; +export const getAssemblyViewScrollToInstructionAddress: Selector< + number | undefined +> = (state) => + getProfileSpecificState(state).assemblyView.scrollToInstructionAddress; +export const getAssemblyViewHighlightedInstruction: Selector< + number | undefined +> = (state) => + getProfileSpecificState(state).assemblyView.highlightedInstruction; export const getShowJsTracerSummary: Selector = (state) => getProfileSpecificState(state).showJsTracerSummary; diff --git a/src/test/store/bottom-box.test.ts b/src/test/store/bottom-box.test.ts index b8130ab41a..baf86fa371 100644 --- a/src/test/store/bottom-box.test.ts +++ b/src/test/store/bottom-box.test.ts @@ -6,10 +6,7 @@ import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profil import { storeWithProfile } from '../fixtures/stores'; import * as UrlStateSelectors from '../../selectors/url-state'; import * as ProfileSelectors from '../../selectors/profile'; -import { - selectedThreadSelectors, - selectedNodeSelectors, -} from '../../selectors/per-thread'; +import { selectedThreadSelectors } from '../../selectors/per-thread'; import { emptyAddressTimings } from '../../profile-logic/address-timings'; import { getBottomBoxInfoForCallNode } from '../../profile-logic/bottom-box'; import { @@ -249,8 +246,12 @@ describe('bottom box', function () { // has 2 samples in line 62 and only 1 sample in line 60. expect(UrlStateSelectors.getIsBottomBoxOpen(getState())).toBeTrue(); expect(ProfileSelectors.getSourceViewFile(getState())).toBe('cde.cpp'); - expect(UrlStateSelectors.getSourceViewScrollToLineNumber(getState())).toBe(62); - expect(UrlStateSelectors.getSourceViewHighlightedLine(getState())).toBe(undefined); + expect(UrlStateSelectors.getSourceViewScrollToLineNumber(getState())).toBe( + 62 + ); + expect(UrlStateSelectors.getSourceViewHighlightedLine(getState())).toBe( + undefined + ); expect(UrlStateSelectors.getAssemblyViewIsOpen(getState())).toBeFalse(); expect( ensureExists(UrlStateSelectors.getAssemblyViewNativeSymbol(getState())) @@ -270,43 +271,78 @@ describe('bottom box', function () { // is what the call tree usually does on its own), and then we open the bottom // box with that info. dispatch(changeSelectedCallNode(threadsKey, [A, B, C, D])); - const bottomBoxInfoABC = getBottomBoxInfoForCallNode( + const bottomBoxInfoABCD = getBottomBoxInfoForCallNode( ensureExists(callNodeInfo.getCallNodeIndexFromPath([A, B, C, D])), callNodeInfo, thread, thread.samples ); - dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfoABC)); + dispatch( + updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfoABCD) + ); - // Check the assembly view address timings, both the (thread-)global timings - // and the timings for the selected call node. - // Note the difference between "selectedThreadSelectors" and "selectedNodeSelectors" below. - // Both timings should be identical here because Dsym is selected and because - // there is no recursion on Dsym. + // Check the assembly view state, including address timings. + expect(UrlStateSelectors.getAssemblyViewNativeSymbol(getState())).toEqual({ + libIndex: 0, + address: 0x40, + name: 'Dsym', + functionSize: 18, + functionSizeIsKnown: false, + }); expect( selectedThreadSelectors.getAssemblyViewAddressTimings(getState()) .totalAddressHits ).toEqual(new Map([[0x51, 1]])); expect( - selectedNodeSelectors.getAssemblyViewAddressTimings(getState()) - .totalAddressHits - ).toEqual(new Map([[0x51, 1]])); + UrlStateSelectors.getAssemblyViewHighlightedInstruction(getState()) + ).toBe(undefined); + expect( + UrlStateSelectors.getAssemblyViewScrollToInstructionAddress(getState()) + ).toBe(0x51); // Select the call node at [A, B, C]. - dispatch(changeSelectedCallNode(threadsKey, [A, B, C])); - // The global timings should still remain the same. + dispatch(changeSelectedCallNode(threadsKey, [A, B, C])); expect( selectedThreadSelectors.getAssemblyViewAddressTimings(getState()) .totalAddressHits ).toEqual(new Map([[0x51, 1]])); - // The timings for the selected call node should have dropped to zero, - // because the call node at [A, B, C] does not have any frames in Dsym. + // Change the selection back to call node [A, B, C, D], and then open the + // bottom box for the call node [A, B, C]. This simulates opening the + // bottom box from the context menu. + dispatch(changeSelectedCallNode(threadsKey, [A, B, C, D])); + const bottomBoxInfoABC = getBottomBoxInfoForCallNode( + ensureExists(callNodeInfo.getCallNodeIndexFromPath([A, B, C])), + callNodeInfo, + thread, + thread.samples + ); + dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfoABC)); + + // Check the assembly view state again. + expect(UrlStateSelectors.getAssemblyViewNativeSymbol(getState())).toEqual({ + libIndex: 0, + address: 0x30, + name: 'Bsym', + functionSize: 22, + functionSizeIsKnown: false, + }); expect( - selectedNodeSelectors.getAssemblyViewAddressTimings(getState()) + selectedThreadSelectors.getAssemblyViewAddressTimings(getState()) .totalAddressHits - ).toEqual(new Map()); + ).toEqual( + new Map([ + [0x40, 1], + [0x45, 1], + ]) + ); + expect( + UrlStateSelectors.getAssemblyViewHighlightedInstruction(getState()) + ).toBe(undefined); + expect( + UrlStateSelectors.getAssemblyViewScrollToInstructionAddress(getState()) + ).toBeOneOf([0x40, 0x45]); }); // Further ideas for tests: diff --git a/src/test/url-handling.test.ts b/src/test/url-handling.test.ts index eb692cfb23..c71786ca12 100644 --- a/src/test/url-handling.test.ts +++ b/src/test/url-handling.test.ts @@ -1801,6 +1801,7 @@ describe('URL persistence of bottom box (source view and assembly view)', functi libIndex: 0, sourceIndex, nativeSymbols: [], + initialNativeSymbol: null, }; dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo)); const newStore = _getStoreFromStateAfterUrlRoundtrip(getState()); @@ -1833,6 +1834,7 @@ describe('URL persistence of bottom box (source view and assembly view)', functi libIndex: 0, sourceIndex, nativeSymbols: [], + initialNativeSymbol: null, }; dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo)); dispatch(closeBottomBox()); @@ -1861,6 +1863,7 @@ describe('URL persistence of bottom box (source view and assembly view)', functi libIndex: 0, sourceIndex: null, nativeSymbols: [nativeSymbolInfo], + initialNativeSymbol: 0, }; dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo)); const newStore = _getStoreFromStateAfterUrlRoundtrip(getState()); @@ -1903,6 +1906,7 @@ describe('URL persistence of bottom box (source view and assembly view)', functi libIndex: 0, sourceIndex, nativeSymbols: [nativeSymbolInfo], + initialNativeSymbol: 0, }; dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo)); const newStore = _getStoreFromStateAfterUrlRoundtrip(getState()); diff --git a/src/types/actions.ts b/src/types/actions.ts index 1709a40394..34ada128db 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -345,6 +345,8 @@ type ProfileAction = readonly shouldOpenAssemblyView: boolean; readonly scrollToLineNumber?: number; readonly highlightLineNumber?: number; + readonly scrollToInstructionAddress?: number; + readonly highlightInstructionAddress?: number; } | { readonly type: 'OPEN_ASSEMBLY_VIEW'; diff --git a/src/types/profile-derived.ts b/src/types/profile-derived.ts index 8a3a37946a..f113607c3b 100644 --- a/src/types/profile-derived.ts +++ b/src/types/profile-derived.ts @@ -775,9 +775,11 @@ export type BottomBoxInfo = { libIndex: IndexIntoLibs | null; sourceIndex: IndexIntoSourceTable | null; nativeSymbols: NativeSymbolInfo[]; - // Optional line number to scroll to in the source view. + initialNativeSymbol: number | null; // index into `nativeSymbols` scrollToLineNumber?: number; highlightLineNumber?: number; + scrollToInstructionAddress?: number; + highlightInstructionAddress?: number; }; /** diff --git a/src/types/state.ts b/src/types/state.ts index 302a6f0e59..05989e2a77 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -263,6 +263,10 @@ export type AssemblyViewState = { isOpen: boolean; // When this is incremented, the assembly view scrolls to the "hotspot" line. scrollGeneration: number; + // Optional instruction address to scroll to in the assembly view. + scrollToInstructionAddress?: number; + // Optional address of the highlighted instruction in the assembly view. + highlightedInstruction?: number; // The list of native symbols whose assembly code should be accessible in the // assembly view. Often empty or just one element, but sometimes this can have // multiple elements, for example the different JIT compilations of one JS function. From 4cb91b52608a4e18969363513f2c12a9e62e40e3 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 14 Jan 2026 15:54:35 -0500 Subject: [PATCH 7/8] Implement highlightedInstruction support in the assembly view. --- .../shared/AssemblyView-codemirror.tsx | 41 ++++++++++++++++++- src/components/shared/AssemblyView.tsx | 9 ++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/components/shared/AssemblyView-codemirror.tsx b/src/components/shared/AssemblyView-codemirror.tsx index b57cb3c850..f0efaa4ec9 100644 --- a/src/components/shared/AssemblyView-codemirror.tsx +++ b/src/components/shared/AssemblyView-codemirror.tsx @@ -19,7 +19,12 @@ * width of the editor, it covers both the gutter and the main area. */ import { EditorView, gutter } from '@codemirror/view'; -import { EditorState, StateField, StateEffect } from '@codemirror/state'; +import { + EditorState, + StateField, + StateEffect, + Compartment, +} from '@codemirror/state'; import { syntaxHighlighting } from '@codemirror/language'; import { classHighlighter } from '@lezer/highlight'; import clamp from 'clamp'; @@ -37,12 +42,16 @@ import { timingsExtension, updateTimingsEffect, StringMarker, + createHighlightedLineExtension, } from 'firefox-profiler/utils/codemirror-shared'; // An "effect" is like a redux action. This effect is used to replace the value // of the state field addressToLineMapField. const updateAddressToLineMapEffect = StateEffect.define(); +// This "compartment" allows us to swap the highlighted line when it changes. +const highlightedLineConf = new Compartment(); + // This "state field" stores the current AddressToLineMap. This field allows the // instructionAddressGutter to map line numbers to addresses. const addressToLineMapField = StateField.define({ @@ -174,23 +183,31 @@ export class AssemblyViewEditor { _view: EditorView; _addressToLineMap: AddressToLineMap; _addressTimings: AddressTimings; + _highlightedAddress: Address | null; // Create a CodeMirror editor and add it as a child element of domParent. constructor( initialAssemblyCode: DecodedInstruction[], addressTimings: AddressTimings, + highlightedAddress: Address | null, domParent: Element ) { this._addressToLineMap = new AddressToLineMap( getInstructionAddresses(initialAssemblyCode) ); this._addressTimings = addressTimings; + this._highlightedAddress = highlightedAddress; + const highlightedLine = + highlightedAddress !== null + ? this._addressToLineMap.addressToLine(highlightedAddress) + : null; let state = EditorState.create({ doc: instructionsToText(initialAssemblyCode), extensions: [ timingsExtension, addressToLineMapField, instructionAddressGutter, + highlightedLineConf.of(createHighlightedLineExtension(highlightedLine)), syntaxHighlighting(classHighlighter), EditorState.readOnly.of(true), EditorView.editable.of(false), @@ -220,6 +237,11 @@ export class AssemblyViewEditor { this._addressTimings, this._addressToLineMap ); + // Recalculate the highlighted line based on the new address-to-line mapping. + const highlightedLine = + this._highlightedAddress !== null + ? this._addressToLineMap.addressToLine(this._highlightedAddress) + : null; // The CodeMirror way of replacing the entire contents is to insert new text // and overwrite the full range of existing text. const text = instructionsToText(assemblyCode); @@ -236,6 +258,9 @@ export class AssemblyViewEditor { effects: [ updateAddressToLineMapEffect.of(this._addressToLineMap), updateTimingsEffect.of(lineTimings), + highlightedLineConf.reconfigure( + createHighlightedLineExtension(highlightedLine) + ), ], }); } @@ -280,4 +305,18 @@ export class AssemblyViewEditor { this.scrollToLine(lineNumber - topSpaceLines); } } + + setHighlightedInstruction(address: Address | null) { + // Store the highlighted address so we can recalculate the line number + // when the address-to-line mapping changes. + this._highlightedAddress = address; + // Convert the address to a line number and update the highlighted line. + const lineNumber = + address !== null ? this._addressToLineMap.addressToLine(address) : null; + this._view.dispatch({ + effects: highlightedLineConf.reconfigure( + createHighlightedLineExtension(lineNumber) + ), + }); + } } diff --git a/src/components/shared/AssemblyView.tsx b/src/components/shared/AssemblyView.tsx index 8d5958d6ce..62fcee9861 100644 --- a/src/components/shared/AssemblyView.tsx +++ b/src/components/shared/AssemblyView.tsx @@ -119,6 +119,7 @@ export class AssemblyView extends React.PureComponent { const editor = new AssemblyViewEditor( this._getAssemblyCodeOrFallback(), this.props.timings, + this.props.highlightedInstruction ?? null, domParent ); this._editor = editor; @@ -164,5 +165,13 @@ export class AssemblyView extends React.PureComponent { if (this.props.timings !== prevProps.timings) { this._editor.setTimings(this.props.timings); } + + if ( + this.props.highlightedInstruction !== prevProps.highlightedInstruction + ) { + this._editor.setHighlightedInstruction( + this.props.highlightedInstruction ?? null + ); + } } } From 98b532835f1556715a9c75903ee649d0bbc9cf60 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 19 Jan 2026 15:22:57 -0500 Subject: [PATCH 8/8] Make it clear that the highlight will be unset when not specified. --- src/actions/profile-view.ts | 12 ++++++------ src/app-logic/url-handling.ts | 2 ++ src/components/app/BottomBox.tsx | 4 ++-- src/components/shared/AssemblyView.tsx | 8 +++----- src/components/shared/SourceView.tsx | 6 +++--- src/profile-logic/bottom-box.ts | 16 ++++++++++------ src/reducers/url-state.ts | 14 ++++++++++---- src/selectors/url-state.ts | 12 +++++------- src/test/store/bottom-box.test.ts | 6 +++--- src/test/url-handling.test.ts | 17 +++++++++++++---- src/types/actions.ts | 4 ++-- src/types/profile-derived.ts | 7 +++++-- src/types/state.ts | 12 ++++++------ 13 files changed, 70 insertions(+), 50 deletions(-) diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts index 12b9cdf299..56bef8ff15 100644 --- a/src/actions/profile-view.ts +++ b/src/actions/profile-view.ts @@ -1923,12 +1923,12 @@ export function updateBottomBoxContentsAndMaybeOpen( const { libIndex, sourceIndex, - scrollToLineNumber, - highlightLineNumber, + nativeSymbols, initialNativeSymbol, + scrollToLineNumber, scrollToInstructionAddress, - highlightInstructionAddress, - nativeSymbols, + highlightedLineNumber, + highlightedInstructionAddress, } = bottomBoxInfo; const haveSource = sourceIndex !== null; @@ -1950,9 +1950,9 @@ export function updateBottomBoxContentsAndMaybeOpen( shouldOpenBottomBox, shouldOpenAssemblyView, scrollToLineNumber, - highlightLineNumber, scrollToInstructionAddress, - highlightInstructionAddress, + highlightedLineNumber, + highlightedInstructionAddress, }; } diff --git a/src/app-logic/url-handling.ts b/src/app-logic/url-handling.ts index 0976efba13..f6782d7a93 100644 --- a/src/app-logic/url-handling.ts +++ b/src/app-logic/url-handling.ts @@ -511,12 +511,14 @@ export function stateFromLocation( scrollGeneration: 0, libIndex: null, sourceIndex: null, + highlightedLine: null, }; const assemblyView: AssemblyViewState = { isOpen: false, scrollGeneration: 0, nativeSymbols: [], currentNativeSymbol: null, + highlightedInstruction: null, }; const isBottomBoxOpenPerPanel: any = {}; tabSlugs.forEach((tabSlug) => (isBottomBoxOpenPerPanel[tabSlug] = false)); diff --git a/src/components/app/BottomBox.tsx b/src/components/app/BottomBox.tsx index 88c36c0c03..2411bd4fe1 100644 --- a/src/components/app/BottomBox.tsx +++ b/src/components/app/BottomBox.tsx @@ -53,14 +53,14 @@ type StateProps = { readonly sourceViewCode: SourceCodeStatus | void; readonly sourceViewScrollGeneration: number; readonly sourceViewScrollToLineNumber?: number; - readonly sourceViewHighlightedLine?: number; + readonly sourceViewHighlightedLine: number | null; readonly globalLineTimings: LineTimings; readonly assemblyViewIsOpen: boolean; readonly assemblyViewNativeSymbol: NativeSymbolInfo | null; readonly assemblyViewCode: AssemblyCodeStatus | void; readonly assemblyViewScrollGeneration: number; readonly assemblyViewScrollToInstructionAddress?: number; - readonly assemblyViewHighlightedInstruction?: number; + readonly assemblyViewHighlightedInstruction: number | null; readonly globalAddressTimings: AddressTimings; }; diff --git a/src/components/shared/AssemblyView.tsx b/src/components/shared/AssemblyView.tsx index 62fcee9861..37108a6903 100644 --- a/src/components/shared/AssemblyView.tsx +++ b/src/components/shared/AssemblyView.tsx @@ -46,7 +46,7 @@ type AssemblyViewProps = { readonly nativeSymbol: NativeSymbolInfo | null; readonly scrollGeneration: number; readonly scrollToInstructionAddress?: number; - readonly highlightedInstruction?: number; + readonly highlightedInstruction: number | null; }; let editorModulePromise: Promise | null = null; @@ -119,7 +119,7 @@ export class AssemblyView extends React.PureComponent { const editor = new AssemblyViewEditor( this._getAssemblyCodeOrFallback(), this.props.timings, - this.props.highlightedInstruction ?? null, + this.props.highlightedInstruction, domParent ); this._editor = editor; @@ -169,9 +169,7 @@ export class AssemblyView extends React.PureComponent { if ( this.props.highlightedInstruction !== prevProps.highlightedInstruction ) { - this._editor.setHighlightedInstruction( - this.props.highlightedInstruction ?? null - ); + this._editor.setHighlightedInstruction(this.props.highlightedInstruction); } } } diff --git a/src/components/shared/SourceView.tsx b/src/components/shared/SourceView.tsx index 7464c6a8c0..9e722f1f8b 100644 --- a/src/components/shared/SourceView.tsx +++ b/src/components/shared/SourceView.tsx @@ -42,7 +42,7 @@ type SourceViewProps = { readonly filePath: string | null; readonly scrollGeneration: number; readonly scrollToLineNumber?: number; - readonly highlightedLine?: number; + readonly highlightedLine: number | null; }; let editorModulePromise: Promise | null = null; @@ -109,7 +109,7 @@ export class SourceView extends React.PureComponent { this._getSourceCodeOrFallback(), this.props.filePath, this.props.timings, - this.props.highlightedLine ?? null, + this.props.highlightedLine, domParent ); this._editor = editor; @@ -157,7 +157,7 @@ export class SourceView extends React.PureComponent { } if (this.props.highlightedLine !== prevProps.highlightedLine) { - this._editor.setHighlightedLine(this.props.highlightedLine ?? null); + this._editor.setHighlightedLine(this.props.highlightedLine); } } } diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts index a953b09bf0..a470105912 100644 --- a/src/profile-logic/bottom-box.ts +++ b/src/profile-logic/bottom-box.ts @@ -110,6 +110,8 @@ export function getBottomBoxInfoForCallNode( initialNativeSymbol, scrollToLineNumber: hottestLine, scrollToInstructionAddress: hottestInstructionAddress, + highlightedLineNumber: null, + highlightedInstructionAddress: null, }; } @@ -154,19 +156,21 @@ export function getBottomBoxInfoForStackFrame( : []; const instructionAddress = - nativeSymbol !== null ? frameTable.address[frameIndex] : undefined; + nativeSymbol !== null ? frameTable.address[frameIndex] : -1; // Extract line number from the frame - const lineNumber = frameTable.line[frameIndex] ?? undefined; + const lineNumber = frameTable.line[frameIndex]; return { libIndex, sourceIndex, nativeSymbols: nativeSymbolInfos, initialNativeSymbol: 0, - scrollToLineNumber: lineNumber, - highlightLineNumber: lineNumber, - scrollToInstructionAddress: instructionAddress, - highlightInstructionAddress: instructionAddress, + scrollToLineNumber: lineNumber ?? undefined, + highlightedLineNumber: lineNumber, + scrollToInstructionAddress: + instructionAddress !== -1 ? instructionAddress : undefined, + highlightedInstructionAddress: + instructionAddress !== -1 ? instructionAddress : null, }; } diff --git a/src/reducers/url-state.ts b/src/reducers/url-state.ts index 463f5a93e9..834290828d 100644 --- a/src/reducers/url-state.ts +++ b/src/reducers/url-state.ts @@ -565,7 +565,12 @@ const profileName: Reducer = (state = null, action) => { }; const sourceView: Reducer = ( - state = { scrollGeneration: 0, libIndex: null, sourceIndex: null }, + state = { + scrollGeneration: 0, + libIndex: null, + sourceIndex: null, + highlightedLine: null, + }, action ) => { switch (action.type) { @@ -578,7 +583,7 @@ const sourceView: Reducer = ( libIndex: action.libIndex, sourceIndex: action.sourceIndex, scrollToLineNumber: action.scrollToLineNumber, - highlightedLine: action.highlightLineNumber, + highlightedLine: action.highlightedLineNumber, }; } default: @@ -588,10 +593,11 @@ const sourceView: Reducer = ( const assemblyView: Reducer = ( state = { + isOpen: false, scrollGeneration: 0, + highlightedInstruction: null, nativeSymbols: [], currentNativeSymbol: null, - isOpen: false, }, action ) => { @@ -608,7 +614,7 @@ const assemblyView: Reducer = ( currentNativeSymbol, isOpen: state.isOpen || shouldOpenAssemblyView, scrollToInstructionAddress: action.scrollToInstructionAddress, - highlightedInstruction: action.highlightInstructionAddress, + highlightedInstruction: action.highlightedInstructionAddress, }; } case 'OPEN_ASSEMBLY_VIEW': { diff --git a/src/selectors/url-state.ts b/src/selectors/url-state.ts index 0c5dd66d39..8f02b53cbf 100644 --- a/src/selectors/url-state.ts +++ b/src/selectors/url-state.ts @@ -78,9 +78,8 @@ export const getSourceViewScrollGeneration: Selector = (state) => export const getSourceViewScrollToLineNumber: Selector = ( state ) => getProfileSpecificState(state).sourceView.scrollToLineNumber; -export const getSourceViewHighlightedLine: Selector = ( - state -) => getProfileSpecificState(state).sourceView.highlightedLine; +export const getSourceViewHighlightedLine: Selector = (state) => + getProfileSpecificState(state).sourceView.highlightedLine; export const getAssemblyViewIsOpen: Selector = (state) => getProfileSpecificState(state).assemblyView.isOpen; export const getAssemblyViewNativeSymbol: Selector = ( @@ -98,10 +97,9 @@ export const getAssemblyViewScrollToInstructionAddress: Selector< number | undefined > = (state) => getProfileSpecificState(state).assemblyView.scrollToInstructionAddress; -export const getAssemblyViewHighlightedInstruction: Selector< - number | undefined -> = (state) => - getProfileSpecificState(state).assemblyView.highlightedInstruction; +export const getAssemblyViewHighlightedInstruction: Selector = ( + state +) => getProfileSpecificState(state).assemblyView.highlightedInstruction; export const getShowJsTracerSummary: Selector = (state) => getProfileSpecificState(state).showJsTracerSummary; diff --git a/src/test/store/bottom-box.test.ts b/src/test/store/bottom-box.test.ts index baf86fa371..f410ee3a3c 100644 --- a/src/test/store/bottom-box.test.ts +++ b/src/test/store/bottom-box.test.ts @@ -250,7 +250,7 @@ describe('bottom box', function () { 62 ); expect(UrlStateSelectors.getSourceViewHighlightedLine(getState())).toBe( - undefined + null ); expect(UrlStateSelectors.getAssemblyViewIsOpen(getState())).toBeFalse(); expect( @@ -295,7 +295,7 @@ describe('bottom box', function () { ).toEqual(new Map([[0x51, 1]])); expect( UrlStateSelectors.getAssemblyViewHighlightedInstruction(getState()) - ).toBe(undefined); + ).toBe(null); expect( UrlStateSelectors.getAssemblyViewScrollToInstructionAddress(getState()) ).toBe(0x51); @@ -339,7 +339,7 @@ describe('bottom box', function () { ); expect( UrlStateSelectors.getAssemblyViewHighlightedInstruction(getState()) - ).toBe(undefined); + ).toBe(null); expect( UrlStateSelectors.getAssemblyViewScrollToInstructionAddress(getState()) ).toBeOneOf([0x40, 0x45]); diff --git a/src/test/url-handling.test.ts b/src/test/url-handling.test.ts index c71786ca12..665069f8f1 100644 --- a/src/test/url-handling.test.ts +++ b/src/test/url-handling.test.ts @@ -39,6 +39,7 @@ import type { State, ThreadIndex, IndexIntoSourceTable, + BottomBoxInfo, } from 'firefox-profiler/types'; import getNiceProfile from './fixtures/profiles/call-nodes'; import queryString from 'query-string'; @@ -1797,11 +1798,13 @@ describe('URL persistence of bottom box (source view and assembly view)', functi const sourceFile = 'hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:5bb3e281dc9ec8a619c781d52882adb1cacf20bb'; const sourceIndex = getSourceIndex(sourceFile); - const bottomBoxInfo = { + const bottomBoxInfo: BottomBoxInfo = { libIndex: 0, sourceIndex, nativeSymbols: [], initialNativeSymbol: null, + highlightedLineNumber: null, + highlightedInstructionAddress: null, }; dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo)); const newStore = _getStoreFromStateAfterUrlRoundtrip(getState()); @@ -1830,11 +1833,13 @@ describe('URL persistence of bottom box (source view and assembly view)', functi const sourceFile = 'hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:5bb3e281dc9ec8a619c781d52882adb1cacf20bb'; const sourceIndex = getSourceIndex(sourceFile); - const bottomBoxInfo = { + const bottomBoxInfo: BottomBoxInfo = { libIndex: 0, sourceIndex, nativeSymbols: [], initialNativeSymbol: null, + highlightedLineNumber: null, + highlightedInstructionAddress: null, }; dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo)); dispatch(closeBottomBox()); @@ -1859,11 +1864,13 @@ describe('URL persistence of bottom box (source view and assembly view)', functi functionSize: 14, functionSizeIsKnown: false, }; - const bottomBoxInfo = { + const bottomBoxInfo: BottomBoxInfo = { libIndex: 0, sourceIndex: null, nativeSymbols: [nativeSymbolInfo], initialNativeSymbol: 0, + highlightedLineNumber: null, + highlightedInstructionAddress: null, }; dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo)); const newStore = _getStoreFromStateAfterUrlRoundtrip(getState()); @@ -1902,11 +1909,13 @@ describe('URL persistence of bottom box (source view and assembly view)', functi functionSize: 14, functionSizeIsKnown: false, }; - const bottomBoxInfo = { + const bottomBoxInfo: BottomBoxInfo = { libIndex: 0, sourceIndex, nativeSymbols: [nativeSymbolInfo], initialNativeSymbol: 0, + highlightedLineNumber: null, + highlightedInstructionAddress: null, }; dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo)); const newStore = _getStoreFromStateAfterUrlRoundtrip(getState()); diff --git a/src/types/actions.ts b/src/types/actions.ts index 34ada128db..54ecff2f67 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -344,9 +344,9 @@ type ProfileAction = readonly shouldOpenBottomBox: boolean; readonly shouldOpenAssemblyView: boolean; readonly scrollToLineNumber?: number; - readonly highlightLineNumber?: number; readonly scrollToInstructionAddress?: number; - readonly highlightInstructionAddress?: number; + readonly highlightedLineNumber: number | null; + readonly highlightedInstructionAddress: number | null; } | { readonly type: 'OPEN_ASSEMBLY_VIEW'; diff --git a/src/types/profile-derived.ts b/src/types/profile-derived.ts index f113607c3b..c0ba94655f 100644 --- a/src/types/profile-derived.ts +++ b/src/types/profile-derived.ts @@ -776,10 +776,13 @@ export type BottomBoxInfo = { sourceIndex: IndexIntoSourceTable | null; nativeSymbols: NativeSymbolInfo[]; initialNativeSymbol: number | null; // index into `nativeSymbols` + // Optional line number + instruction address to scroll into view. scrollToLineNumber?: number; - highlightLineNumber?: number; scrollToInstructionAddress?: number; - highlightInstructionAddress?: number; + // Which lines / instructions to highlight (or none). This is used when clicking + // a stack frame in a marker stack. + highlightedLineNumber: number | null; + highlightedInstructionAddress: number | null; }; /** diff --git a/src/types/state.ts b/src/types/state.ts index 05989e2a77..792a1ded08 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -241,6 +241,8 @@ export type ZippedProfilesState = { export type SourceViewState = { scrollGeneration: number; + // Optional line number to scroll to in the source view. + scrollToLineNumber?: number; // Non-null if this source file was opened for a function from native code. // In theory, multiple different libraries can have source files with the same // path but different content. @@ -251,10 +253,8 @@ export type SourceViewState = { // Index into source table. Contains information (filename and uuid) about the // source. Null if a function without a file path was double clicked. sourceIndex: IndexIntoSourceTable | null; - // Optional line number to scroll to in the source view. - scrollToLineNumber?: number; - // Optional line number of the highlighted line in the source view. - highlightedLine?: number; + // The line number of the highlighted line in the source view. + highlightedLine: number | null; }; export type AssemblyViewState = { @@ -265,8 +265,8 @@ export type AssemblyViewState = { scrollGeneration: number; // Optional instruction address to scroll to in the assembly view. scrollToInstructionAddress?: number; - // Optional address of the highlighted instruction in the assembly view. - highlightedInstruction?: number; + // The address of the highlighted instruction in the assembly view. + highlightedInstruction: number | null; // The list of native symbols whose assembly code should be accessible in the // assembly view. Often empty or just one element, but sometimes this can have // multiple elements, for example the different JIT compilations of one JS function.