Bug 1498166 - Add multi-storage to plain histograms r=chutten
authorJan-Erik Rediger <jrediger@mozilla.com>
Mon, 19 Nov 2018 14:44:25 +0000
changeset 503882 137130a36a2264710b41e8e38cdbb242e7dcf78c
parent 503881 b362f996a039cd46e06787cacc5b395a4491633b
child 503883 544d3ecf27ab29902f07237a3170b1c8217672a7
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1498166
milestone65.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 1498166 - Add multi-storage to plain histograms r=chutten This introduces a new wrapper `Histogram` that keeps track of the multiple stores a histogram can be in. Histograms are stored in a hash table, indexed by the name of the store. It has one optimization to support the majority of cases: a single `main` store. For that it stores a direct pointer to the underlying base::Histogram and skips populating the map. This saves an indirection and memory overhead of actually placing it into the hash table. For now a snapshot only ever returns data from the main store. Clearing a snapshot only clears the main store. (This will both change in a follow-up) Depends on D11904 Differential Revision: https://phabricator.services.mozilla.com/D11905
toolkit/components/telemetry/core/TelemetryHistogram.cpp
--- a/toolkit/components/telemetry/core/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/core/TelemetryHistogram.cpp
@@ -148,16 +148,17 @@ struct HistogramInfo {
   uint8_t dataset;
   RecordedProcessType record_in_processes;
   SupportedProduct products;
 
   const char *name() const;
   const char *expiration() const;
   nsresult label_id(const char* label, uint32_t* labelId) const;
   bool allows_key(const nsACString& key) const;
+  bool is_single_store() const;
 };
 
 // Structs used to keep information about the histograms for which a
 // snapshot should be created.
 struct HistogramSnapshotData {
   nsTArray<base::Histogram::Sample> mBucketRanges;
   nsTArray<base::Histogram::Count> mBucketCounts;
   int64_t mSampleSum; // Same type as base::Histogram::SampleSet::sum_
@@ -177,16 +178,64 @@ typedef nsDataHashtable<nsCStringHashKey
 struct KeyedHistogramSnapshotInfo {
   KeyedHistogramSnapshotData data;
   HistogramID histogramId;
 };
 
 typedef mozilla::Vector<KeyedHistogramSnapshotInfo> KeyedHistogramSnapshotsArray;
 typedef mozilla::Vector<KeyedHistogramSnapshotsArray> KeyedHistogramProcessSnapshotsArray;
 
+/**
+ * A Histogram storage engine.
+ *
+ * Takes care of recording data into multiple stores if necessary.
+ */
+class Histogram {
+public:
+  /*
+   * Create a new histogram instance from the given info.
+   *
+   * If the histogram is already expired, this does not allocate.
+   */
+  Histogram(HistogramID histogramId, const HistogramInfo& info, bool expired);
+  ~Histogram();
+
+  /**
+   * Add a sample to this histogram in all registered stores.
+   */
+  void Add(uint32_t sample);
+
+  /**
+   * Clear the main store for this histogram.
+   * TODO(bug 1498173): Add clearing of specific store.
+   */
+  void Clear();
+
+  /**
+   * Get the histogram instance from the named store.
+   */
+  bool GetHistogram(const nsACString& store, base::Histogram** h);
+
+  bool IsExpired() const { return mIsExpired; }
+
+  size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf);
+
+private:
+  // String -> Histogram*
+  typedef nsClassHashtable<nsCStringHashKey, base::Histogram> HistogramStore;
+  HistogramStore mStorage;
+
+  // A valid pointer if this histogram belongs to only the main store
+  base::Histogram* mSingleStore;
+
+  // We don't track stores for expired histograms.
+  // We just store a single flag and all other operations become a no-op.
+  bool mIsExpired;
+};
+
 class KeyedHistogram {
 public:
   KeyedHistogram(HistogramID id, const HistogramInfo& info, bool expired);
   ~KeyedHistogram();
   nsresult GetHistogram(const nsCString& name, base::Histogram** histogram);
   base::Histogram* GetHistogram(const nsCString& name);
   uint32_t GetHistogramType() const { return mHistogramInfo.histogramType; }
   nsresult GetKeys(const StaticMutexAutoLock& aLock, nsTArray<nsCString>& aKeys);
@@ -232,22 +281,23 @@ bool gInitDone = false;
 
 // Whether we are collecting the base, opt-out, Histogram data.
 bool gCanRecordBase = false;
 // Whether we are collecting the extended, opt-in, Histogram data.
 bool gCanRecordExtended = false;
 
 // The storage for actual Histogram instances.
 // We use separate ones for plain and keyed histograms.
-base::Histogram** gHistogramStorage;
+Histogram** gHistogramStorage;
 // Keyed histograms internally map string keys to individual Histogram instances.
 KeyedHistogram** gKeyedHistogramStorage;
 
 // To simplify logic below we use a single histogram instance for all expired histograms.
-base::Histogram* gExpiredHistogram = nullptr;
+base::Histogram* gExpiredBaseHistogram = nullptr;
+Histogram* gExpiredHistogram = nullptr;
 
 // The single placeholder for expired keyed histograms.
 KeyedHistogram* gExpiredKeyedHistogram = nullptr;
 
 // This tracks whether recording is enabled for specific histograms.
 // To utilize C++ initialization rules, we invert the meaning to "disabled".
 bool gHistogramRecordingDisabled[HistogramCount] = {};
 
@@ -297,28 +347,28 @@ size_t internal_HistogramStorageIndex(co
   static_assert(
     HistogramCount <
       std::numeric_limits<size_t>::max() / size_t(ProcessID::Count),
         "Too many histograms and processes to store in a 1D array.");
 
   return aHistogramId * size_t(ProcessID::Count) + size_t(aProcessId);
 }
 
-base::Histogram* internal_GetHistogramFromStorage(const StaticMutexAutoLock& aLock,
-                                                  HistogramID aHistogramId,
-                                                  ProcessID aProcessId)
+Histogram* internal_GetHistogramFromStorage(const StaticMutexAutoLock& aLock,
+                                            HistogramID aHistogramId,
+                                            ProcessID aProcessId)
 {
   size_t index = internal_HistogramStorageIndex(aLock, aHistogramId, aProcessId);
   return gHistogramStorage[index];
 }
 
 void internal_SetHistogramInStorage(const StaticMutexAutoLock& aLock,
                                     HistogramID aHistogramId,
                                     ProcessID aProcessId,
-                                    base::Histogram* aHistogram)
+                                    Histogram* aHistogram)
 {
   MOZ_ASSERT(XRE_IsParentProcess(),
     "Histograms are stored only in the parent process.");
 
   size_t index = internal_HistogramStorageIndex(aLock, aHistogramId, aProcessId);
   MOZ_ASSERT(!gHistogramStorage[index],
     "Mustn't overwrite storage without clearing it first.");
   gHistogramStorage[index] = aHistogram;
@@ -339,48 +389,51 @@ void internal_SetKeyedHistogramInStorage
     "Keyed Histograms are stored only in the parent process.");
 
   size_t index = internal_KeyedHistogramStorageIndex(aHistogramId, aProcessId);
   MOZ_ASSERT(!gKeyedHistogramStorage[index],
     "Mustn't overwrite storage without clearing it first");
   gKeyedHistogramStorage[index] = aKeyedHistogram;
 }
 
