Bug 911641 (part 2) - Prefix some reporters with "redundant/", and make about:memory ignore them. r=johns.
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 03 Sep 2013 20:06:36 -0700
changeset 146733 00c58bce0d2d1e0d13ee29656c8c26d7d9ce70bf
parent 146732 21e1bf789adde5ee3a5ed39f1b2e89131df55a9d
child 146734 5ec98e8b1be9f619dca62e255bf22e46a96fb1e4
push id25270
push useremorley@mozilla.com
push dateThu, 12 Sep 2013 11:04:52 +0000
treeherdermozilla-central@b83f6d80af5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohns
bugs911641
milestone26.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 911641 (part 2) - Prefix some reporters with "redundant/", and make about:memory ignore them. r=johns.
js/xpconnect/src/XPCJSRuntime.cpp
toolkit/components/aboutmemory/content/aboutMemory.js
toolkit/components/aboutmemory/tests/test_aboutmemory.xul
toolkit/components/telemetry/TelemetryPing.js
xpcom/base/nsIMemoryReporter.idl
xpcom/base/nsMemoryReporterManager.cpp
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -1581,37 +1581,40 @@ private:
 
 // Nb: js-main-runtime-compartments/system + js-main-runtime-compartments/user
 // could be different to the number of compartments reported by JSReporter if a
 // garbage collection occurred between them being consulted.  We could move
 // these reporters into JSReporter to avoid that problem, but then we couldn't
 // easily report them via telemetry, so we live with the small risk of
 // inconsistencies.
 
-class JSMainRuntimeCompartmentsCountSystemReporter MOZ_FINAL : public MemoryUniReporter
+class RedundantJSMainRuntimeCompartmentsSystemReporter MOZ_FINAL : public MemoryUniReporter
 {
 public:
-    JSMainRuntimeCompartmentsCountSystemReporter()
-      : MemoryUniReporter("js-main-runtime-compartments-count/system",
-                          KIND_OTHER, UNITS_COUNT,
-"The number of JavaScript compartments for system code in the main JSRuntime.")
+    // An empty description is ok because this is a "redundant/"-prefixed
+    // reporter and so is ignored by about:memory.
+    RedundantJSMainRuntimeCompartmentsSystemReporter()
+      : MemoryUniReporter("redundant/js-main-runtime-compartments/system",
+                          KIND_OTHER, UNITS_COUNT, "")
     {}
 private:
     int64_t Amount() MOZ_OVERRIDE
     {
         JSRuntime *rt = nsXPConnect::GetRuntimeInstance()->Runtime();
         return JS::SystemCompartmentCount(rt);
     }
 };
 
