Bug 1069873 - Add counter histogram type. r=froydnj

ipc/chromium/src/base/histogram.cc

@@ -1048,6 +1048,61 @@ FlagHistogram::AddSampleSet(const SampleSet& sample) {
     Accumulate(1, 1, one_index);
   }
 }
+
+//------------------------------------------------------------------------------
+// CountHistogram:
+//------------------------------------------------------------------------------
+
+Histogram *
+CountHistogram::FactoryGet(const std::string &name, Flags flags)
+{
+  Histogram *h(nullptr);
+
+  if (!StatisticsRecorder::FindHistogram(name, &h)) {
+    CountHistogram *fh = new CountHistogram(name);
+    fh->InitializeBucketRange();
+    fh->SetFlags(flags);
+    h = StatisticsRecorder::RegisterOrDeleteDuplicate(fh);
+  }
+
+  return h;
+}
+
+CountHistogram::CountHistogram(const std::string &name)
+  : LinearHistogram(name, 1, 2, 3) {
+}
+
+Histogram::ClassType
+CountHistogram::histogram_type() const
+{
+  return COUNT_HISTOGRAM;
+}
+
+void
+CountHistogram::Accumulate(Sample value, Count count, size_t index)
+{
+  size_t zero_index = BucketIndex(0);
+  LinearHistogram::Accumulate(1, 1, zero_index);
+}
+
+void
+CountHistogram::AddSampleSet(const SampleSet& sample) {
+  DCHECK_EQ(bucket_count(), sample.size());
+  // We can't be sure the SampleSet provided came from another CountHistogram,
+  // so we at least check that the unused buckets are empty.
+
+  const size_t indices[] = { BucketIndex(0), BucketIndex(1), BucketIndex(2) };
+
+  if (sample.counts(indices[1]) != 0 || sample.counts(indices[2]) != 0) {
+    return;
+  }
+
+  if (sample.counts(indices[0]) != 0) {
+    Accumulate(1, sample.counts(indices[0]), indices[0]);
+  }
+}
+
+
 //------------------------------------------------------------------------------
 // CustomHistogram:
 //------------------------------------------------------------------------------

ipc/chromium/src/base/histogram.h

@@ -278,6 +278,7 @@ class Histogram {
     LINEAR_HISTOGRAM,
     BOOLEAN_HISTOGRAM,
     FLAG_HISTOGRAM,
+    COUNT_HISTOGRAM,
     CUSTOM_HISTOGRAM,
     NOT_VALID_IN_RENDERER
   };
@@ -690,6 +691,24 @@ private:
   DISALLOW_COPY_AND_ASSIGN(FlagHistogram);
 };
 
+// CountHistogram only allows a single monotic counter value.
+class CountHistogram : public LinearHistogram
+{
+public:
+  static Histogram *FactoryGet(const std::string &name, Flags flags);
+
+  virtual ClassType histogram_type() const;
+
+  virtual void Accumulate(Sample value, Count count, size_t index);
+
+  virtual void AddSampleSet(const SampleSet& sample);
+
+private:
+  explicit CountHistogram(const std::string &name);
+
+  DISALLOW_COPY_AND_ASSIGN(CountHistogram);
+};
+
 //------------------------------------------------------------------------------
 
 // CustomHistogram is a histogram for a set of custom integers.

toolkit/components/telemetry/Histograms.json

@@ -4368,6 +4368,11 @@
     "kind": "flag",
     "description": "a testing histogram; not meant to be touched"
   },
