Bug 1129029 - Telemetry probes for reader mode performance. r=Gijs, a=sledru FIREFOX_BETA_38_END
authorMargaret Leibovic <margaret.leibovic@gmail.com>
Thu, 30 Apr 2015 14:51:02 -0700
changeset 260461 85229fbaf017
parent 260460 5fc66f6dd277
child 260462 fff143cacb66
child 267424 43ff9e99e73f
push id786
push userryanvm@gmail.com
push date2015-05-11 14:41 +0000
treeherdermozilla-release@85229fbaf017 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs, sledru
bugs1129029
milestone38.0.5
Bug 1129029 - Telemetry probes for reader mode performance. r=Gijs, a=sledru
toolkit/components/reader/ReaderMode.jsm
toolkit/components/telemetry/Histograms.json
--- a/toolkit/components/reader/ReaderMode.jsm
+++ b/toolkit/components/reader/ReaderMode.jsm
@@ -3,25 +3,36 @@
  * 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/. */
 "use strict";
 
 this.EXPORTED_SYMBOLS = ["ReaderMode"];
 
 const { classes: Cc, interfaces: Ci, utils: Cu } = Components;
 
+// Constants for telemetry.
+const DOWNLOAD_SUCCESS = 0;
+const DOWNLOAD_ERROR_XHR = 1;
+const DOWNLOAD_ERROR_NO_DOC = 2;
+
+const PARSE_SUCCESS = 0;
+const PARSE_ERROR_TOO_MANY_ELEMENTS = 1;
+const PARSE_ERROR_WORKER = 2;
+const PARSE_ERROR_NO_ARTICLE = 3;
+
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 Cu.importGlobalProperties(["XMLHttpRequest"]);
 
 XPCOMUtils.defineLazyModuleGetter(this, "CommonUtils", "resource://services-common/utils.js");
 XPCOMUtils.defineLazyModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ReaderWorker", "resource://gre/modules/reader/ReaderWorker.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task", "resource://gre/modules/Task.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "TelemetryStopwatch", "resource://gre/modules/TelemetryStopwatch.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "Readability", function() {
   let scope = {};
   scope.dump = this.dump;
   Services.scriptloader.loadSubScript("resource://gre/modules/reader/Readability.js", scope);
   return scope["Readability"];
 });
 
