Backed out changeset 712073c4f0b4 (bug 1213780) for causing rooting hazard failures on a CLOSED TREE
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Fri, 23 Oct 2015 14:38:29 +0200
changeset 304332 9a21365f7a4ad6b8eb3d97bf360468228afcb89d
parent 304331 e9cae6188ca4434009b0308dc33d31e61035ac76
child 304333 9200ea11d07f7273a7e404bb8ba07a0ab2d7c36b
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1213780
milestone44.0a1
backs out712073c4f0b41cfa50f3d99332b44882cbeb0644
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
Backed out changeset 712073c4f0b4 (bug 1213780) for causing rooting hazard failures on a CLOSED TREE
toolkit/components/telemetry/Telemetry.cpp
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -5,17 +5,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include <algorithm>
 
 #include <fstream>
 
 #include <prio.h>
 
-#include "mozilla/dom/ToJSValue.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MathAlgorithms.h"
 
 #include "base/histogram.h"
 #include "base/pickle.h"
 #include "nsIComponentManager.h"
@@ -216,145 +215,100 @@ CombinedStacks::SizeOfExcludingThis() co
   for (std::vector<Stack>::const_iterator i = mStacks.begin(),
          e = mStacks.end(); i != e; ++i) {
     const Stack& s = *i;
     n += s.capacity() * sizeof(Telemetry::ProcessedStack::Frame);
   }
   return n;
 }
 
