bug 1218576 - Ensure remaining batched telemetry is flushed on content process shutdown r=gfritzsche
authorChris H-C <chutten@mozilla.com>
Tue, 12 Jul 2016 11:49:38 -0400
changeset 314445 83d24a3dc1a99dd5669cc718e459996e26ad44e2
parent 314444 92ed900459dfcf59a3a28263d0855ee279d74f49
child 314446 02bee0c4865ad16db50a47b25c6410dd1b1b3dce
push id20574
push usercbook@mozilla.com
push dateTue, 20 Sep 2016 10:05:16 +0000
treeherderfx-team@14705f779a46 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1218576
milestone51.0a1
bug 1218576 - Ensure remaining batched telemetry is flushed on content process shutdown r=gfritzsche On content process shutdown we send a content process ping to ensure we have up-to-date data from the content process before it goes away. Now we need to also flush the batched telemetry accumulations to the parent so that it can be present in the ping. No attempt is made to synchronize access to IPCTimerFired. It is safe to re-enter. No attempt is made to cancel the timer as its firing is benign. MozReview-Commit-ID: 1gjNH9IPhKf
toolkit/components/telemetry/Telemetry.cpp
toolkit/components/telemetry/TelemetryHistogram.cpp
toolkit/components/telemetry/TelemetrySession.jsm
toolkit/components/telemetry/nsITelemetry.idl
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -2343,16 +2343,23 @@ TelemetryImpl::SnapshotScalars(unsigned 
 
 NS_IMETHODIMP
 TelemetryImpl::ClearScalars()
 {
   TelemetryScalar::ClearScalars();
   return NS_OK;
 }
 
+NS_IMETHODIMP
+TelemetryImpl::FlushBatchedChildTelemetry()
+{
+  TelemetryHistogram::IPCTimerFired(nullptr, nullptr);
+  return NS_OK;
+}
+
 size_t
 TelemetryImpl::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf)
 {
   size_t n = aMallocSizeOf(this);
 
   // Ignore the hashtables in mAddonMap; they are not significant.
   n += TelemetryHistogram::GetMapShallowSizesOfExcludingThis(aMallocSizeOf);
   n += TelemetryScalar::GetMapShallowSizesOfExcludingThis(aMallocSizeOf);
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -2549,18 +2549,21 @@ TelemetryHistogram::GetHistogramSizesofI
   }
   return n;
 }
 
 // This method takes the lock only to double-buffer the batched telemetry.
 // It releases the lock before calling out to IPC code which can (and does)
 // Accumulate (which would deadlock)
 //
-// To ensure non-reentrancy, the timer is not released until the method
-// completes
+// To ensure we don't loop IPCTimerFired->AccumulateChild->arm timer, we don't
+// unset gIPCTimerArmed until the IPC completes
+//
+// This function may be re-entered. The shared datastructures gAccumulations and
+// gKeyedAccumulations are guarded by the lock.
 void
 TelemetryHistogram::IPCTimerFired(nsITimer* aTimer, void* aClosure)
 {
   nsTArray<Accumulation> accumulationsToSend;
   nsTArray<KeyedAccumulation> keyedAccumulationsToSend;
   {
     StaticMutexAutoLock locker(gTelemetryHistogramMutex);
     if (gAccumulations) {
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -1833,17 +1833,17 @@ var Impl = {
       this._log.trace("observe - " + aTopic + " notified.");
     }
 
     switch (aTopic) {
     case "content-child-shutdown":
       // content-child-shutdown is only registered for content processes.
       Services.obs.removeObserver(this, "content-child-shutdown");
       this.uninstall();
-
+      Telemetry.flushBatchedChildTelemetry();
       this.sendContentProcessPing(REASON_SAVED_SESSION);
       break;
     case TOPIC_CYCLE_COLLECTOR_BEGIN:
       let now = new Date();
       if (!gLastMemoryPoll
           || (TELEMETRY_INTERVAL <= now - gLastMemoryPoll)) {
         gLastMemoryPoll = now;
         this.gatherMemory();
--- a/toolkit/components/telemetry/nsITelemetry.idl
+++ b/toolkit/components/telemetry/nsITelemetry.idl
@@ -391,9 +391,15 @@ interface nsITelemetry : nsISupports
    */
   [implicit_jscontext, optional_argc]
   jsval snapshotScalars(in uint32_t aDataset, [optional] in boolean aClear);
 
   /**
    * Resets all the stored scalars. This is intended to be only used in tests.
    */
   void clearScalars();
+
+  /**
+   * Immediately sends any Telemetry batched on this process to the parent
+   * process. This is intended only to be used on process shutdown.
+   */
+  void flushBatchedChildTelemetry();
 };