Bug 1300491 - Removed histogramFrom and resulting unused functions, all STARTUP_* histograms, and any tests using these. r?chutten draft
authorAdamG2 <adamgj.wong@gmail.com>
Tue, 25 Oct 2016 23:48:20 -0400
changeset 429574 74e3518ed3d0e4bb8ae883239d333f8d04d7f733
parent 423365 723c2e894079d0c870a1b78679f971c7a1d3d31f
child 534999 f2c352d5c9831b3be1bf941678aac8a5c7ddab61
push id33602
push userbmo:adamgj.wong@gmail.com
push dateWed, 26 Oct 2016 03:49:27 +0000
reviewerschutten
bugs1300491
milestone52.0a1
Bug 1300491 - Removed histogramFrom and resulting unused functions, all STARTUP_* histograms, and any tests using these. r?chutten MozReview-Commit-ID: EnVYxGIOWgs
toolkit/components/telemetry/Telemetry.cpp
toolkit/components/telemetry/TelemetryHistogram.cpp
toolkit/components/telemetry/TelemetryHistogram.h
toolkit/components/telemetry/TelemetrySession.jsm
toolkit/components/telemetry/gen-histogram-bucket-ranges.py
toolkit/components/telemetry/nsITelemetry.idl
toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
toolkit/components/telemetry/tests/unit/test_nsITelemetry.js
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -1080,23 +1080,16 @@ TelemetryImpl::AddSQLInfo(JSContext *cx,
   }
 
   return JS_DefineProperty(cx, rootObj,
                            mainThread ? "mainThread" : "otherThreads",
                            statsObj, JSPROP_ENUMERATE);
 }
 
 NS_IMETHODIMP
-TelemetryImpl::HistogramFrom(const nsACString &name, const nsACString &existing_name,
-                             JSContext *cx, JS::MutableHandle<JS::Value> ret)
-{
-  return TelemetryHistogram::HistogramFrom(name, existing_name, cx, ret);
-}
-
-NS_IMETHODIMP
 TelemetryImpl::RegisterAddonHistogram(const nsACString &id,
                                       const nsACString &name,
                                       uint32_t histogramType,
                                       uint32_t min, uint32_t max,
                                       uint32_t bucketCount,
                                       uint8_t optArgCount)
 {
   return TelemetryHistogram::RegisterAddonHistogram
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -550,16 +550,19 @@ internal_GetHistogramByName(const nsACSt
                                 NS_LITERAL_CSTRING(CHILD_HISTOGRAM_SUFFIX));
   rv = internal_GetHistogramByEnumId(id, ret, isChild);
   if (NS_FAILED(rv))
     return rv;
 
   return NS_OK;
 }
 
