From 8593e43ebd25ca6d27720a0d9214c5cd3c3a307f Mon Sep 17 00:00:00 2001 From: Jordan Santell Date: Sat, 2 May 2015 16:47:41 -0700 Subject: [PATCH] Bug 1159052 - Performance recording should stop rendering and recording as soon as the recording stops. r=vp --- .../devtools/performance/modules/actors.js | 15 ++- browser/devtools/performance/modules/front.js | 20 ++-- .../performance/modules/recording-model.js | 32 +++++- .../performance/performance-controller.js | 97 +++++++++---------- .../devtools/performance/performance-view.js | 58 +++++------ browser/devtools/performance/performance.xul | 5 +- browser/devtools/performance/test/browser.ini | 1 + .../test/browser_perf-console-record-01.js | 4 +- .../test/browser_perf-console-record-02.js | 8 +- .../test/browser_perf-console-record-03.js | 8 +- .../test/browser_perf-highlighted.js | 4 +- .../test/browser_perf-recording-model-01.js | 62 ++++++++++++ browser/devtools/performance/test/head.js | 7 +- .../views/details-abstract-subview.js | 6 +- browser/devtools/performance/views/details.js | 20 ++-- .../devtools/performance/views/overview.js | 80 +++++++-------- .../devtools/performance/views/recordings.js | 4 - 17 files changed, 260 insertions(+), 171 deletions(-) create mode 100644 browser/devtools/performance/test/browser_perf-recording-model-01.js diff --git a/browser/devtools/performance/modules/actors.js b/browser/devtools/performance/modules/actors.js index 61fec66b177f..26fd96fff5c0 100644 --- a/browser/devtools/performance/modules/actors.js +++ b/browser/devtools/performance/modules/actors.js @@ -223,10 +223,17 @@ MemoryFrontFacade.prototype = { yield this.attach(); - let startTime = yield this.startRecordingAllocations({ - probability: options.allocationsSampleProbability, - maxLogLength: options.allocationsMaxLogLength - }); + // Reconstruct our options because the server actor fails when being passed + // undefined values in objects. + let allocationOptions = {}; + if (options.allocationsSampleProbability !== void 0) { + allocationOptions.probability = options.allocationsSampleProbability; + } + if (options.allocationsMaxLogLength !== void 0) { + allocationOptions.maxLogLength = options.allocationsMaxLogLength; + } + + let startTime = yield this.startRecordingAllocations(allocationOptions); yield this._pullAllocationSites(); diff --git a/browser/devtools/performance/modules/front.js b/browser/devtools/performance/modules/front.js index 0ce6e8307c8a..73a5a67b0218 100644 --- a/browser/devtools/performance/modules/front.js +++ b/browser/devtools/performance/modules/front.js @@ -27,9 +27,8 @@ const DEFAULT_ALLOCATION_SITES_PULL_TIMEOUT = 200; // ms // Events to pipe from PerformanceActorsConnection to the PerformanceFront const CONNECTION_PIPE_EVENTS = [ - "console-profile-start", "console-profile-ending", "console-profile-end", "timeline-data", "profiler-already-active", "profiler-activated", - "recording-started", "recording-stopped" + "recording-starting", "recording-started", "recording-stopping", "recording-stopped" ]; /** @@ -227,8 +226,6 @@ PerformanceActorsConnection.prototype = { console: true, label: profileLabel })); - - this.emit("console-profile-start", model); }), /** @@ -271,9 +268,7 @@ PerformanceActorsConnection.prototype = { return; } - this.emit("console-profile-ending", model); yield this.stopRecording(model); - this.emit("console-profile-end", model); }), /** @@ -309,6 +304,8 @@ PerformanceActorsConnection.prototype = { */ startRecording: Task.async(function*(options = {}) { let model = new RecordingModel(options); + this.emit("recording-starting", model); + // All actors are started asynchronously over the remote debugging protocol. // Get the corresponding start times from each one of them. // The timeline and memory actors are target-dependent, so start those as well, @@ -343,6 +340,13 @@ PerformanceActorsConnection.prototype = { return; } + // Flag the recording as no longer recording, so that `model.isRecording()` + // is false. Do this before we fetch all the data, and then subsequently + // the recording can be considered "completed". + let endTime = Date.now(); + model._onStoppingRecording(endTime); + this.emit("recording-stopping", model); + // Currently there are two ways profiles stop recording. Either manually in the // performance tool, or via console.profileEnd. Once a recording is done, // we want to deliver the model to the performance tool (either as a return @@ -485,7 +489,9 @@ PerformanceFront.prototype = { } let actor = this._connection[`_${actorName}`]; return actor[method].apply(actor, args); - } + }, + + toString: () => "[object PerformanceFront]" }; /** diff --git a/browser/devtools/performance/modules/recording-model.js b/browser/devtools/performance/modules/recording-model.js index 6d92bc901be6..bd29ff7a0de2 100644 --- a/browser/devtools/performance/modules/recording-model.js +++ b/browser/devtools/performance/modules/recording-model.js @@ -38,6 +38,7 @@ RecordingModel.prototype = { _console: false, _imported: false, _recording: false, + _completed: false, _profilerStartTime: 0, _timelineStartTime: 0, _memoryStartTime: 0, @@ -94,7 +95,7 @@ RecordingModel.prototype = { // However, we also want to update the view with the elapsed time // even when the actor is not generating data. To do this we get // the local time and use it to compute a reasonable elapsed time. - this._localStartTime = Date.now() + this._localStartTime = Date.now(); this._profilerStartTime = info.profilerStartTime; this._timelineStartTime = info.timelineStartTime; @@ -108,19 +109,30 @@ RecordingModel.prototype = { this._allocations = { sites: [], timestamps: [], frames: [], counts: [] }; }, + /** + * Called when the signal was sent to the front to no longer record more + * data, and begin fetching the data. There's some delay during fetching, + * even though the recording is stopped, the model is not yet completed until + * all the data is fetched. + */ + _onStoppingRecording: function (endTime) { + this._duration = endTime - this._localStartTime; + this._recording = false; + }, + /** * Sets results available from stopping a recording from SharedPerformanceConnection. * Should only be called by SharedPerformanceConnection. */ _onStopRecording: Task.async(function *(info) { this._profile = info.profile; - this._duration = info.profilerEndTime - this._profilerStartTime; - this._recording = false; + this._completed = true; // We filter out all samples that fall out of current profile's range // since the profiler is continuously running. Because of this, sample // times are not guaranteed to have a zero epoch, so offset the // timestamps. + // TODO move this into FakeProfilerFront in ./actors.js after bug 1154115 RecordingUtils.offsetSampleTimes(this._profile, this._profilerStartTime); // Markers need to be sorted ascending by time, to be properly displayed @@ -248,9 +260,21 @@ RecordingModel.prototype = { return this._console; }, + /** + * Returns a boolean indicating whether or not this recording model + * has finished recording. + * There is some delay in fetching data between when the recording stops, and + * when the recording is considered completed once it has all the profiler and timeline data. + */ + isCompleted: function () { + return this._completed || this.isImported(); + }, + /** * Returns a boolean indicating whether or not this recording model * is recording. + * A model may no longer be recording, yet still not have the profiler data. In that + * case, use `isCompleted()`. */ isRecording: function () { return this._recording; @@ -262,7 +286,7 @@ RecordingModel.prototype = { addTimelineData: function (eventName, ...data) { // If this model isn't currently recording, // ignore the timeline data. - if (!this._recording) { + if (!this.isRecording()) { return; } diff --git a/browser/devtools/performance/performance-controller.js b/browser/devtools/performance/performance-controller.js index 3a5e8864e574..189a7055c616 100644 --- a/browser/devtools/performance/performance-controller.js +++ b/browser/devtools/performance/performance-controller.js @@ -63,13 +63,6 @@ const EVENTS = { // Fired by the PerformanceController when the devtools theme changes. THEME_CHANGED: "Performance:ThemeChanged", - // When the SharedPerformanceConnection handles profiles created via `console.profile()`, - // the controller handles those events and emits the below events for consumption - // by other views. - CONSOLE_RECORDING_STARTED: "Performance:ConsoleRecordingStarted", - CONSOLE_RECORDING_WILL_STOP: "Performance:ConsoleRecordingWillStop", - CONSOLE_RECORDING_STOPPED: "Performance:ConsoleRecordingStopped", - // Emitted by the PerformanceView when the state (display mode) changes, // for example when switching between "empty", "recording" or "recorded". // This causes certain panels to be hidden or visible. @@ -190,9 +183,7 @@ let PerformanceController = { this._onRecordingSelectFromView = this._onRecordingSelectFromView.bind(this); this._onPrefChanged = this._onPrefChanged.bind(this); this._onThemeChanged = this._onThemeChanged.bind(this); - this._onConsoleProfileStart = this._onConsoleProfileStart.bind(this); - this._onConsoleProfileEnd = this._onConsoleProfileEnd.bind(this); - this._onConsoleProfileEnding = this._onConsoleProfileEnding.bind(this); + this._onRecordingStateChange = this._onRecordingStateChange.bind(this); // All boolean prefs should be handled via the OptionsView in the // ToolbarView, so that they may be accessible via the "gear" menu. @@ -208,9 +199,10 @@ let PerformanceController = { this._nonBooleanPrefs.registerObserver(); this._nonBooleanPrefs.on("pref-changed", this._onPrefChanged); - gFront.on("console-profile-start", this._onConsoleProfileStart); - gFront.on("console-profile-ending", this._onConsoleProfileEnding); - gFront.on("console-profile-end", this._onConsoleProfileEnd); + gFront.on("recording-starting", this._onRecordingStateChange); + gFront.on("recording-started", this._onRecordingStateChange); + gFront.on("recording-stopping", this._onRecordingStateChange); + gFront.on("recording-stopped", this._onRecordingStateChange); ToolbarView.on(EVENTS.PREF_CHANGED, this._onPrefChanged); PerformanceView.on(EVENTS.UI_START_RECORDING, this.startRecording); PerformanceView.on(EVENTS.UI_STOP_RECORDING, this.stopRecording); @@ -229,9 +221,10 @@ let PerformanceController = { this._nonBooleanPrefs.unregisterObserver(); this._nonBooleanPrefs.off("pref-changed", this._onPrefChanged); - gFront.off("console-profile-start", this._onConsoleProfileStart); - gFront.off("console-profile-ending", this._onConsoleProfileEnding); - gFront.off("console-profile-end", this._onConsoleProfileEnd); + gFront.off("recording-starting", this._onRecordingStateChange); + gFront.off("recording-started", this._onRecordingStateChange); + gFront.off("recording-stopping", this._onRecordingStateChange); + gFront.off("recording-stopped", this._onRecordingStateChange); ToolbarView.off(EVENTS.PREF_CHANGED, this._onPrefChanged); PerformanceView.off(EVENTS.UI_START_RECORDING, this.startRecording); PerformanceView.off(EVENTS.UI_STOP_RECORDING, this.stopRecording); @@ -300,12 +293,7 @@ let PerformanceController = { sampleFrequency: this.getPref("profiler-sample-frequency") }; - this.emit(EVENTS.RECORDING_WILL_START); - - let recording = yield gFront.startRecording(options); - this._recordings.push(recording); - - this.emit(EVENTS.RECORDING_STARTED, recording); + yield gFront.startRecording(options); }), /** @@ -315,9 +303,7 @@ let PerformanceController = { stopRecording: Task.async(function *() { let recording = this.getLatestManualRecording(); - this.emit(EVENTS.RECORDING_WILL_STOP, recording); yield gFront.stopRecording(recording); - this.emit(EVENTS.RECORDING_STOPPED, recording); }), /** @@ -344,6 +330,11 @@ let PerformanceController = { if (latest && latest.isRecording()) { yield this.stopRecording(); } + // If last recording is not recording, but finalizing itself, + // wait for that to finish + if (latest && !latest.isCompleted()) { + yield this.once(EVENTS.RECORDING_STOPPED); + } this._recordings.length = 0; this.setCurrentRecording(null); @@ -440,27 +431,33 @@ let PerformanceController = { }, /** - * Fired when `console.profile()` is executed. + * Fired when a recording model changes state. + * + * @param {string} state + * @param {RecordingModel} model */ - _onConsoleProfileStart: function (_, recording) { - this._recordings.push(recording); - this.emit(EVENTS.CONSOLE_RECORDING_STARTED, recording); - }, - - /** - * Fired when `console.profileEnd()` is executed, and the profile - * is stopping soon, as it fetches profiler data. - */ - _onConsoleProfileEnding: function (_, recording) { - this.emit(EVENTS.CONSOLE_RECORDING_WILL_STOP, recording); - }, - - /** - * Fired when `console.profileEnd()` is executed, and - * has a corresponding `console.profile()` session. - */ - _onConsoleProfileEnd: function (_, recording) { - this.emit(EVENTS.CONSOLE_RECORDING_STOPPED, recording); + _onRecordingStateChange: function (state, model) { + switch (state) { + // Fired when a RecordingModel was just created from the front + case "recording-starting": + // When a recording is just starting, store it internally + this._recordings.push(model); + this.emit(EVENTS.RECORDING_WILL_START, model); + break; + // Fired when a RecordingModel has started recording + case "recording-started": + this.emit(EVENTS.RECORDING_STARTED, model); + break; + // Fired when a RecordingModel is no longer recording, and + // starting to fetch all the profiler data + case "recording-stopping": + this.emit(EVENTS.RECORDING_WILL_STOP, model); + break; + // Fired when a RecordingModel is finished fetching all of its data + case "recording-stopped": + this.emit(EVENTS.RECORDING_STOPPED, model); + break; + } }, /** @@ -480,21 +477,21 @@ let PerformanceController = { * model, like `withTicks`, or `withMemory`. * @option {Array} actors * An array of strings indicating what actors must exist. - * @option {boolean} isRecording - * A boolean indicating whether the recording must be either recording or not - * recording. Setting to undefined will allow either state. + * @option {boolean} mustBeCompleted + * A boolean indicating whether the recording must be either completed or not. + * Setting to undefined will allow either state. * @param {RecordingModel} recording * An optional recording model to use instead of the currently selected. * * @return boolean */ - isFeatureSupported: function ({ features, actors, isRecording: shouldBeRecording }, recording) { + isFeatureSupported: function ({ features, actors, mustBeCompleted }, recording) { recording = recording || this.getCurrentRecording(); let recordingConfig = recording ? recording.getConfiguration() : {}; - let currentRecordingState = recording ? recording.isRecording() : void 0; + let currentCompletedState = recording ? recording.isCompleted() : void 0; let actorsSupported = gFront.getActorSupport(); - if (shouldBeRecording != null && shouldBeRecording !== currentRecordingState) { + if (mustBeCompleted != null && mustBeCompleted !== currentCompletedState) { return false; } if (actors && !actors.every(a => actorsSupported[a])) { diff --git a/browser/devtools/performance/performance-view.js b/browser/devtools/performance/performance-view.js index 91b1198e8c5e..64b38f06cdd1 100644 --- a/browser/devtools/performance/performance-view.js +++ b/browser/devtools/performance/performance-view.js @@ -41,12 +41,11 @@ let PerformanceView = { this._onRecordButtonClick = this._onRecordButtonClick.bind(this); this._onImportButtonClick = this._onImportButtonClick.bind(this); this._onClearButtonClick = this._onClearButtonClick.bind(this); - this._lockRecordButton = this._lockRecordButton.bind(this); - this._unlockRecordButton = this._unlockRecordButton.bind(this); + this._lockRecordButtons = this._lockRecordButtons.bind(this); + this._unlockRecordButtons = this._unlockRecordButtons.bind(this); this._onRecordingSelected = this._onRecordingSelected.bind(this); this._onRecordingStopped = this._onRecordingStopped.bind(this); - this._onRecordingWillStop = this._onRecordingWillStop.bind(this); - this._onRecordingWillStart = this._onRecordingWillStart.bind(this); + this._onRecordingStarted = this._onRecordingStarted.bind(this); for (let button of $$(".record-button")) { button.addEventListener("click", this._onRecordButtonClick); @@ -55,11 +54,8 @@ let PerformanceView = { this._clearButton.addEventListener("click", this._onClearButtonClick); // Bind to controller events to unlock the record button - PerformanceController.on(EVENTS.RECORDING_WILL_START, this._onRecordingWillStart); - PerformanceController.on(EVENTS.RECORDING_WILL_STOP, this._onRecordingWillStop); - PerformanceController.on(EVENTS.RECORDING_STARTED, this._unlockRecordButton); + PerformanceController.on(EVENTS.RECORDING_STARTED, this._onRecordingStarted); PerformanceController.on(EVENTS.RECORDING_STOPPED, this._onRecordingStopped); - PerformanceController.on(EVENTS.CONSOLE_RECORDING_STOPPED, this._onRecordingStopped); PerformanceController.on(EVENTS.RECORDING_SELECTED, this._onRecordingSelected); this.setState("empty"); @@ -82,11 +78,8 @@ let PerformanceView = { this._importButton.removeEventListener("click", this._onImportButtonClick); this._clearButton.removeEventListener("click", this._onClearButtonClick); - PerformanceController.off(EVENTS.RECORDING_WILL_START, this._onRecordingWillStart); - PerformanceController.off(EVENTS.RECORDING_WILL_STOP, this._onRecordingWillStop); - PerformanceController.off(EVENTS.RECORDING_STARTED, this._unlockRecordButton); + PerformanceController.off(EVENTS.RECORDING_STARTED, this._onRecordingStarted); PerformanceController.off(EVENTS.RECORDING_STOPPED, this._onRecordingStopped); - PerformanceController.off(EVENTS.CONSOLE_RECORDING_STOPPED, this._onRecordingStopped); PerformanceController.off(EVENTS.RECORDING_SELECTED, this._onRecordingSelected); yield ToolbarView.destroy(); @@ -130,31 +123,30 @@ let PerformanceView = { * Adds the `locked` attribute on the record button. This prevents it * from being clicked while recording is started or stopped. */ - _lockRecordButton: function () { - this._recordButton.setAttribute("locked", "true"); + _lockRecordButtons: function () { + for (let button of $$(".record-button")) { + button.setAttribute("locked", "true"); + } }, /** * Removes the `locked` attribute on the record button. */ - _unlockRecordButton: function () { - this._recordButton.removeAttribute("locked"); + _unlockRecordButtons: function () { + for (let button of $$(".record-button")) { + button.removeAttribute("locked"); + } }, /** - * Fired when a recording is starting, but not yet completed. + * When a recording has started. */ - _onRecordingWillStart: function () { - this._lockRecordButton(); - this._recordButton.setAttribute("checked", "true"); - }, - - /** - * Fired when a recording is stopping, but not yet completed. - */ - _onRecordingWillStop: function () { - this._lockRecordButton(); - this._recordButton.removeAttribute("checked"); + _onRecordingStarted: function (_, recording) { + // A stopped recording can be from `console.profileEnd` -- only unlock + // the button if it's the main recording that was started via UI. + if (!recording.isConsole()) { + this._unlockRecordButtons(); + } }, /** @@ -164,7 +156,7 @@ let PerformanceView = { // A stopped recording can be from `console.profileEnd` -- only unlock // the button if it's the main recording that was started via UI. if (!recording.isConsole()) { - this._unlockRecordButton(); + this._unlockRecordButtons(); } // If the currently selected recording is the one that just stopped, @@ -187,7 +179,15 @@ let PerformanceView = { _onRecordButtonClick: function (e) { if (this._recordButton.hasAttribute("checked")) { this.emit(EVENTS.UI_STOP_RECORDING); + this._lockRecordButtons(); + for (let button of $$(".record-button")) { + button.removeAttribute("checked"); + } } else { + this._lockRecordButtons(); + for (let button of $$(".record-button")) { + button.setAttribute("checked", "true"); + } this.emit(EVENTS.UI_START_RECORDING); } }, diff --git a/browser/devtools/performance/performance.xul b/browser/devtools/performance/performance.xul index 6b6c72968c37..426bad84315c 100644 --- a/browser/devtools/performance/performance.xul +++ b/browser/devtools/performance/performance.xul @@ -77,7 +77,7 @@