-// Factory function for histogram instances.
+// Factory function for base::Histogram instances.
 base::Histogram*
-internal_CreateHistogramInstance(const HistogramInfo& info, int bucketsOffset);
+internal_CreateBaseHistogramInstance(const HistogramInfo& info, int bucketsOffset);
+
+// Factory function for histogram instances.
+Histogram*
+internal_CreateHistogramInstance(HistogramID histogramId);
 
 bool
 internal_IsHistogramEnumId(HistogramID aID)
 {
   static_assert(((HistogramID)-1 > 0), "ID should be unsigned.");
   return aID < HistogramCount;
 }
 
 // Look up a plain histogram by id.
-base::Histogram*
+Histogram*
 internal_GetHistogramById(const StaticMutexAutoLock& aLock,
                           HistogramID histogramId,
                           ProcessID processId,
                           bool instantiate = true)
 {
   MOZ_ASSERT(internal_IsHistogramEnumId(histogramId));
   MOZ_ASSERT(!gHistogramInfos[histogramId].keyed);
   MOZ_ASSERT(processId < ProcessID::Count);
 
-  base::Histogram* h = internal_GetHistogramFromStorage(aLock, histogramId, processId);
+  Histogram* h = internal_GetHistogramFromStorage(aLock, histogramId, processId);
   if (h || !instantiate) {
     return h;
   }
 
-  const HistogramInfo& info = gHistogramInfos[histogramId];
-  const int bucketsOffset = gHistogramBucketLowerBoundIndex[histogramId];
-  h = internal_CreateHistogramInstance(info, bucketsOffset);
+  h = internal_CreateHistogramInstance(histogramId);
   MOZ_ASSERT(h);
   internal_SetHistogramInStorage(aLock, histogramId, processId, h);
+
   return h;
 }
 
 // Look up a keyed histogram by id.
 KeyedHistogram*
 internal_GetKeyedHistogramById(HistogramID histogramId, ProcessID processId,
                                bool instantiate = true)
 {
@@ -430,31 +483,16 @@ internal_GetHistogramIdByName(const Stat
   if (name.Equals(gHistogramInfos[idx].name())) {
     *id = HistogramID(idx);
     return NS_OK;
   }
 
   return NS_ERROR_ILLEGAL_VALUE;
 }
 
-// Clear a histogram from storage.
-void
-internal_ClearHistogramById(const StaticMutexAutoLock& aLock,
-                            HistogramID histogramId,
-                            ProcessID processId)
-{
-  size_t index = internal_HistogramStorageIndex(aLock, histogramId, processId);
-  if (gHistogramStorage[index] == gExpiredHistogram) {
-    // We keep gExpiredHistogram until TelemetryHistogram::DeInitializeGlobalState
-    return;
-  }
-  delete gHistogramStorage[index];
-  gHistogramStorage[index] = nullptr;
-}
-
 }
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
 //
 // PRIVATE: Misc small helpers
 
 namespace {
@@ -484,17 +522,17 @@ bool
 internal_IsEmpty(const StaticMutexAutoLock& aLock, const base::Histogram *h)
 {
   return h->is_empty();
 }
 
 bool
 internal_IsExpired(const StaticMutexAutoLock& aLock, base::Histogram* h)
 {
-  return h == gExpiredHistogram;
+  return h == gExpiredBaseHistogram;
 }
 
 void
 internal_SetHistogramRecordingEnabled(const StaticMutexAutoLock& aLock,
                                       HistogramID id,
                                       bool aEnabled)
 {
   MOZ_ASSERT(internal_IsHistogramEnumId(id));
@@ -566,16 +604,22 @@ HistogramInfo::allows_key(const nsACStri
       return true;
     }
   }
 
   // |key| was not found.
   return false;
 }
 
