diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts index 0885bcc50f..56bef8ff15 100644 --- a/src/actions/profile-view.ts +++ b/src/actions/profile-view.ts @@ -1918,8 +1918,19 @@ export function changeTableViewOptions( export function updateBottomBoxContentsAndMaybeOpen( currentTab: TabSlug, - { libIndex, sourceIndex, nativeSymbols, lineNumber }: BottomBoxInfo + bottomBoxInfo: BottomBoxInfo ): Action { + const { + libIndex, + sourceIndex, + nativeSymbols, + initialNativeSymbol, + scrollToLineNumber, + scrollToInstructionAddress, + highlightedLineNumber, + highlightedInstructionAddress, + } = bottomBoxInfo; + const haveSource = sourceIndex !== null; const haveAssembly = nativeSymbols.length !== 0; @@ -1929,22 +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, - lineNumber, + scrollToLineNumber, + scrollToInstructionAddress, + 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 c2631c42fb..2411bd4fe1 100644 --- a/src/components/app/BottomBox.tsx +++ b/src/components/app/BottomBox.tsx @@ -14,25 +14,22 @@ import { CodeLoadingOverlay } from './CodeLoadingOverlay'; import { CodeErrorOverlay } from './CodeErrorOverlay'; import { getSourceViewScrollGeneration, - getSourceViewLineNumber, + getSourceViewScrollToLineNumber, + getSourceViewHighlightedLine, 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 { 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'; @@ -55,16 +52,16 @@ type StateProps = { readonly sourceViewFile: string | null; readonly sourceViewCode: SourceCodeStatus | void; readonly sourceViewScrollGeneration: number; - readonly sourceViewLineNumber?: number; + readonly sourceViewScrollToLineNumber?: number; + readonly sourceViewHighlightedLine: number | null; readonly globalLineTimings: LineTimings; - readonly selectedCallNodeLineTimings: LineTimings; readonly assemblyViewIsOpen: boolean; readonly assemblyViewNativeSymbol: NativeSymbolInfo | null; readonly assemblyViewCode: AssemblyCodeStatus | void; readonly assemblyViewScrollGeneration: number; + readonly assemblyViewScrollToInstructionAddress?: number; + readonly assemblyViewHighlightedInstruction: number | null; readonly globalAddressTimings: AddressTimings; - readonly selectedCallNodeAddressTimings: AddressTimings; - readonly disableOverscan: boolean; }; type DispatchProps = { @@ -160,16 +157,16 @@ class BottomBoxImpl extends React.PureComponent { sourceViewFile, sourceViewCode, globalLineTimings, - disableOverscan, sourceViewScrollGeneration, - sourceViewLineNumber, - selectedCallNodeLineTimings, + sourceViewScrollToLineNumber, + sourceViewHighlightedLine, assemblyViewIsOpen, assemblyViewScrollGeneration, + assemblyViewScrollToInstructionAddress, + assemblyViewHighlightedInstruction, assemblyViewNativeSymbol, assemblyViewCode, globalAddressTimings, - selectedCallNodeAddressTimings, } = this.props; const sourceCode = sourceViewCode && sourceViewCode.type === 'AVAILABLE' @@ -223,20 +220,17 @@ class BottomBoxImpl extends React.PureComponent { {displayIonGraph ? ( ) : null} {displaySourceView ? ( ) : null} @@ -264,12 +258,14 @@ class BottomBoxImpl extends React.PureComponent {
{assemblyViewNativeSymbol !== null ? ( ) : null} @@ -302,19 +298,19 @@ export const BottomBox = explicitConnect<{}, StateProps, DispatchProps>({ sourceViewFile: getSourceViewFile(state), sourceViewCode: getSourceViewCode(state), globalLineTimings: selectedThreadSelectors.getSourceViewLineTimings(state), - selectedCallNodeLineTimings: - selectedNodeSelectors.getSourceViewLineTimings(state), sourceViewScrollGeneration: getSourceViewScrollGeneration(state), - sourceViewLineNumber: getSourceViewLineNumber(state), + sourceViewScrollToLineNumber: getSourceViewScrollToLineNumber(state), + sourceViewHighlightedLine: getSourceViewHighlightedLine(state), assemblyViewNativeSymbol: getAssemblyViewNativeSymbol(state), assemblyViewCode: getAssemblyViewCode(state), globalAddressTimings: selectedThreadSelectors.getAssemblyViewAddressTimings(state), - selectedCallNodeAddressTimings: - selectedNodeSelectors.getAssemblyViewAddressTimings(state), assemblyViewScrollGeneration: getAssemblyViewScrollGeneration(state), + assemblyViewScrollToInstructionAddress: + getAssemblyViewScrollToInstructionAddress(state), + assemblyViewHighlightedInstruction: + getAssemblyViewHighlightedInstruction(state), assemblyViewIsOpen: getAssemblyViewIsOpen(state), - disableOverscan: getPreviewSelectionIsBeingModified(state), }), mapDispatchToProps: { closeBottomBox, 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/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 729699d172..37108a6903 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'; @@ -44,10 +43,10 @@ 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; + readonly scrollGeneration: number; + readonly scrollToInstructionAddress?: number; + readonly highlightedInstruction: number | null; }; let editorModulePromise: Promise | null = null; @@ -56,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); @@ -150,10 +119,16 @@ export class AssemblyView extends React.PureComponent { const editor = new AssemblyViewEditor( this._getAssemblyCodeOrFallback(), this.props.timings, + this.props.highlightedInstruction, domParent ); this._editor = editor; - this._scrollToHotSpot(this.props.hotSpotTimings); + if (this.props.scrollToInstructionAddress !== undefined) { + this._scrollToAddressWithSpaceOnTop( + this.props.scrollToInstructionAddress, + 5 + ); + } })(); } @@ -177,14 +152,24 @@ 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) { this._editor.setTimings(this.props.timings); } + + if ( + this.props.highlightedInstruction !== prevProps.highlightedInstruction + ) { + this._editor.setHighlightedInstruction(this.props.highlightedInstruction); + } } } diff --git a/src/components/shared/CallNodeContextMenu.tsx b/src/components/shared/CallNodeContextMenu.tsx index 7e2633f6cf..ebbe889971 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'; @@ -52,12 +49,12 @@ import type { TransformType, ImplementationFilter, IndexIntoCallNodeTable, - CallNodePath, Thread, ThreadsKey, CategoryList, InnerWindowID, Page, + SamplesLikeTable, } from 'firefox-profiler/types'; import type { TabSlug } from 'firefox-profiler/app-logic/tabs-handling'; @@ -68,9 +65,9 @@ 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 rightClickedCallNodePath: CallNodePath | null; readonly rightClickedCallNodeIndex: IndexIntoCallNodeTable | null; readonly implementation: ImplementationFilter; readonly inverted: boolean; @@ -91,6 +88,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; @@ -226,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); } @@ -337,9 +337,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 +504,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; } @@ -533,30 +535,30 @@ class CallNodeContextMenuImpl extends React.PureComponent { getRightClickedCallNodeInfo(): null | { readonly thread: Thread; readonly threadsKey: ThreadsKey; + readonly previewFilteredCtssSamples: SamplesLikeTable; readonly callNodeInfo: CallNodeInfo; - readonly callNodePath: CallNodePath; readonly callNodeIndex: IndexIntoCallNodeTable; } { const { thread, threadsKey, + previewFilteredCtssSamples, callNodeInfo, - rightClickedCallNodePath, rightClickedCallNodeIndex, } = this.props; if ( thread && threadsKey !== null && + previewFilteredCtssSamples !== null && callNodeInfo && - rightClickedCallNodePath && - typeof rightClickedCallNodeIndex === 'number' + rightClickedCallNodeIndex !== null ) { return { thread, threadsKey, + previewFilteredCtssSamples, callNodeInfo, - callNodePath: rightClickedCallNodePath, callNodeIndex: rightClickedCallNodeIndex, }; } @@ -897,8 +899,8 @@ export const CallNodeContextMenu = explicitConnect< let thread = null; let threadsKey = null; + let previewFilteredCtssSamples = null; let callNodeInfo = null; - let rightClickedCallNodePath = null; let rightClickedCallNodeIndex = null; if (rightClickedCallNodeInfo !== null) { @@ -908,17 +910,18 @@ export const CallNodeContextMenu = explicitConnect< thread = selectors.getFilteredThread(state); threadsKey = rightClickedCallNodeInfo.threadsKey; + previewFilteredCtssSamples = + selectors.getPreviewFilteredCtssSamples(state); callNodeInfo = selectors.getCallNodeInfo(state); - rightClickedCallNodePath = rightClickedCallNodeInfo.callNodePath; rightClickedCallNodeIndex = selectors.getRightClickedCallNodeIndex(state); } return { thread, threadsKey, + previewFilteredCtssSamples, categories: getCategories(state), callNodeInfo, - rightClickedCallNodePath, rightClickedCallNodeIndex, implementation: getImplementationFilter(state), inverted: getInvertCallstack(state), 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 7563e42d79..9e722f1f8b 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'; @@ -40,12 +39,10 @@ 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 scrollGeneration: number; readonly scrollToLineNumber?: number; - readonly hotSpotTimings: LineTimings; - readonly highlightedLine?: number; + readonly highlightedLine: number | null; }; let editorModulePromise: Promise | null = null; @@ -54,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); @@ -142,15 +109,13 @@ 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; // 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); } })(); } @@ -179,14 +144,11 @@ 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); } } @@ -195,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/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..d027f0b83c 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, @@ -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 new file mode 100644 index 0000000000..a470105912 --- /dev/null +++ b/src/profile-logic/bottom-box.ts @@ -0,0 +1,176 @@ +/* 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, + 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'; +import { + getAddressTimings, + getStackAddressInfoForCallNode, +} from './address-timings'; + +/** + * 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, + samples: SamplesLikeTable +): 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 + ); + + // 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( + nativeSymbolIndex, + nativeSymbols, + frameTable, + stringTable + ) + ); + + // Compute the hottest line, so we can 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); + + // 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, + highlightedLineNumber: null, + highlightedInstructionAddress: null, + }; +} + +/** + * 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 + ), + ] + : []; + + const instructionAddress = + nativeSymbol !== null ? frameTable.address[frameIndex] : -1; + + // Extract line number from the frame + const lineNumber = frameTable.line[frameIndex]; + + return { + libIndex, + sourceIndex, + nativeSymbols: nativeSymbolInfos, + initialNativeSymbol: 0, + scrollToLineNumber: lineNumber ?? undefined, + highlightedLineNumber: lineNumber, + scrollToInstructionAddress: + instructionAddress !== -1 ? instructionAddress : undefined, + highlightedInstructionAddress: + instructionAddress !== -1 ? instructionAddress : null, + }; +} diff --git a/src/profile-logic/call-tree.ts b/src/profile-logic/call-tree.ts index 5802354430..71ab7b3f7a 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[]; @@ -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/profile-logic/profile-data.ts b/src/profile-logic/profile-data.ts index 97525c5502..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,109 +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, - lineNumber, - }; -} - /** * Determines the timeline type by looking at the profile data. * diff --git a/src/reducers/url-state.ts b/src/reducers/url-state.ts index 8823e926ce..834290828d 100644 --- a/src/reducers/url-state.ts +++ b/src/reducers/url-state.ts @@ -565,16 +565,25 @@ 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) { 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.highlightedLineNumber, }; } default: @@ -584,10 +593,11 @@ const sourceView: Reducer = ( const assemblyView: Reducer = ( state = { + isOpen: false, scrollGeneration: 0, + highlightedInstruction: null, nativeSymbols: [], currentNativeSymbol: null, - isOpen: false, }, action ) => { @@ -595,11 +605,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.highlightedInstructionAddress, }; } case 'OPEN_ASSEMBLY_VIEW': { diff --git a/src/selectors/per-thread/index.ts b/src/selectors/per-thread/index.ts index df364eccb4..130c55a0a0 100644 --- a/src/selectors/per-thread/index.ts +++ b/src/selectors/per-thread/index.ts @@ -23,26 +23,13 @@ import { getComposedSelectorsPerThread, type ComposedSelectorsPerThread, } from './composed'; -import { - getStackLineInfoForCallNode, - getLineTimings, -} from '../../profile-logic/line-timings'; -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, - StackLineInfo, - LineTimings, - StackAddressInfo, - AddressTimings, State, } from 'firefox-profiler/types'; @@ -205,10 +192,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; }; export const selectedNodeSelectors: NodeSelectors = (() => { @@ -268,85 +251,10 @@ 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, - 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, - 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/selectors/url-state.ts b/src/selectors/url-state.ts index 2587b5462c..8f02b53cbf 100644 --- a/src/selectors/url-state.ts +++ b/src/selectors/url-state.ts @@ -75,8 +75,11 @@ 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 = ( @@ -90,6 +93,13 @@ 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 = ( + state +) => getProfileSpecificState(state).assemblyView.highlightedInstruction; export const getShowJsTracerSummary: Selector = (state) => getProfileSpecificState(state).showJsTracerSummary; 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 2007e02f84..f410ee3a3c 100644 --- a/src/test/store/bottom-box.test.ts +++ b/src/test/store/bottom-box.test.ts @@ -6,12 +6,9 @@ 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/profile-data'; +import { getBottomBoxInfoForCallNode } from '../../profile-logic/bottom-box'; import { changeSelectedCallNode, updateBottomBoxContentsAndMaybeOpen, @@ -33,7 +30,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 +92,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 +180,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 +231,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 +242,16 @@ 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( + null + ); expect(UrlStateSelectors.getAssemblyViewIsOpen(getState())).toBeFalse(); expect( ensureExists(UrlStateSelectors.getAssemblyViewNativeSymbol(getState())) @@ -263,42 +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, + thread.samples + ); + dispatch( + updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfoABCD) ); - dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfoABC)); - // 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(null); + 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(null); + expect( + UrlStateSelectors.getAssemblyViewScrollToInstructionAddress(getState()) + ).toBeOneOf([0x40, 0x45]); }); // Further ideas for tests: 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/test/url-handling.test.ts b/src/test/url-handling.test.ts index eb692cfb23..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,10 +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()); @@ -1829,10 +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()); @@ -1857,10 +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()); @@ -1899,10 +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 3804235664..54ecff2f67 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -343,7 +343,10 @@ type ProfileAction = readonly currentTab: TabSlug; readonly shouldOpenBottomBox: boolean; readonly shouldOpenAssemblyView: boolean; - readonly lineNumber?: number; + readonly scrollToLineNumber?: number; + readonly scrollToInstructionAddress?: 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 c37e45319e..c0ba94655f 100644 --- a/src/types/profile-derived.ts +++ b/src/types/profile-derived.ts @@ -775,9 +775,14 @@ export type BottomBoxInfo = { libIndex: IndexIntoLibs | null; 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. - lineNumber?: number; + initialNativeSymbol: number | null; // index into `nativeSymbols` + // Optional line number + instruction address to scroll into view. + scrollToLineNumber?: number; + scrollToInstructionAddress?: 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 0989fd0472..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,9 +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. - // If not specified, the source view will scroll to the hottest line. - lineNumber?: number; + // The line number of the highlighted line in the source view. + highlightedLine: number | null; }; export type AssemblyViewState = { @@ -262,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; + // 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.