Bug 1432791 - Remove the Microsecond AutoTimer resolution. r?dexter draft
authorJeremy Lempereur <jeremy.lempereur@gmail.com>
Wed, 07 Feb 2018 21:11:22 +0100
changeset 753776 c0ac844aa10ff58ee595ad722c7745920635af65
parent 752021 4fe6f6560083f8c8257282bef1d4e0ced9d1b975
push id98678
push userbmo:jeremy.lempereur@gmail.com
push dateMon, 12 Feb 2018 11:53:07 +0000
reviewersdexter
bugs1432791
milestone60.0a1
Bug 1432791 - Remove the Microsecond AutoTimer resolution. r?dexter Telemetry's AutoTimer Microsecond resolution was not used. I removed it with the TimerResolution enum, which allowed me to remove a couple of templates as well. MozReview-Commit-ID: 76qHgmYEsE3
dom/storage/LocalStorageCache.cpp
toolkit/components/reputationservice/LoginReputation.cpp
toolkit/components/telemetry/Telemetry.cpp
toolkit/components/telemetry/Telemetry.h
toolkit/components/telemetry/docs/collection/histograms.rst
toolkit/components/telemetry/docs/collection/measuring-time.rst
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
--- a/dom/storage/LocalStorageCache.cpp
+++ b/dom/storage/LocalStorageCache.cpp
@@ -258,17 +258,17 @@ class TelemetryAutoTimer
 {
 public:
   explicit TelemetryAutoTimer(Telemetry::HistogramID aId)
     : id(aId), start(TimeStamp::Now())
   {}
 
   ~TelemetryAutoTimer()
   {
-    Telemetry::AccumulateDelta_impl<Telemetry::Millisecond>::compute(id, start);
+    Telemetry::AccumulateTimeDelta(id, start);
   }
 
 private:
   Telemetry::HistogramID id;
   const TimeStamp start;
 };
 
 } // namespace
--- a/toolkit/components/reputationservice/LoginReputation.cpp
+++ b/toolkit/components/reputationservice/LoginReputation.cpp
@@ -417,18 +417,18 @@ LoginReputationService::QueryLoginWhitel
     [self, aRequest, startTimeMs](VerdictType aResolveValue) -> void {
       // Promise is resolved if url is found in google-provided whitelist.
       MOZ_ASSERT(NS_IsMainThread());
       MOZ_ASSERT(aResolveValue == nsILoginReputationVerdictType::SAFE);
 
       LR_LOG(("Query login whitelist [request = %p, result = SAFE]",
               aRequest));
 
-      AccumulateDelta_impl<Millisecond>::compute(
-        LOGIN_REPUTATION_LOGIN_WHITELIST_LOOKUP_TIME, startTimeMs);
+      AccumulateTimeDelta(LOGIN_REPUTATION_LOGIN_WHITELIST_LOOKUP_TIME,
+                          startTimeMs);
 
       Accumulate(LOGIN_REPUTATION_LOGIN_WHITELIST_RESULT,
         nsILoginReputationVerdictType::SAFE);
 
       self->Finish(aRequest, NS_OK, nsILoginReputationVerdictType::SAFE);
     },
     [self, aRequest, startTimeMs](nsresult rv) -> void {
       // Promise is rejected if url cannot be found in google-provided whitelist.
@@ -440,18 +440,18 @@ LoginReputationService::QueryLoginWhitel
           LR_LOG(("Error in QueryLoginWhitelist() [request = %p, rv = %s]",
                   aRequest, errorName.get()));
         }
 
         // Don't record the lookup time when there is an error, only record the
         // result here.
         Accumulate(LOGIN_REPUTATION_LOGIN_WHITELIST_RESULT, 2); // 2 is error
       } else {
-        AccumulateDelta_impl<Millisecond>::compute(
-          LOGIN_REPUTATION_LOGIN_WHITELIST_LOOKUP_TIME, startTimeMs);
+        AccumulateTimeDelta(LOGIN_REPUTATION_LOGIN_WHITELIST_LOOKUP_TIME,
+                            startTimeMs);
 
         Accumulate(LOGIN_REPUTATION_LOGIN_WHITELIST_RESULT,
           nsILoginReputationVerdictType::UNSPECIFIED);
 
         LR_LOG(("Query login whitelist cannot find the URL [request = %p]",
                 aRequest));
       }
 
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -1982,20 +1982,37 @@ void
 AccumulateCategorical(HistogramID id, const nsTArray<nsCString>& labels)
 {
   TelemetryHistogram::AccumulateCategorical(id, labels);
 }
 
 void
 AccumulateTimeDelta(HistogramID aHistogram, TimeStamp start, TimeStamp end)
 {
+  if (start > end) {
+    Accumulate(aHistogram, 0);
+    return;
+  }
   Accumulate(aHistogram,
              static_cast<uint32_t>((end - start).ToMilliseconds()));
 }
 
