Bug 1310011 - Allow annotations on a hang to accumulate. r?aklotz draft
authorMike Conley <mconley@mozilla.com>
Thu, 13 Oct 2016 18:10:20 -0400
changeset 425341 950d9807d4a6ce8d7a8350e06f8c5ec8140a6ff8
parent 425340 b860edc388bddd00c5bc797ac1312c482561bde3
child 533902 4ff21ed32a27eab84fea92ccfa21e4fabd94f4c7
push id32406
push usermconley@mozilla.com
push dateFri, 14 Oct 2016 15:50:11 +0000
reviewersaklotz
bugs1310011
milestone52.0a1
Bug 1310011 - Allow annotations on a hang to accumulate. r?aklotz MozReview-Commit-ID: EE4bMOjCPrV
xpcom/threads/BackgroundHangMonitor.cpp
xpcom/threads/HangAnnotations.cpp
xpcom/threads/HangAnnotations.h
xpcom/threads/HangMonitor.cpp
--- a/xpcom/threads/BackgroundHangMonitor.cpp
+++ b/xpcom/threads/BackgroundHangMonitor.cpp
@@ -292,34 +292,39 @@ BackgroundHangManager::RunMonitorThread(
       }
       PRIntervalTime interval = currentThread->mInterval;
       PRIntervalTime hangTime = intervalNow - interval;
       if (MOZ_UNLIKELY(hangTime >= currentThread->mMaxTimeout)) {
         // A permahang started
         // Skip subsequent iterations and tolerate a race on mWaiting here
         currentThread->mWaiting = true;
         currentThread->mHanging = false;
+        currentThread->mAnnotations = currentThread->mAnnotators.GatherAnnotations();
         currentThread->ReportPermaHang();
         continue;
       }
 
       if (MOZ_LIKELY(!currentThread->mHanging)) {
         if (MOZ_UNLIKELY(hangTime >= currentThread->mTimeout)) {
           // A hang started
           currentThread->mStackHelper.GetStack(currentThread->mHangStack);
           currentThread->mHangStart = interval;
           currentThread->mHanging = true;
-          currentThread->mAnnotations =
-            currentThread->mAnnotators.GatherAnnotations();
+          currentThread->mAnnotators.Accumulate();
         }
       } else {
         if (MOZ_LIKELY(interval != currentThread->mHangStart)) {
           // A hang ended
+          currentThread->mAnnotations = currentThread->mAnnotators.GatherAnnotations();
           currentThread->ReportHang(intervalNow - currentThread->mHangStart);
           currentThread->mHanging = false;
+        } else {
+          // We're still hanging. Poll any annotators in case conditions have changed
+          // in the meantime.
+          currentThread->mAnnotators.Accumulate();
         }
       }
 
       /* If we are hanging, the next time we check for hang status is when
          the hang turns into a permahang. If we're not hanging, the next
          recheck timeout is when we may be entering a hang. */
       PRIntervalTime nextRecheck;
       if (currentThread->mHanging) {
--- a/xpcom/threads/HangAnnotations.cpp
+++ b/xpcom/threads/HangAnnotations.cpp
@@ -190,32 +190,43 @@ Annotators::Unregister(Annotator& aAnnot
 {
   MutexAutoLock lock(mMutex);
   DebugOnly<std::set<Annotator*>::size_type> numErased;
   numErased = mAnnotators.erase(&aAnnotator);
   MOZ_ASSERT(numErased == 1);
   return mAnnotators.empty();
 }
 
-UniquePtr<HangAnnotations>
-Annotators::GatherAnnotations()
+void
+Annotators::Accumulate()
 {
-  auto annotations = MakeUnique<BrowserHangAnnotations>();
+  if (!mAnnotations) {
+    mAnnotations = MakeUnique<BrowserHangAnnotations>();
+  }
+
   { // Scope for lock
     MutexAutoLock lock(mMutex);
     for (std::set<Annotator*>::iterator i = mAnnotators.begin(),
                                         e = mAnnotators.end();
          i != e; ++i) {
-      (*i)->AnnotateHang(*annotations);
+      (*i)->AnnotateHang(*mAnnotations);
     }
   }
-  if (annotations->IsEmpty()) {
+}
+
+UniquePtr<HangAnnotations>
+Annotators::GatherAnnotations()
+{
+  Accumulate();
+
+  // Accumulate will have created this mAnnotations for us.
+  if (mAnnotations->IsEmpty()) {
     return nullptr;
   }
-  return Move(annotations);
+  return Move(mAnnotations);
 }
 
 } // namespace Observer
 
 void
 RegisterAnnotator(Annotator& aAnnotator)
 {
   BackgroundHangMonitor::RegisterAnnotator(aAnnotator);
@@ -237,18 +248,27 @@ UnregisterAnnotator(Annotator& aAnnotato
   if (NS_IsMainThread() &&
       GeckoProcessType_Default == XRE_GetProcessType()) {
     if (gChromehangAnnotators->Unregister(aAnnotator)) {
       gChromehangAnnotators = nullptr;
     }
   }
 }
 
+void
+ChromeHangAnnotatorAccumulateCallout()
+{
+  if (!gChromehangAnnotators) {
+    return;
+  }
+  gChromehangAnnotators->Accumulate();
+}
+
 UniquePtr<HangAnnotations>
-ChromeHangAnnotatorCallout()
+ChromeHangAnnotatorGatherCallout()
 {
   if (!gChromehangAnnotators) {
     return nullptr;
   }
   return gChromehangAnnotators->GatherAnnotations();
 }
 
 } // namespace HangMonitor
--- a/xpcom/threads/HangAnnotations.h
+++ b/xpcom/threads/HangAnnotations.h
@@ -68,37 +68,60 @@ void RegisterAnnotator(Annotator& aAnnot
 /**
  * Registers an Annotator that was previously registered via RegisterAnnotator.
  * @param aAnnotator Reference to an object that implements the
  * HangMonitor::Annotator interface.
  */
 void UnregisterAnnotator(Annotator& aAnnotator);
 
 /**
- * Gathers annotations. This function should be called by ChromeHangs.
+ * Polls annotators for annotations. This function should be called by ChromeHangs.
+ */
+void ChromeHangAnnotatorAccumulateCallout();
+
+/**
+ * Gather captured annotations. This function should be called by ChromeHangs.
  * @return UniquePtr to HangAnnotations object or nullptr if none.
  */
-HangAnnotationsPtr ChromeHangAnnotatorCallout();
+HangAnnotationsPtr ChromeHangAnnotatorGatherCallout();
 
 namespace Observer {
 
 class Annotators
 {
 public:
   Annotators();
   ~Annotators();
 
   bool Register(Annotator& aAnnotator);
   bool Unregister(Annotator& aAnnotator);
 
+  /**
+   * Does one final accumulation, and then returns a pointer to the
+   * hang annotations that were accumulated. Will return nullptr if
+   * no annotations were accumulated.
+   * @return UniquePtr to HangAnnotations object or nullptr
+   */
   HangAnnotationsPtr GatherAnnotations();
 
+  /**
+   * Polls registered annotators for annotations. This will be called
+   * periodically during the lifetime of a hang. Once a hang has finished,
+   * GatherAnnotations should be called to collect the accumulated
+   * annotations and clear the accumulation.
+   *
+   * Note that if an annotation with the same key is accumulated more than
+   * once, the last usage is what will ultimately be gathered.
+   */
+  void Accumulate();
+
 private:
   Mutex                mMutex;
   std::set<Annotator*> mAnnotators;
+  HangAnnotationsPtr   mAnnotations;
 };
 
 } // namespace Observer
 
 } // namespace HangMonitor
 } // namespace mozilla
 
 #endif // mozilla_HangAnnotations_h
--- a/xpcom/threads/HangMonitor.cpp
+++ b/xpcom/threads/HangMonitor.cpp
@@ -246,33 +246,34 @@ ThreadMain(void*)
         timestamp == lastTimestamp &&
         gTimeout > 0) {
       ++waitCount;
 #ifdef REPORT_CHROME_HANGS
       // Capture the chrome-hang stack + Firefox & system uptimes after
       // the minimum hang duration has been reached (not when the hang ends)
       if (waitCount == 2) {
         GetChromeHangReport(stack, systemUptime, firefoxUptime);
-        annotations = ChromeHangAnnotatorCallout();
+        ChromeHangAnnotatorAccumulateCallout();
       }
 #else
       // This is the crash-on-hang feature.
       // See bug 867313 for the quirk in the waitCount comparison
       if (waitCount >= 2) {
         int32_t delay =
           int32_t(PR_IntervalToSeconds(now - timestamp));
         if (delay >= gTimeout) {
           MonitorAutoUnlock unlock(*gMonitor);
           Crash();
         }
       }
 #endif
     } else {
 #ifdef REPORT_CHROME_HANGS
       if (waitCount >= 2) {
+        annotations = ChromeHangAnnotatorGatherCallout();
         uint32_t hangDuration = PR_IntervalToSeconds(now - lastTimestamp);
         Telemetry::RecordChromeHang(hangDuration, stack, systemUptime,
                                     firefoxUptime, Move(annotations));
         stack.Clear();
       }
 #endif
       lastTimestamp = timestamp;
       waitCount = 0;