Bug 869266 - Reduce number of SQL statements to record crashes; r=rne…

…wman
dothq · May 9, 2013 · 579e450 · 579e450
1 parent 4381ba2
commit 579e450
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 9 deletions.
diff --git a/services/healthreport/providers.jsm b/services/healthreport/providers.jsm
@@ -927,6 +927,12 @@ CrashesProvider.prototype = Object.freeze({
 
     let m = this.getMeasurement("crashes", 1);
 
+    // Aggregate counts locally to avoid excessive storage interaction.
+    let counts = {
+      pending: new Metrics.DailyValues(),
+      submitted: new Metrics.DailyValues(),
+    };
+
     // FUTURE detect mtimes in the future and react more intelligently.
     for (let filename in pending) {
       let modified = pending[filename].modified;
@@ -935,7 +941,7 @@ CrashesProvider.prototype = Object.freeze({
         continue;
       }
 
-      yield m.incrementDailyCounter("pending", modified);
+      counts.pending.appendValue(modified, 1);
     }
 
     for (let filename in submitted) {
@@ -945,7 +951,15 @@ CrashesProvider.prototype = Object.freeze({
         continue;
       }
 
-      yield m.incrementDailyCounter("submitted", modified);
+      counts.submitted.appendValue(modified, 1);
+    }
+
+    for (let [date, values] in counts.pending) {
+      yield m.incrementDailyCounter("pending", date, values.length);
+    }
+
+    for (let [date, values] in counts.submitted) {
+      yield m.incrementDailyCounter("submitted", date, values.length);
     }
 
     yield this.setState("lastCheck", "" + now.getTime());

diff --git a/services/healthreport/tests/xpcshell/test_provider_crashes.js b/services/healthreport/tests/xpcshell/test_provider_crashes.js
@@ -126,13 +126,17 @@ add_task(function test_collect() {
   let tomorrow = new Date(now.getTime() + MILLISECONDS_PER_DAY);
   let yesterday = new Date(now.getTime() - MILLISECONDS_PER_DAY);
 
-  let yesterdayID = createFakeCrash(false, yesterday);
-  let tomorrowID = createFakeCrash(false, tomorrow);
+  createFakeCrash(false, yesterday);
+
+  // Create multiple to test that multiple are handled properly.
+  createFakeCrash(false, tomorrow);
+  createFakeCrash(false, tomorrow);
+  createFakeCrash(false, tomorrow);
 
   yield provider.collectConstantData();
   values = yield m.getValues();
   do_check_eq(values.days.size, 11);
-  do_check_eq(values.days.getDay(tomorrow).get("pending"), 1);
+  do_check_eq(values.days.getDay(tomorrow).get("pending"), 3);
 
   for each (let date in gPending) {
     let day = values.days.getDay(date);

diff --git a/services/metrics/dataprovider.jsm b/services/metrics/dataprovider.jsm
@@ -233,11 +233,13 @@ Measurement.prototype = Object.freeze({
    *        (string) The name of the field whose value to increment.
    * @param date
    *        (Date) Day on which to increment the counter.
+   * @param by
+   *        (integer) How much to increment by.
    * @return Promise<>
    */
-  incrementDailyCounter: function (field, date=new Date()) {
+  incrementDailyCounter: function (field, date=new Date(), by=1) {
     return this.storage.incrementDailyCounterFromFieldID(this.fieldID(field),
-                                                         date);
+                                                         date, by);
   },
 
   /**

diff --git a/services/metrics/storage.jsm b/services/metrics/storage.jsm
@@ -491,7 +491,7 @@ const SQL = {
           "field_id = :field_id AND day = :days " +
         "), " +
         "0" +
-      ") + 1)",
+      ") + :by)",
 
   deleteLastNumericFromFieldID:
     "DELETE FROM last_numeric WHERE field_id = :field_id",
@@ -1614,13 +1614,16 @@ MetricsStorageSqliteBackend.prototype = Object.freeze({
    * @param date
    *        (Date) When the increment occurred. This is typically "now" but can
    *        be explicitly defined for events that occurred in the past.
+   * @param by
+   *        (integer) How much to increment the value by. Defaults to 1.
    */
-  incrementDailyCounterFromFieldID: function (id, date=new Date()) {
+  incrementDailyCounterFromFieldID: function (id, date=new Date(), by=1) {
     this._ensureFieldType(id, this.FIELD_DAILY_COUNTER);
 
     let params = {
       field_id: id,
       days: dateToDays(date),
+      by: by,
     };
 
     return this._connection.executeCached(SQL.incrementDailyCounterFromFieldID,

diff --git a/services/metrics/tests/xpcshell/test_metrics_provider.js b/services/metrics/tests/xpcshell/test_metrics_provider.js
@@ -97,6 +97,10 @@ add_task(function test_measurement_storage_basic() {
   count = yield provider.storage.getDailyCounterCountFromFieldID(counterID, yesterday);
   do_check_eq(count, 1);
 
+  yield m.incrementDailyCounter("daily-counter", now, 4);
+  count = yield provider.storage.getDailyCounterCountFromFieldID(counterID, now);
+  do_check_eq(count, 6);
+
   // Daily discrete numeric.
   let dailyDiscreteNumericID = m.fieldID("daily-discrete-numeric");
   yield m.addDailyDiscreteNumeric("daily-discrete-numeric", 5, now);

diff --git a/services/metrics/tests/xpcshell/test_metrics_storage.js b/services/metrics/tests/xpcshell/test_metrics_storage.js
@@ -445,6 +445,10 @@ add_task(function test_increment_daily_counter_basic() {
   count = yield backend.getDailyCounterCountFromFieldID(fieldID, now);
   do_check_eq(count, 2);
 
+  yield backend.incrementDailyCounterFromFieldID(fieldID, now, 10);
+  count = yield backend.getDailyCounterCountFromFieldID(fieldID, now);
+  do_check_eq(count, 12);
+
   yield backend.close();
 });