-class JSMainRuntimeCompartmentsCountUserReporter MOZ_FINAL : public MemoryUniReporter
+class RedundantJSMainRuntimeCompartmentsUserReporter MOZ_FINAL : public MemoryUniReporter
 {
 public:
-    JSMainRuntimeCompartmentsCountUserReporter()
-      : MemoryUniReporter("js-main-runtime-compartments-count/user",
+    // An empty description is ok because this is a "redundant/"-prefixed
+    // reporter and so is ignored by about:memory.
+    RedundantJSMainRuntimeCompartmentsUserReporter()
+      : MemoryUniReporter("redundant/js-main-runtime-compartments/user",
                           KIND_OTHER, UNITS_COUNT,
 "The number of JavaScript compartments for user code in the main JSRuntime.")
     {}
 private:
     int64_t Amount() MOZ_OVERRIDE
     {
         JSRuntime *rt = nsXPConnect::GetRuntimeInstance()->Runtime();
         return JS::UserCompartmentCount(rt);
@@ -3038,18 +3041,18 @@ XPCJSRuntime::XPCJSRuntime(nsXPConnect* 
 
     // Set up locale information and callbacks for the newly-created runtime so
     // that the various toLocaleString() methods, localeCompare(), and other
     // internationalization APIs work as desired.
     if (!xpc_LocalizeRuntime(runtime))
         NS_RUNTIMEABORT("xpc_LocalizeRuntime failed.");
 
     NS_RegisterMemoryReporter(new JSMainRuntimeGCHeapReporter());
-    NS_RegisterMemoryReporter(new JSMainRuntimeCompartmentsCountSystemReporter());
-    NS_RegisterMemoryReporter(new JSMainRuntimeCompartmentsCountUserReporter());
+    NS_RegisterMemoryReporter(new RedundantJSMainRuntimeCompartmentsSystemReporter());
+    NS_RegisterMemoryReporter(new RedundantJSMainRuntimeCompartmentsUserReporter());
     NS_RegisterMemoryReporter(new JSMainRuntimeTemporaryPeakReporter());
     NS_RegisterMemoryReporter(new JSMainRuntimeCompartmentsReporter);
 
     // Install a JavaScript 'debugger' keyword handler in debug builds only
 #ifdef DEBUG
     if (!JS_GetGlobalDebugHooks(runtime)->debuggerHandler)
         xpc_InstallJSDebuggerKeywordHandler(runtime);
 #endif
--- a/toolkit/components/aboutmemory/content/aboutMemory.js
+++ b/toolkit/components/aboutmemory/content/aboutMemory.js
@@ -1012,34 +1012,29 @@ function getPCollsByProcess(aProcessRepo
   // be in parentheses, so a ')' might appear after the '.'.)
   const gSentenceRegExp = /^[A-Z].*\.\)?$/m;
 
   // Ignore the "smaps" reporter in non-verbose mode unless we're reading from
   // a file or the clipboard.  (Note that reports from these reporters can
   // reach here via a "content-child" reporter if they were in a child
   // process.)
   //
-  // Also ignore the "resident-fast" reporter; we use the vanilla "resident"
-  // reporter because it's more important that we get accurate results than
-  // that we avoid the (small) possibility of a long pause when loading
-  // about:memory.  Furthermore, the "resident" reporter can purge pages on
-  // MacOS, which affects the results of the "resident-fast" reporter, and we
-  // don't want the measurements shown in about:memory to be affected by the
-  // (arbitrary) order of memory reporter execution.
+  // Ignore any "redundant/"-prefixed reporters and reports, which are only
+  // used by telemetry.
 
   function ignoreReporter(aName)
   {
     return (aName === "smaps" && !gVerbose.checked && !aForceShowSmaps) ||
-           aName === "resident-fast";
+           aName.startsWith("redundant/");
   }
 
   function ignoreReport(aUnsafePath)
   {
     return (isSmapsPath(aUnsafePath) && !gVerbose.checked && !aForceShowSmaps) ||
-           aUnsafePath == "resident-fast";
+           aUnsafePath.startsWith("redundant/");
   }
 
   function handleReport(aProcess, aUnsafePath, aKind, aUnits, aAmount,
                         aDescription, aPresence)
   {
     if (isExplicitPath(aUnsafePath)) {
       assertInput(aKind === KIND_HEAP || aKind === KIND_NONHEAP,
                   "bad explicit kind");
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
@@ -160,22 +160,22 @@
     is(ex.result, Cr.NS_ERROR_NOT_AVAILABLE, "mgr.explicit exception");
   }
  
   // The main process always comes first when we display about:memory.  The
   // remaining processes are sorted by their |resident| values (starting with
   // the largest).  Processes without a |resident| memory reporter are saved
   // for the end.
   let fakeReporters2 = [
-    { name: "resident-fast",
+    { name: "redundant/foobar",
       collectReports: function(aCbObj, aClosure) {
         // Shouldn't reach here;  aboutMemory.js should skip the reporter.
         // (Nb: this must come after |mgr.explicit| is accessed, otherwise it
         // *will* be run by nsMemoryReporterManager::GetExplicit()).
-        ok(false, "'resident-fast' reporter was run");
+        ok(false, "'redundant/foobar' reporter was run");
       }
     },
     { name: "fake3",
       collectReports: function(aCbObj, aClosure) {
         function f(aP1, aP2, aK, aU, aA) {
           aCbObj.callback(aP1, aP2, aK, aU, aA, "Desc.", aClosure);
         }
         f("2nd", "heap-allocated",  OTHER,   BYTES,1000* MB);
@@ -190,17 +190,17 @@
         f("2nd", "other0",          OTHER,   BYTES,666 * MB);
         f("2nd", "other1",          OTHER,   BYTES,111 * MB);
         // If the "smaps" reporter is in a child process it'll be passed to
         // the main process under a different name.  The fact that we skip
         // the "smaps" reporter in the main process won't cause these
         // to be skipped;  the report-level skipping will be hit instead.
         f("2nd", "size/e",          NONHEAP, BYTES,24*4*KB);
         f("2nd", "size/f",          NONHEAP, BYTES,24*4*KB);
-        f("2nd", "resident-fast",   NONHEAP, BYTES,24*4*KB); // ignored!
+        f("2nd", "redundant/blah",  NONHEAP, BYTES,24*4*KB); // ignored!
 
         // Check that we can handle "heap-allocated" not being present.
         f("3rd", "explicit/a/b",    HEAP,    BYTES,333 * MB);
         f("3rd", "explicit/a/c",    HEAP,    BYTES,444 * MB);
         f("3rd", "other1",          OTHER,   BYTES,  1 * MB);
         f("3rd", "resident",        OTHER,   BYTES,100 * MB);
 
         // Invalid values (negative, too-big) should be identified.
--- a/toolkit/components/telemetry/TelemetryPing.js
+++ b/toolkit/components/telemetry/TelemetryPing.js
@@ -50,20 +50,20 @@ const TELEMETRY_DELAY = 60000;
 //
 //   * MEMORY_JS_GC_HEAP, and
 //   * MEMORY_JS_COMPARTMENTS_SYSTEM.
 //
 // We used to measure "explicit" too, but it could cause hangs, and the data
 // was always really noisy anyway.  See bug 859657.
 const MEM_HISTOGRAMS = {
   "js-main-runtime-gc-heap": "MEMORY_JS_GC_HEAP",
-  "js-main-runtime-compartments-count/system": "MEMORY_JS_COMPARTMENTS_SYSTEM",
-  "js-main-runtime-compartments-count/user": "MEMORY_JS_COMPARTMENTS_USER",
+  "redundant/js-main-runtime-compartments/system": "MEMORY_JS_COMPARTMENTS_SYSTEM",
+  "redundant/js-main-runtime-compartments/user": "MEMORY_JS_COMPARTMENTS_USER",
   "js-main-runtime-temporary-peak": "MEMORY_JS_MAIN_RUNTIME_TEMPORARY_PEAK",
-  "resident-fast": "MEMORY_RESIDENT",
+  "redundant/resident-fast": "MEMORY_RESIDENT",
   "vsize": "MEMORY_VSIZE",
   "storage-sqlite": "MEMORY_STORAGE_SQLITE",
   "images-content-used-uncompressed":
     "MEMORY_IMAGES_CONTENT_USED_UNCOMPRESSED",
   "heap-allocated": "MEMORY_HEAP_ALLOCATED",
   "heap-overhead": "MEMORY_HEAP_COMMITTED_UNUSED",
   "heap-overhead-ratio": "MEMORY_HEAP_COMMITTED_UNUSED_RATIO",
   "page-faults-hard": "PAGE_FAULTS_HARD",
--- a/xpcom/base/nsIMemoryReporter.idl
+++ b/xpcom/base/nsIMemoryReporter.idl
@@ -143,16 +143,21 @@ interface nsIMemoryReporterCallback : ns
  *   description that is a sentence (i.e. starts with a capital letter and
  *   ends with a period, or similar).
  *
  * - The "size", "rss", "pss" and "swap" trees are optional.  They
  *   represent regions of virtual memory that the process has mapped.
  *   Reporters in this category must have kind NONHEAP, units BYTES, and a
  *   non-empty description.
  *
+ * - The "redundant" tree is optional, and can be used for reports that are
+ *   redundant w.r.t. other reports.  These are useful for telemetry, and are
+ *   not shown in about:memory.  Reports in this tree are entirely
+ *   unconstrained.
+ *
  * - All other reports are unconstrained except that they must have a
  *   description that is a sentence.
  */
 [scriptable, uuid(53248304-124b-43cd-99dc-6e5797b91618)]
 interface nsIMemoryReporter : nsISupports
 {
   /*
    * The name of the reporter.  Useful when only one reporter needs to be run.
--- a/xpcom/base/nsMemoryReporterManager.cpp
+++ b/xpcom/base/nsMemoryReporterManager.cpp
@@ -384,21 +384,26 @@ public:
 "other processes being run and details of the OS kernel and so is best used "
 "for comparing the memory usage of a single process at different points in "
 "time.")
     {}
 
     NS_IMETHOD GetAmount(int64_t* aAmount) { return GetResident(aAmount); }
 };
 
+// This is a "redundant/"-prefixed reporter, which means it's ignored by
+// about:memory.  This is good because the "resident" reporter can purge pages
+// on MacOS, which affects the "resident-fast" results, and we don't want the
+// measurements shown in about:memory to be affected by the (arbitrary) order
+// of memory reporter execution.  This reporter is used by telemetry.
 class ResidentFastReporter MOZ_FINAL : public MemoryUniReporter
 {
 public:
     ResidentFastReporter()
-      : MemoryUniReporter("resident-fast", KIND_OTHER, UNITS_BYTES,
+      : MemoryUniReporter("redundant/resident-fast", KIND_OTHER, UNITS_BYTES,
 "This is the same measurement as 'resident', but it tries to be as fast as "
 "possible at the expense of accuracy.  On most platforms this is identical to "
 "the 'resident' measurement, but on Mac it may over-count.  You should use "
 "'resident-fast' where you care about latency of collection (e.g. in "
 "telemetry).  Otherwise you should use 'resident'.")
     {}
 
     NS_IMETHOD GetAmount(int64_t* aAmount) { return GetResidentFast(aAmount); }