From aaf592495f9f529468ec967f44d49374a29c456e Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 9 Mar 2023 17:14:47 -0500 Subject: [PATCH 1/9] Move mapGetKeyWithMaxValue into src/utils/. --- src/components/shared/SourceView.js | 17 +++-------------- src/utils/index.js | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/components/shared/SourceView.js b/src/components/shared/SourceView.js index 1d72b6c1fd..9b21cb1496 100644 --- a/src/components/shared/SourceView.js +++ b/src/components/shared/SourceView.js @@ -6,6 +6,7 @@ import * as React from 'react'; import { ensureExists } from 'firefox-profiler/utils/flow'; +import { mapGetKeyWithMaxValue } from 'firefox-profiler/utils'; import type { LineTimings } from 'firefox-profiler/types'; import type { SourceViewEditor } from './SourceView-codemirror'; @@ -47,18 +48,6 @@ type SourceViewProps = {| +hotSpotTimings: LineTimings, |}; -function _mapGetKeyWithMaxValue(map: Map): K | void { - let maxValue = -Infinity; - let keyForMaxValue; - for (const [key, value] of map) { - if (value > maxValue) { - maxValue = value; - keyForMaxValue = key; - } - } - return keyForMaxValue; -} - let editorModulePromise: Promise | null = null; export class SourceView extends React.PureComponent { @@ -88,8 +77,8 @@ export class SourceView extends React.PureComponent { */ _scrollToHotSpot(timingsForScrolling: LineTimings) { const heaviestLine = - _mapGetKeyWithMaxValue(timingsForScrolling.totalLineHits) ?? - _mapGetKeyWithMaxValue(this.props.timings.totalLineHits); + mapGetKeyWithMaxValue(timingsForScrolling.totalLineHits) ?? + mapGetKeyWithMaxValue(this.props.timings.totalLineHits); if (heaviestLine !== undefined) { this._scrollToLine(heaviestLine - 5); } diff --git a/src/utils/index.js b/src/utils/index.js index a2f2922f78..6356a69c8c 100644 --- a/src/utils/index.js +++ b/src/utils/index.js @@ -120,3 +120,21 @@ export function countPositiveValues(arr: Array): number { } return count; } + +/** + * Find the highest number among the map values and return its key. + * Returns undefined if the map is empty. + * If multiple entries with the highest value exist, it returns the key of the + * first encountered highest value. + */ +export function mapGetKeyWithMaxValue(map: Map): K | void { + let maxValue = -Infinity; + let keyForMaxValue; + for (const [key, value] of map) { + if (value > maxValue) { + maxValue = value; + keyForMaxValue = key; + } + } + return keyForMaxValue; +} From f9ad9da0594f15de4b9709d865c2017f088d6f50 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 9 Mar 2023 17:19:03 -0500 Subject: [PATCH 2/9] Rename source to code or sourceCode in more places. --- src/actions/code.js | 4 ++-- src/components/app/BottomBox.js | 6 +++--- src/components/shared/SourceView.js | 24 ++++++++++++------------ src/reducers/code.js | 4 ++-- src/types/actions.js | 2 +- src/types/state.js | 2 +- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/actions/code.js b/src/actions/code.js index fddbe025d8..2afa84c49b 100644 --- a/src/actions/code.js +++ b/src/actions/code.js @@ -23,8 +23,8 @@ export function beginLoadingSourceCodeFromBrowserConnection( return { type: 'SOURCE_CODE_LOADING_BEGIN_BROWSER_CONNECTION', file }; } -export function finishLoadingSourceCode(file: string, source: string): Action { - return { type: 'SOURCE_CODE_LOADING_SUCCESS', file, source }; +export function finishLoadingSourceCode(file: string, code: string): Action { + return { type: 'SOURCE_CODE_LOADING_SUCCESS', file, code }; } export function failLoadingSourceCode( diff --git a/src/components/app/BottomBox.js b/src/components/app/BottomBox.js index 5ef89658ba..75ff32e515 100644 --- a/src/components/app/BottomBox.js +++ b/src/components/app/BottomBox.js @@ -243,9 +243,9 @@ class BottomBoxImpl extends React.PureComponent { sourceViewScrollGeneration, selectedCallNodeLineTimings, } = this.props; - const source = + const sourceCode = sourceViewCode && sourceViewCode.type === 'AVAILABLE' - ? sourceViewCode.source + ? sourceViewCode.code : ''; const path = sourceViewFile !== null @@ -273,7 +273,7 @@ class BottomBoxImpl extends React.PureComponent { { } _getMaxLineNumber() { - const { source, timings } = this.props; - const sourceLines = source.split('\n'); + const { sourceCode, timings } = this.props; + const sourceLines = sourceCode.split('\n'); let maxLineNumber = sourceLines.length; if (maxLineNumber <= 1) { // We probably don't have the true source code yet, and don't really know @@ -107,10 +107,10 @@ export class SourceView extends React.PureComponent { return maxLineNumber; } - _getSourceOrFallback() { - const { source } = this.props; - if (source !== '') { - return source; + _getSourceCodeOrFallback() { + const { sourceCode } = this.props; + if (sourceCode !== '') { + return sourceCode; } return '\n'.repeat(this._getMaxLineNumber()); } @@ -139,7 +139,7 @@ export class SourceView extends React.PureComponent { } const { SourceViewEditor } = codeMirrorModule; const editor = new SourceViewEditor( - this._getSourceOrFallback(), + this._getSourceCodeOrFallback(), this.props.filePath, this.props.timings, domParent @@ -161,12 +161,12 @@ export class SourceView extends React.PureComponent { } if ( - this.props.source !== prevProps.source || - (this.props.source === '' && - prevProps.source === '' && + this.props.sourceCode !== prevProps.sourceCode || + (this.props.sourceCode === '' && + prevProps.sourceCode === '' && this.props.timings !== prevProps.timings) ) { - this._editor.setContents(this._getSourceOrFallback()); + this._editor.setContents(this._getSourceCodeOrFallback()); } if ( diff --git a/src/reducers/code.js b/src/reducers/code.js index 4aff3d86e7..2bf31b39f7 100644 --- a/src/reducers/code.js +++ b/src/reducers/code.js @@ -32,9 +32,9 @@ const sourceCodeCache: Reducer> = ( return newState; } case 'SOURCE_CODE_LOADING_SUCCESS': { - const { file, source } = action; + const { file, code } = action; const newState = new Map(state); - newState.set(file, { type: 'AVAILABLE', source }); + newState.set(file, { type: 'AVAILABLE', code }); return newState; } case 'SOURCE_CODE_LOADING_ERROR': { diff --git a/src/types/actions.js b/src/types/actions.js index 05e8c3cbf6..c3a881b96a 100644 --- a/src/types/actions.js +++ b/src/types/actions.js @@ -633,7 +633,7 @@ type L10nAction = type SourcesAction = | {| +type: 'SOURCE_CODE_LOADING_BEGIN_URL', file: string, url: string |} | {| +type: 'SOURCE_CODE_LOADING_BEGIN_BROWSER_CONNECTION', file: string |} - | {| +type: 'SOURCE_CODE_LOADING_SUCCESS', file: string, source: string |} + | {| +type: 'SOURCE_CODE_LOADING_SUCCESS', file: string, code: string |} | {| +type: 'SOURCE_CODE_LOADING_ERROR', file: string, diff --git a/src/types/state.js b/src/types/state.js index d71a86d5e4..beccd8345c 100644 --- a/src/types/state.js +++ b/src/types/state.js @@ -284,7 +284,7 @@ export type DecodedInstruction = {| export type SourceCodeStatus = | {| type: 'LOADING', source: CodeLoadingSource |} | {| type: 'ERROR', errors: SourceCodeLoadingError[] |} - | {| type: 'AVAILABLE', source: string |}; + | {| type: 'AVAILABLE', code: string |}; export type AssemblyCodeStatus = | {| type: 'LOADING', source: CodeLoadingSource |} From 450f89cbe516131d79b48de1a4d172765164449c Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 9 Mar 2023 17:21:09 -0500 Subject: [PATCH 3/9] Move some CodeMirror code. --- .../shared/SourceView-codemirror.js | 164 +----------------- src/utils/codemirror-shared.js | 164 ++++++++++++++++++ 2 files changed, 170 insertions(+), 158 deletions(-) create mode 100644 src/utils/codemirror-shared.js diff --git a/src/components/shared/SourceView-codemirror.js b/src/components/shared/SourceView-codemirror.js index 1c76e95fd3..fe7a2bf47d 100644 --- a/src/components/shared/SourceView-codemirror.js +++ b/src/components/shared/SourceView-codemirror.js @@ -20,21 +20,8 @@ * a cm-nonZeroLine class to them. This highlight line goes across the entire * width of the editor, it covers both the gutter and the main area. */ -import { - EditorView, - Decoration, - lineNumbers, - GutterMarker, - gutter, - gutterLineClass, -} from '@codemirror/view'; -import { - EditorState, - StateField, - StateEffect, - Compartment, - RangeSet, -} from '@codemirror/state'; +import { EditorView, lineNumbers } from '@codemirror/view'; +import { EditorState, Compartment } from '@codemirror/state'; import { syntaxHighlighting } from '@codemirror/language'; import { classHighlighter } from '@lezer/highlight'; import { cpp } from '@codemirror/lang-cpp'; @@ -43,7 +30,10 @@ import { javascript } from '@codemirror/lang-javascript'; import clamp from 'clamp'; import type { LineTimings } from 'firefox-profiler/types'; -import { emptyLineTimings } from 'firefox-profiler/profile-logic/line-timings'; +import { + timingsExtension, + updateTimingsEffect, +} from 'firefox-profiler/utils/codemirror-shared'; // This "compartment" allows us to swap the syntax highlighting language when // the file path changes. @@ -84,148 +74,6 @@ function _languageExtForPath( return []; } -// This gutter marker applies the "cm-nonZeroLine" class to gutter elements. -const nonZeroLineGutterMarker = new (class extends GutterMarker { - elementClass = 'cm-nonZeroLine'; -})(); - -// This "decoration" applies the "cm-nonZeroLine" class to the line of source -// code in the main editor contents (not the gutter). -const nonZeroLineDecoration = Decoration.line({ class: 'cm-nonZeroLine' }); - -// An "effect" is like a redux action. This effect is used to replace the value -// of the timingsField state field. -const updateTimingsEffect = StateEffect.define(); - -// A "state field" for the timings. -const timingsField = StateField.define({ - create() { - return emptyLineTimings; - }, - update(timings, transaction) { - // This is like a reducer. Find an updateTimingsEffect in the transaction - // and set this field to the timings in it. - let newTimings = timings; - for (const effect of transaction.effects) { - if (effect.is(updateTimingsEffect)) { - newTimings = effect.value; - } - } - return newTimings; - }, -}); - -// Finds all lines with non-zero line timings, for the highlight line. -// The line numbers are then converted into "positions", i.e. character offsets -// in the document, for the start of the line. -// Then they are sorted, because our caller wants to have a sorted list. -function getSortedStartPositionsOfNonZeroLines(state: EditorState): number[] { - const timings = state.field(timingsField); - const nonZeroLines = new Set(); - for (const lineNumber of timings.totalLineHits.keys()) { - nonZeroLines.add(lineNumber); - } - for (const lineNumber of timings.selfLineHits.keys()) { - nonZeroLines.add(lineNumber); - } - const lineCount = state.doc.lines; - const positions = [...nonZeroLines] - .filter((l) => l <= lineCount) - .map((lineNumber) => state.doc.line(lineNumber).from); - positions.sort((a, b) => a - b); - return positions; -} - -// This is an "extension" which applies the "cm-nonZeroLine" class to all gutter -// elements for lines with non-zero timings. It is like a piece of derived state; -// it needs to be recomputed whenever one of the input states change. The input -// states are the editor contents ("doc") and the value of the timings field. -// The editor contents are relevant because the output is expressed in terms of -// positions, i.e. character offsets from the document start, and those positions -// need to be updated if the amount of text in a line changes. This happens when -// we replace the file placeholder content with the actual file content. -const nonZeroLineGutterHighlighter = gutterLineClass.compute( - ['doc', timingsField], - (state) => { - const positions = getSortedStartPositionsOfNonZeroLines(state); - return RangeSet.of(positions.map((p) => nonZeroLineGutterMarker.range(p))); - } -); - -// Same as the previous extension, but this one is for the main editor. There -// doesn't seem to be a way to set a class for the entire line, i.e. both the -// gutter elements and the main editor elements of that line. -const nonZeroLineDecorationHighlighter = EditorView.decorations.compute( - ['doc', timingsField], - (state) => { - const positions = getSortedStartPositionsOfNonZeroLines(state); - return RangeSet.of(positions.map((p) => nonZeroLineDecoration.range(p))); - } -); - -// This is a "gutter marker" which renders just a string and nothing else. -// It is used for the LineTimings annotations, i.e. for the numbers in the -// gutter. -class StringMarker extends GutterMarker { - _s; - - constructor(s) { - super(); - this._s = s; - } - - toDOM() { - return document.createTextNode(this._s); - } -} - -// The "extension" which manages the elements in the gutter for the "total" -// column. -const totalTimingsGutter = gutter({ - class: 'cm-total-timings-gutter', - lineMarker(view, line) { - // Return a gutter marker for this line, or null. - const lineNumber = view.state.doc.lineAt(line.from).number; - const timings = view.state.field(timingsField); - const totalTime = timings.totalLineHits.get(lineNumber); - return totalTime !== undefined ? new StringMarker(totalTime) : null; - }, - lineMarkerChange(update) { - // Return true if the update affects the total timings in the gutter. - return update.transactions.some((t) => - t.effects.some((e) => e.is(updateTimingsEffect)) - ); - }, -}); - -// The "extension" which manages the elements in the gutter for the "self" -// column. -const selfTimingsGutter = gutter({ - class: 'cm-self-timings-gutter', - lineMarker(view, line) { - // Return a gutter marker for this line, or null. - const lineNumber = view.state.doc.lineAt(line.from).number; - const timings = view.state.field(timingsField); - const selfTime = timings.selfLineHits.get(lineNumber); - return selfTime !== undefined ? new StringMarker(selfTime) : null; - }, - lineMarkerChange(update) { - // Return true if the update affects the self timings in the gutter. - return update.transactions.some((t) => - t.effects.some((e) => e.is(updateTimingsEffect)) - ); - }, -}); - -// All extensions which have to do with timings, grouped into one extension. -const timingsExtension = [ - timingsField, - totalTimingsGutter, - selfTimingsGutter, - nonZeroLineGutterHighlighter, - nonZeroLineDecorationHighlighter, -]; - // Adjustments to make a CodeMirror editor work as a non-editable code viewer. const codeViewerExtension = [ // Make the editor non-editable. diff --git a/src/utils/codemirror-shared.js b/src/utils/codemirror-shared.js new file mode 100644 index 0000000000..f47213ae33 --- /dev/null +++ b/src/utils/codemirror-shared.js @@ -0,0 +1,164 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +// @flow + +import { + EditorView, + Decoration, + GutterMarker, + gutter, + gutterLineClass, +} from '@codemirror/view'; +import { + EditorState, + StateField, + StateEffect, + RangeSet, +} from '@codemirror/state'; + +import type { LineTimings } from 'firefox-profiler/types'; + +import { emptyLineTimings } from 'firefox-profiler/profile-logic/line-timings'; + +// This gutter marker applies the "cm-nonZeroLine" class to gutter elements. +const nonZeroLineGutterMarker = new (class extends GutterMarker { + elementClass = 'cm-nonZeroLine'; +})(); + +// This "decoration" applies the "cm-nonZeroLine" class to the line of assembly +// code in the main editor contents (not the gutter). +const nonZeroLineDecoration = Decoration.line({ class: 'cm-nonZeroLine' }); + +// An "effect" is like a redux action. This effect is used to replace the value +// of the timingsField state field. +export const updateTimingsEffect = StateEffect.define(); + +// A "state field" for the timings. +const timingsField = StateField.define({ + create() { + return emptyLineTimings; + }, + update(timings, transaction) { + // This is like a reducer. Find an updateTimingsEffect in the transaction + // and set this field to the timings in it. + let newTimings = timings; + for (const effect of transaction.effects) { + if (effect.is(updateTimingsEffect)) { + newTimings = effect.value; + } + } + return newTimings; + }, +}); + +// Finds all lines with non-zero line timings, for the highlight line. +// The line numbers are then converted into "positions", i.e. character offsets +// in the document, for the start of the line. +// Then they are sorted, because our caller wants to have a sorted list. +function getSortedStartPositionsOfNonZeroLines(state: EditorState): number[] { + const timings = state.field(timingsField); + const nonZeroLines = new Set(); + for (const lineNumber of timings.totalLineHits.keys()) { + nonZeroLines.add(lineNumber); + } + for (const lineNumber of timings.selfLineHits.keys()) { + nonZeroLines.add(lineNumber); + } + const lineCount = state.doc.lines; + const positions = [...nonZeroLines] + .filter((l) => l <= lineCount) + .map((lineNumber) => state.doc.line(lineNumber).from); + positions.sort((a, b) => a - b); + return positions; +} + +// This is an "extension" which applies the "cm-nonZeroLine" class to all gutter +// elements for lines with non-zero timings. It is like a piece of derived state; +// it needs to be recomputed whenever one of the input states change. The input +// states are the editor contents ("doc") and the value of the timings field. +// The editor contents are relevant because the output is expressed in terms of +// positions, i.e. character offsets from the document start, and those positions +// need to be updated if the amount of text in a line changes. This happens when +// we replace the file placeholder content with the actual file content. +const nonZeroLineGutterHighlighter = gutterLineClass.compute( + ['doc', timingsField], + (state) => { + const positions = getSortedStartPositionsOfNonZeroLines(state); + return RangeSet.of(positions.map((p) => nonZeroLineGutterMarker.range(p))); + } +); + +// Same as the previous extension, but this one is for the main editor. There +// doesn't seem to be a way to set a class for the entire line, i.e. both the +// gutter elements and the main editor elements of that line. +const nonZeroLineDecorationHighlighter = EditorView.decorations.compute( + ['doc', timingsField], + (state) => { + const positions = getSortedStartPositionsOfNonZeroLines(state); + return RangeSet.of(positions.map((p) => nonZeroLineDecoration.range(p))); + } +); + +// This is a "gutter marker" which renders just a string and nothing else. +// It is used for the AddressTimings annotations, i.e. for the numbers in the +// gutter. +export class StringMarker extends GutterMarker { + _s: string; + + constructor(s: string) { + super(); + this._s = s; + } + + toDOM() { + return document.createTextNode(this._s); + } +} + +// The "extension" which manages the elements in the gutter for the "total" +// column. +const totalTimingsGutter = gutter({ + class: 'cm-total-timings-gutter', + lineMarker(view, line) { + // Return a gutter marker for this line, or null. + const lineNumber = view.state.doc.lineAt(line.from).number; + const timings = view.state.field(timingsField); + const totalTime = timings.totalLineHits.get(lineNumber); + return totalTime !== undefined ? new StringMarker(totalTime) : null; + }, + lineMarkerChange(update) { + // Return true if the update affects the total timings in the gutter. + return update.transactions.some((t) => + t.effects.some((e) => e.is(updateTimingsEffect)) + ); + }, +}); + +// The "extension" which manages the elements in the gutter for the "self" +// column. +const selfTimingsGutter = gutter({ + class: 'cm-self-timings-gutter', + lineMarker(view, line) { + // Return a gutter marker for this line, or null. + const lineNumber = view.state.doc.lineAt(line.from).number; + const timings = view.state.field(timingsField); + const selfTime = timings.selfLineHits.get(lineNumber); + return selfTime !== undefined ? new StringMarker(selfTime) : null; + }, + lineMarkerChange(update) { + // Return true if the update affects the self timings in the gutter. + return update.transactions.some((t) => + t.effects.some((e) => e.is(updateTimingsEffect)) + ); + }, +}); + +// All extensions which have to do with timings, grouped into one extension. +export const timingsExtension = [ + timingsField, + totalTimingsGutter, + selfTimingsGutter, + nonZeroLineGutterHighlighter, + nonZeroLineDecorationHighlighter, +]; From 18d5cccf9dce52c3c09105281ba89a5a4bc8071d Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 9 Mar 2023 17:38:54 -0500 Subject: [PATCH 4/9] Move loading and error UI code. This also updates some L10nIDs and some strings. In particular, it changes the string "Source not available" to "Source code not available", which is less ambiguous. --- locales/en-US/app.ftl | 26 ++- src/components/app/BottomBox.css | 13 +- src/components/app/BottomBox.js | 226 +++++------------------ src/components/app/CodeErrorOverlay.js | 124 +++++++++++++ src/components/app/CodeLoadingOverlay.js | 44 +++++ 5 files changed, 234 insertions(+), 199 deletions(-) create mode 100644 src/components/app/CodeErrorOverlay.js create mode 100644 src/components/app/CodeLoadingOverlay.js diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 196f00f2f3..3c799d0245 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -926,21 +926,25 @@ TransformNavigator--collapse-indirect-recursion = Collapse indirect recursion: { # $item (String) - Name of the function that transform applied to. TransformNavigator--collapse-function-subtree = Collapse subtree: { $item } -## Source code view in a box at the bottom of the UI. +## "Bottom box" - a view which contains the source view and the assembly view, +## at the bottom of the profiler UI +## +## Some of these string IDs still start with SourceView, even though the strings +## are used for both the source view and the assembly view. -# Displayed while the source view is waiting for the network request which -# delivers the source code. +# Displayed while a view in the bottom box is waiting for code to load from +# the network. # Variables: # $host (String) - The "host" part of the URL, e.g. hg.mozilla.org SourceView--loading-url = Waiting for { $host }… -# Displayed while the source view is waiting for the browser to deliver -# the source code. +# Displayed while a view in the bottom box is waiting for code to load from +# the browser. SourceView--loading-browser-connection = Waiting for { -firefox-brand-name }… # Displayed whenever the source view was not able to get the source code for # a file. -SourceView--source-not-available-title = Source not available +BottomBox--source-code-not-available-title = Source code not available # Displayed whenever the source view was not able to get the source code for # a file. @@ -949,6 +953,13 @@ SourceView--source-not-available-title = Source not available SourceView--source-not-available-text = See issue #3741 for supported scenarios and planned improvements. +SourceView--close-button = + .title = Close the source view + +## Code loading errors +## These are displayed both in the source view and in the assembly view. +## The string IDs here currently all start with SourceView for historical reasons. + # Displayed below SourceView--cannot-obtain-source, if the profiler does not # know which URL to request source code from. SourceView--no-known-cors-url = @@ -1016,9 +1027,6 @@ SourceView--not-in-archive-error-when-obtaining-source = SourceView--archive-parsing-error-when-obtaining-source = The archive at { $url } could not be parsed: { $parsingErrorMessage } -SourceView--close-button = - .title = Close the source view - ## UploadedRecordingsHome ## This is the page that displays all the profiles that user has uploaded. ## See: https://profiler.firefox.com/uploaded-recordings/ diff --git a/src/components/app/BottomBox.css b/src/components/app/BottomBox.css index 15caca382f..09da65225c 100644 --- a/src/components/app/BottomBox.css +++ b/src/components/app/BottomBox.css @@ -42,7 +42,8 @@ background: url(/res/img/svg/close-dark.svg) no-repeat center / 16px 16px; } -.sourceStatusOverlay { +.codeLoadingOverlay, +.sourceCodeErrorOverlay { /** * Put the overlay on top of everything in .bottom-main, but centered * horizontally and vertically. We center using margin: auto, and enforce @@ -69,8 +70,8 @@ word-break: break-word; } -/* Use :before to add a loading spinner image */ -.sourceStatusOverlay.loading::before { +/* Use ::before to add a loading spinner image */ +.codeLoadingOverlay::before { display: block; width: 32px; height: 32px; @@ -79,8 +80,8 @@ content: ''; } -/* Use :before to add an alert icon */ -.sourceStatusOverlay.error::before { +/* Use ::before to add an alert icon */ +.sourceCodeErrorOverlay::before { display: block; width: 50px; height: 50px; @@ -90,6 +91,6 @@ filter: brightness(70%) drop-shadow(0 1px rgb(255 255 255 / 0.5)); } -.sourceStatusOverlay ul { +.codeErrorOverlay { padding-left: 20px; } diff --git a/src/components/app/BottomBox.js b/src/components/app/BottomBox.js index 75ff32e515..de4e517feb 100644 --- a/src/components/app/BottomBox.js +++ b/src/components/app/BottomBox.js @@ -7,6 +7,8 @@ import React from 'react'; import classNames from 'classnames'; import { SourceView } from '../shared/SourceView'; +import { CodeLoadingOverlay } from './CodeLoadingOverlay'; +import { CodeErrorOverlay } from './CodeErrorOverlay'; import { getSourceViewFile, getSourceViewScrollGeneration, @@ -19,11 +21,11 @@ import { closeBottomBox } from 'firefox-profiler/actions/profile-view'; import { parseFileNameFromSymbolication } from 'firefox-profiler/utils/special-paths'; import { getSourceViewCode } from 'firefox-profiler/selectors/code'; import { getPreviewSelection } from 'firefox-profiler/selectors/profile'; -import { assertExhaustiveCheck } from 'firefox-profiler/utils/flow'; import explicitConnect from 'firefox-profiler/utils/connect'; import type { ConnectedProps } from 'firefox-profiler/utils/connect'; import type { LineTimings, SourceCodeStatus } from 'firefox-profiler/types'; +import type { CodeErrorOverlayProps } from './CodeErrorOverlay'; import { Localized } from '@fluent/react'; @@ -44,187 +46,41 @@ type DispatchProps = {| type Props = ConnectedProps<{||}, StateProps, DispatchProps>; -type SourceStatusOverlayProps = {| status: SourceCodeStatus |}; - -function SourceStatusOverlay({ status }: SourceStatusOverlayProps) { - switch (status.type) { - case 'AVAILABLE': - return null; // No overlay if we have source code. - case 'LOADING': { - const { source } = status; - switch (source.type) { - case 'URL': { - const { url } = source; - let host; - try { - host = new URL(url).host; - } catch (e) { - host = url; - } - return ( - -
- {`Waiting for ${host}…`} -
- - ); - } - case 'BROWSER_CONNECTION': { - return ( - -
- Waiting for browser… -
- - ); - } - default: - throw assertExhaustiveCheck(source.type); - } - } - case 'ERROR': { - return ( -
-
- -

Source not available

- - - ), - }} +export function SourceCodeErrorOverlay({ errors }: CodeErrorOverlayProps) { + return ( +
+
+ +

Source code not available

+ + + ), + }} + > +

+ See + -

- See - - issue #3741 - - for supported scenarios and planned improvements. -

- -
    - {status.errors.map((error, key) => { - switch (error.type) { - case 'NO_KNOWN_CORS_URL': { - return ( - -
  • No known cross-origin-accessible URL.
    • -     - ); - } - case 'NETWORK_ERROR': { - const { url, networkErrorMessage } = error; - return ( - -
  • {`There was a network error when fetching the URL ${url}: ${networkErrorMessage}`}
    • -     - ); - } - case 'BROWSER_CONNECTION_ERROR': { - const { browserConnectionErrorMessage } = error; - return ( - -
  • {`Could not query the browser’s symbolication API: ${browserConnectionErrorMessage}`}
    • -     - ); - } - case 'BROWSER_API_ERROR': { - const { apiErrorMessage } = error; - return ( - -
  • {`The browser’s symbolication API returned an error: ${apiErrorMessage}`}
    • -     - ); - } - case 'SYMBOL_SERVER_API_ERROR': { - const { apiErrorMessage } = error; - return ( - -
  • {`The local symbol server’s symbolication API returned an error: ${apiErrorMessage}`}
    • -     - ); - } - case 'BROWSER_API_MALFORMED_RESPONSE': { - const { errorMessage } = error; - return ( - -
  • {`The browser’s symbolication API returned a malformed response: ${errorMessage}`}
    • -     - ); - } - case 'SYMBOL_SERVER_API_MALFORMED_RESPONSE': { - const { errorMessage } = error; - return ( - -
  • {`The local symbol server’s symbolication API returned a malformed response: ${errorMessage}`}
    • -     - ); - } - case 'NOT_PRESENT_IN_ARCHIVE': { - const { url, pathInArchive } = error; - return ( - -
  • {`The file ${pathInArchive} was not found in the archive from ${url}.`}
    • -     - ); - } - case 'ARCHIVE_PARSING_ERROR': { - const { url, parsingErrorMessage } = error; - return ( - -
  • {`The archive at ${url} could not be parsed: ${parsingErrorMessage}`}
    • -     - ); - } - default: - throw assertExhaustiveCheck(error.type); - } - })} -
-
-
- ); - } - default: - throw assertExhaustiveCheck(status.type); - } + issue #3741 + + for supported scenarios and planned improvements. +