+bool
+HistogramInfo::is_single_store() const
+{
+  return store_count == 1 && store_index == UINT16_MAX;
+}
+
 } // namespace
 
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
 //
 // PRIVATE: Histogram Get, Add, Clone, Clear functions
 
@@ -599,33 +643,58 @@ internal_CheckHistogramArguments(const H
     if (info.min < 1) {
       return NS_ERROR_ILLEGAL_VALUE;
     }
   }
 
   return NS_OK;
 }
 
+Histogram*
+internal_CreateHistogramInstance(HistogramID histogramId)
+{
+  const HistogramInfo& info = gHistogramInfos[histogramId];
+
+  if (NS_FAILED(internal_CheckHistogramArguments(info))) {
+    MOZ_ASSERT(false, "Failed histogram argument checks.");
+    return nullptr;
+  }
+
+  const bool isExpired = IsExpiredVersion(info.expiration());
+
+  if (isExpired) {
+    if (!gExpiredHistogram) {
+      gExpiredHistogram = new Histogram(histogramId, info, /* expired */ true);
+    }
+
+    return gExpiredHistogram;
+  }
+
+  Histogram *wrapper = new Histogram(histogramId, info, /* expired */ false);
+
+  return wrapper;
+}
+
 base::Histogram*
-internal_CreateHistogramInstance(const HistogramInfo& passedInfo, int bucketsOffset)
+internal_CreateBaseHistogramInstance(const HistogramInfo& passedInfo, int bucketsOffset)
 {
   if (NS_FAILED(internal_CheckHistogramArguments(passedInfo))) {
     MOZ_ASSERT(false, "Failed histogram argument checks.");
     return nullptr;
   }
 
   // To keep the code simple we map all the calls to expired histograms to the same histogram instance.
   // We create that instance lazily when needed.
   const bool isExpired = IsExpiredVersion(passedInfo.expiration());
   HistogramInfo info = passedInfo;
   const int* buckets = &gHistogramBucketLowerBounds[bucketsOffset];
 
   if (isExpired) {
-    if (gExpiredHistogram) {
-      return gExpiredHistogram;
+    if (gExpiredBaseHistogram) {
+      return gExpiredBaseHistogram;
     }
 
     // The first values in gHistogramBucketLowerBounds are reserved for
     // expired histograms.
     buckets = gHistogramBucketLowerBounds;
     info.min = 1;
     info.max = 2;
     info.bucketCount = 3;
@@ -652,25 +721,25 @@ internal_CreateHistogramInstance(const H
     h = CountHistogram::FactoryGet(flags, buckets);
     break;
   default:
     MOZ_ASSERT(false, "Invalid histogram type");
     return nullptr;
   }
 
   if (isExpired) {
-    gExpiredHistogram = h;
+    gExpiredBaseHistogram = h;
   }
 
   return h;
 }
 
 nsresult
 internal_HistogramAdd(const StaticMutexAutoLock& aLock,
-                      base::Histogram& histogram,
+                      Histogram& histogram,
                       const HistogramID id,
                       uint32_t value,
                       ProcessID aProcessType)
 {
   // Check if we are allowed to record the data.
   bool canRecordDataset = CanRecordDataset(gHistogramInfos[id].dataset,
                                            internal_CanRecordBase(),
                                            internal_CanRecordExtended());
@@ -899,19 +968,29 @@ internal_GetHistogramsSnapshot(const Sta
       }
 
       if (!IsInDataset(info.dataset, aDataset)) {
         continue;
       }
 
       bool shouldInstantiate =
         info.histogramType == nsITelemetry::HISTOGRAM_FLAG;
-      base::Histogram* h = internal_GetHistogramById(aLock, id, ProcessID(process),
+      Histogram* w = internal_GetHistogramById(aLock, id, ProcessID(process),
                                                shouldInstantiate);
-      if (!h || internal_IsExpired(aLock, h) || !internal_ShouldReflectHistogram(aLock, h, id)) {
+      if (!w || w->IsExpired()) {
+        continue;
+      }
+
+      base::Histogram *h = nullptr;
+      NS_NAMED_LITERAL_CSTRING(store, "main");
+      if (!w->GetHistogram(store, &h)) {
+        continue;
+      }
+
+      if (!internal_ShouldReflectHistogram(aLock, h, id)) {
         continue;
       }
 
       const char* name = info.name();
       if (aFilterTest && strncmp(TEST_HISTOGRAM_PREFIX, name, strlen(TEST_HISTOGRAM_PREFIX)) == 0) {
         if (aClearSubsession) {
           h->Clear();
         }
@@ -936,16 +1015,135 @@ internal_GetHistogramsSnapshot(const Sta
   return NS_OK;
 }
 
 } // namespace
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
 //
+// PRIVATE: class Histogram
+
+namespace {
+
+Histogram::Histogram(HistogramID histogramId, const HistogramInfo& info, bool expired)
+  : mStorage()
+  , mSingleStore(nullptr)
+  , mIsExpired(expired)
+{
+  if (IsExpired()) {
+    return;
+  }
+
+  base::Histogram* h;
+  const int bucketsOffset = gHistogramBucketLowerBoundIndex[histogramId];
+
+  if (info.is_single_store()) {
+    mSingleStore = internal_CreateBaseHistogramInstance(info, bucketsOffset);
+  } else {
+    for (uint32_t i = 0; i < info.store_count; i++) {
+      auto store = nsDependentCString(&gHistogramStringTable[gHistogramStoresTable[info.store_index + i]]);
+      h = internal_CreateBaseHistogramInstance(info, bucketsOffset);
+      mStorage.Put(store, h);
+    }
+  }
+}
+
+Histogram::~Histogram()
+{
+  delete mSingleStore;
+}
+
+void
+Histogram::Add(uint32_t sample)
+{
+  MOZ_ASSERT(XRE_IsParentProcess(), "Only add to histograms in the parent process");
+  if (!XRE_IsParentProcess()) {
+    return;
+  }
+
+  if (IsExpired()) {
+    return;
+  }
+
+  if (mSingleStore != nullptr) {
+    mSingleStore->Add(sample);
+  } else {
+    for (auto iter = mStorage.Iter(); !iter.Done(); iter.Next()) {
+      auto& h = iter.Data();
+      h->Add(sample);
+    }
+  }
+}
+
+void
+Histogram::Clear()
+{
+  MOZ_ASSERT(XRE_IsParentProcess(), "Only clear histograms in the parent process");
+  if (!XRE_IsParentProcess()) {
+    return;
+  }
+
+  if (mSingleStore != nullptr) {
+    mSingleStore->Clear();
+  } else {
+    base::Histogram* h = nullptr;
+    bool found = GetHistogram(NS_LITERAL_CSTRING("main"), &h);
+    if (!found) {
+      return;
+    }
+    MOZ_ASSERT(h, "Should have found a valid histogram in the main store");
+
+    h->Clear();
+  }
+}
+
+bool
+Histogram::GetHistogram(const nsACString& store, base::Histogram** h)
+{
+  MOZ_ASSERT(!IsExpired());
+  if (IsExpired()) {
+    return false;
+  }
+
+  if (mSingleStore != nullptr){
+    *h = mSingleStore;
+    return true;
+  }
+
+  return mStorage.Get(store, h);
+}
+
+size_t
+Histogram::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf)
+{
+  size_t n = 0;
+  n += aMallocSizeOf(this);
+  /*
+   * In theory mStorage.SizeOfExcludingThis should included the data part of the map,
+   * but the numbers seemed low, so we are only taking the shallow size and do
+   * the iteration here.
+   */
+  n += mStorage.ShallowSizeOfExcludingThis(aMallocSizeOf);
+  for (auto iter = mStorage.Iter(); !iter.Done(); iter.Next()) {
+    auto& h = iter.Data();
+    n += h->SizeOfIncludingThis(aMallocSizeOf);
+  }
+  if (mSingleStore != nullptr) {
+    // base::Histogram doesn't have SizeOfExcludingThis, so we are overcounting the pointer here.
+    n += mSingleStore->SizeOfIncludingThis(aMallocSizeOf);
+  }
+  return n;
+}
+
+} // namespace
+
+////////////////////////////////////////////////////////////////////////
+////////////////////////////////////////////////////////////////////////
+//
 // PRIVATE: class KeyedHistogram and internal_ReflectKeyedHistogram
 
 namespace {
 
 nsresult
 internal_ReflectKeyedHistogram(const KeyedHistogramSnapshotData& aSnapshot,
                                const HistogramInfo& info,
                                JSContext* aCx, JS::Handle<JSObject*> aObj)
@@ -981,17 +1179,17 @@ KeyedHistogram::KeyedHistogram(Histogram
   , mIsExpired(expired)
 {
 }
 
 KeyedHistogram::~KeyedHistogram()
 {
   for (auto iter = mHistogramMap.Iter(); !iter.Done(); iter.Next()) {
     base::Histogram* h = iter.Get()->mData;
-    if (h == gExpiredHistogram) {
+    if (h == gExpiredBaseHistogram) {
       continue;
     }
     delete h;
   }
   mHistogramMap.Clear();
 }
 
 nsresult
@@ -999,17 +1197,17 @@ KeyedHistogram::GetHistogram(const nsCSt
 {
   KeyedHistogramEntry* entry = mHistogramMap.GetEntry(key);
   if (entry) {
     *histogram = entry->mData;
     return NS_OK;
   }
 
   int bucketsOffset = gHistogramBucketLowerBoundIndex[mId];
-  base::Histogram* h = internal_CreateHistogramInstance(mHistogramInfo, bucketsOffset);
+  base::Histogram* h = internal_CreateBaseHistogramInstance(mHistogramInfo, bucketsOffset);
   if (!h) {
     return NS_ERROR_FAILURE;
   }
 
   h->ClearFlags(base::Histogram::kUmaTargetedHistogramFlag);
   *histogram = h;
 
   entry = mHistogramMap.PutEntry(key);
@@ -1080,17 +1278,17 @@ KeyedHistogram::Clear()
 {
   MOZ_ASSERT(XRE_IsParentProcess());
   if (!XRE_IsParentProcess()) {
     return;
   }
 
   for (auto iter = mHistogramMap.Iter(); !iter.Done(); iter.Next()) {
     base::Histogram* h = iter.Get()->mData;
-    if (h == gExpiredHistogram) {
+    if (h == gExpiredBaseHistogram) {
       continue;
     }
     delete h;
   }
   mHistogramMap.Clear();
 }
 
 size_t
@@ -1284,19 +1482,19 @@ internal_RemoteAccumulate(const StaticMu
 
 void internal_Accumulate(const StaticMutexAutoLock& aLock, HistogramID aId, uint32_t aSample)
 {
   if (!internal_CanRecordBase() ||
       internal_RemoteAccumulate(aLock, aId, aSample)) {
     return;
   }
 
-  base::Histogram *h = internal_GetHistogramById(aLock, aId, ProcessID::Parent);
-  MOZ_ASSERT(h);
-  internal_HistogramAdd(aLock, *h, aId, aSample, ProcessID::Parent);
+  Histogram *w = internal_GetHistogramById(aLock, aId, ProcessID::Parent);
+  MOZ_ASSERT(w);
+  internal_HistogramAdd(aLock, *w, aId, aSample, ProcessID::Parent);
 }
 
 void
 internal_Accumulate(const StaticMutexAutoLock& aLock, HistogramID aId,
                     const nsCString& aKey, uint32_t aSample)
 {
   if (!gInitDone || !internal_CanRecordBase() ||
       internal_RemoteAccumulate(aLock, aId, aKey, aSample)) {
@@ -1313,20 +1511,21 @@ internal_AccumulateChild(const StaticMut
                          ProcessID aProcessType,
                          HistogramID aId,
                          uint32_t aSample)
 {
   if (!internal_CanRecordBase()) {
     return;
   }
 
-  if (base::Histogram* h = internal_GetHistogramById(aLock, aId, aProcessType)) {
-    internal_HistogramAdd(aLock, *h, aId, aSample, aProcessType);
+  Histogram *w = internal_GetHistogramById(aLock, aId, aProcessType);
+  if (w == nullptr) {
+    NS_WARNING("Failed GetHistogramById for CHILD");
   } else {
-    NS_WARNING("Failed GetHistogramById for CHILD");
+    internal_HistogramAdd(aLock, *w, aId, aSample, aProcessType);
   }
 }
 
 void
 internal_AccumulateChildKeyed(const StaticMutexAutoLock& aLock, ProcessID aProcessType,
                               HistogramID aId, const nsCString& aKey, uint32_t aSample)
 {
   if (!gInitDone || !internal_CanRecordBase()) {
@@ -1349,21 +1548,24 @@ internal_ClearHistogram(const StaticMute
   // Handle keyed histograms.
   if (gHistogramInfos[id].keyed) {
     for (uint32_t process = 0; process < static_cast<uint32_t>(ProcessID::Count); ++process) {
       KeyedHistogram* kh = internal_GetKeyedHistogramById(id, static_cast<ProcessID>(process), /* instantiate = */ false);
       if (kh) {
         kh->Clear();
       }
     }
-  }
-
-  // Now reset the histograms instances for all processes.
-  for (uint32_t process = 0; process < static_cast<uint32_t>(ProcessID::Count); ++process) {
-    internal_ClearHistogramById(aLock, id, static_cast<ProcessID>(process));
+  } else {
+    // Reset the histograms instances for all processes.
+    for (uint32_t process = 0; process < static_cast<uint32_t>(ProcessID::Count); ++process) {
+      Histogram* h = internal_GetHistogramById(aLock, id, static_cast<ProcessID>(process), /* instantiate = */ false);
+      if (h) {
+        h->Clear();
+      }
+    }
   }
 }
 
 } // namespace
 
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
@@ -1583,17 +1785,25 @@ internal_JSHistogram_Snapshot(JSContext 
   HistogramSnapshotData dataSnapshot;
   {
     StaticMutexAutoLock locker(gTelemetryHistogramMutex);
     MOZ_ASSERT(internal_IsHistogramEnumId(id));
 
     // This is not good standard behavior given that we have histogram instances
     // covering multiple processes.
     // However, changing this requires some broader changes to callers.
-    base::Histogram* h = internal_GetHistogramById(locker, id, ProcessID::Parent);
+    Histogram* w = internal_GetHistogramById(locker, id, ProcessID::Parent);
+    base::Histogram *h = nullptr;
+    NS_NAMED_LITERAL_CSTRING(store, "main");
+    if (!w->GetHistogram(store, &h)) {
+      // When it's not in the 'main' store, let's skip the snapshot completely,
+      // but don't fail
+      args.rval().setUndefined();
+      return true;
+    }
     // Take a snapshot of the data here, protected by the lock, and then,
     // outside of the lock protection, mirror it to a JS structure
     if (NS_FAILED(internal_GetHistogramAndSamples(locker, h, dataSnapshot))) {
       return false;
     }
   }
 
   JS::Rooted<JSObject*> snapshot(cx, JS_NewPlainObject(cx));
@@ -2046,17 +2256,17 @@ void TelemetryHistogram::InitializeGloba
   MOZ_ASSERT(!gInitDone, "TelemetryHistogram::InitializeGlobalState "
              "may only be called once");
 
   gCanRecordBase = canRecordBase;
   gCanRecordExtended = canRecordExtended;
 
   if (XRE_IsParentProcess()) {
     gHistogramStorage =
-      new base::Histogram*[HistogramCount * size_t(ProcessID::Count)] {};
+      new Histogram*[HistogramCount * size_t(ProcessID::Count)] {};
     gKeyedHistogramStorage =
       new KeyedHistogram*[HistogramCount * size_t(ProcessID::Count)] {};
   }
 
     // 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.
@@ -2096,16 +2306,18 @@ void TelemetryHistogram::DeInitializeGlo
       }
       if (gHistogramStorage[i] != gExpiredHistogram) {
         delete gHistogramStorage[i];
       }
     }
     delete[] gHistogramStorage;
     delete[] gKeyedHistogramStorage;
   }
+  delete gExpiredBaseHistogram;
+  gExpiredBaseHistogram = nullptr;
   delete gExpiredHistogram;
   gExpiredHistogram = nullptr;
   delete gExpiredKeyedHistogram;
   gExpiredKeyedHistogram = nullptr;
 }
 
 #ifdef DEBUG
 bool TelemetryHistogram::GlobalStateHasBeenInitialized() {
@@ -2638,17 +2850,17 @@ TelemetryHistogram::GetHistogramSizesOfI
       if (gKeyedHistogramStorage[i] && gKeyedHistogramStorage[i] != gExpiredKeyedHistogram) {
         n += gKeyedHistogramStorage[i]->SizeOfIncludingThis(aMallocSizeOf);
       }
     }
   }
 
   // If we allocated the array, let's count the number of pointers in there.
   if (gHistogramStorage) {
-    n += HistogramCount * size_t(ProcessID::Count) * sizeof(base::Histogram*);
+    n += HistogramCount * size_t(ProcessID::Count) * sizeof(Histogram*);
     for (size_t i = 0; i < HistogramCount * size_t(ProcessID::Count); ++i) {
       if (gHistogramStorage[i] && gHistogramStorage[i] != gExpiredHistogram) {
         n += gHistogramStorage[i]->SizeOfIncludingThis(aMallocSizeOf);
       }
     }
   }
 
   // We only allocate the expired (keyed) histogram once.
@@ -3057,20 +3269,31 @@ TelemetryHistogram::DeserializeHistogram
 
         ProcessID procID = static_cast<ProcessID>(process);
         if (!internal_CanRecordHistogram(id, procID)) {
           // We're not allowed to record this, so don't try to restore it.
           continue;
         }
 
         // Get the Histogram instance: this will instantiate it if it doesn't exist.
-        base::Histogram* h = internal_GetHistogramById(locker, id, procID);
+        Histogram* w = internal_GetHistogramById(locker, id, procID);
+        MOZ_ASSERT(w);
+
+        if (!w || w->IsExpired()) {
+          continue;
+        }
+
+        base::Histogram *h = nullptr;
+        NS_NAMED_LITERAL_CSTRING(store, "main");
+        if (!w->GetHistogram(store, &h)) {
+          continue;
+        }
         MOZ_ASSERT(h);
 
-        if (!h || internal_IsExpired(locker, h)) {
+        if (!h) {
           // Don't restore expired histograms.
           continue;
         }
 
         // Make sure that histogram counts have matching sizes. If not,
         // |AddSampleSet| will fail and crash.
         size_t numCounts = mozilla::Get<1>(histogramData).Length();
         if (h->bucket_count() != numCounts) {