Bug 1380880 - Use process type to distinguish keyed histograms (r=chutten)
authorBill McCloskey <billm@mozilla.com>
Thu, 13 Jul 2017 16:23:50 -0700
changeset 417762 5c1415579a9bc9e6cdd3ff18c7817789076c93ad
parent 417714 b292385f25f7621f56409342a964ce6c6973c7a4
child 417763 fe2266f8d8e407ec226b956c21fe76dc481a396c
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1380880
milestone56.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 1380880 - Use process type to distinguish keyed histograms (r=chutten) MozReview-Commit-ID: 8pcou7DsU7S
toolkit/components/telemetry/TelemetryHistogram.cpp
toolkit/components/telemetry/tests/unit/test_ChildHistograms.js
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -860,17 +860,18 @@ internal_ShouldReflectHistogram(Histogra
 ////////////////////////////////////////////////////////////////////////
 //
 // PRIVATE: class KeyedHistogram
 
 namespace {
 
 class KeyedHistogram {
 public:
-  KeyedHistogram(const nsACString &name, const nsACString &expiration,
+  KeyedHistogram(ProcessID processType, const nsACString &name,
+                 const nsACString &expiration,
                  uint32_t histogramType, uint32_t min, uint32_t max,
                  uint32_t bucketCount, uint32_t dataset);
   nsresult GetHistogram(const nsCString& name, Histogram** histogram, bool subsession);
   Histogram* GetHistogram(const nsCString& name, bool subsession);
   uint32_t GetHistogramType() const { return mHistogramType; }
   nsresult GetJSKeys(JSContext* cx, JS::CallArgs& args);
   nsresult GetJSSnapshot(JSContext* cx, JS::Handle<JSObject*> obj,
                          bool subsession, bool clearSubsession);
@@ -890,35 +891,38 @@ private:
 #if !defined(MOZ_WIDGET_ANDROID)
   KeyedHistogramMapType mSubsessionMap;
 #endif
 
   static bool ReflectKeyedHistogram(KeyedHistogramEntry* entry,
                                     JSContext* cx,
                                     JS::Handle<JSObject*> obj);
 
+  const ProcessID mProcessType;
   const nsCString mName;
   const nsCString mExpiration;
   const uint32_t mHistogramType;
   const uint32_t mMin;
   const uint32_t mMax;
   const uint32_t mBucketCount;
   const uint32_t mDataset;
   mozilla::Atomic<bool, mozilla::Relaxed> mRecordingEnabled;
 };
 
-KeyedHistogram::KeyedHistogram(const nsACString &name,
+KeyedHistogram::KeyedHistogram(ProcessID processType,
+                               const nsACString &name,
                                const nsACString &expiration,
                                uint32_t histogramType,
                                uint32_t min, uint32_t max,
                                uint32_t bucketCount, uint32_t dataset)
   : mHistogramMap()
 #if !defined(MOZ_WIDGET_ANDROID)
   , mSubsessionMap()
 #endif
+  , mProcessType(processType)
   , mName(name)
   , mExpiration(expiration)
   , mHistogramType(histogramType)
   , mMin(min)
   , mMax(max)
   , mBucketCount(bucketCount)
   , mDataset(dataset)
   , mRecordingEnabled(true)
@@ -942,16 +946,17 @@ KeyedHistogram::GetHistogram(const nsCSt
 
   nsCString histogramName;
 #if !defined(MOZ_WIDGET_ANDROID)
   if (subsession) {
     histogramName.AppendLiteral(SUBSESSION_HISTOGRAM_PREFIX);
   }
 #endif
   histogramName.Append(mName);
+  histogramName.Append(SuffixForProcessType(mProcessType));
   histogramName.AppendLiteral(KEYED_HISTOGRAM_NAME_SEPARATOR);
   histogramName.Append(key);
 
   Histogram* h;
   nsresult rv = internal_HistogramGet(histogramName.get(), mExpiration.get(),
                                       mHistogramType, mMin, mMax, mBucketCount,
                                       true, &h);
   if (NS_FAILED(rv)) {
@@ -1852,38 +1857,38 @@ void TelemetryHistogram::InitializeGloba
   // Create registered keyed histograms
   for (const auto & h : gHistograms) {
     if (!h.keyed) {
       continue;
     }
 
     const nsDependentCString id(h.id());
     const nsDependentCString expiration(h.expiration());
-    gKeyedHistograms.Put(id, new KeyedHistogram(id, expiration, h.histogramType,
+    gKeyedHistograms.Put(id, new KeyedHistogram(ProcessID::Parent, id, expiration, h.histogramType,
                                                 h.min, h.max, h.bucketCount, h.dataset));
     if (XRE_IsParentProcess()) {
       // We must create registered child keyed histograms as well or else the
       // same code in TelemetrySession.jsm that fails without parent keyed
       // histograms will fail without child keyed histograms.
       nsCString contentId(id);
       contentId.AppendLiteral(CONTENT_HISTOGRAM_SUFFIX);
       gKeyedHistograms.Put(contentId,
-                           new KeyedHistogram(id, expiration, h.histogramType,
+                           new KeyedHistogram(ProcessID::Content, id, expiration, h.histogramType,
                                               h.min, h.max, h.bucketCount, h.dataset));
 
       nsCString gpuId(id);
       gpuId.AppendLiteral(GPU_HISTOGRAM_SUFFIX);
       gKeyedHistograms.Put(gpuId,
-                           new KeyedHistogram(id, expiration, h.histogramType,
+                           new KeyedHistogram(ProcessID::Gpu, id, expiration, h.histogramType,
                                               h.min, h.max, h.bucketCount, h.dataset));
 
       nsCString extensionId(id);
       extensionId.AppendLiteral(EXTENSION_HISTOGRAM_SUFFIX);
       gKeyedHistograms.Put(extensionId,
-                           new KeyedHistogram(id, expiration, h.histogramType,
+                           new KeyedHistogram(ProcessID::Extension, id, expiration, h.histogramType,
                                               h.min, h.max, h.bucketCount, h.dataset));
     }
   }
 
     // Some Telemetry histograms depend on the value of C++ constants and hardcode
     // their values in Histograms.json.
     // We add static asserts here for those values to match so that future changes
     // don't go unnoticed.
--- a/toolkit/components/telemetry/tests/unit/test_ChildHistograms.js
+++ b/toolkit/components/telemetry/tests/unit/test_ChildHistograms.js
@@ -120,16 +120,18 @@ add_task(async function() {
   let contentFlag = Telemetry.getHistogramById("TELEMETRY_TEST_FLAG_CONTENT_PROCESS");
   contentFlag.add(true);
   let mainFlag = Telemetry.getHistogramById("TELEMETRY_TEST_FLAG_MAIN_PROCESS");
   mainFlag.add(true);
   let allLinear = Telemetry.getHistogramById("TELEMETRY_TEST_ALL_PROCESSES");
   allLinear.add(20);
   let allChildLinear = Telemetry.getHistogramById("TELEMETRY_TEST_ALL_CHILD_PROCESSES");
   allChildLinear.add(20);
+  let countKeyed = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_COUNT");
+  countKeyed.add("a");
 
   const payload = TelemetrySession.getPayload("test-ping");
   Assert.ok("processes" in payload, "Should have processes section");
   Assert.ok("content" in payload.processes, "Should have child process section");
   Assert.ok("histograms" in payload.processes.content, "Child process section should have histograms.");
   Assert.ok("keyedHistograms" in payload.processes.content, "Child process section should have keyed histograms.");
   check_histogram_values(payload.processes.content);
 
@@ -155,11 +157,12 @@ add_task(async function() {
   Assert.ok(!("TELEMETRY_TEST_CONTENT_PROCESS" in mainHs), "Should not have content process histogram in main process payload");
   Assert.ok(!("TELEMETRY_TEST_KEYED_CONTENT_PROCESS" in mainKhs), "Should not have keyed content process histogram in main process payload");
   Assert.ok(!("TELEMETRY_TEST_FLAG_CONTENT_PROCESS" in mainHs), "Should not have content process histogram in main process payload");
   Assert.ok("TELEMETRY_TEST_ALL_PROCESSES" in mainHs, "Should have all-process histogram in main process payload");
   Assert.equal(mainHs.TELEMETRY_TEST_ALL_PROCESSES.sum, 20, "Should have correct value");
   Assert.ok(!("TELEMETRY_TEST_ALL_CHILD_PROCESSES" in mainHs), "Should not have all-child process histogram in main process payload");
   Assert.ok("TELEMETRY_TEST_FLAG_MAIN_PROCESS" in mainHs, "Should have main process histogram in main process payload");
   Assert.equal(mainHs.TELEMETRY_TEST_FLAG_MAIN_PROCESS.sum, 1, "Should have correct value");
+  Assert.equal(mainKhs.TELEMETRY_TEST_KEYED_COUNT.a.sum, 1, "Should have correct value in parent");
 
   do_test_finished();
 });