Bug 836186 - Don't load FHR providers until they are used; r=rnewman
authorGregory Szorc <gps@mozilla.com>
Thu, 31 Jan 2013 08:58:19 -0800
changeset 130796 3446cda2c8b811dbeae1f0d726face752ce448bf
parent 130795 e8482b89eaed5d8c90a5ab46741555cfdefbdce9
child 130797 018a9b5a299cf3ae8a5c7619536d9364b29adb08
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs836186
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 836186 - Don't load FHR providers until they are used; r=rnewman
services/healthreport/healthreporter.jsm
services/healthreport/profile.jsm
services/healthreport/providers.jsm
services/healthreport/tests/xpcshell/test_healthreporter.js
services/metrics/collector.jsm
services/metrics/dataprovider.jsm
services/metrics/modules-testing/mocks.jsm
services/metrics/tests/xpcshell/test_metrics_collector.js
--- a/services/healthreport/healthreporter.jsm
+++ b/services/healthreport/healthreporter.jsm
@@ -124,16 +124,18 @@ function HealthReporter(branch, policy, 
   this._initialized = false;
   this._initializeHadError = false;
   this._initializedDeferred = Promise.defer();
   this._shutdownRequested = false;
   this._shutdownInitiated = false;
   this._shutdownComplete = false;
   this._shutdownCompleteCallback = null;
 
+  this._constantOnlyProviders = {};
+
   this._ensureDirectoryExists(this._stateDir)
       .then(this._onStateDirCreated.bind(this),
             this._onInitError.bind(this));
 
 }
 
 HealthReporter.prototype = Object.freeze({
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver]),
@@ -544,39 +546,87 @@ HealthReporter.prototype = Object.freeze
       let uri = cm.getCategoryEntry(category, entry);
       this._log.info("Attempting to load provider from category manager: " +
                      entry + " from " + uri);
 
       try {
         let ns = {};
         Cu.import(uri, ns);
 
-        let provider = new ns[entry]();
-        provider.initPreferences(this._branch + "provider.");
-        provider.healthReporter = this;
-        promises.push(this.registerProvider(provider));
+        let proto = ns[entry].prototype;
+        if (proto.constantOnly) {
+          this._log.info("Provider is constant-only. Deferring initialization: " +
+                         proto.name);
+          this._constantOnlyProviders[proto.name] = ns[entry];
+        } else {
+          let provider = this.initProviderFromType(ns[entry]);
+          promises.push(this.registerProvider(provider));
+        }
       } catch (ex) {
         this._log.warn("Error registering provider from category manager: " +
                        entry + "; " + CommonUtils.exceptionStr(ex));
         continue;
       }
     }
 
     return Task.spawn(function wait() {
       for (let promise of promises) {
         yield promise;
       }
     });
   },
 