+ + +
+
+ ); } class BottomBoxImpl extends React.PureComponent { @@ -280,9 +136,11 @@ class BottomBoxImpl extends React.PureComponent { ref={this._sourceView} /> ) : null} - {sourceViewCode !== undefined && - sourceViewCode.type !== 'AVAILABLE' ? ( - + {sourceViewCode !== undefined && sourceViewCode.type === 'LOADING' ? ( + + ) : null} + {sourceViewCode !== undefined && sourceViewCode.type === 'ERROR' ? ( + ) : null} diff --git a/src/components/app/CodeErrorOverlay.js b/src/components/app/CodeErrorOverlay.js new file mode 100644 index 0000000000..f833fd19c6 --- /dev/null +++ b/src/components/app/CodeErrorOverlay.js @@ -0,0 +1,124 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +// @flow + +import React from 'react'; + +import { assertExhaustiveCheck } from 'firefox-profiler/utils/flow'; +import type { SourceCodeLoadingError } from 'firefox-profiler/types'; +import { Localized } from '@fluent/react'; + +export type CodeErrorOverlayProps = {| + errors: SourceCodeLoadingError[], +|}; + +export function CodeErrorOverlay({ errors }: CodeErrorOverlayProps) { + return ( +
    + {errors.map((error, key) => { + switch (error.type) { + case 'NO_KNOWN_CORS_URL': { + return ( + +
  • No known cross-origin-accessible URL.
    • +     + ); + } + case 'NETWORK_ERROR': { + const { url, networkErrorMessage } = error; + return ( + +
  • {`There was a network error when fetching the URL ${url}: ${networkErrorMessage}`}
    • +     + ); + } + case 'BROWSER_CONNECTION_ERROR': { + const { browserConnectionErrorMessage } = error; + return ( + +
  • {`Could not query the browser’s symbolication API: ${browserConnectionErrorMessage}`}
    • +     + ); + } + case 'BROWSER_API_ERROR': { + const { apiErrorMessage } = error; + return ( + +
  • {`The browser’s symbolication API returned an error: ${apiErrorMessage}`}
    • +     + ); + } + case 'SYMBOL_SERVER_API_ERROR': { + const { apiErrorMessage } = error; + return ( + +
  • {`The local symbol server’s symbolication API returned an error: ${apiErrorMessage}`}
    • +     + ); + } + case 'BROWSER_API_MALFORMED_RESPONSE': { + const { errorMessage } = error; + return ( + +
  • {`The browser’s symbolication API returned a malformed response: ${errorMessage}`}
    • +     + ); + } + case 'SYMBOL_SERVER_API_MALFORMED_RESPONSE': { + const { errorMessage } = error; + return ( + +
  • {`The local symbol server’s symbolication API returned a malformed response: ${errorMessage}`}
    • +     + ); + } + case 'NOT_PRESENT_IN_ARCHIVE': { + const { url, pathInArchive } = error; + return ( + +
  • {`The file ${pathInArchive} was not found in the archive from ${url}.`}
    • +     + ); + } + case 'ARCHIVE_PARSING_ERROR': { + const { url, parsingErrorMessage } = error; + return ( + +
  • {`The archive at ${url} could not be parsed: ${parsingErrorMessage}`}
    • +     + ); + } + default: + throw assertExhaustiveCheck(error.type); + } + })} +
+ ); +} diff --git a/src/components/app/CodeLoadingOverlay.js b/src/components/app/CodeLoadingOverlay.js new file mode 100644 index 0000000000..8857e7f3cc --- /dev/null +++ b/src/components/app/CodeLoadingOverlay.js @@ -0,0 +1,44 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +// @flow + +import React from 'react'; + +import { assertExhaustiveCheck } from 'firefox-profiler/utils/flow'; +import { Localized } from '@fluent/react'; +import type { CodeLoadingSource } from 'firefox-profiler/types'; + +type CodeLoadingOverlayProps = {| + source: CodeLoadingSource, +|}; + +export function CodeLoadingOverlay({ source }: CodeLoadingOverlayProps) { + switch (source.type) { + case 'URL': { + const { url } = source; + let host; + try { + host = new URL(url).host; + } catch (e) { + host = url; + } + return ( + +
{`Waiting for ${host}…`}
+ + ); + } + case 'BROWSER_CONNECTION': { + return ( + +
Waiting for browser…
+ + ); + } + default: + throw assertExhaustiveCheck(source.type); + } +} + +export default CodeLoadingOverlay; From 9647b432b830075bb58ee1fc0ec6f55f992b2930 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 9 Mar 2023 18:08:25 -0500 Subject: [PATCH 5/9] Remove unused SourceView CSS. --- src/components/shared/SourceView.css | 52 +--------------------------- 1 file changed, 1 insertion(+), 51 deletions(-) diff --git a/src/components/shared/SourceView.css b/src/components/shared/SourceView.css index c9edff7266..7bde148c35 100644 --- a/src/components/shared/SourceView.css +++ b/src/components/shared/SourceView.css @@ -110,57 +110,7 @@ text-overflow: ellipsis; } -.sourceViewBody { - position: relative; - z-index: 0; - overflow: auto; - flex: 1; - line-height: 16px; - outline: 0; - will-change: scroll-position; -} - -.sourceViewBodyInnerWrapper { - position: absolute; - top: 0; - left: 0; - display: flex; - min-width: 100%; - flex-flow: row; -} - -.sourceViewBodyInner0 { - position: sticky; - z-index: 2; - left: 0; -} - -.sourceViewBodyInner1 { - /* This allows the right column to expand more than its content, so that the - * background always extends to the right edge. */ - flex-grow: 1; -} - -.sourceViewBodyInner { - overflow: hidden; - background: white; -} - -.sourceViewRowFixedColumns { - display: flex; - flex-flow: row; - align-items: stretch; - justify-content: flex-start; - white-space: nowrap; -} - -.sourceViewRowScrolledColumns { - padding-left: 3px; - white-space: pre; -} - -.cm-nonZeroLine, -.sourceViewRowNonZero { +.cm-nonZeroLine { background-color: #edf6ff; } From d9292576229f3db18ec475e27cce866fb4c4c0b8 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 9 Mar 2023 18:14:39 -0500 Subject: [PATCH 6/9] Make SourceView CSS shareable. --- .../shared/{SourceView.css => CodeView.css} | 14 +++++++------- src/components/shared/SourceView.js | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) rename src/components/shared/{SourceView.css => CodeView.css} (95%) diff --git a/src/components/shared/SourceView.css b/src/components/shared/CodeView.css similarity index 95% rename from src/components/shared/SourceView.css rename to src/components/shared/CodeView.css index 7bde148c35..f363ba02c9 100644 --- a/src/components/shared/SourceView.css +++ b/src/components/shared/CodeView.css @@ -2,7 +2,7 @@ * 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/. */ -.sourceView { +.codeView { display: flex; min-height: 0; flex: 1; @@ -24,7 +24,7 @@ --theme-highlight-punctuation: var(--grey-70); } -.sourceViewHeader { +.codeViewHeader { display: flex; height: 16px; flex-flow: row; @@ -55,11 +55,11 @@ background-color: white !important; } -.sourceViewHeaderMainColumn { +.codeViewHeaderMainColumn { flex: 1; } -.sourceViewHeaderColumn { +.codeViewHeaderColumn { position: relative; box-sizing: border-box; padding: 1px 5px; @@ -68,7 +68,7 @@ } /* A small vertical separator line */ -.sourceViewHeaderColumn.sourceViewFixedColumn::after { +.codeViewHeaderColumn.codeViewFixedColumn::after { position: absolute; top: 3px; right: 0; @@ -78,8 +78,8 @@ content: ''; } -.sourceViewHeaderColumn.total, -.sourceViewHeaderColumn.self, +.codeViewHeaderColumn.total, +.codeViewHeaderColumn.self, .cm-total-timings-gutter, .cm-self-timings-gutter { width: 50px; diff --git a/src/components/shared/SourceView.js b/src/components/shared/SourceView.js index 13aa8acf93..cc4bee80cf 100644 --- a/src/components/shared/SourceView.js +++ b/src/components/shared/SourceView.js @@ -11,13 +11,13 @@ import type { LineTimings } from 'firefox-profiler/types'; import type { SourceViewEditor } from './SourceView-codemirror'; -import './SourceView.css'; +import './CodeView.css'; const SourceViewHeader = () => { return ( -
+
Self - +
); }; @@ -117,7 +117,7 @@ export class SourceView extends React.PureComponent { render() { return ( -
+
From cdd28123c8ede7824dfa83b5bd432e9dd006e082 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 16 Feb 2023 14:51:49 -0500 Subject: [PATCH 7/9] Add an AssemblyViewToggleButton component. --- locales/en-US/app.ftl | 12 +++ res/css/photon/button.css | 3 +- res/img/svg/asm-icon.svg | 8 ++ .../app/AssemblyViewToggleButton.js | 87 +++++++++++++++++++ 4 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 res/img/svg/asm-icon.svg create mode 100644 src/components/app/AssemblyViewToggleButton.js diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 3c799d0245..e69dbdaf99 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -1027,6 +1027,18 @@ SourceView--not-in-archive-error-when-obtaining-source = SourceView--archive-parsing-error-when-obtaining-source = The archive at { $url } could not be parsed: { $parsingErrorMessage } +## Toggle buttons in the top right corner of the bottom box + +# The toggle button for the assembly view, while the assembly view is hidden. +# Assembly refers to the low-level programming language. +AssemblyView--show-button = + .title = Show the assembly view + +# The toggle button for the assembly view, while the assembly view is shown. +# Assembly refers to the low-level programming language. +AssemblyView--hide-button = + .title = Hide the assembly view + ## UploadedRecordingsHome ## This is the page that displays all the profiles that user has uploaded. ## See: https://profiler.firefox.com/uploaded-recordings/ diff --git a/res/css/photon/button.css b/res/css/photon/button.css index 024e0819a2..9a37ddf7d2 100644 --- a/res/css/photon/button.css +++ b/res/css/photon/button.css @@ -80,7 +80,8 @@ background-color: transparent; } -.photon-button-ghost:hover:not(:disabled) { +.photon-button-ghost:hover:not(:disabled), +.photon-button-ghost--checked { background-color: var(--grey-90-a10); } diff --git a/res/img/svg/asm-icon.svg b/res/img/svg/asm-icon.svg new file mode 100644 index 0000000000..f7adf26afd --- /dev/null +++ b/res/img/svg/asm-icon.svg @@ -0,0 +1,8 @@ + + + + + + \ No newline at end of file diff --git a/src/components/app/AssemblyViewToggleButton.js b/src/components/app/AssemblyViewToggleButton.js new file mode 100644 index 0000000000..c511f80d98 --- /dev/null +++ b/src/components/app/AssemblyViewToggleButton.js @@ -0,0 +1,87 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +// @flow + +import React from 'react'; +import classNames from 'classnames'; + +import { getAssemblyViewIsOpen } from 'firefox-profiler/selectors/url-state'; +import { + openAssemblyView, + closeAssemblyView, +} from 'firefox-profiler/actions/profile-view'; +import explicitConnect from 'firefox-profiler/utils/connect'; + +import type { ConnectedProps } from 'firefox-profiler/utils/connect'; + +import { Localized } from '@fluent/react'; + +type StateProps = {| + +assemblyViewIsOpen: boolean, +|}; + +type DispatchProps = {| + +openAssemblyView: typeof openAssemblyView, + +closeAssemblyView: typeof closeAssemblyView, +|}; + +type Props = ConnectedProps<{||}, StateProps, DispatchProps>; + +class AssemblyViewToggleButtonImpl extends React.PureComponent { + _onClick = () => { + if (this.props.assemblyViewIsOpen) { + this.props.closeAssemblyView(); + } else { + this.props.openAssemblyView(); + } + }; + + render() { + const { assemblyViewIsOpen } = this.props; + + return assemblyViewIsOpen ? ( + +
+ ); + return (
-
-

{path ?? '(no file selected)'}

- -
-
- {sourceViewFile !== null ? ( - - ) : null} - {sourceViewCode !== undefined && sourceViewCode.type === 'LOADING' ? ( - - ) : null} - {sourceViewCode !== undefined && sourceViewCode.type === 'ERROR' ? ( - + +
+
+

{path ?? '(no source file)'}

+ {assemblyViewIsOpen ? null : trailingHeaderButtons} +
+
+ {sourceViewFile !== null ? ( + + ) : null} + {sourceViewCode !== undefined && + sourceViewCode.type === 'LOADING' ? ( + + ) : null} + {sourceViewCode !== undefined && + sourceViewCode.type === 'ERROR' ? ( + + ) : null} +
+
+ + {assemblyViewIsOpen ? ( +
+
+

+ {assemblyViewNativeSymbol !== null + ? assemblyViewNativeSymbol.name + : '(no native symbol)'} +

+ {trailingHeaderButtons} +
+
+ {assemblyViewNativeSymbol !== null ? ( + + ) : null} + {assemblyViewCode !== undefined && + assemblyViewCode.type === 'LOADING' ? ( + + ) : null} + {assemblyViewCode !== undefined && + assemblyViewCode.type === 'ERROR' ? ( + + ) : null} +
+
) : null} -
+
); } } +function convertErrors(errors: ApiQueryError[]): SourceCodeLoadingError[] { + // Copy the array so that the types work out. + return errors.map((e) => e); +} + export const BottomBox = explicitConnect<{||}, StateProps, DispatchProps>({ mapStateToProps: (state) => ({ sourceViewFile: getSourceViewFile(state), @@ -156,6 +288,14 @@ export const BottomBox = explicitConnect<{||}, StateProps, DispatchProps>({ selectedCallNodeLineTimings: selectedNodeSelectors.getSourceViewLineTimings(state), sourceViewScrollGeneration: getSourceViewScrollGeneration(state), + assemblyViewNativeSymbol: getAssemblyViewNativeSymbol(state), + assemblyViewCode: getAssemblyViewCode(state), + globalAddressTimings: + selectedThreadSelectors.getAssemblyViewAddressTimings(state), + selectedCallNodeAddressTimings: + selectedNodeSelectors.getAssemblyViewAddressTimings(state), + assemblyViewScrollGeneration: getAssemblyViewScrollGeneration(state), + assemblyViewIsOpen: getAssemblyViewIsOpen(state), disableOverscan: getPreviewSelection(state).isModifying, }), mapDispatchToProps: { diff --git a/src/components/shared/AssemblyView-codemirror.js b/src/components/shared/AssemblyView-codemirror.js new file mode 100644 index 0000000000..e53cc956a6 --- /dev/null +++ b/src/components/shared/AssemblyView-codemirror.js @@ -0,0 +1,287 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +// @flow + +/** + * This module wraps all the interaction with the CodeMirror API into a + * AssemblyViewEditor class. + * + * This module is intended to be imported asynchronously, so that all the + * CodeMirror code can be split into a separate bundle chunk. + * + * This file implements the following features: + * - Display assembly code. + * - Display a gutter with: + * - "Total" timings for each instruction + * - "Self" timings for each instruction + * - The address for each instruction + * - Highlight assembly code lines which have a non-zero timing, by applying + * a cm-nonZeroLine class to them. This highlight line goes across the entire + * 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 { syntaxHighlighting } from '@codemirror/language'; +import { classHighlighter } from '@lezer/highlight'; +import clamp from 'clamp'; + +import type { + AddressTimings, + Address, + LineTimings, + LineNumber, + NativeSymbolInfo, + DecodedInstruction, +} from 'firefox-profiler/types'; + +import { bisectionRight } from 'firefox-profiler/utils/bisect'; +import { + timingsExtension, + updateTimingsEffect, + StringMarker, +} 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 "state field" stores the current AddressToLineMap. This field allows the +// instructionAddressGutter to map line numbers to addresses. +const addressToLineMapField = StateField.define({ + create() { + return []; + }, + update(instructionAddresses, transaction) { + // Get the new value from an effect in the transaction. + let newSortedAddresses = instructionAddresses; + for (const effect of transaction.effects) { + if (effect.is(updateAddressToLineMapEffect)) { + newSortedAddresses = effect.value; + } + } + return newSortedAddresses; + }, +}); + +// A gutter which displays the address of each instruction. +const instructionAddressGutter = gutter({ + class: 'cm-instruction-address-gutter', + + // Returns a gutter marker for this line, or null. + lineMarker(view, line) { + const lineNumber = view.state.doc.lineAt(line.from).number; + const map = view.state.field(addressToLineMapField); + const address = map.lineToAddress(lineNumber); + return address !== null + ? new StringMarker(`0x${address.toString(16)}`) + : null; + }, + + // Returns true if the update affects the instruction addresses in the gutter. + lineMarkerChange(update) { + return update.transactions.some((t) => + t.effects.some((e) => e.is(updateAddressToLineMapEffect)) + ); + }, +}); + +function instructionsToText(assemblyCode: DecodedInstruction[]): string { + return assemblyCode.map((instr) => instr.decodedString).join('\n'); +} + +/** + * This map is used to convert between instruction addresses and editor line + * numbers. + */ +class AddressToLineMap { + // The address of each instruction. This stays constant for the entire lifetime + // of this AddressToLineMap instance. + // + // _instructionAddresses[0] contains the address of the instruction which is + // displayed in line 1. (Line numbers are 1-based.) + // + // The addresses need to be ordered from low to high, so that the binary search + // works. + _instructionAddresses: Address[]; + + constructor(instructionAddresses) { + this._instructionAddresses = instructionAddresses; + } + + // Find the line which displays the instruction which covers `address`. + // `address` doesn't need to be a perfect match for the instruction address; + // for example, in the example below, address 0x10e4 is mapped to line 3: + // + // 1: 0x10da: mov r14, rdi + // 2: 0x10dd: mov rdi, rsi + // 3: 0x10e0: call _malloc_usable_size + // 4: 0x10e5: test rax, rax + // 5: 0x10e8: je loc_10f6 + addressToLine(address: Address): LineNumber | null { + const insertionIndex = bisectionRight(this._instructionAddresses, address); + if (insertionIndex === 0) { + // address < instructionAddresses[0] + return null; + } + + const elementIndex = insertionIndex - 1; + const lineNumber = elementIndex + 1; + return lineNumber; + } + + // Return the address of the instruction which is displayed in line `lineNumber`. + lineToAddress(lineNumber: LineNumber): Address | null { + if (lineNumber < 1 || lineNumber > this._instructionAddresses.length) { + return null; + } + + const elementIndex = lineNumber - 1; + return this._instructionAddresses[elementIndex]; + } +} + +function getInstructionAddresses( + assemblyCode: DecodedInstruction[] +): Address[] { + return assemblyCode.map((instr) => instr.address); +} + +// Convert AddressTimings to LineTimings with the help of an AddressToLineMap. +function addressTimingsToLineTimings( + addressTimings: AddressTimings, + map: AddressToLineMap +): LineTimings { + const totalLineHits = new Map(); + for (const [address, hitCount] of addressTimings.totalAddressHits) { + const line = map.addressToLine(address); + if (line !== null) { + const currentHitCount = totalLineHits.get(line) ?? 0; + totalLineHits.set(line, currentHitCount + hitCount); + } + } + + const selfLineHits = new Map(); + for (const [address, hitCount] of addressTimings.selfAddressHits) { + const line = map.addressToLine(address); + if (line !== null) { + const currentHitCount = selfLineHits.get(line) ?? 0; + selfLineHits.set(line, currentHitCount + hitCount); + } + } + + return { totalLineHits, selfLineHits }; +} + +export class AssemblyViewEditor { + _view: EditorView; + _addressToLineMap: AddressToLineMap; + _addressTimings: AddressTimings; + + // Create a CodeMirror editor and add it as a child element of domParent. + constructor( + initialAssemblyCode: DecodedInstruction[], + nativeSymbol: NativeSymbolInfo, + addressTimings: AddressTimings, + domParent: Element + ) { + this._addressToLineMap = new AddressToLineMap( + getInstructionAddresses(initialAssemblyCode) + ); + this._addressTimings = addressTimings; + let state = EditorState.create({ + doc: instructionsToText(initialAssemblyCode), + extensions: [ + timingsExtension, + addressToLineMapField, + instructionAddressGutter, + syntaxHighlighting(classHighlighter), + EditorState.readOnly.of(true), + EditorView.editable.of(false), + ], + }); + const lineTimings = addressTimingsToLineTimings( + this._addressTimings, + this._addressToLineMap + ); + state = state.update({ + effects: [ + updateAddressToLineMapEffect.of(this._addressToLineMap), + updateTimingsEffect.of(lineTimings), + ], + }).state; + this._view = new EditorView({ + state, + parent: domParent, + }); + } + + setContents(assemblyCode: DecodedInstruction[]) { + this._addressToLineMap = new AddressToLineMap( + getInstructionAddresses(assemblyCode) + ); + const lineTimings = addressTimingsToLineTimings( + this._addressTimings, + this._addressToLineMap + ); + // 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); + this._view.dispatch( + this._view.state.update({ + changes: { + insert: text, + from: 0, + to: this._view.state.doc.length, + }, + }) + ); + this._view.dispatch({ + effects: [ + updateAddressToLineMapEffect.of(this._addressToLineMap), + updateTimingsEffect.of(lineTimings), + ], + }); + } + + setTimings(addressTimings: AddressTimings) { + // Update the value of the timings field by dispatching an updateTimingsEffect. + this._addressTimings = addressTimings; + const lineTimings = addressTimingsToLineTimings( + this._addressTimings, + this._addressToLineMap + ); + this._view.dispatch({ + effects: updateTimingsEffect.of(lineTimings), + }); + } + + scrollToLine(lineNumber: number) { + // Clamp the line number to the document's line count. + lineNumber = clamp(lineNumber, 1, this._view.state.doc.lines); + + // Convert the line number into a position. + const pos = this._view.state.doc.line(lineNumber).from; + // Dispatch the scroll action. + this._view.dispatch({ + effects: EditorView.scrollIntoView(pos, { y: 'start', yMargin: 0 }), + }); + // Trigger a measure flush, to work around + // https://github.com/codemirror/codemirror.next/issues/676 + this._view.coordsAtPos(0); + } + + scrollToAddress(address: Address) { + const lineNumber = this._addressToLineMap.addressToLine(address); + if (lineNumber !== null) { + this.scrollToLine(lineNumber); + } + } + + scrollToAddressWithSpaceOnTop(address: Address, topSpaceLines: number) { + const lineNumber = this._addressToLineMap.addressToLine(address); + if (lineNumber !== null) { + this.scrollToLine(lineNumber - topSpaceLines); + } + } +} diff --git a/src/components/shared/AssemblyView.js b/src/components/shared/AssemblyView.js new file mode 100644 index 0000000000..5018671217 --- /dev/null +++ b/src/components/shared/AssemblyView.js @@ -0,0 +1,193 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +// @flow + +import * as React from 'react'; + +import { ensureExists } from 'firefox-profiler/utils/flow'; +import type { + AddressTimings, + NativeSymbolInfo, + DecodedInstruction, +} from 'firefox-profiler/types'; +import { mapGetKeyWithMaxValue } from 'firefox-profiler/utils'; + +import type { AssemblyViewEditor } from './AssemblyView-codemirror'; + +import './CodeView.css'; + +const AssemblyViewHeader = () => { + return ( +
+ + Total + + + Self + + +
+ ); +}; + +type AssemblyViewProps = {| + +timings: AddressTimings, + +assemblyCode: DecodedInstruction[], + +disableOverscan: boolean, + +nativeSymbol: NativeSymbolInfo | null, + +scrollToHotSpotGeneration: number, + +hotSpotTimings: AddressTimings, +|}; + +let editorModulePromise: Promise | null = null; + +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); + } + } + + _getAssemblyCodeOrFallback(): DecodedInstruction[] { + const { assemblyCode } = this.props; + if (assemblyCode.length !== 0) { + // We have assembly code for the selected symbol. Good. + return assemblyCode; + } + + // We don't have the true assembly code yet, and don't really know + // at which address each instruction starts. + // Compute a fallback by getting known addresses from the timings. + + const { timings, nativeSymbol } = this.props; + const addresses = [...timings.totalAddressHits.keys()]; + + // Also include the start address of the symbol, if it's not already present. + if ( + nativeSymbol !== null && + !timings.totalAddressHits.has(nativeSymbol.address) + ) { + addresses.push(nativeSymbol.address); + } + + addresses.sort((a, b) => a - b); + + // Create fallback assembly code where each known address is mapped to an + // empty string instruction. + return addresses.map((address) => ({ + address, + decodedString: '', + })); + } + + render() { + return ( +
+ +
+
+ ); + } + + componentDidMount() { + // Load the module with all the @codemirror imports asynchronously, so that + // it can be split into a separate bundle chunk. + if (editorModulePromise === null) { + editorModulePromise = import('./AssemblyView-codemirror'); + } + (async () => { + const codeMirrorModulePromise = ensureExists(editorModulePromise); + const codeMirrorModule = await codeMirrorModulePromise; + const domParent = this._ref.current; + if (!domParent) { + return; + } + const { AssemblyViewEditor } = codeMirrorModule; + const editor = new AssemblyViewEditor( + this._getAssemblyCodeOrFallback(), + this.props.nativeSymbol, + this.props.timings, + domParent + ); + this._editor = editor; + this._scrollToHotSpot(this.props.hotSpotTimings); + })(); + } + + // CodeMirror's API is not based on React. When our props change, we need to + // translate those changes into CodeMirror API calls manually. + componentDidUpdate(prevProps: AssemblyViewProps) { + if (!this._editor) { + return; + } + + let contentsChanged = false; + if ( + this.props.assemblyCode !== prevProps.assemblyCode || + (this.props.assemblyCode.length === 0 && + prevProps.assemblyCode.length === 0 && + this.props.timings !== prevProps.timings) + ) { + this._editor.setContents(this._getAssemblyCodeOrFallback()); + contentsChanged = true; + } + + if ( + contentsChanged || + this.props.scrollToHotSpotGeneration !== + prevProps.scrollToHotSpotGeneration + ) { + this._scrollToHotSpot(this.props.hotSpotTimings); + } + + if (this.props.timings !== prevProps.timings) { + this._editor.setTimings(this.props.timings); + } + } +} diff --git a/src/components/shared/CodeView.css b/src/components/shared/CodeView.css index f363ba02c9..17bb1813a4 100644 --- a/src/components/shared/CodeView.css +++ b/src/components/shared/CodeView.css @@ -101,6 +101,17 @@ min-width: 45px !important; } +.cm-instruction-address-gutter { + color: #aaa; + font-family: ui-monospace, 'Roboto Mono', monospace; + font-variant-numeric: tabular-nums; +} + +.cm-instruction-address-gutter .cm-gutterElement { + min-width: 45px !important; + padding: 0 8px; +} + .cm-total-timings-gutter .cm-gutterElement, .cm-self-timings-gutter .cm-gutterElement { overflow: hidden; From d67512773d40e64c5a98aca587f3e47b6505bc99 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 9 Mar 2023 18:43:14 -0500 Subject: [PATCH 9/9] Add a test for the assembly view. --- .../{SourceView.test.js => BottomBox.test.js} | 85 ++++++++++++++++--- ...ew.test.js.snap => BottomBox.test.js.snap} | 63 +++++++++++++- 2 files changed, 137 insertions(+), 11 deletions(-) rename src/test/components/{SourceView.test.js => BottomBox.test.js} (57%) rename src/test/components/__snapshots__/{SourceView.test.js.snap => BottomBox.test.js.snap} (60%) diff --git a/src/test/components/SourceView.test.js b/src/test/components/BottomBox.test.js similarity index 57% rename from src/test/components/SourceView.test.js rename to src/test/components/BottomBox.test.js index cc9c337bc1..fb28fbc87b 100644 --- a/src/test/components/SourceView.test.js +++ b/src/test/components/BottomBox.test.js @@ -29,7 +29,7 @@ jest.mock('../../components/timeline', () => ({ Timeline: 'custom-timeline', })); -describe('SourceView', () => { +describe('BottomBox', () => { autoMockDomRect(); afterEach(() => { delete Range.prototype.getClientRects; @@ -50,9 +50,11 @@ describe('SourceView', () => { const revision = '997f00815e6bc28806b75448c8829f0259d2cb28'; const filepath = 'widget/cocoa/nsAppShell.mm'; - window.fetch.get( - `https://hg.mozilla.org/mozilla-central/raw-file/${revision}/${filepath}`, - stripIndent` + window.fetch + .post('http://127.0.0.1:8000/source/v1', 500) + .get( + `https://hg.mozilla.org/mozilla-central/raw-file/${revision}/${filepath}`, + stripIndent` line 1 line 2 line 3 @@ -61,10 +63,30 @@ describe('SourceView', () => { line 6 line 7 ` - ); + ) + .post( + 'http://127.0.0.1:8000/asm/v1', + JSON.stringify({ + startAddress: '0x20', + size: '0x1a', + arch: 'x86_64', + syntax: ['Intel'], + instructions: [ + [0, 'push rsi'], + [1, 'push rdi'], + [2, 'push rbx'], + [3, 'sub rsp, 0x20'], + [7, 'mov rsi, rcx'], + [10, 'mov rdi, qword [rcx + 0x58]'], + [14, 'mov ebx, edi'], + [16, 'and ebx, 0x1400'], + [22, 'call 0x16c5d35'], + ], + }) + ); const { profile } = getProfileFromTextSamples(` - A[file:hg:hg.mozilla.org/mozilla-central:${filepath}:${revision}][line:4] + A[file:hg:hg.mozilla.org/mozilla-central:${filepath}:${revision}][line:4][address:30][sym:Asym:20:1a][lib:libA.so] B[file:git:github.com/rust-lang/rust:library/std/src/sys/unix/thread.rs:53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b] C[lib:libC.so][file:s3:gecko-generated-sources:a5d3747707d6877b0e5cb0a364e3cb9fea8aa4feb6ead138952c2ba46d41045297286385f0e0470146f49403e46bd266e654dfca986de48c230f3a71c2aafed4/ipc/ipdl/PBackgroundChild.cpp:] D[lib:libD.so] @@ -75,7 +97,7 @@ describe('SourceView', () => { updateUrlState( stateFromLocation({ pathname: '/from-browser', - search: '', + search: '?symbolServer=http://127.0.0.1:8000', hash: '', }) ) @@ -89,22 +111,25 @@ describe('SourceView', () => { return { sourceView: () => document.querySelector('.sourceView'), + assemblyView: () => document.querySelector('.assemblyView'), }; } it('does not show the source view at loadtime', () => { - const { sourceView } = setup(); + const { sourceView, assemblyView } = setup(); expect(sourceView()).not.toBeInTheDocument(); + expect(assemblyView()).not.toBeInTheDocument(); }); - it('should show the source view when double clicking on a line in the tree view', async () => { - const { sourceView } = setup(); + it('should show the source view when a line in the tree view is double-clicked', async () => { + const { sourceView, assemblyView } = setup(); const frameElement = screen.getByRole('treeitem', { name: /^A/ }); fireFullClick(frameElement); fireFullClick(frameElement, { detail: 2 }); expect(sourceView()).toBeInTheDocument(); + expect(assemblyView()).not.toBeInTheDocument(); const sourceViewElement = ensureExists(sourceView()); const sourceViewContent = await within(sourceViewElement).findByRole( @@ -117,4 +142,44 @@ describe('SourceView', () => { expect(sourceViewContent).toMatchSnapshot(); }); + + it('should show the assembly view when pressing the toggle button', async () => { + const { sourceView, assemblyView } = setup(); + + const frameElement = screen.getByRole('treeitem', { name: /^A/ }); + + fireFullClick(frameElement); + fireFullClick(frameElement, { detail: 2 }); + expect(sourceView()).toBeInTheDocument(); + expect(assemblyView()).not.toBeInTheDocument(); + + const asmViewShowButton = ensureExists( + document.querySelector('.bottom-assembly-button') + ); + fireFullClick(asmViewShowButton); + + expect(sourceView()).toBeInTheDocument(); + expect(assemblyView()).toBeInTheDocument(); + + const assemblyViewElement = ensureExists(assemblyView()); + const assemblyViewContent = await within(assemblyViewElement).findByRole( + 'textbox' + ); + + // Find one of the instructions. Once we have assembly syntax highlighting, + // we'll probably have to match on a smaller string. + await within(assemblyViewContent).findAllByText( + 'mov rdi, qword [rcx + 0x58]' + ); + + expect(assemblyViewContent).toMatchSnapshot(); + + // Click the toggle button again and make sure the assembly view hides. + const asmViewHideButton = ensureExists( + document.querySelector('.bottom-assembly-button') + ); + fireFullClick(asmViewHideButton); + + expect(assemblyView()).not.toBeInTheDocument(); + }); }); diff --git a/src/test/components/__snapshots__/SourceView.test.js.snap b/src/test/components/__snapshots__/BottomBox.test.js.snap similarity index 60% rename from src/test/components/__snapshots__/SourceView.test.js.snap rename to src/test/components/__snapshots__/BottomBox.test.js.snap index 2db3a04310..9620f6c330 100644 --- a/src/test/components/__snapshots__/SourceView.test.js.snap +++ b/src/test/components/__snapshots__/BottomBox.test.js.snap @@ -1,6 +1,67 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`SourceView should show the source view when double clicking on a line in the tree view 1`] = ` +exports[`BottomBox should show the assembly view when pressing the toggle button 1`] = ` +
+
+ push rsi +
+
+ push rdi +
+
+ push rbx +
+
+ sub rsp, 0x20 +
+
+ mov rsi, rcx +
+
+ mov rdi, qword [rcx + 0x58] +
+
+ mov ebx, edi +
+
+ and ebx, 0x1400 +
+
+ call 0x16c5d35 +
+
+`; + +exports[`BottomBox should show the source view when a line in the tree view is double-clicked 1`] = `