From 1723b72c7b40b0b767e2ba8dd5d143b5aed58c04 Mon Sep 17 00:00:00 2001 From: Steven MacLeod Date: Wed, 31 Jul 2013 21:15:26 -0400 Subject: [PATCH] Bug 898184 - Restore telemetry measurements that were removed when transitioning to the SessionWorker. r=ttaubert --- .../sessionstore/src/SessionWorker.js | 55 +++++++++++++------ .../sessionstore/src/_SessionFile.jsm | 24 +++++--- 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/browser/components/sessionstore/src/SessionWorker.js b/browser/components/sessionstore/src/SessionWorker.js index 6fd765e778f8..b0381be28a9e 100644 --- a/browser/components/sessionstore/src/SessionWorker.js +++ b/browser/components/sessionstore/src/SessionWorker.js @@ -21,8 +21,8 @@ let Decoder = new TextDecoder(); * {fun:function_name, args:array_of_arguments_or_null, id: custom_id} * * Sends messages: - * {ok: result, id: custom_id} / {fail: serialized_form_of_OS.File.Error, - * id: custom_id} + * {ok: result, id: custom_id, telemetry: {}} / + * {fail: serialized_form_of_OS.File.Error, id: custom_id} */ self.onmessage = function (msg) { let data = msg.data; @@ -34,7 +34,7 @@ self.onmessage = function (msg) { let id = data.id; try { - result = Agent[data.fun].apply(Agent, data.args); + result = Agent[data.fun].apply(Agent, data.args) || {}; } catch (ex if ex instanceof OS.File.Error) { // Instances of OS.File.Error know how to serialize themselves // (deserialization ensures that we end up with OS-specific @@ -46,7 +46,11 @@ self.onmessage = function (msg) { // Other exceptions do not, and should be propagated through DOM's // built-in mechanism for uncaught errors, although this mechanism // may lose interesting information. - self.postMessage({ok: result, id: id}); + self.postMessage({ + ok: result.result, + id: id, + telemetry: result.telemetry || {} + }); }; let Agent = { @@ -100,24 +104,34 @@ let Agent = { read: function () { for (let path of [this.path, this.backupPath]) { try { - return this.initialState = Decoder.decode(File.read(path)); + let durationMs = Date.now(); + let bytes = File.read(path); + durationMs = Date.now() - durationMs; + this.initialState = Decoder.decode(bytes); + + return { + result: this.initialState, + telemetry: {FX_SESSION_RESTORE_READ_FILE_MS: durationMs} + }; } catch (ex if isNoSuchFileEx(ex)) { // Ignore exceptions about non-existent files. } } - // No sessionstore data files found. Return an empty string. - return ""; + return {result: ""}; }, /** * Write the session to disk. */ write: function (stateString, options) { + let telemetry = {}; if (!this.hasWrittenState) { if (options && options.backupOnFirstWrite) { try { + let startMs = Date.now(); File.move(this.path, this.backupPath); + telemetry.FX_SESSION_RESTORE_BACKUP_FILE_MS = Date.now() - startMs; } catch (ex if isNoSuchFileEx(ex)) { // Ignore exceptions about non-existent files. } @@ -126,8 +140,7 @@ let Agent = { this.hasWrittenState = true; } - let bytes = Encoder.encode(stateString); - return File.writeAtomic(this.path, bytes, {tmpPath: this.path + ".tmp"}); + return this._write(stateString, telemetry); }, /** @@ -158,8 +171,18 @@ let Agent = { state.session = state.session || {}; state.session.state = loadState; - let bytes = Encoder.encode(JSON.stringify(state)); - return File.writeAtomic(this.path, bytes, {tmpPath: this.path + ".tmp"}); + return this._write(JSON.stringify(state)); + }, + + /** + * Write a stateString to disk + */ + _write: function (stateString, telemetry = {}) { + let bytes = Encoder.encode(stateString); + let startMs = Date.now(); + let result = File.writeAtomic(this.path, bytes, {tmpPath: this.path + ".tmp"}); + telemetry.FX_SESSION_RESTORE_WRITE_FILE_MS = Date.now() - startMs; + return {result: result, telemetry: telemetry}; }, /** @@ -167,10 +190,10 @@ let Agent = { */ createBackupCopy: function (ext) { try { - return File.copy(this.path, this.backupPath + ext); + return {result: File.copy(this.path, this.backupPath + ext)}; } catch (ex if isNoSuchFileEx(ex)) { // Ignore exceptions about non-existent files. - return true; + return {result: true}; } }, @@ -179,10 +202,10 @@ let Agent = { */ removeBackupCopy: function (ext) { try { - return File.remove(this.backupPath + ext); + return {result: File.remove(this.backupPath + ext)}; } catch (ex if isNoSuchFileEx(ex)) { // Ignore exceptions about non-existent files. - return true; + return {result: true}; } }, @@ -219,7 +242,7 @@ let Agent = { throw exn; } - return true; + return {result: true}; } }; diff --git a/browser/components/sessionstore/src/_SessionFile.jsm b/browser/components/sessionstore/src/_SessionFile.jsm index c6dbd5b80a56..0fe3ac1d1230 100644 --- a/browser/components/sessionstore/src/_SessionFile.jsm +++ b/browser/components/sessionstore/src/_SessionFile.jsm @@ -203,13 +203,15 @@ let SessionFileInternal = { }, read: function () { - return SessionWorker.post("read").then(msg => msg.ok); + return SessionWorker.post("read").then(msg => { + this._recordTelemetry(msg.telemetry); + return msg.ok; + }); }, write: function (aData, aOptions) { let refObj = {}; return TaskUtils.spawn(function task() { - TelemetryStopwatch.start("FX_SESSION_RESTORE_WRITE_FILE_MS", refObj); TelemetryStopwatch.start("FX_SESSION_RESTORE_WRITE_FILE_LONGEST_OP_MS", refObj); try { @@ -217,12 +219,11 @@ let SessionFileInternal = { // At this point, we measure how long we stop the main thread TelemetryStopwatch.finish("FX_SESSION_RESTORE_WRITE_FILE_LONGEST_OP_MS", refObj); - // Now wait for the result and measure how long we had to wait for the result - yield promise; - TelemetryStopwatch.finish("FX_SESSION_RESTORE_WRITE_FILE_MS", refObj); + // Now wait for the result and record how long the write took + let msg = yield promise; + this._recordTelemetry(msg.telemetry); } catch (ex) { TelemetryStopwatch.cancel("FX_SESSION_RESTORE_WRITE_FILE_LONGEST_OP_MS", refObj); - TelemetryStopwatch.cancel("FX_SESSION_RESTORE_WRITE_FILE_MS", refObj); Cu.reportError("Could not write session state file " + this.path + ": " + ex); } @@ -230,7 +231,10 @@ let SessionFileInternal = { }, writeLoadStateOnceAfterStartup: function (aLoadState) { - return SessionWorker.post("writeLoadStateOnceAfterStartup", [aLoadState]); + return SessionWorker.post("writeLoadStateOnceAfterStartup", [aLoadState]).then(msg => { + this._recordTelemetry(msg.telemetry); + return msg; + }); }, createBackupCopy: function (ext) { @@ -243,6 +247,12 @@ let SessionFileInternal = { wipe: function () { return SessionWorker.post("wipe"); + }, + + _recordTelemetry: function(telemetry) { + for (let histogramId in telemetry){ + Telemetry.getHistogramById(histogramId).add(telemetry[histogramId]); + } } };