+  initProviderFromType: function (providerType) {
+    let provider = new providerType();
+    provider.initPreferences(this._branch + "provider.");
+    provider.healthReporter = this;
+
+    return provider;
+  },
+
   /**
    * Collect all measurements for all registered providers.
    */
   collectMeasurements: function () {
-    return this._collector.collectConstantData();
+    return Task.spawn(function doCollection() {
+      for each (let providerType in this._constantOnlyProviders) {
+        try {
+          let provider = this.initProviderFromType(providerType);
+          yield this.registerProvider(provider);
+        } catch (ex) {
+          this._log.warn("Error registering constant-only provider: " +
+                         CommonUtils.exceptionStr(ex));
+        }
+      }
+
+      let result;
+      try {
+        result = yield this._collector.collectConstantData();
+      } finally {
+        for (let provider of this._collector.providers) {
+          if (!provider.constantOnly) {
+            continue;
+          }
+
+          this._log.info("Shutting down constant-only provider: " +
+                         provider.name);
+
+          try {
+            yield provider.shutdown();
+          } catch (ex) {
+            this._log.warn("Error when shutting down provider: " +
+                           CommonUtils.exceptionStr(ex));
+          } finally {
+            this._collector.unregisterProvider(provider.name);
+          }
+        }
+      }
+
+      throw new Task.Result(result);
+    }.bind(this));
   },
 
   /**
    * Called to initiate a data upload.
    *
    * The passed argument is a `DataSubmissionRequest` from policy.jsm.
    */
   requestDataUpload: function (request) {
--- a/services/healthreport/profile.jsm
+++ b/services/healthreport/profile.jsm
@@ -209,16 +209,18 @@ function ProfileMetadataProvider() {
 }
 ProfileMetadataProvider.prototype = {
   __proto__: Metrics.Provider.prototype,
 
   name: "org.mozilla.profile",
 
   measurementTypes: [ProfileMetadataMeasurement],
 
+  constantOnly: true,
+
   getProfileCreationDays: function () {
     let accessor = new ProfileCreationTimeAccessor(null, this._log);
 
     return accessor.created
                    .then(truncate);
   },
 
   collectConstantData: function () {
--- a/services/healthreport/providers.jsm
+++ b/services/healthreport/providers.jsm
@@ -116,16 +116,18 @@ this.AppInfoProvider = function AppInfoP
 }
 AppInfoProvider.prototype = Object.freeze({
   __proto__: Metrics.Provider.prototype,
 
   name: "org.mozilla.appInfo",
 
   measurementTypes: [AppInfoMeasurement, AppVersionMeasurement],
 
+  constantOnly: true,
+
   appInfoFields: {
     // From nsIXULAppInfo.
     vendor: "vendor",
     name: "name",
     id: "ID",
     version: "version",
     appBuildID: "appBuildID",
     platformVersion: "platformVersion",
@@ -291,16 +293,18 @@ this.SysInfoProvider = function SysInfoP
 
 SysInfoProvider.prototype = Object.freeze({
   __proto__: Metrics.Provider.prototype,
 
   name: "org.mozilla.sysinfo",
 
   measurementTypes: [SysInfoMeasurement],
 
+  constantOnly: true,
+
   sysInfoFields: {
     cpucount: "cpuCount",
     memsize: "memoryMB",
     manufacturer: "manufacturer",
     device: "device",
     hardware: "hardware",
     name: "name",
     version: "version",
@@ -472,16 +476,18 @@ this.SessionsProvider = function () {
 
 SessionsProvider.prototype = Object.freeze({
   __proto__: Metrics.Provider.prototype,
 
   name: "org.mozilla.appSessions",
 
   measurementTypes: [CurrentSessionMeasurement, PreviousSessionsMeasurement],
 
+  constantOnly: true,
+
   collectConstantData: function () {
     let previous = this.getMeasurement("previous", 2);
 
     return this.storage.enqueueTransaction(this._recordAndPruneSessions.bind(this));
   },
 
   _recordAndPruneSessions: function () {
     this._log.info("Moving previous sessions from session recorder to storage.");
@@ -741,16 +747,18 @@ this.CrashesProvider = function () {
 
 CrashesProvider.prototype = Object.freeze({
   __proto__: Metrics.Provider.prototype,
 
   name: "org.mozilla.crashes",
 
   measurementTypes: [DailyCrashesMeasurement],
 
+  constantOnly: true,
+
   collectConstantData: function () {
     return Task.spawn(this._populateCrashCounts.bind(this));
   },
 
   _populateCrashCounts: function () {
     let now = new Date();
     let service = new CrashDirectoryService();
 
--- a/services/healthreport/tests/xpcshell/test_healthreporter.js
+++ b/services/healthreport/tests/xpcshell/test_healthreporter.js
@@ -166,16 +166,49 @@ add_task(function test_register_provider
   let reporter = yield getReporter("category_manager");
   do_check_eq(reporter._collector._providers.size, 0);
   yield reporter.registerProvidersFromCategoryManager(category);
   do_check_eq(reporter._collector._providers.size, 1);
 
   reporter._shutdown();
 });
 
+// Constant only providers are only initialized at constant collect
+// time.
+add_task(function test_constant_only_providers() {
+  const category = "healthreporter-constant-only";
+
+  let cm = Cc["@mozilla.org/categorymanager;1"]
+             .getService(Ci.nsICategoryManager);
+  cm.addCategoryEntry(category, "DummyProvider",
+                      "resource://testing-common/services/metrics/mocks.jsm",
+                      false, true);
+  cm.addCategoryEntry(category, "DummyConstantProvider",
+                      "resource://testing-common/services/metrics/mocks.jsm",
+                      false, true);
+
+  let reporter = yield getReporter("constant_only_providers");
+  do_check_eq(reporter._collector._providers.size, 0);
+  yield reporter.registerProvidersFromCategoryManager(category);
+  do_check_eq(reporter._collector._providers.size, 1);
+  do_check_true(reporter._storage.hasProvider("DummyProvider"));
+  do_check_false(reporter._storage.hasProvider("DummyConstantProvider"));
+
+  yield reporter.collectMeasurements();
+
+  do_check_eq(reporter._collector._providers.size, 1);
+  do_check_true(reporter._storage.hasProvider("DummyConstantProvider"));
+
+  let mID = reporter._storage.measurementID("DummyConstantProvider", "DummyMeasurement", 1);
+  let values = yield reporter._storage.getMeasurementValues(mID);
+  do_check_true(values.singular.size > 0);
+
+  reporter._shutdown();
+});
+
 add_task(function test_json_payload_simple() {
   let reporter = yield getReporter("json_payload_simple");
 
   let now = new Date();
   let payload = yield reporter.getJSONPayload();
   let original = JSON.parse(payload);
 
   do_check_eq(original.version, 1);
--- a/services/metrics/collector.jsm
+++ b/services/metrics/collector.jsm
@@ -76,16 +76,26 @@ Collector.prototype = Object.freeze({
 
     if (this._providerInitQueue.length == 1) {
       this._popAndInitProvider();
     }
 
     return deferred.promise;
   },
 
+  /**
+   * Remove a named provider from the collector.
+   *
+   * It is the caller's responsibility to shut down the provider
+   * instance.
+   */
+  unregisterProvider: function (name) {
+    this._providers.delete(name);
+  },
+
   _popAndInitProvider: function () {
     if (!this._providerInitQueue.length || this._providerInitializing) {
       return;
     }
 
     let [provider, deferred] = this._providerInitQueue.shift();
     this._providerInitializing = true;
 
--- a/services/metrics/dataprovider.jsm
+++ b/services/metrics/dataprovider.jsm
@@ -387,16 +387,30 @@ this.Provider = function () {
   this._log = Log4Moz.repository.getLogger("Services.Metrics.Provider." + this.name);
 
   this.measurements = null;
   this.storage = null;
 }
 
 Provider.prototype = Object.freeze({
   /**
+   * Whether the provider provides only constant data.
+   *
+   * If this is true, the provider likely isn't instantiated until
+   * `collectConstantData` is called and the provider may be torn down after
+   * this function has finished.
+   *
+   * This is an optimization so provider instances aren't dead weight while the
+   * application is running.
+   *
+   * This must be set on the prototype for the optimization to be realized.
+   */
+  constantOnly: false,
+
+  /**
    * Obtain a `Measurement` from its name and version.
    *
    * If the measurement is not found, an Error is thrown.
    */
   getMeasurement: function (name, version) {
     if (!Number.isInteger(version)) {
       throw new Error("getMeasurement expects an integer version. Got: " + version);
     }
--- a/services/metrics/modules-testing/mocks.jsm
+++ b/services/metrics/modules-testing/mocks.jsm
@@ -2,16 +2,17 @@
  * 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 = [
   "DummyMeasurement",
   "DummyProvider",
+  "DummyConstantProvider",
 ];
 
 const {utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/commonjs/promise/core.js");
 Cu.import("resource://gre/modules/Metrics.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 
@@ -82,8 +83,19 @@ DummyProvider.prototype = {
       m.setDailyLastText("daily-last-text", "biz", now);
       m.setLastNumeric("last-numeric", 4, now);
       return m.setLastText("last-text", "bazfoo", now);
     }.bind(this));
   },
 
 };
 
+
+this.DummyConstantProvider = function () {
+  DummyProvider.call(this, "DummyConstantProvider");
+}
+
+DummyConstantProvider.prototype = {
+  __proto__: DummyProvider.prototype,
+
+  constantOnly: true,
+};
+
--- a/services/metrics/tests/xpcshell/test_metrics_collector.js
+++ b/services/metrics/tests/xpcshell/test_metrics_collector.js
@@ -37,16 +37,19 @@ add_task(function test_register_provider
   } catch (ex) {
     do_check_true(ex.message.startsWith("Argument must be a Provider"));
     failed = true;
   } finally {
     do_check_true(failed);
     failed = false;
   }
 
+  collector.unregisterProvider(dummy.name);
+  do_check_eq(collector._providers.size, 0);
+
   yield storage.close();
 });
 
 add_task(function test_collect_constant_data() {
   let storage = yield Metrics.Storage("collect_constant_data");
   let collector = new Metrics.Collector(storage);
   let provider = new DummyProvider();
   yield collector.registerProvider(provider);