-// This utility function generates a string key that is used to index the annotations
-// in a hash map from |HangReports::AddHang|.
-nsresult
-ComputeAnnotationsKey(const HangAnnotationsPtr& aAnnotations, nsAString& aKeyOut)
-{
-  UniquePtr<HangAnnotations::Enumerator> annotationsEnum = aAnnotations->GetEnumerator();
-  if (!annotationsEnum) {
-    return NS_ERROR_FAILURE;
-  }
-
-  // Append all the attributes to the key, to uniquely identify this annotation.
-  nsAutoString  key;
-  nsAutoString  value;
-  while (annotationsEnum->Next(key, value)) {
-    aKeyOut.Append(key);
-    aKeyOut.Append(value);
-  }
-
-  return NS_OK;
-}
-
 class HangReports {
 public:
   /**
    * This struct encapsulates information for an individual ChromeHang annotation.
    * mHangIndex is the index of the corresponding ChromeHang.
    */
   struct AnnotationInfo {
     AnnotationInfo(uint32_t aHangIndex,
                    HangAnnotationsPtr aAnnotations)
-      : mAnnotations(Move(aAnnotations))
-    {
-      mHangIndices.AppendElement(aHangIndex);
-    }
+      : mHangIndex(aHangIndex)
+      , mAnnotations(Move(aAnnotations))
+    {}
     AnnotationInfo(AnnotationInfo&& aOther)
-      : mHangIndices(aOther.mHangIndices)
+      : mHangIndex(aOther.mHangIndex)
       , mAnnotations(Move(aOther.mAnnotations))
     {}
     ~AnnotationInfo() {}
     AnnotationInfo& operator=(AnnotationInfo&& aOther)
     {
-      mHangIndices = aOther.mHangIndices;
+      mHangIndex = aOther.mHangIndex;
       mAnnotations = Move(aOther.mAnnotations);
       return *this;
     }
-    // To save memory, a single AnnotationInfo can be associated to multiple chrome
-    // hangs. The following array holds the index of each related chrome hang.
-    nsTArray<uint32_t> mHangIndices;
+    uint32_t mHangIndex;
     HangAnnotationsPtr mAnnotations;
 
   private:
     // Force move constructor
     AnnotationInfo(const AnnotationInfo& aOther) = delete;
     void operator=(const AnnotationInfo& aOther) = delete;
   };
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
   void AddHang(const Telemetry::ProcessedStack& aStack, uint32_t aDuration,
                int32_t aSystemUptime, int32_t aFirefoxUptime,
                HangAnnotationsPtr aAnnotations);
   uint32_t GetDuration(unsigned aIndex) const;
   int32_t GetSystemUptime(unsigned aIndex) const;
   int32_t GetFirefoxUptime(unsigned aIndex) const;
-  const nsClassHashtable<nsStringHashKey, AnnotationInfo>& GetAnnotationInfo() const;
+  const nsTArray<AnnotationInfo>& GetAnnotationInfo() const;
   const CombinedStacks& GetStacks() const;
 private:
   /**
    * This struct encapsulates the data for an individual ChromeHang, excluding
    * annotations.
    */
   struct HangInfo {
     // Hang duration (in seconds)
     uint32_t mDuration;
     // System uptime (in minutes) at the time of the hang
     int32_t mSystemUptime;
     // Firefox uptime (in minutes) at the time of the hang
     int32_t mFirefoxUptime;
   };
   std::vector<HangInfo> mHangInfo;
-  nsClassHashtable<nsStringHashKey, AnnotationInfo> mAnnotationInfo;
+  nsTArray<AnnotationInfo> mAnnotationInfo;
   CombinedStacks mStacks;
 };
 
 void
 HangReports::AddHang(const Telemetry::ProcessedStack& aStack,
                      uint32_t aDuration,
                      int32_t aSystemUptime,
                      int32_t aFirefoxUptime,
                      HangAnnotationsPtr aAnnotations) {
   HangInfo info = { aDuration, aSystemUptime, aFirefoxUptime };
   mHangInfo.push_back(info);
+  if (aAnnotations) {
+    AnnotationInfo ainfo(static_cast<uint32_t>(mHangInfo.size() - 1),
+                         Move(aAnnotations));
+    mAnnotationInfo.AppendElement(Move(ainfo));
+  }
   mStacks.AddStack(aStack);
-
-  if (!aAnnotations) {
-    return;
-  }
-
-  nsAutoString annotationsKey;
-  uint32_t hangIndex = static_cast<uint32_t>(mHangInfo.size() - 1);
-
-  // Generate a key to index aAnnotations in the hash map.
-  nsresult rv = ComputeAnnotationsKey(aAnnotations, annotationsKey);
-  if (NS_FAILED(rv)) {
-    return;
-  }
-
-  AnnotationInfo* annotationsEntry = mAnnotationInfo.Get(annotationsKey);
-  if (annotationsEntry) {
-    // If the key is already in the hash map, append the index of the chrome hang
-    // to its indices.
-    annotationsEntry->mHangIndices.AppendElement(hangIndex);
-    return;
-  }
-
-  // If the key was not found, add the annotations to the hash map.
-  mAnnotationInfo.Put(annotationsKey, new AnnotationInfo(hangIndex, Move(aAnnotations)));
 }
 
 size_t
 HangReports::SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const {
   size_t n = 0;
   n += mStacks.SizeOfExcludingThis();
   // This is a crude approximation. See comment on
   // CombinedStacks::SizeOfExcludingThis.
   n += mHangInfo.capacity() * sizeof(HangInfo);
-  n += mAnnotationInfo.ShallowSizeOfExcludingThis(aMallocSizeOf);
-  n += mAnnotationInfo.Count() * sizeof(AnnotationInfo);
-  for (auto iter = mAnnotationInfo.ConstIter(); !iter.Done(); iter.Next()) {
-    n += iter.Key().SizeOfExcludingThisIfUnshared(aMallocSizeOf);
-    n += iter.Data()->mAnnotations->SizeOfIncludingThis(aMallocSizeOf);
+  n += mAnnotationInfo.Capacity() * sizeof(AnnotationInfo);
+  for (int32_t i = 0, l = mAnnotationInfo.Length(); i < l; ++i) {
+    n += mAnnotationInfo[i].mAnnotations->SizeOfIncludingThis(aMallocSizeOf);
   }
   return n;
 }
 
 const CombinedStacks&
 HangReports::GetStacks() const {
   return mStacks;
 }
@@ -369,17 +323,17 @@ HangReports::GetSystemUptime(unsigned aI
   return mHangInfo[aIndex].mSystemUptime;
 }
 
 int32_t
 HangReports::GetFirefoxUptime(unsigned aIndex) const {
   return mHangInfo[aIndex].mFirefoxUptime;
 }
 
-const nsClassHashtable<nsStringHashKey, HangReports::AnnotationInfo>&
+const nsTArray<HangReports::AnnotationInfo>&
 HangReports::GetAnnotationInfo() const {
   return mAnnotationInfo;
 }
 
 /**
  * IOInterposeObserver recording statistics of main-thread I/O during execution,
  * aimed at consumption by TelemetryImpl
  */
@@ -2577,70 +2531,52 @@ TelemetryImpl::GetChromeHangs(JSContext 
     if (!JS_DefineElement(cx, systemUptimeArray, i, mHangReports.GetSystemUptime(i),
                           JSPROP_ENUMERATE)) {
       return NS_ERROR_FAILURE;
     }
     if (!JS_DefineElement(cx, firefoxUptimeArray, i, mHangReports.GetFirefoxUptime(i),
                           JSPROP_ENUMERATE)) {
       return NS_ERROR_FAILURE;
     }
-
-    size_t annotationIndex = 0;
-    const nsClassHashtable<nsStringHashKey, HangReports::AnnotationInfo>& annotationInfo =
-      mHangReports.GetAnnotationInfo();
-
-    for (auto iter = annotationInfo.ConstIter(); !iter.Done(); iter.Next()) {
-      const HangReports::AnnotationInfo* info = iter.Data();
-
+    const nsTArray<HangReports::AnnotationInfo>& annotationInfo =
+                                                mHangReports.GetAnnotationInfo();
+    for (uint32_t iterIndex = 0, arrayLen = annotationInfo.Length();
+         iterIndex < arrayLen; ++iterIndex) {
       JS::Rooted<JSObject*> keyValueArray(cx, JS_NewArrayObject(cx, 0));
       if (!keyValueArray) {
         return NS_ERROR_FAILURE;
       }
-
-      // Create an array containing all the indices of the chrome hangs relative to this
-      // annotation.
-      JS::Rooted<JS::Value> indicesArray(cx);
-      if (!mozilla::dom::ToJSValue(cx, info->mHangIndices, &indicesArray)) {
-        return NS_ERROR_OUT_OF_MEMORY;
-      }
-
-      // We're saving the annotation as [[indices], {annotation-data}], so add the indices
-      // array as the first element of that structure.
-      if (!JS_DefineElement(cx, keyValueArray, 0, indicesArray, JSPROP_ENUMERATE)) {
+      JS::RootedValue indexValue(cx);
+      indexValue.setNumber(annotationInfo[iterIndex].mHangIndex);
+      if (!JS_DefineElement(cx, keyValueArray, 0, indexValue, JSPROP_ENUMERATE)) {
         return NS_ERROR_FAILURE;
       }
-
-      // Create the annotations object...
       JS::Rooted<JSObject*> jsAnnotation(cx, JS_NewPlainObject(cx));
       if (!jsAnnotation) {
         return NS_ERROR_FAILURE;
       }
       UniquePtr<HangAnnotations::Enumerator> annotationsEnum =
-        info->mAnnotations->GetEnumerator();
+        annotationInfo[iterIndex].mAnnotations->GetEnumerator();
       if (!annotationsEnum) {
         return NS_ERROR_FAILURE;
       }
-
-      // ... fill it with key:value pairs...
-      nsAutoString key;
-      nsAutoString value;
+      nsAutoString  key;
+      nsAutoString  value;
       while (annotationsEnum->Next(key, value)) {
         JS::RootedValue jsValue(cx);
         jsValue.setString(JS_NewUCStringCopyN(cx, value.get(), value.Length()));
         if (!JS_DefineUCProperty(cx, jsAnnotation, key.get(), key.Length(),
                                  jsValue, JSPROP_ENUMERATE)) {
           return NS_ERROR_FAILURE;
         }
       }
-
-      // ... and append it after the indices array.
       if (!JS_DefineElement(cx, keyValueArray, 1, jsAnnotation, JSPROP_ENUMERATE)) {
         return NS_ERROR_FAILURE;
       }
-      if (!JS_DefineElement(cx, annotationsArray, annotationIndex++,
+      if (!JS_DefineElement(cx, annotationsArray, iterIndex,
                          keyValueArray, JSPROP_ENUMERATE)) {
         return NS_ERROR_FAILURE;
       }
     }
   }
 
   return NS_OK;
 }