+void
+AccumulateTimeDelta(HistogramID aHistogram,
+                    const nsCString& key,
+                    TimeStamp start,
+                    TimeStamp end)
+{
+  if (start > end) {
+    Accumulate(aHistogram, key, 0);
+    return;
+  }
+  Accumulate(
+    aHistogram, key, static_cast<uint32_t>((end - start).ToMilliseconds()));
+}
 const char*
 GetHistogramName(HistogramID id)
 {
   return TelemetryHistogram::GetHistogramName(id);
 }
 
 bool
 CanRecordBase()
--- a/toolkit/components/telemetry/Telemetry.h
+++ b/toolkit/components/telemetry/Telemetry.h
@@ -35,21 +35,16 @@ namespace HangMonitor {
 namespace Telemetry {
 
 struct HistogramAccumulation;
 struct KeyedHistogramAccumulation;
 struct ScalarAction;
 struct KeyedScalarAction;
 struct ChildEventData;
 
-enum TimerResolution {
-  Millisecond,
-  Microsecond
-};
-
 /**
  * Initialize the Telemetry service on the main thread at startup.
  */
 void Init();
 
 /**
  * Adds sample to a histogram defined in TelemetryHistogramEnums.h
  *
@@ -208,79 +203,45 @@ void AccumulateCategorical(HistogramID i
  *
  * @param id - histogram id
  * @param start - start time
  * @param end - end time
  */
 void AccumulateTimeDelta(HistogramID id, TimeStamp start, TimeStamp end = TimeStamp::Now());
 
 /**
+ * Adds time delta in milliseconds to a keyed histogram defined in
+ * TelemetryHistogramEnums.h
+ *
+ * @param id - histogram id
+ * @param key - the string key
+ * @param start - start time
+ * @param end - end time
+ */
+void
+AccumulateTimeDelta(HistogramID id,
+                    const nsCString& key,
+                    TimeStamp start,
+                    TimeStamp end = TimeStamp::Now());
+
+/**
  * Enable/disable recording for this histogram in this process at runtime.
- * Recording is enabled by default, unless listed at kRecordingInitiallyDisabledIDs[].
- * id must be a valid telemetry enum, otherwise an assertion is triggered.
+ * Recording is enabled by default, unless listed at
+ * kRecordingInitiallyDisabledIDs[]. id must be a valid telemetry enum,
  *
  * @param id - histogram id
  * @param enabled - whether or not to enable recording from now on.
  */
 void SetHistogramRecordingEnabled(HistogramID id, bool enabled);
 
 const char* GetHistogramName(HistogramID id);
 
-/**
- * Those wrappers are needed because the VS versions we use do not support free
- * functions with default template arguments.
- */
-template<TimerResolution res>
-struct AccumulateDelta_impl
-{
-  static void compute(HistogramID id, TimeStamp start, TimeStamp end = TimeStamp::Now());
-  static void compute(HistogramID id, const nsCString& key, TimeStamp start, TimeStamp end = TimeStamp::Now());
-};
-
-template<>
-struct AccumulateDelta_impl<Millisecond>
+template<HistogramID id>
+class MOZ_RAII AutoTimer
 {
-  static void compute(HistogramID id, TimeStamp start, TimeStamp end = TimeStamp::Now()) {
-    if (start > end) {
-      Accumulate(id, 0);
-      return;
-    }
-    Accumulate(id, static_cast<uint32_t>((end - start).ToMilliseconds()));
-  }
-  static void compute(HistogramID id, const nsCString& key, TimeStamp start, TimeStamp end = TimeStamp::Now()) {
-    if (start > end) {
-      Accumulate(id, key, 0);
-      return;
-    }
-    Accumulate(id, key, static_cast<uint32_t>((end - start).ToMilliseconds()));
-  }
-};
-
-template<>
-struct AccumulateDelta_impl<Microsecond>
-{
-  static void compute(HistogramID id, TimeStamp start, TimeStamp end = TimeStamp::Now()) {
-    if (start > end) {
-      Accumulate(id, 0);
-      return;
-    }
-    Accumulate(id, static_cast<uint32_t>((end - start).ToMicroseconds()));
-  }
-  static void compute(HistogramID id, const nsCString& key, TimeStamp start, TimeStamp end = TimeStamp::Now()) {
-    if (start > end) {
-      Accumulate(id, key, 0);
-      return;
-    }
-    Accumulate(id, key, static_cast<uint32_t>((end - start).ToMicroseconds()));
-  }
-};
-
-
-template<HistogramID id, TimerResolution res = Millisecond>
-class MOZ_RAII AutoTimer {
 public:
   explicit AutoTimer(TimeStamp aStart = TimeStamp::Now() MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
      : start(aStart)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
   }
 
   explicit AutoTimer(const nsCString& aKey, TimeStamp aStart = TimeStamp::Now() MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
@@ -288,19 +249,19 @@ public:
     , key(aKey)
   {
     MOZ_ASSERT(!aKey.IsEmpty(), "The key must not be empty.");
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
   }
 
   ~AutoTimer() {
     if (key.IsEmpty()) {
-      AccumulateDelta_impl<res>::compute(id, start);
+      AccumulateTimeDelta(id, start);
     } else {
-      AccumulateDelta_impl<res>::compute(id, key, start);
+      AccumulateTimeDelta(id, key, start);
     }
   }
 
 private:
   const TimeStamp start;
   const nsCString key;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
--- a/toolkit/components/telemetry/docs/collection/histograms.rst
+++ b/toolkit/components/telemetry/docs/collection/histograms.rst
@@ -248,35 +248,62 @@ Probes in native code can also use the `
   #include "mozilla/Telemetry.h"
 
   /**
    * Adds sample to a histogram defined in Histograms.json
    *
    * @param id - histogram id
    * @param sample - value to record.
    */
-  void Accumulate(ID id, uint32_t sample);
+  void Accumulate(HistogramID id, uint32_t sample);
+
+  /**
+   * Adds samples to a histogram defined in Histograms.json
+   *
+   * @param id - histogram id
+   * @param samples - values to record.
+   */
+  void Accumulate(HistogramID id, const nsTArray<uint32_t>& samples);
 
   /**
    * Adds sample to a keyed histogram defined in Histograms.h
    *
    * @param id - keyed histogram id
    * @param key - the string key
    * @param sample - (optional) value to record, defaults to 1.
    */
-  void Accumulate(ID id, const nsCString& key, uint32_t sample = 1);
+  void Accumulate(HistogramID id, const nsCString& key, uint32_t sample = 1);
 
   /**
    * Adds time delta in milliseconds to a histogram defined in Histograms.json
    *
    * @param id - histogram id
    * @param start - start time
-   * @param end - end time
+   * @param end - (optional) end time, defaults to TimeStamp::Now().
    */
-  void AccumulateTimeDelta(ID id, TimeStamp start, TimeStamp end = TimeStamp::Now());
+  void AccumulateTimeDelta(HistogramID id, TimeStamp start, TimeStamp end = TimeStamp::Now());
+
+  /**
+   * Adds time delta in milliseconds to a keyed histogram defined in Histograms.json
+   *
+   * @param id - histogram id
+   * @param key - the string key
+   * @param start - start time
+   * @param end - (optional) end time, defaults to TimeStamp::Now().
+   */
+  void AccumulateTimeDelta(HistogramID id, const cs TimeStamp start, TimeStamp end = TimeStamp::Now());
+
+  /** Adds time delta in milliseconds to a histogram defined in TelemetryHistogramEnums.h
+   *
+   * @param id - histogram id
+   * @param key - the string key
+   * @param start - start time
+   * @param end - (optional) end time, defaults to TimeStamp::Now().
+   */
+  void AccumulateTimeDelta(HistogramID id, const nsCString& key, TimeStamp start, TimeStamp end = TimeStamp::Now());
 
 The histogram names declared in ``Histograms.json`` are translated into constants in the ``mozilla::Telemetry`` namespace:
 
 .. code-block:: cpp
 
   mozilla::Telemetry::Accumulate(mozilla::Telemetry::STARTUP_CRASH_DETECTED, true);
 
 .. warning::
--- a/toolkit/components/telemetry/docs/collection/measuring-time.rst
+++ b/toolkit/components/telemetry/docs/collection/measuring-time.rst
@@ -70,26 +70,27 @@ Example:
 From C++
 ========
 
 API:
 
 .. code-block:: cpp
 
     // This helper class is the preferred way to record elapsed time.
-    template<ID id, TimerResolution res = MilliSecond>
+    template<HistogramID id>
     class AutoTimer {
       // Record into a plain histogram.
       explicit AutoTimer(TimeStamp aStart = TimeStamp::Now());
       // Record into a keyed histogram, with key |aKey|.
       explicit AutoTimer(const nsCString& aKey,
                          TimeStamp aStart = TimeStamp::Now());
     };
 
-    void AccumulateTimeDelta(ID id, TimeStamp start, TimeStamp end = TimeStamp::Now());
+    void AccumulateTimeDelta(HistogramID id, TimeStamp start, TimeStamp end = TimeStamp::Now());
+    void AccumulateTimeDelta(HistogramID id, const nsCString& key, TimeStamp start, TimeStamp end = TimeStamp::Now());
 
 Example:
 
 .. code-block:: cpp
 
     {
       Telemetry::AutoTimer<Telemetry::FIND_PLUGINS> telemetry;
       // ... scan disk for plugins.
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -1873,18 +1873,18 @@ nsUrlClassifierDBService::AsyncClassifyL
           }
         }
       }
 
       nsCOMPtr<nsIRunnable> cbRunnable = NS_NewRunnableFunction(
         "nsUrlClassifierDBService::AsyncClassifyLocalWithTables",
         [callback, matchedLists, startTime]() -> void {
           // Measure the time diff between calling and callback.
-          AccumulateDelta_impl<Millisecond>::compute(
-            Telemetry::URLCLASSIFIER_ASYNC_CLASSIFYLOCAL_TIME, startTime);
+          AccumulateTimeDelta(Telemetry::URLCLASSIFIER_ASYNC_CLASSIFYLOCAL_TIME,
+                              startTime);
 
           // |callback| is captured as const value so ...
           auto cb = const_cast<nsIURIClassifierCallback*>(callback.get());
           cb->OnClassifyComplete(NS_OK, // Not used.
                                  matchedLists,
                                  EmptyCString(),  // provider. (Not used)
                                  EmptyCString()); // prefix. (Not used)
         });