From 57b03a170c24078bda3f9ca8f97b82cce15f6cf6 Mon Sep 17 00:00:00 2001 From: Greg Tatum Date: Wed, 22 Jan 2020 23:11:49 +0000 Subject: [PATCH] Bug 1597381 - Create profiler presets for about:profiling; r=julienw This commit creates profiler presets for the current browser. It does not handle remote profiling yet, as there is no UI surface for it yet. The preset setting updates happen inside of the reducers and presets, and not the selectors. This design was chosen so that as the presets were changed, it would actually change the settings below, making it easy to change and customize the settings by chosing the nearest preset. The UI designs are still in a bit of a flux, so this UI design was the easiest to implement for the initial pass, but we don't yet have consensus on what the UI will look like for the final design. Differential Revision: https://phabricator.services.mozilla.com/D58465 --HG-- extra : moz-landing-system : lando --- .../client/performance-new/@types/perf.d.ts | 26 ++++ .../components/AboutProfiling.js | 5 +- .../performance-new/components/Presets.js | 126 ++++++++++++++++++ .../performance-new/components/moz.build | 1 + .../performance-new/popup/background.jsm.js | 52 +++++++- .../client/performance-new/store/actions.js | 15 +++ .../client/performance-new/store/reducers.js | 30 +++++ .../client/performance-new/store/selectors.js | 23 ++++ devtools/client/themes/aboutprofiling.css | 30 ++++- .../shared/performance-new/recording-utils.js | 34 +++++ modules/libpref/init/all.js | 3 + 11 files changed, 342 insertions(+), 3 deletions(-) create mode 100644 devtools/client/performance-new/components/Presets.js diff --git a/devtools/client/performance-new/@types/perf.d.ts b/devtools/client/performance-new/@types/perf.d.ts index bdcf5eef44f3..b8750f8e96e2 100644 --- a/devtools/client/performance-new/@types/perf.d.ts +++ b/devtools/client/performance-new/@types/perf.d.ts @@ -110,6 +110,7 @@ export interface State { features: string[]; threads: string[]; objdirs: string[]; + presetName: string; initializedValues: InitializedValues | null; promptEnvRestart: null | string } @@ -189,6 +190,7 @@ interface GeckoProfilerFrameScriptInterface { } export interface RecordingStateFromPreferences { + presetName: string; entries: number; interval: number; features: string[]; @@ -268,6 +270,11 @@ export type Action = recordingSettingsFromPreferences: RecordingStateFromPreferences; getSymbolTableGetter: (profile: object) => GetSymbolTableCallback; supportedFeatures: string[] | null; + } + | { + type: "CHANGE_PRESET"; + presetName: string; + preset: PresetDefinition | undefined; }; export interface InitializeStoreValues { @@ -306,6 +313,11 @@ export interface ContentFrameMessageManager { * one of the properties of this interface. */ export interface PerformancePref { + /** + * The recording preferences by default are controlled by different presets. + * This pref stores that preset. + */ + Preset: "devtools.performance.recording.preset"; /** * Stores the total number of entries to be used in the profile buffer. */ @@ -378,3 +390,17 @@ export interface ScaleFunctions { fromValueToFraction: NumberScaler, fromFractionToSingleDigitValue: NumberScaler, } + +export interface PresetDefinition { + label: string; + description: string; + entries: number; + interval: number; + features: string[]; + threads: string[]; + duration: number; +} + +export interface PresetDefinitions { + [presetName: string]: PresetDefinition; +} diff --git a/devtools/client/performance-new/components/AboutProfiling.js b/devtools/client/performance-new/components/AboutProfiling.js index 91c7e16c2c1c..f4661796ea09 100644 --- a/devtools/client/performance-new/components/AboutProfiling.js +++ b/devtools/client/performance-new/components/AboutProfiling.js @@ -37,6 +37,9 @@ const { const Settings = createFactory( require("devtools/client/performance-new/components/Settings.js") ); +const Presets = createFactory( + require("devtools/client/performance-new/components/Presets") +); const selectors = require("devtools/client/performance-new/store/selectors"); const { @@ -112,7 +115,7 @@ class AboutProfiling extends PureComponent { ), // The presets aren't built yet, but this div serves as a place to put the // visual divider that is in the UX mock-up. - div({ className: "perf-presets" }), + Presets(), Settings() ); } diff --git a/devtools/client/performance-new/components/Presets.js b/devtools/client/performance-new/components/Presets.js new file mode 100644 index 000000000000..28c4cf3eecb9 --- /dev/null +++ b/devtools/client/performance-new/components/Presets.js @@ -0,0 +1,126 @@ +/* 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/. */ +// @ts-check + +/** + * @template P + * @typedef {import("react-redux").ResolveThunks

} ResolveThunks