+  "TELEMETRY_TEST_COUNT": {
+    "expires_in_version": "default",
+    "kind": "count",
+    "description": "a testing histogram; not meant to be touched"
+  },
   "STARTUP_CRASH_DETECTED": {
     "expires_in_version": "never",
     "kind": "flag",

toolkit/components/telemetry/Telemetry.cpp

@@ -728,7 +728,8 @@ HistogramGet(const char *name, const char *expiration, uint32_t min, uint32_t ma
              uint32_t bucketCount, uint32_t histogramType, Histogram **result)
 {
   if (histogramType != nsITelemetry::HISTOGRAM_BOOLEAN
-      && histogramType != nsITelemetry::HISTOGRAM_FLAG) {
+      && histogramType != nsITelemetry::HISTOGRAM_FLAG
+      && histogramType != nsITelemetry::HISTOGRAM_COUNT) {
     // Sanity checks for histogram parameters.
     if (min >= max)
       return NS_ERROR_ILLEGAL_VALUE;
@@ -761,6 +762,9 @@ HistogramGet(const char *name, const char *expiration, uint32_t min, uint32_t ma
   case nsITelemetry::HISTOGRAM_FLAG:
     *result = FlagHistogram::FactoryGet(name, Histogram::kUmaTargetedHistogramFlag);
     break;
+  case nsITelemetry::HISTOGRAM_COUNT:
+    *result = CountHistogram::FactoryGet(name, Histogram::kUmaTargetedHistogramFlag);
+    break;
   default:
     return NS_ERROR_INVALID_ARG;
   }
@@ -904,33 +908,37 @@ IsEmpty(const Histogram *h)
 bool
 JSHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp)
 {
-  JS::CallArgs args = CallArgsFromVp(argc, vp);
-  if (!args.length()) {
-    JS_ReportError(cx, "Expected one argument");
+  JSObject *obj = JS_THIS_OBJECT(cx, vp);
+  if (!obj) {
     return false;
   }
 
-  if (!(args[0].isNumber() || args[0].isBoolean())) {
-    JS_ReportError(cx, "Not a number");
-    return false;
-  }
+  Histogram *h = static_cast<Histogram*>(JS_GetPrivate(obj));
+  Histogram::ClassType type = h->histogram_type();
 
-  int32_t value;
-  if (!JS::ToInt32(cx, args[0], &value)) {
-    return false;
-  }
-
-  if (TelemetryImpl::CanRecord()) {
-    JSObject *obj = JS_THIS_OBJECT(cx, vp);
-    if (!obj) {
+  int32_t value = 1;
+  if (type != base::CountHistogram::COUNT_HISTOGRAM) {
+    JS::CallArgs args = CallArgsFromVp(argc, vp);
+    if (!args.length()) {
+      JS_ReportError(cx, "Expected one argument");
       return false;
     }
 
-    Histogram *h = static_cast<Histogram*>(JS_GetPrivate(obj));
+    if (!(args[0].isNumber() || args[0].isBoolean())) {
+      JS_ReportError(cx, "Not a number");
+      return false;
+    }
+
+    if (!JS::ToInt32(cx, args[0], &value)) {
+      return false;
+    }
+  }
+
+  if (TelemetryImpl::CanRecord()) {
     h->Add(value);
   }
-  return true;
 
+  return true;
 }
 
 bool
@@ -1583,10 +1591,12 @@ TelemetryImpl::GetHistogramSnapshots(JSContext *cx, JS::MutableHandle<JS::Value>
     return NS_ERROR_FAILURE;
   ret.setObject(*root_obj);
 
-  // Ensure that all the HISTOGRAM_FLAG histograms have been created, so
-  // that their values are snapshotted.
+  // Ensure that all the HISTOGRAM_FLAG & HISTOGRAM_COUNT histograms have
+  // been created, so that their values are snapshotted.
   for (size_t i = 0; i < Telemetry::HistogramCount; ++i) {
-    if (gHistograms[i].histogramType == nsITelemetry::HISTOGRAM_FLAG) {
+    const uint32_t type = gHistograms[i].histogramType;
+    if (type == nsITelemetry::HISTOGRAM_FLAG ||
+        type == nsITelemetry::HISTOGRAM_COUNT) {
       Histogram *h;
       DebugOnly<nsresult> rv = GetHistogramByEnumId(Telemetry::ID(i), &h);
       MOZ_ASSERT(NS_SUCCEEDED(rv));

toolkit/components/telemetry/gen-histogram-bucket-ranges.py

@@ -28,7 +28,8 @@ def main(argv):
             'flag': '3',
             'enumerated': '1',
             'linear': '1',
-            'exponential': '0'
+            'exponential': '0',
+            'count': '4',
             }
         # Use __setitem__ because Python lambdas are so limited.
         histogram_tools.table_dispatch(histogram.kind(), table,

toolkit/components/telemetry/gen-histogram-data.py

@@ -97,6 +97,9 @@ def static_asserts_for_boolean(histogram):
 def static_asserts_for_flag(histogram):
     pass
 
+def static_asserts_for_count(histogram):
+    pass
+
 def static_asserts_for_enumerated(histogram):
     n_values = histogram.high()
     static_assert("%s > 2" % n_values,
@@ -128,6 +131,7 @@ def write_histogram_static_asserts(histograms):
     table = {
         'boolean' : static_asserts_for_boolean,
         'flag' : static_asserts_for_flag,
+        'count': static_asserts_for_count,
         'enumerated' : static_asserts_for_enumerated,
         'linear' : static_asserts_for_linear,
         'exponential' : static_asserts_for_exponential,

toolkit/components/telemetry/histogram_tools.py

@@ -65,7 +65,7 @@ class Histogram:
 definition is a dict-like object that must contain at least the keys:
 
  - 'kind': The kind of histogram.  Must be one of 'boolean', 'flag',
-   'enumerated', 'linear', or 'exponential'.
+   'count', 'enumerated', 'linear', or 'exponential'.
  - 'description': A textual description of the histogram.
 
 The key 'cpp_guard' is optional; if present, it denotes a preprocessor
@@ -80,6 +80,7 @@ symbol that should guard C/C++ definitions associated with the histogram."""
         self.compute_bucket_parameters(definition)
         table = { 'boolean': 'BOOLEAN',
                   'flag': 'FLAG',
+                  'count': 'COUNT',
                   'enumerated': 'LINEAR',
                   'linear': 'LINEAR',
                   'exponential': 'EXPONENTIAL' }
@@ -96,7 +97,7 @@ symbol that should guard C/C++ definitions associated with the histogram."""
 
     def kind(self):
         """Return the kind of the histogram.
-Will be one of 'boolean', 'flag', 'enumerated', 'linear', or 'exponential'."""
+Will be one of 'boolean', 'flag', 'count', 'enumerated', 'linear', or 'exponential'."""
         return self._kind
 
     def expiration(self):
@@ -137,6 +138,7 @@ is enabled."""
         """Return an array of lower bounds for each bucket in the histogram."""
         table = { 'boolean': linear_buckets,
                   'flag': linear_buckets,
+                  'count': linear_buckets,
                   'enumerated': linear_buckets,
                   'linear': linear_buckets,
                   'exponential': exponential_buckets }
@@ -147,6 +149,7 @@ is enabled."""
         table = {
             'boolean': Histogram.boolean_flag_bucket_parameters,
             'flag': Histogram.boolean_flag_bucket_parameters,
+            'count': Histogram.boolean_flag_bucket_parameters,
             'enumerated': Histogram.enumerated_bucket_parameters,
             'linear': Histogram.linear_bucket_parameters,
             'exponential': Histogram.exponential_bucket_parameters
@@ -161,6 +164,7 @@ is enabled."""
         table = {
             'boolean': always_allowed_keys,
             'flag': always_allowed_keys,
+            'count': always_allowed_keys,
             'enumerated': always_allowed_keys + ['n_values'],
             'linear': general_keys,
             'exponential': general_keys + ['extended_statistics_ok']

toolkit/components/telemetry/nsITelemetry.idl

@@ -12,7 +12,7 @@ interface nsIFetchTelemetryDataCallback : nsISupports
   void complete();
 };
 
-[scriptable, uuid(4e4bfc35-dac6-4b28-ade4-7e45760051d5)]
+[scriptable, uuid(df355bbf-437d-4332-80e6-a8a54db959f3)]
 interface nsITelemetry : nsISupports
 {
   /**
@@ -21,11 +21,13 @@ interface nsITelemetry : nsISupports
    * HISTOGRAM_LINEAR - buckets increase linearly
    * HISTOGRAM_BOOLEAN - For storing 0/1 values
    * HISTOGRAM_FLAG - For storing a single value; its count is always == 1.
+   * HISTOGRAM_COUNT - For storing counter values without bucketing.
    */
   const unsigned long HISTOGRAM_EXPONENTIAL = 0;
   const unsigned long HISTOGRAM_LINEAR = 1;
   const unsigned long HISTOGRAM_BOOLEAN = 2;
   const unsigned long HISTOGRAM_FLAG = 3;
+  const unsigned long HISTOGRAM_COUNT = 4;
 
   /*
    * An object containing a snapshot from all of the currently registered histograms.
@@ -33,7 +35,8 @@ interface nsITelemetry : nsISupports
    * where data is consists of the following properties:
    *   min - Minimal bucket size
    *   max - Maximum bucket size
-   *   histogram_type - HISTOGRAM_EXPONENTIAL, HISTOGRAM_LINEAR, or HISTOGRAM_BOOLEAN
+   *   histogram_type - HISTOGRAM_EXPONENTIAL, HISTOGRAM_LINEAR, HISTOGRAM_BOOLEAN
+   *                    or HISTOGRAM_COUNT
    *   counts - array representing contents of the buckets in the histogram
    *   ranges -  an array with calculated bucket sizes
    *   sum - sum of the bucket contents
@@ -143,7 +146,7 @@ interface nsITelemetry : nsISupports
    * @param min - Minimal bucket size
    * @param max - Maximum bucket size
    * @param bucket_count - number of buckets in the histogram.
-   * @param type - HISTOGRAM_EXPONENTIAL, HISTOGRAM_LINEAR or HISTOGRAM_BOOLEAN
+   * @param type - HISTOGRAM_EXPONENTIAL, HISTOGRAM_LINEAR, HISTOGRAM_BOOLEAN or HISTOGRAM_COUNT
    * 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()
@@ -192,8 +195,8 @@ interface nsITelemetry : nsISupports
    * @param min - Minimal bucket size
    * @param max - Maximum bucket size
    * @param bucket_count - number of buckets in the histogram
-   * @param histogram_type - HISTOGRAM_EXPONENTIAL, HISTOGRAM_LINEAR, or
-   *        HISTOGRAM_BOOLEAN
+   * @param histogram_type - HISTOGRAM_EXPONENTIAL, HISTOGRAM_LINEAR,
+   *        HISTOGRAM_BOOLEAN or HISTOGRAM_COUNT
    */
   void registerAddonHistogram(in ACString addon_id, in ACString name,
                               in uint32_t min, in uint32_t max,

toolkit/components/telemetry/tests/unit/head.js

@@ -5,6 +5,7 @@
 // copied from toolkit/mozapps/extensions/test/xpcshell/head_addons.js
 const XULAPPINFO_CONTRACTID = "@mozilla.org/xre/app-info;1";
 const XULAPPINFO_CID = Components.ID("{c763b610-9d49-455a-bbd2-ede71682a1ac}");
+let gAppInfo;
 
 function createAppInfo(id, name, version, platformVersion) {
   gAppInfo = {

toolkit/components/telemetry/tests/unit/test_TelemetryPing.js

@@ -80,8 +80,10 @@ function setupTestData() {
   Services.startup.interrupted = true;
   Telemetry.registerAddonHistogram(ADDON_NAME, ADDON_HISTOGRAM, 1, 5, 6,
                                    Telemetry.HISTOGRAM_LINEAR);
-  h1 = Telemetry.getAddonHistogram(ADDON_NAME, ADDON_HISTOGRAM);
+  let h1 = Telemetry.getAddonHistogram(ADDON_NAME, ADDON_HISTOGRAM);
   h1.add(1);
+  let h2 = Telemetry.getHistogramById("TELEMETRY_TEST_COUNT");
+  h2.add();
 }
 
 function getSavedHistogramsFile(basename) {
@@ -209,9 +211,14 @@ function checkPayload(request, reason, successfulPings) {
   const TELEMETRY_PING = "TELEMETRY_PING";
   const TELEMETRY_SUCCESS = "TELEMETRY_SUCCESS";
   const TELEMETRY_TEST_FLAG = "TELEMETRY_TEST_FLAG";
+  const TELEMETRY_TEST_COUNT = "TELEMETRY_TEST_COUNT";
   const READ_SAVED_PING_SUCCESS = "READ_SAVED_PING_SUCCESS";
+
   do_check_true(TELEMETRY_PING in payload.histograms);
   do_check_true(READ_SAVED_PING_SUCCESS in payload.histograms);
+  do_check_true(TELEMETRY_TEST_FLAG in payload.histograms);
+  do_check_true(TELEMETRY_TEST_COUNT in payload.histograms);
+
   let rh = Telemetry.registeredHistograms([]);
   for (let name of rh) {
     if (/SQLITE/.test(name) && name in payload.histograms) {
@@ -234,6 +241,19 @@ function checkPayload(request, reason, successfulPings) {
   let flag = payload.histograms[TELEMETRY_TEST_FLAG];
   do_check_eq(uneval(flag), uneval(expected_flag));
 
+  // We should have a test count.
+  const expected_count = {
+    range: [1, 2],
+    bucket_count: 3,
+    histogram_type: 4,
+    values: {0:1, 1:0},
+    sum: 1,
+    sum_squares_lo: 1,
+    sum_squares_hi: 0,
+  };
+  let count = payload.histograms[TELEMETRY_TEST_COUNT];
+  do_check_eq(uneval(count), uneval(expected_count));
+
   // There should be one successful report from the previous telemetry ping.
   const expected_tc = {
     range: [1, 2],

toolkit/components/telemetry/tests/unit/test_nsITelemetry.js

@@ -151,7 +151,7 @@ function test_flag_histogram()
   var h = Telemetry.newHistogram("test::flag histogram", "never", 130, 4, 5, Telemetry.HISTOGRAM_FLAG);
   var r = h.snapshot().ranges;
   // Flag histograms ignore numeric parameters.
-  do_check_eq(uneval(r), uneval([0, 1, 2]))
+  do_check_eq(uneval(r), uneval([0, 1, 2]));
   // Should already have a 0 counted.
   var c = h.snapshot().counts;
   var s = h.snapshot().sum;
@@ -172,6 +172,23 @@ function test_flag_histogram()
   do_check_eq(h.snapshot().histogram_type, Telemetry.HISTOGRAM_FLAG);
 }
 
+function test_count_histogram()
+{
+  let h = Telemetry.newHistogram("test::count histogram", "never", 1, 2, 3, Telemetry.HISTOGRAM_COUNT);
+  let s = h.snapshot();
+  do_check_eq(uneval(s.ranges), uneval([0, 1, 2]));
+  do_check_eq(uneval(s.counts), uneval([0, 0, 0]));
+  do_check_eq(s.sum, 0);
+  h.add();
+  s = h.snapshot();
+  do_check_eq(uneval(s.counts), uneval([1, 0, 0]));
+  do_check_eq(s.sum, 1);
+  h.add();
+  s = h.snapshot();
+  do_check_eq(uneval(s.counts), uneval([2, 0, 0]));
+  do_check_eq(s.sum, 2);
+}
+
 function test_getHistogramById() {
   try {
     Telemetry.getHistogramById("nonexistent");
@@ -217,7 +234,8 @@ function test_histogramFrom() {
       "CYCLE_COLLECTOR",      // EXPONENTIAL
       "GC_REASON_2",          // LINEAR
       "GC_RESET",             // BOOLEAN
-      "TELEMETRY_TEST_FLAG"   // FLAG
+      "TELEMETRY_TEST_FLAG",  // FLAG
+      "TELEMETRY_TEST_COUNT", // COUNT
   ];
 
   for each (let name in names) {
@@ -227,11 +245,16 @@ function test_histogramFrom() {
     compareHistograms(original, clone);
   }
 
-  // Additionally, set the flag on TELEMETRY_TEST_FLAG, and check it gets set on the 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);
 }
 
 function test_getSlowSQL() {
@@ -379,6 +402,7 @@ function run_test()
 
   test_boolean_histogram();
   test_flag_histogram();
+  test_count_histogram();
   test_getHistogramById();
   test_histogramFrom();
   test_getSlowSQL();