Bug 1157870 - Performance Groups should have a unique ID (high-level). r=mossop
☠☠ backed out by d6e9eea07a71 ☠ ☠
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Mon, 18 May 2015 16:40:34 +0200
changeset 279671 819d7887fdc417049f32b1dbe8883e7f86ab7909
parent 279670 6a0397d889c9305b66f6f2af6d2a5e0a286da041
child 279672 6d8206038e6c9247f97308edacce8bd4b586d094
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmossop
bugs1157870
milestone41.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 1157870 - Performance Groups should have a unique ID (high-level). r=mossop
toolkit/components/aboutperformance/content/aboutPerformance.js
toolkit/components/perfmonitoring/PerformanceStats.jsm
toolkit/components/perfmonitoring/nsIPerformanceStats.idl
toolkit/components/perfmonitoring/nsPerformanceStats.cpp
toolkit/components/perfmonitoring/nsPerformanceStats.h
toolkit/components/perfmonitoring/tests/browser/browser_compartments.js
--- a/toolkit/components/aboutperformance/content/aboutPerformance.js
+++ b/toolkit/components/aboutperformance/content/aboutPerformance.js
@@ -115,18 +115,17 @@ let State = {
    * - `deltaT`: the number of milliseconds elapsed since the data
    *   was last displayed.
    */
   update: Task.async(function*() {
     let snapshot = yield this._monitor.promiseSnapshot();
     let newData = new Map();
     let deltas = [];
     for (let componentNew of snapshot.componentsData) {
-      let {name, addonId, isSystem} = componentNew;
-      let key = JSON.stringify({name, addonId, isSystem});
+      let key = componentNew.groupId;
       let componentOld = State._componentsData.get(key);
       deltas.push(componentNew.subtract(componentOld));
       newData.set(key, componentNew);
     }
     State._componentsData = newData;
     let now = window.performance.now();
     let process = snapshot.processData.subtract(State._processData);
     let result = {
--- a/toolkit/components/perfmonitoring/PerformanceStats.jsm
+++ b/toolkit/components/perfmonitoring/PerformanceStats.jsm
@@ -37,17 +37,17 @@ XPCOMUtils.defineLazyServiceGetter(this,
   Ci.nsIFinalizationWitnessService
 );
 
 
 // The topic used to notify that a PerformanceMonitor has been garbage-collected
 // and that we can release/close the probes it holds.
 const FINALIZATION_TOPIC = "performancemonitor-finalize";
 
-const PROPERTIES_META_IMMUTABLE = ["name", "addonId", "isSystem"];
+const PROPERTIES_META_IMMUTABLE = ["name", "addonId", "isSystem", "groupId"];
 const PROPERTIES_META = [...PROPERTIES_META_IMMUTABLE, "windowId", "title"];
 
 /**
  * Access to a low-level performance probe.
  *
  * Each probe is dedicated to some form of performance monitoring.
  * As each probe may have a performance impact, a probe is activated
  * only when a client has requested a PerformanceMonitor for this probe,
--- a/toolkit/components/perfmonitoring/nsIPerformanceStats.idl
+++ b/toolkit/components/perfmonitoring/nsIPerformanceStats.idl
@@ -17,19 +17,27 @@
 
 /**
  * Snapshot of the performance of a component, e.g. an add-on, a web
  * page, system built-ins, or the entire process itself.
  *
  * All values are monotonic and are updated only when
  * `nsIPerformanceStatsService.isStopwatchActive` is `true`.
  */
-[scriptable, uuid(b060d75d-55bc-4c82-a4ff-458fc5ab2a69)]
+[scriptable, uuid(47f8d36d-1d67-43cb-befd-d2f4720ac568)]
 interface nsIPerformanceStats: nsISupports {
   /**
+   * An identifier unique to the component.
+   *
+   * This identifier is somewhat human-readable to aid with debugging,
+   * but clients should not rely upon the format.
+   */
+  readonly attribute AString groupId;
+
+  /**
    * The name of the component:
    * - for the process itself, "<process>";
    * - for platform code, "<platform>";
    * - for an add-on, the identifier of the addon (e.g. "myaddon@foo.bar");
    * - for a webpage, the url of the page.
    */
   readonly attribute AString name;
 
--- a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp
+++ b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp
@@ -17,25 +17,33 @@
 #include "nsJSUtils.h"
 #include "xpcpublic.h"
 #include "jspubtd.h"
 #include "nsIJSRuntimeService.h"
 
 #include "nsIDOMWindow.h"
 #include "nsGlobalWindow.h"
 
+#if defined(XP_WIN)
+#include "Windows.h"
+#else
+#include <unistd.h>
+#endif
+
 class nsPerformanceStats: public nsIPerformanceStats {
 public:
   nsPerformanceStats(const nsAString& aName,
+                     const nsAString& aGroupId,
                      const nsAString& aAddonId,
                      const nsAString& aTitle,
                      const uint64_t aWindowId,
                      const bool aIsSystem,
                      const js::PerformanceData& aPerformanceData)
     : mName(aName)
+    , mGroupId(aGroupId)
     , mAddonId(aAddonId)
     , mTitle(aTitle)
     , mWindowId(aWindowId)
     , mIsSystem(aIsSystem)
     , mPerformanceData(aPerformanceData)
   {
   }
   explicit nsPerformanceStats() {}
@@ -43,17 +51,23 @@ public:
   NS_DECL_ISUPPORTS
 
   /* readonly attribute AString name; */
   NS_IMETHOD GetName(nsAString& aName) override {
     aName.Assign(mName);
     return NS_OK;
   };
 
-  /* readonly attribute AString addon id; */
+  /* readonly attribute AString groupId; */
+  NS_IMETHOD GetGroupId(nsAString& aGroupId) override {
+    aGroupId.Assign(mGroupId);
+    return NS_OK;
+  };
+
+  /* readonly attribute AString addonId; */
   NS_IMETHOD GetAddonId(nsAString& aAddonId) override {
     aAddonId.Assign(mAddonId);
     return NS_OK;
   };
 
   /* readonly attribute uint64_t windowId; */
   NS_IMETHOD GetWindowId(uint64_t *aWindowId) override {
     *aWindowId = mWindowId;
@@ -106,16 +120,17 @@ public:
     for (size_t i = 0; i < length; ++i) {
       (*aNumberOfOccurrences)[i] = mPerformanceData.durations[i];
     }
     return NS_OK;
   };
 
 private:
   nsString mName;
+  nsString mGroupId;
   nsString mAddonId;
   nsString mTitle;
   uint64_t mWindowId;
   bool mIsSystem;
 
   js::PerformanceData mPerformanceData;
 
   virtual ~nsPerformanceStats() {}
@@ -126,57 +141,60 @@ NS_IMPL_ISUPPORTS(nsPerformanceStats, ns
 
 class nsPerformanceSnapshot : public nsIPerformanceSnapshot
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIPERFORMANCESNAPSHOT
 
   nsPerformanceSnapshot();
-  nsresult Init(JSContext*);
+  nsresult Init(JSContext*, uint64_t processId);
 private:
   virtual ~nsPerformanceSnapshot();
 
   /**
    * Import a `js::PerformanceStats` as a `nsIPerformanceStats`.
    *
    * Precondition: this method assumes that we have entered the JSCompartment for which data `c`
    * has been collected.
    *
    * `cx` may be `nullptr` if we are importing the statistics for the
    * entire process, rather than the statistics for a specific set of
    * compartments.
    */
-  already_AddRefed<nsIPerformanceStats> ImportStats(JSContext* cx, const js::PerformanceData& data);
+  already_AddRefed<nsIPerformanceStats> ImportStats(JSContext* cx, const js::PerformanceData& data, uint64_t uid);
 
   /**
    * Callbacks for iterating through the `PerformanceStats` of a runtime.
    */
-  bool IterPerformanceStatsCallbackInternal(JSContext* cx, const js::PerformanceData& stats);
-  static bool IterPerformanceStatsCallback(JSContext* cx, const js::PerformanceData& stats, void* self);
+  bool IterPerformanceStatsCallbackInternal(JSContext* cx, const js::PerformanceData& stats, uint64_t uid);
+  static bool IterPerformanceStatsCallback(JSContext* cx, const js::PerformanceData& stats, uint64_t uid, void* self);
 
   // If the context represents a window, extract the title and window ID.
   // Otherwise, extract "" and 0.
   static void GetWindowData(JSContext*,
                             nsString& title,
                             uint64_t* windowId);
-
+  void GetGroupId(JSContext*,
+                  uint64_t uid,
+                  nsString& groupId);
   // If the context presents an add-on, extract the addon ID.
   // Otherwise, extract "".
   static void GetAddonId(JSContext*,
                          JS::Handle<JSObject*> global,
                          nsAString& addonId);
 
   // Determine whether a context is part of the system principals.
   static bool GetIsSystem(JSContext*,
                           JS::Handle<JSObject*> global);
 
 private:
   nsCOMArray<nsIPerformanceStats> mComponentsData;
   nsCOMPtr<nsIPerformanceStats> mProcessData;
+  uint64_t mProcessId;
 };
 
 NS_IMPL_ISUPPORTS(nsPerformanceSnapshot, nsIPerformanceSnapshot)
 
 nsPerformanceSnapshot::nsPerformanceSnapshot()
 {
 }
 
@@ -228,75 +246,96 @@ nsPerformanceSnapshot::GetAddonId(JSCont
 
   JSAddonId* jsid = AddonIdOfObject(global);
   if (!jsid) {
     return;
   }
   AssignJSFlatString(addonId, (JSFlatString*)jsid);
 }
 
+void
+nsPerformanceSnapshot::GetGroupId(JSContext* cx,
+                                  uint64_t uid,
+                                  nsString& groupId)
+{
+  JSRuntime* rt = JS_GetRuntime(cx);
+  uint64_t runtimeId = reinterpret_cast<uintptr_t>(rt);
+
+  groupId.AssignLiteral("process: ");
+  groupId.AppendInt(mProcessId);
+  groupId.AssignLiteral(", thread: ");
+  groupId.AppendInt(runtimeId);
+  groupId.AppendLiteral(", group: ");
+  groupId.AppendInt(uid);
+}
+
 /* static */ bool
 nsPerformanceSnapshot::GetIsSystem(JSContext*,
                                    JS::Handle<JSObject*> global)
 {
   return nsContentUtils::IsSystemPrincipal(nsContentUtils::ObjectPrincipal(global));
 }
 
 already_AddRefed<nsIPerformanceStats>
-nsPerformanceSnapshot::ImportStats(JSContext* cx, const js::PerformanceData& performance) {
+nsPerformanceSnapshot::ImportStats(JSContext* cx, const js::PerformanceData& performance, const uint64_t uid) {
   JS::RootedObject global(cx, JS::CurrentGlobalOrNull(cx));
 
   if (!global) {
     // While it is possible for a compartment to have no global
     // (e.g. atoms), this compartment is not very interesting for us.
     return nullptr;
   }
 
+  nsString groupId;
+  GetGroupId(cx, uid, groupId);
+
   nsString addonId;
   GetAddonId(cx, global, addonId);
 
   nsString title;
   uint64_t windowId;
   GetWindowData(cx, title, &windowId);
 
   nsAutoString name;
   nsAutoCString cname;
   xpc::GetCurrentCompartmentName(cx, cname);
   name.Assign(NS_ConvertUTF8toUTF16(cname));
 
   bool isSystem = GetIsSystem(cx, global);
 
   nsCOMPtr<nsIPerformanceStats> result =
-    new nsPerformanceStats(name, addonId, title, windowId, isSystem, performance);
+    new nsPerformanceStats(name, groupId, addonId, title, windowId, isSystem, performance);
   return result.forget();
 }
 
 /*static*/ bool
-nsPerformanceSnapshot::IterPerformanceStatsCallback(JSContext* cx, const js::PerformanceData& stats, void* self) {
-  return reinterpret_cast<nsPerformanceSnapshot*>(self)->IterPerformanceStatsCallbackInternal(cx, stats);
+nsPerformanceSnapshot::IterPerformanceStatsCallback(JSContext* cx, const js::PerformanceData& stats, const uint64_t uid, void* self) {
+  return reinterpret_cast<nsPerformanceSnapshot*>(self)->IterPerformanceStatsCallbackInternal(cx, stats, uid);
 }
 
 bool
-nsPerformanceSnapshot::IterPerformanceStatsCallbackInternal(JSContext* cx, const js::PerformanceData& stats) {
-  nsCOMPtr<nsIPerformanceStats> result = ImportStats(cx, stats);
+nsPerformanceSnapshot::IterPerformanceStatsCallbackInternal(JSContext* cx, const js::PerformanceData& stats, const uint64_t uid) {
+  nsCOMPtr<nsIPerformanceStats> result = ImportStats(cx, stats, uid);
   if (result) {
     mComponentsData.AppendElement(result);
   }
 
   return true;
 }
 
 nsresult
-nsPerformanceSnapshot::Init(JSContext* cx) {
+nsPerformanceSnapshot::Init(JSContext* cx, uint64_t processId) {
+  mProcessId = processId;
   js::PerformanceData processStats;
   if (!js::IterPerformanceStats(cx, nsPerformanceSnapshot::IterPerformanceStatsCallback, &processStats, this)) {
     return NS_ERROR_UNEXPECTED;
   }
 
   mProcessData = new nsPerformanceStats(NS_LITERAL_STRING("<process>"), // name
+                                        NS_LITERAL_STRING("<process:?>"), // group id
                                         NS_LITERAL_STRING(""),          // add-on id
                                         NS_LITERAL_STRING(""),          // title
                                         0,                              // window id
                                         true,                           // isSystem
                                         processStats);
   return NS_OK;
 }
 
@@ -321,16 +360,21 @@ NS_IMETHODIMP nsPerformanceSnapshot::Get
   NS_IF_ADDREF(*aProcess = mProcessData);
   return NS_OK;
 }
 
 
 NS_IMPL_ISUPPORTS(nsPerformanceStatsService, nsIPerformanceStatsService)
 
 nsPerformanceStatsService::nsPerformanceStatsService()
+#if defined(XP_WIN)
+  : mProcessId(GetCurrentProcessId())
+#else
+  : mProcessId(getpid())
+#endif
 {
 }
 
 nsPerformanceStatsService::~nsPerformanceStatsService()
 {
 }
 
 //[implicit_jscontext] attribute bool isMonitoringCPOW;
@@ -362,17 +406,17 @@ NS_IMETHODIMP nsPerformanceStatsService:
   }
   return NS_OK;
 }
 
 /* readonly attribute nsIPerformanceSnapshot snapshot; */
 NS_IMETHODIMP nsPerformanceStatsService::GetSnapshot(JSContext* cx, nsIPerformanceSnapshot * *aSnapshot)
 {
   nsRefPtr<nsPerformanceSnapshot> snapshot = new nsPerformanceSnapshot();
-  nsresult rv = snapshot->Init(cx);
+  nsresult rv = snapshot->Init(cx, mProcessId);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   snapshot.forget(aSnapshot);
   return NS_OK;
 }
 
--- a/toolkit/components/perfmonitoring/nsPerformanceStats.h
+++ b/toolkit/components/perfmonitoring/nsPerformanceStats.h
@@ -14,12 +14,13 @@ public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIPERFORMANCESTATSSERVICE
 
   nsPerformanceStatsService();
 
 private:
   virtual ~nsPerformanceStatsService();
 
+  const uint64_t mProcessId;
 protected:
 };
 
 #endif
--- a/toolkit/components/perfmonitoring/tests/browser/browser_compartments.js
+++ b/toolkit/components/perfmonitoring/tests/browser/browser_compartments.js
@@ -142,47 +142,35 @@ function monotinicity_tester(source, tes
     // Sanity check on the process data.
     sanityCheck(previous.processData, snapshot.processData);
     SilentAssert.equal(snapshot.processData.isSystem, true);
     SilentAssert.equal(snapshot.processData.name, "<process>");
     SilentAssert.equal(snapshot.processData.addonId, "");
     previous.procesData = snapshot.processData;
 
     // Sanity check on components data.
-    let set = new Set();
     let map = new Map();
     for (let item of snapshot.componentsData) {
 	 for (let [probe, k] of [
         ["jank", "totalUserTime"],
         ["jank", "totalSystemTime"],
         ["cpow", "totalCPOWTime"]
       ]) {
         SilentAssert.leq(item[probe][k], snapshot.processData[probe][k],
           `Sanity check (${testName}): component has a lower ${k} than process`);
       }
 
-      let key = `{name: ${item.name}, window: ${item.windowId}, addonId: ${item.addonId}, isSystem: ${item.isSystem}}`;
-      if (set.has(key)) {
-        // There are at least two components with the same name (e.g. about:blank).
-        // Don't perform sanity checks on that name until we know how to make
-        // the difference.
-        map.delete(key);
-        continue;
-      }
+      let key = item.groupId;
+      SilentAssert.ok(!map.has(key), "The component hasn't been seen yet.");
       map.set(key, item);
-      set.add(key);
     }
     for (let [key, item] of map) {
       sanityCheck(previous.componentsMap.get(key), item);
       previous.componentsMap.set(key, item);
     }
-    info(`Deactivating deduplication check (Bug 1150045)`);
-    if (false) {
-      SilentAssert.equal(set.size, snapshot.componentsData.length);
-    }
   });
   let interval = window.setInterval(frameCheck, 300);
   registerCleanupFunction(() => {
     window.clearInterval(interval);
   });
 }
 
 add_task(function* test() {