+ */ + +/** + * @typedef {Object} StateProps + * @property {string} presetName + */ + +/** + * @typedef {Object} ThunkDispatchProps + * @property {typeof actions.changePreset} changePreset + */ + +/** + * @typedef {ResolveThunks} DispatchProps + * @typedef {StateProps & DispatchProps} Props + * @typedef {import("../@types/perf").State} StoreState + */ + +"use strict"; +const { PureComponent } = require("devtools/client/shared/vendor/react"); +const { + div, + label, + input, +} = require("devtools/client/shared/vendor/react-dom-factories"); +const selectors = require("devtools/client/performance-new/store/selectors"); +const actions = require("devtools/client/performance-new/store/actions"); +const { connect } = require("devtools/client/shared/vendor/react-redux"); +const { presets } = require("devtools/shared/performance-new/recording-utils"); +/** + * Switch between various profiler presets, which will override the individualized + * settings for the profiler. + * + * @extends {React.PureComponent} + */ +class Presets extends PureComponent { + /** @param {Props} props */ + constructor(props) { + super(props); + this.onChange = this.onChange.bind(this); + } + + /** + * Handle the checkbox change. + * @param {React.ChangeEvent} event + */ + onChange(event) { + this.props.changePreset(event.target.value); + } + + /** + * @param {string} presetName + * @returns {React.ReactNode} + */ + renderPreset(presetName) { + const preset = presets[presetName]; + let labelText, description; + if (preset) { + labelText = preset.label; + description = preset.description; + } else { + labelText = "Custom"; + } + return label( + { className: "perf-presets-label" }, + div( + { className: "perf-presets-input-container" }, + input({ + className: "perf-presets-input", + type: "radio", + name: "presets", + value: presetName, + checked: this.props.presetName === presetName, + onChange: this.onChange, + }) + ), + div( + { className: "perf-presets-text" }, + div({ className: "pref-preset-text-label" }, labelText), + description + ? div({ className: "perf-presets-description" }, description) + : null + ) + ); + } + + render() { + return div( + { className: "perf-presets" }, + this.renderPreset("web-developer"), + this.renderPreset("firefox-platform"), + this.renderPreset("firefox-front-end"), + this.renderPreset("custom") + ); + } +} + +/** + * @param {StoreState} state + * @returns {StateProps} + */ +function mapStateToProps(state) { + return { + presetName: selectors.getPresetName(state), + }; +} + +/** + * @type {ThunkDispatchProps} + */ +const mapDispatchToProps = { + changePreset: actions.changePreset, +}; + +module.exports = connect( + mapStateToProps, + mapDispatchToProps +)(Presets); diff --git a/devtools/client/performance-new/components/moz.build b/devtools/client/performance-new/components/moz.build index 4fa76fb6c514..0410c06df4b4 100644 --- a/devtools/client/performance-new/components/moz.build +++ b/devtools/client/performance-new/components/moz.build @@ -8,6 +8,7 @@ DevToolsModules( 'Description.js', 'DevToolsAndPopup.js', 'DirectoryPicker.js', + 'Presets.js', 'ProfilerEventHandling.js', 'Range.js', 'RecordingButton.js', diff --git a/devtools/client/performance-new/popup/background.jsm.js b/devtools/client/performance-new/popup/background.jsm.js index cd69c20b3f29..cc5f83bb74c1 100644 --- a/devtools/client/performance-new/popup/background.jsm.js +++ b/devtools/client/performance-new/popup/background.jsm.js @@ -25,6 +25,7 @@ const { AppConstants } = ChromeUtils.import( * @typedef {import("../@types/perf").PopupBackgroundFeatures} PopupBackgroundFeatures * @typedef {import("../@types/perf").SymbolTableAsTuple} SymbolTableAsTuple * @typedef {import("../@types/perf").PerformancePref} PerformancePref + * @typedef {import("../@types/perf").PresetDefinitions} PresetDefinitions */ /** @type {PerformancePref["Entries"]} */ @@ -39,6 +40,8 @@ const THREADS_PREF = "devtools.performance.recording.threads"; const OBJDIRS_PREF = "devtools.performance.recording.objdirs"; /** @type {PerformancePref["Duration"]} */ const DURATION_PREF = "devtools.performance.recording.duration"; +/** @type {PerformancePref["Preset"]} */ +const PRESET_PREF = "devtools.performance.recording.preset"; // The following utilities are lazily loaded as they are not needed when controlling the // global state of the profiler, and only are used during specific funcationality like @@ -262,16 +265,26 @@ function _getArrayOfStringsHostPref(prefName) { function getRecordingPreferencesFromBrowser() { // If you add a new preference here, please do not forget to update // `revertRecordingPreferences` as well. + const objdirs = _getArrayOfStringsHostPref(OBJDIRS_PREF); + const presetName = Services.prefs.getCharPref(PRESET_PREF); + + // First try to get the values from a preset. + const recordingPrefs = getRecordingPrefsFromPreset(presetName, objdirs); + if (recordingPrefs) { + return recordingPrefs; + } + + // Next use the preferences to get the values. const entries = Services.prefs.getIntPref(ENTRIES_PREF); const interval = Services.prefs.getIntPref(INTERVAL_PREF); const features = _getArrayOfStringsPref(FEATURES_PREF); const threads = _getArrayOfStringsPref(THREADS_PREF); - const objdirs = _getArrayOfStringsHostPref(OBJDIRS_PREF); const duration = Services.prefs.getIntPref(DURATION_PREF); const supportedFeatures = new Set(Services.profiler.GetFeatures()); return { + presetName: "custom", entries, interval, // Validate the features before passing them to the profiler. @@ -282,10 +295,45 @@ function getRecordingPreferencesFromBrowser() { }; } +/** + * @param {string} presetName + * @param {string[]} objdirs + * @return {RecordingStateFromPreferences | null} + */ +function getRecordingPrefsFromPreset(presetName, objdirs) { + const { presets } = lazyRecordingUtils(); + + if (presetName === "custom") { + return null; + } + + const preset = presets[presetName]; + if (!preset) { + console.error(`Unknown profiler preset was encountered: "${presetName}"`); + return null; + } + + const supportedFeatures = new Set(Services.profiler.GetFeatures()); + + return { + presetName, + entries: preset.entries, + // The interval is stored in preferences as microseconds, but the preset + // defines it in terms of milliseconds. Make the conversion here. + interval: preset.interval * 1000, + // Validate the features before passing them to the profiler. + features: preset.features.filter(feature => supportedFeatures.has(feature)), + threads: preset.threads, + objdirs, + duration: preset.duration, + }; +} + /** * @param {RecordingStateFromPreferences} prefs */ function setRecordingPreferencesOnBrowser(prefs) { + Services.prefs.setCharPref(PRESET_PREF, prefs.presetName); Services.prefs.setIntPref(ENTRIES_PREF, prefs.entries); // The interval pref stores the value in microseconds for extra precision. Services.prefs.setIntPref(INTERVAL_PREF, prefs.interval); @@ -300,6 +348,7 @@ const platform = AppConstants.platform; * @type {() => void} */ function revertRecordingPreferences() { + Services.prefs.clearUserPref(PRESET_PREF); Services.prefs.clearUserPref(ENTRIES_PREF); Services.prefs.clearUserPref(INTERVAL_PREF); Services.prefs.clearUserPref(FEATURES_PREF); @@ -326,6 +375,7 @@ let _defaultPrefsForOlderFirefox; function getDefaultRecordingPreferencesForOlderFirefox() { if (!_defaultPrefsForOlderFirefox) { _defaultPrefsForOlderFirefox = { + presetName: "custom", entries: 10000000, // ~80mb, // Do not expire markers, let them roll off naturally from the circular buffer. duration: 0, diff --git a/devtools/client/performance-new/store/actions.js b/devtools/client/performance-new/store/actions.js index 14555c21adee..c627f87b0cad 100644 --- a/devtools/client/performance-new/store/actions.js +++ b/devtools/client/performance-new/store/actions.js @@ -12,6 +12,7 @@ const { const { getEnvironmentVariable, } = require("devtools/client/performance-new/browser"); +const { presets } = require("devtools/shared/performance-new/recording-utils"); /** * @typedef {import("../@types/perf").Action} Action @@ -139,6 +140,20 @@ exports.changeThreads = threads => threads, }); +/** + * Change the preset. + * @param {string} presetName + * @return {ThunkAction} + */ +exports.changePreset = presetName => + _dispatchAndUpdatePreferences({ + type: "CHANGE_PRESET", + presetName, + // Also dispatch the preset so that the reducers can pre-fill the values + // from a preset. + preset: presets[presetName], + }); + /** * Updates the recording settings for the objdirs. * @param {string[]} objdirs diff --git a/devtools/client/performance-new/store/reducers.js b/devtools/client/performance-new/store/reducers.js index 7627502e1cbc..f62a41911849 100644 --- a/devtools/client/performance-new/store/reducers.js +++ b/devtools/client/performance-new/store/reducers.js @@ -72,6 +72,8 @@ function interval(state = 1, action) { switch (action.type) { case "CHANGE_INTERVAL": return action.interval; + case "CHANGE_PRESET": + return action.preset ? action.preset.interval : state; case "INITIALIZE_STORE": return action.recordingSettingsFromPreferences.interval; default: @@ -87,6 +89,8 @@ function entries(state = 0, action) { switch (action.type) { case "CHANGE_ENTRIES": return action.entries; + case "CHANGE_PRESET": + return action.preset ? action.preset.entries : state; case "INITIALIZE_STORE": return action.recordingSettingsFromPreferences.entries; default: @@ -102,6 +106,8 @@ function features(state = [], action) { switch (action.type) { case "CHANGE_FEATURES": return action.features; + case "CHANGE_PRESET": + return action.preset ? action.preset.features : state; case "INITIALIZE_STORE": return action.recordingSettingsFromPreferences.features; default: @@ -117,6 +123,8 @@ function threads(state = [], action) { switch (action.type) { case "CHANGE_THREADS": return action.threads; + case "CHANGE_PRESET": + return action.preset ? action.preset.threads : state; case "INITIALIZE_STORE": return action.recordingSettingsFromPreferences.threads; default: @@ -139,6 +147,27 @@ function objdirs(state = [], action) { } } +/** + * The current preset name, used to select + * @type {Reducer} + */ +function presetName(state = "", action) { + switch (action.type) { + case "INITIALIZE_STORE": + return action.recordingSettingsFromPreferences.presetName; + case "CHANGE_PRESET": + return action.presetName; + case "CHANGE_INTERVAL": + case "CHANGE_ENTRIES": + case "CHANGE_FEATURES": + case "CHANGE_THREADS": + // When updating any values, switch the preset over to "custom". + return "custom"; + default: + return state; + } +} + /** * These are all the values used to initialize the profiler. They should never * change once added to the store. @@ -192,6 +221,7 @@ module.exports = combineReducers({ features, threads, objdirs, + presetName, initializedValues, promptEnvRestart, }); diff --git a/devtools/client/performance-new/store/selectors.js b/devtools/client/performance-new/store/selectors.js index c6860c89f9b6..cbda8eb52244 100644 --- a/devtools/client/performance-new/store/selectors.js +++ b/devtools/client/performance-new/store/selectors.js @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ // @ts-check "use strict"; +const { presets } = require("devtools/shared/performance-new/recording-utils"); /** * @typedef {import("../@types/perf").RecordingState} RecordingState @@ -49,6 +50,9 @@ const getThreadsString = state => getThreads(state).join(","); /** @type {Selector} */ const getObjdirs = state => state.objdirs; +/** @type {Selector} */ +const getPresetName = state => state.presetName; + /** * Warning! This function returns a new object on every run, and so should not * be used directly as a React prop. @@ -56,7 +60,25 @@ const getObjdirs = state => state.objdirs; * @type {Selector} */ const getRecordingSettings = state => { + const presetName = getPresetName(state); + const preset = presets[presetName]; + if (preset) { + // Use the the settings from the preset. + return { + presetName: presetName, + entries: preset.entries, + interval: preset.interval, + features: preset.features, + threads: preset.threads, + objdirs: getObjdirs(state), + // The client doesn't implement durations yet. See Bug 1587165. + duration: preset.duration, + }; + } + + // Use the the custom settings from the panel. return { + presetName: "custom", entries: getEntries(state), interval: getInterval(state), features: getFeatures(state), @@ -110,6 +132,7 @@ module.exports = { getThreads, getThreadsString, getObjdirs, + getPresetName, getRecordingSettings, getInitializedValues, getPerfFront, diff --git a/devtools/client/themes/aboutprofiling.css b/devtools/client/themes/aboutprofiling.css index 78673b21ffe8..74409676209e 100644 --- a/devtools/client/themes/aboutprofiling.css +++ b/devtools/client/themes/aboutprofiling.css @@ -45,6 +45,7 @@ h2 { .perf-presets { margin: 2em 0; border-bottom: 1px solid var(--in-content-border-color); + padding-bottom: 1em; } .perf-settings-row { @@ -105,7 +106,6 @@ h2 { .perf-settings-feature-label { margin: 1.3em 0; - } .perf-settings-checkbox-and-name { @@ -168,3 +168,31 @@ h2 { .perf-settings-dir-list-button-group input[type=button] { margin-left: 0; } + +.perf-presets-input-container { + height: 20px; + display: flex; +} + +.perf-presets-input { + width: 15px; + margin: 0 10px; +} + +.pref-preset-text-label { + height: 20px; + display: flex; + align-items: center; +} + +.perf-presets-description { + font-size: 13px; + color: var(--grey-60); + margin: 5px 0; +} + +.perf-presets-label { + padding: 7px 0; + display: flex; + align-items: start; +} diff --git a/devtools/shared/performance-new/recording-utils.js b/devtools/shared/performance-new/recording-utils.js index f0c13646c251..93049b7d6601 100644 --- a/devtools/shared/performance-new/recording-utils.js +++ b/devtools/shared/performance-new/recording-utils.js @@ -11,6 +11,7 @@ /** * @typedef {import("../../client/performance-new/@types/perf").GetActiveBrowsingContextID} GetActiveBrowsingContextID + * @typedef {import("../../client/performance-new/@types/perf").PresetDefinitions} PresetDefinitions */ /** @@ -64,6 +65,39 @@ function getActiveBrowsingContextID() { return 0; } +/** @type {PresetDefinitions} */ +const presets = { + "web-developer": { + label: "Web Developer", + description: + "Recommended preset for most web app debuggging, with low overhead.", + entries: 10000000, + interval: 1, + features: ["js"], + threads: ["GeckoMain", "Compositor", "Renderer", "DOM Worker"], + duration: 0, + }, + "firefox-platform": { + label: "Firefox Platform", + description: "Recommended preset for internal Firefox platform debugging.", + entries: 10000000, + interval: 1, + features: ["js", "leaf", "stackwalk"], + threads: ["GeckoMain", "Compositor", "Renderer"], + duration: 0, + }, + "firefox-front-end": { + label: "Firefox Front-End", + description: "Recommended preset for internal Firefox front-end debugging.", + entries: 10000000, + interval: 1, + features: ["js", "leaf", "stackwalk"], + threads: ["GeckoMain", "Compositor", "Renderer", "DOM Worker"], + duration: 0, + }, +}; + module.exports = { getActiveBrowsingContextID, + presets, }; diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 08e721607920..a0386c1e88fa 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -860,6 +860,9 @@ pref("devtools.recordreplay.cloudServer", ""); // profiler.firefox.com, or in tests. This isn't exposed directly to the user. pref("devtools.performance.recording.ui-base-url", "https://profiler.firefox.com"); +// The preset to use for the recording settings. If set to "custom" then the pref +// values below will be used. +pref("devtools.performance.recording.preset", "web-developer"); // Profiler buffer size. It is the maximum number of 8-bytes entries in the // profiler's buffer. 10000000 is ~80mb. pref("devtools.performance.recording.entries", 10000000);