+
+#if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID)
+
 /**
  * This clones a histogram |existing| with the id |existingId| to a
  * new histogram with the name |newName|.
  * For simplicity this is limited to registered histograms.
  */
 Histogram*
 internal_CloneHistogram(const nsACString& newName,
                         mozilla::Telemetry::ID existingId,
@@ -580,36 +583,16 @@ internal_CloneHistogram(const nsACString
 
   Histogram::SampleSet ss;
   existing.SnapshotSample(&ss);
   clone->AddSampleSet(ss);
 
   return clone;
 }
 
-/**
- * This clones a histogram with the id |existingId| to a new histogram
- * with the name |newName|.
- * For simplicity this is limited to registered histograms.
- */
-Histogram*
-internal_CloneHistogram(const nsACString& newName,
-                        mozilla::Telemetry::ID existingId)
-{
-  Histogram *existing = nullptr;
-  nsresult rv = internal_GetHistogramByEnumId(existingId, &existing);
-  if (NS_FAILED(rv)) {
-    return nullptr;
-  }
-
-  return internal_CloneHistogram(newName, existingId, *existing);
-}
-
-#if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID)
-
 Histogram*
 internal_GetSubsessionHistogram(Histogram& existing)
 {
   mozilla::Telemetry::ID id;
   nsresult rv
     = internal_GetHistogramEnumId(existing.histogram_name().c_str(), &id);
   if (NS_FAILED(rv) || gHistograms[id].keyed) {
     return nullptr;
@@ -2284,43 +2267,16 @@ const char*
 TelemetryHistogram::GetHistogramName(mozilla::Telemetry::ID id)
 {
   StaticMutexAutoLock locker(gTelemetryHistogramMutex);
   const HistogramInfo& h = gHistograms[id];
   return h.id();
 }
 
 nsresult
-TelemetryHistogram::HistogramFrom(const nsACString &name,
-                                  const nsACString &existing_name,
-                                  JSContext *cx,
-                                  JS::MutableHandle<JS::Value> ret)
-{
-  Histogram* clone = nullptr;
-  {
-    StaticMutexAutoLock locker(gTelemetryHistogramMutex);
-    mozilla::Telemetry::ID id;
-    nsresult rv
-      = internal_GetHistogramEnumId(PromiseFlatCString(existing_name).get(),
-                                    &id);
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-
-    clone = internal_CloneHistogram(name, id);
-    if (!clone) {
-      return NS_ERROR_FAILURE;
-    }
-  }
-
-  // Runs without protection from |gTelemetryHistogramMutex|
-  return internal_WrapAndReturnHistogram(clone, cx, ret);
-}
-
-nsresult
 TelemetryHistogram::CreateHistogramSnapshots(JSContext *cx,
                                              JS::MutableHandle<JS::Value> ret,
                                              bool subsession,
                                              bool clearSubsession)
 {
   // Runs without protection from |gTelemetryHistogramMutex|
   JS::Rooted<JSObject*> root_obj(cx, JS_NewPlainObject(cx));
   if (!root_obj)
--- a/toolkit/components/telemetry/TelemetryHistogram.h
+++ b/toolkit/components/telemetry/TelemetryHistogram.h
@@ -54,20 +54,16 @@ GetHistogramById(const nsACString &name,
 nsresult
 GetKeyedHistogramById(const nsACString &name, JSContext *cx,
                       JS::MutableHandle<JS::Value> ret);
 
 const char*
 GetHistogramName(mozilla::Telemetry::ID id);
 
 nsresult
-HistogramFrom(const nsACString &name, const nsACString &existing_name,
-              JSContext *cx, JS::MutableHandle<JS::Value> ret);
-
-nsresult
 CreateHistogramSnapshots(JSContext *cx, JS::MutableHandle<JS::Value> ret,
                          bool subsession, bool clearSubsession);
 
 nsresult
 RegisteredHistograms(uint32_t aDataset, uint32_t *aCount,
                      char*** aHistograms);
 
 nsresult
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -645,19 +645,16 @@ this.TelemetrySession = Object.freeze({
   },
 });
 
 var Impl = {
   _histograms: {},
   _initialized: false,
   _logger: null,
   _prevValues: {},
-  // Regex that matches histograms we care about during startup.
-  // Keep this in sync with gen-histogram-bucket-ranges.py.
-  _startupHistogramRegex: /SQLITE|HTTP|SPDY|CACHE|DNS/,
   _slowSQLStartup: {},
   _hasWindowRestoredObserver: false,
   _hasXulWindowVisibleObserver: false,
   _startupIO : {},
   // The previous build ID, if this is the first run with a new build.
   // Null if this is the first run, or the previous build ID is unknown.
   _previousBuildId: null,
   // Telemetry payloads sent by child processes.
@@ -1211,46 +1208,21 @@ var Impl = {
     let h = this._histograms[id];
     if (!h) {
       h = Telemetry.getHistogramById(id);
       this._histograms[id] = h;
     }
     h.add(val);
   },
 
-  /**
-   * Return true if we're interested in having a STARTUP_* histogram for
-   * the given histogram name.
-   */
-  isInterestingStartupHistogram: function isInterestingStartupHistogram(name) {
-    return this._startupHistogramRegex.test(name);
-  },
-
   getChildPayloads: function getChildPayloads() {
     return this._childTelemetry.map(child => child.payload);
   },
 
   /**
-   * Make a copy of interesting histograms at startup.
-   */
-  gatherStartupHistograms: function gatherStartupHistograms() {
-    this._log.trace("gatherStartupHistograms");
-
-    let info =
-      Telemetry.registeredHistograms(this.getDatasetType(), []);
-    let snapshots = Telemetry.histogramSnapshots;
-    for (let name of info) {
-      // Only duplicate histograms with actual data.
-      if (this.isInterestingStartupHistogram(name) && name in snapshots) {
-        Telemetry.histogramFrom("STARTUP_" + name, name);
-      }
-    }
-  },
-
-  /**
    * Get the current session's payload using the provided
    * simpleMeasurements and info, which are typically obtained by a call
    * to |this.getSimpleMeasurements| and |this.getMetadata|,
    * respectively.
    */
   assemblePayloadWithMeasurements: function(simpleMeasurements, info, reason, clearSubsession) {
     const isSubsession = IS_UNIFIED_TELEMETRY && !this._isClassicReason(reason);
     clearSubsession = IS_UNIFIED_TELEMETRY && clearSubsession;
@@ -1541,18 +1513,16 @@ var Impl = {
       this._log.trace("setupContentProcess - base recording is disabled, not initializing");
       return;
     }
 
     Services.obs.addObserver(this, "content-child-shutdown", false);
     cpml.addMessageListener(MESSAGE_TELEMETRY_GET_CHILD_THREAD_HANGS, this);
     cpml.addMessageListener(MESSAGE_TELEMETRY_GET_CHILD_USS, this);
 
-    this.gatherStartupHistograms();
-
     let delayedTask = new DeferredTask(function* () {
       this._initialized = true;
 
       this.attachObservers();
       this.gatherMemory();
     }.bind(this), testing ? TELEMETRY_TEST_DELAY : TELEMETRY_DELAY);
 
     delayedTask.arm();
@@ -1759,17 +1729,16 @@ var Impl = {
   },
 
   getPayload: function getPayload(reason, clearSubsession) {
     this._log.trace("getPayload - clearSubsession: " + clearSubsession);
     reason = reason || REASON_GATHER_PAYLOAD;
     // This function returns the current Telemetry payload to the caller.
     // We only gather startup info once.
     if (Object.keys(this._slowSQLStartup).length == 0) {
-      this.gatherStartupHistograms();
       this._slowSQLStartup = Telemetry.slowSQL;
     }
     this.gatherMemory();
     return this.getSessionPayload(reason, clearSubsession);
   },
 
   getChildThreadHangs: function getChildThreadHangs() {
     return new Promise((resolve) => {
@@ -1817,17 +1786,16 @@ var Impl = {
 
   gatherStartup: function gatherStartup() {
     this._log.trace("gatherStartup");
     let counters = processInfo.getCounters();
     if (counters) {
       [this._startupIO.startupSessionRestoreReadBytes,
         this._startupIO.startupSessionRestoreWriteBytes] = counters;
     }
-    this.gatherStartupHistograms();
     this._slowSQLStartup = Telemetry.slowSQL;
   },
 
   setAddOns: function setAddOns(aAddOns) {
     this._addons = aAddOns;
   },
 
   testPing: function testPing() {
--- a/toolkit/components/telemetry/gen-histogram-bucket-ranges.py
+++ b/toolkit/components/telemetry/gen-histogram-bucket-ranges.py
@@ -8,19 +8,16 @@
 
 import sys
 import re
 import histogram_tools
 import json
 
 from collections import OrderedDict
 
-# Keep this in sync with TelemetryController.
-startup_histogram_re = re.compile("SQLITE|HTTP|SPDY|CACHE|DNS")
-
 def main(argv):
     filenames = argv
 
     all_histograms = OrderedDict()
 
     for histogram in histogram_tools.from_files(filenames):
         name = histogram.name()
         parameters = OrderedDict()
@@ -45,14 +42,11 @@ def main(argv):
             parameters['buckets'] = buckets
             parameters['max'] = buckets[-1]
             parameters['bucket_count'] = len(buckets)
         except histogram_tools.DefinitionException:
             continue
 
         all_histograms.update({ name: parameters });
 
-        if startup_histogram_re.search(name) is not None:
-            all_histograms.update({ "STARTUP_" + name: parameters })
-
     print json.dumps({ 'histograms': all_histograms})
 
 main(sys.argv[1:])
--- a/toolkit/components/telemetry/nsITelemetry.idl
+++ b/toolkit/components/telemetry/nsITelemetry.idl
@@ -174,31 +174,16 @@ interface nsITelemetry : nsISupports
    *
    * @param dataset - DATASET_RELEASE_CHANNEL_OPTOUT or DATASET_RELEASE_CHANNEL_OPTIN
    */
   void registeredHistograms(in uint32_t dataset,
                             out uint32_t count,
                             [retval, array, size_is(count)] out string histograms);
 
   /**
-   * Create a histogram using the current state of an existing histogram.  The
-   * existing histogram must be registered in TelemetryHistograms.h.
-   *
-   * @param name Unique histogram name
-   * @param existing_name Existing histogram name
-   * The returned object has the following functions:
-   *   add(int) - Adds an int value to the appropriate bucket
-   *   snapshot() - Returns a snapshot of the histogram with the same data fields as in histogramSnapshots()
-   *   clear() - Zeros out the histogram's buckets and sum
-   *   dataset() - identifies what dataset this is in: DATASET_RELEASE_CHANNEL_OPTOUT or ...OPTIN
-   */
-  [implicit_jscontext]
-  jsval histogramFrom(in ACString name, in ACString existing_name);
-
-  /**
    * Create and return a histogram registered in TelemetryHistograms.h.
    *
    * @param id - unique identifier from TelemetryHistograms.h
    * The returned object has the following functions:
    *   add(int) - Adds an int value to the appropriate bucket
    *   snapshot() - Returns a snapshot of the histogram with the same data fields as in histogramSnapshots()
    *   clear() - Zeros out the histogram's buckets and sum
    *   dataset() - identifies what dataset this is in: DATASET_RELEASE_CHANNEL_OPTOUT or ...OPTIN
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ -104,17 +104,16 @@ function fakeGenerateUUID(sessionFunc, s
 
 function fakeIdleNotification(topic) {
   let session = Cu.import("resource://gre/modules/TelemetrySession.jsm");
   return session.TelemetryScheduler.observe(null, topic, null);
 }
 
 function setupTestData() {
 
-  Telemetry.histogramFrom(IGNORE_CLONED_HISTOGRAM, IGNORE_HISTOGRAM_TO_CLONE);
   Services.startup.interrupted = true;
   Telemetry.registerAddonHistogram(ADDON_NAME, ADDON_HISTOGRAM,
                                    Telemetry.HISTOGRAM_LINEAR,
                                    1, 5, 6);
   let h1 = Telemetry.getAddonHistogram(ADDON_NAME, ADDON_HISTOGRAM);
   h1.add(1);
   let h2 = Telemetry.getHistogramById("TELEMETRY_TEST_COUNT");
   h2.add();
@@ -328,24 +327,16 @@ function checkPayload(payload, reason, s
   const TELEMETRY_TEST_KEYED_COUNT = "TELEMETRY_TEST_KEYED_COUNT";
 
   if (successfulPings > 0) {
     Assert.ok(TELEMETRY_PING in payload.histograms);
   }
   Assert.ok(TELEMETRY_TEST_FLAG in payload.histograms);
   Assert.ok(TELEMETRY_TEST_COUNT in payload.histograms);
 
-  let rh = Telemetry.registeredHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, []);
-  for (let name of rh) {
-    if (/SQLITE/.test(name) && name in payload.histograms) {
-      let histogramName = ("STARTUP_" + name);
-      Assert.ok(histogramName in payload.histograms, histogramName + " must be available.");
-    }
-  }
-
   Assert.ok(!(IGNORE_CLONED_HISTOGRAM in payload.histograms));
 
   // Flag histograms should automagically spring to life.
   const expected_flag = {
     range: [1, 2],
     bucket_count: 3,
     histogram_type: 3,
     values: {0:1, 1:0},
--- a/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js
+++ b/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js
@@ -240,45 +240,16 @@ add_task(function* test_getHistogramById
   }
   var h = Telemetry.getHistogramById("CYCLE_COLLECTOR");
   var s = h.snapshot();
   do_check_eq(s.histogram_type, Telemetry.HISTOGRAM_EXPONENTIAL);
   do_check_eq(s.min, 1);
   do_check_eq(s.max, 10000);
 });
 
-add_task(function* test_histogramFrom() {
-  // Test one histogram of each type.
-  let names = [
-      "CYCLE_COLLECTOR",      // EXPONENTIAL
-      "GC_REASON_2",          // LINEAR
-      "GC_RESET",             // BOOLEAN
-      "TELEMETRY_TEST_FLAG",  // FLAG
-      "TELEMETRY_TEST_COUNT", // COUNT
-  ];
-
-  for (let name of names) {
-    let [min, max, bucket_count] = [1, INT_MAX - 1, 10]
-    let original = Telemetry.getHistogramById(name);
-    let clone = Telemetry.histogramFrom("clone" + name, name);
-    compareHistograms(original, clone);
-  }
-
-  // Additionally, set TELEMETRY_TEST_FLAG and TELEMETRY_TEST_COUNT
-  // and check they get set on the clone.
-  let testFlag = Telemetry.getHistogramById("TELEMETRY_TEST_FLAG");
-  testFlag.add(1);
-  let testCount = Telemetry.getHistogramById("TELEMETRY_TEST_COUNT");
-  testCount.add();
-  let clone = Telemetry.histogramFrom("FlagClone", "TELEMETRY_TEST_FLAG");
-  compareHistograms(testFlag, clone);
-  clone = Telemetry.histogramFrom("CountClone", "TELEMETRY_TEST_COUNT");
-  compareHistograms(testCount, clone);
-});
-
 add_task(function* test_getSlowSQL() {
   var slow = Telemetry.slowSQL;
   do_check_true(("mainThread" in slow) && ("otherThreads" in slow));
 });
 
 add_task(function* test_getWebrtc() {
   var webrtc = Telemetry.webrtcStats;
   do_check_true("IceCandidatesStats" in webrtc);
@@ -437,28 +408,24 @@ add_task(function* test_addons() {
   snapshots = Telemetry.addonHistogramSnapshots;
   do_check_false(addon_id in snapshots);
   // Make sure other addons are unaffected.
   do_check_true(extra_addon in snapshots);
 });
 
 add_task(function* test_expired_histogram() {
   var test_expired_id = "TELEMETRY_TEST_EXPIRED";
-  var clone_id = "ExpiredClone";
   var dummy = Telemetry.getHistogramById(test_expired_id);
-  var dummy_clone = Telemetry.histogramFrom(clone_id, test_expired_id);
   var rh = Telemetry.registeredHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, []);
   Assert.ok(!!rh);
 
   dummy.add(1);
-  dummy_clone.add(1);
 
   do_check_eq(Telemetry.histogramSnapshots["__expired__"], undefined);
   do_check_eq(Telemetry.histogramSnapshots[test_expired_id], undefined);
-  do_check_eq(Telemetry.histogramSnapshots[clone_id], undefined);
   do_check_eq(rh[test_expired_id], undefined);
 });
 
 add_task(function* test_keyed_histogram() {
   // Check that invalid names get rejected.
 
   let threw = false;
   try {