@@ -152,48 +163,61 @@ this.ReaderMode = {
    * Downloads and parses a document from a URL.
    *
    * @param url URL to download and parse.
    * @return {Promise}
    * @resolves JS object representing the article, or null if no article is found.
    */
   downloadAndParseDocument: Task.async(function* (url) {
     let uri = Services.io.newURI(url, null, null);
-    let doc = yield this._downloadDocument(url);
+    TelemetryStopwatch.start("READER_MODE_DOWNLOAD_MS");
+    let doc = yield this._downloadDocument(url).catch(e => {
+      TelemetryStopwatch.finish("READER_MODE_DOWNLOAD_MS");
+      throw e;
+    });
+    TelemetryStopwatch.finish("READER_MODE_DOWNLOAD_MS");
     return yield this._readerParse(uri, doc);
   }),
 
   _downloadDocument: function (url) {
+    let histogram = Services.telemetry.getHistogramById("READER_MODE_DOWNLOAD_RESULT");
     return new Promise((resolve, reject) => {
       let xhr = new XMLHttpRequest();
       xhr.open("GET", url, true);
       xhr.onerror = evt => reject(evt.error);
       xhr.responseType = "document";
       xhr.onload = evt => {
         if (xhr.status !== 200) {
           reject("Reader mode XHR failed with status: " + xhr.status);
+          histogram.add(DOWNLOAD_ERROR_XHR);
           return;
         }
 
         let doc = xhr.responseXML;
+        if (!doc) {
+          reject("Reader mode XHR didn't return a document");
+          histogram.add(DOWNLOAD_ERROR_NO_DOC);
+          return;
+        }
 
         // Manually follow a meta refresh tag if one exists.
         let meta = doc.querySelector("meta[http-equiv=refresh]");
         if (meta) {
           let content = meta.getAttribute("content");
           if (content) {
             let urlIndex = content.indexOf("URL=");
             if (urlIndex > -1) {
               let url = content.substring(urlIndex + 4);
               this._downloadDocument(url).then((doc) => resolve(doc));
               return;
             }
           }
         }
         resolve(doc);
+        histogram.add(DOWNLOAD_SUCCESS);
       }
       xhr.send();
     });
   },
 
 
   /**
    * Retrieves an article from the cache given an article URI.
@@ -271,55 +295,65 @@ this.ReaderMode = {
    * in readerWorker.js.
    *
    * @param uri The article URI.
    * @param doc The document to parse.
    * @return {Promise}
    * @resolves JS object representing the article, or null if no article is found.
    */
   _readerParse: Task.async(function* (uri, doc) {
+    let histogram = Services.telemetry.getHistogramById("READER_MODE_PARSE_RESULT");
     if (this.parseNodeLimit) {
       let numTags = doc.getElementsByTagName("*").length;
       if (numTags > this.parseNodeLimit) {
         this.log("Aborting parse for " + uri.spec + "; " + numTags + " elements found");
+        histogram.add(PARSE_ERROR_TOO_MANY_ELEMENTS);
         return null;
       }
     }
 
     let uriParam = {
       spec: uri.spec,
       host: uri.host,
       prePath: uri.prePath,
       scheme: uri.scheme,
       pathBase: Services.io.newURI(".", null, uri).spec
     };
 
+    TelemetryStopwatch.start("READER_MODE_SERIALIZE_DOM_MS");
     let serializer = Cc["@mozilla.org/xmlextras/xmlserializer;1"].
                      createInstance(Ci.nsIDOMSerializer);
-    let serializedDoc = yield Promise.resolve(serializer.serializeToString(doc));
+    let serializedDoc = serializer.serializeToString(doc);
+    TelemetryStopwatch.finish("READER_MOD_SERIALIZE_DOM_MS");
 
+    TelemetryStopwatch.start("READER_MODE_WORKER_PARSE_MS");
     let article = null;
     try {
       article = yield ReaderWorker.post("parseDocument", [uriParam, serializedDoc]);
     } catch (e) {
       Cu.reportError("Error in ReaderWorker: " + e);
+      histogram.add(PARSE_ERROR_WORKER);
     }
+    TelemetryStopwatch.finish("READER_MODE_WORKER_PARSE_MS");
 
     if (!article) {
       this.log("Worker did not return an article");
+      histogram.add(PARSE_ERROR_NO_ARTICLE);
       return null;
     }
 
     // Readability returns a URI object, but we only care about the URL.
     article.url = article.uri.spec;
     delete article.uri;
 
     let flags = Ci.nsIDocumentEncoder.OutputSelectionOnly | Ci.nsIDocumentEncoder.OutputAbsoluteLinks;
     article.title = Cc["@mozilla.org/parserutils;1"].getService(Ci.nsIParserUtils)
                                                     .convertToPlainText(article.title, flags, 0);
+
+    histogram.add(PARSE_SUCCESS);
     return article;
   }),
 
   get _cryptoHash() {
     delete this._cryptoHash;
     return this._cryptoHash = Cc["@mozilla.org/security/hash;1"].createInstance(Ci.nsICryptoHash);
   },
 
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -7542,10 +7542,44 @@
     "description": "Number of concurrent UDP socket closing threads"
   },
   "UDP_SOCKET_CLOSE_TIME": {
     "expires_in_version": "45",
     "kind": "exponential",
     "high": "60000",
     "n_buckets": 30,
     "description": "Time PR_Close of a UDP socket taken (ms)"
+  },
+  "READER_MODE_SERIALIZE_DOM_MS": {
+    "expires_in_version": "42",
+    "kind": "exponential",
+    "high": "5000",
+    "n_buckets": 15,
+    "description": "Time (ms) to serialize a DOM to send to the reader worker"
+  },
+  "READER_MODE_WORKER_PARSE_MS": {
+    "expires_in_version": "42",
+    "kind": "exponential",
+    "high": "10000",
+    "n_buckets": 30,
+    "description": "Time (ms) for the reader worker to parse a document"
+  },
+  "READER_MODE_DOWNLOAD_MS": {
+    "expires_in_version": "42",
+    "kind": "exponential",
+    "low": 50,
+    "high": "40000",
+    "n_buckets": 60,
+    "description": "Time (ms) to download a document to show in reader mode"
+  },
+  "READER_MODE_PARSE_RESULT" : {
+    "expires_in_version": "42",
+    "kind": "enumerated",
+    "n_values": 5,
+    "description": "The result of trying to parse a document to show in reader view (0=Success, 1=Error too many elements, 2=Error in worker, 3=Error no article)"
+  },
+  "READER_MODE_DOWNLOAD_RESULT" : {
+    "expires_in_version": "42",
+    "kind": "enumerated",
+    "n_values": 5,
+    "description": "The result of trying to download a document to show in reader view (0=Success, 1=Error XHR, 2=Error no document)"
   }
 }