Bug 1215540 - Fix Telemetry reporting repeated hang annotations for Thread hangs. r=aklotz
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Tue, 27 Oct 2015 04:35:00 +0100
changeset 304914 56707eeec6e53453878731864eaac19ef1a0c5ab
parent 304913 0393e5088a20f01fbc0bef806005d6c3126b64ac
child 304915 f6d32a2fa56177e545400e655f3f57ebaa6b5a39
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)
reviewersaklotz
bugs1215540
milestone44.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 1215540 - Fix Telemetry reporting repeated hang annotations for Thread hangs. r=aklotz
toolkit/components/telemetry/Telemetry.cpp
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -2898,66 +2898,87 @@ CreateJSHangStack(JSContext* cx, const T
     JS::RootedString string(cx, JS_NewStringCopyZ(cx, stack[i]));
     if (!JS_DefineElement(cx, ret, i, string, JSPROP_ENUMERATE)) {
       return nullptr;
     }
   }
   return ret;
 }
 
-static JSObject*
-CreateJSHangAnnotations(JSContext* cx, const HangAnnotationsVector& annotations)
+static void
+CreateJSHangAnnotations(JSContext* cx, const HangAnnotationsVector& annotations,
+                        JS::MutableHandleObject returnedObject)
 {
   JS::RootedObject annotationsArray(cx, JS_NewArrayObject(cx, 0));
   if (!annotationsArray) {
-    return nullptr;
-  }
+    returnedObject.set(nullptr);
+    return;
+  }
+  // We keep track of the annotations we reported in this hash set, so we can
+  // discard duplicated ones.
+  nsTHashtable<nsStringHashKey> reportedAnnotations;
   size_t annotationIndex = 0;
   for (const HangAnnotationsPtr *i = annotations.begin(), *e = annotations.end();
        i != e; ++i) {
     JS::RootedObject jsAnnotation(cx, JS_NewPlainObject(cx));
     if (!jsAnnotation) {
       continue;
     }
     const HangAnnotationsPtr& curAnnotations = *i;
+    // Build a key to index the current annotations in our hash set.
+    nsAutoString annotationsKey;
+    nsresult rv = ComputeAnnotationsKey(curAnnotations, annotationsKey);
+    if (NS_FAILED(rv)) {
+      continue;
+    }
+    // Check if the annotations are in the set. If that's the case, don't double report.
+    if (reportedAnnotations.GetEntry(annotationsKey)) {
+      continue;
+    }
+    // If not, report them.
+    reportedAnnotations.PutEntry(annotationsKey);
     UniquePtr<HangAnnotations::Enumerator> annotationsEnum =
       curAnnotations->GetEnumerator();
     if (!annotationsEnum) {
       continue;
     }
     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 nullptr;
+        returnedObject.set(nullptr);
+        return;
       }
     }
     if (!JS_SetElement(cx, annotationsArray, annotationIndex, jsAnnotation)) {
       continue;
     }
     ++annotationIndex;
   }
-  return annotationsArray;
+  // Return the array using a |MutableHandleObject| to avoid triggering a false
+  // positive rooting issue in the hazard analysis build.
+  returnedObject.set(annotationsArray);
 }
 
 static JSObject*
 CreateJSHangHistogram(JSContext* cx, const Telemetry::HangHistogram& hang)
 {
   JS::RootedObject ret(cx, JS_NewPlainObject(cx));
   if (!ret) {
     return nullptr;
   }
 
   JS::RootedObject stack(cx, CreateJSHangStack(cx, hang.GetStack()));
   JS::RootedObject time(cx, CreateJSTimeHistogram(cx, hang));
   auto& hangAnnotations = hang.GetAnnotations();
-  JS::RootedObject annotations(cx, CreateJSHangAnnotations(cx, hangAnnotations));
+  JS::RootedObject annotations(cx);
+  CreateJSHangAnnotations(cx, hangAnnotations, &annotations);
 
   if (!stack ||
       !time ||
       !annotations ||
       !JS_DefineProperty(cx, ret, "stack", stack, JSPROP_ENUMERATE) ||
       !JS_DefineProperty(cx, ret, "histogram", time, JSPROP_ENUMERATE) ||
       (!hangAnnotations.empty() && // <-- Only define annotations when nonempty
         !JS_DefineProperty(cx, ret, "annotations", annotations, JSPROP_ENUMERATE))) {