Bug 972712 (part 3) - Rework notable string reporting. r=till.
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 26 Feb 2014 18:11:01 -0800
changeset 171477 609b72cffe1f98dcc3188c1df3f99cdcf4053402
parent 171476 83b4c208e0c353efe7fb3625258abbb59f1ddf30
child 171478 655050f4a7467edcea8894b51f3734970d832876
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewerstill
bugs972712
milestone30.0a1
Bug 972712 (part 3) - Rework notable string reporting. r=till.
js/public/MemoryMetrics.h
js/src/vm/MemoryMetrics.cpp
js/xpconnect/src/XPCJSRuntime.cpp
--- a/js/public/MemoryMetrics.h
+++ b/js/public/MemoryMetrics.h
@@ -87,67 +87,28 @@ struct InefficientNonFlatteningStringHas
 //
 // In some classes, one or more of the macro arguments aren't used.  We use '_'
 // for those.
 //
 #define DECL_SIZE(kind, gc, mSize)                      size_t mSize;
 #define ZERO_SIZE(kind, gc, mSize)                      mSize(0),
 #define COPY_OTHER_SIZE(kind, gc, mSize)                mSize(other.mSize),
 #define ADD_OTHER_SIZE(kind, gc, mSize)                 mSize += other.mSize;
+#define SUB_OTHER_SIZE(kind, gc, mSize)                 MOZ_ASSERT(mSize >= other.mSize); \
+                                                        mSize -= other.mSize;
+#define ADD_SIZE_TO_N(kind, gc, mSize)                  n += mSize;
 #define ADD_SIZE_TO_N_IF_LIVE_GC_THING(kind, gc, mSize) n += (js::gc) ? mSize : 0;
 #define ADD_TO_TAB_SIZES(kind, gc, mSize)               sizes->add(JS::TabSizes::kind, mSize);
 
 // Used to annotate which size_t fields measure live GC things and which don't.
 enum {
     NotLiveGCThing = false,
     IsLiveGCThing = true
 };
 
-struct ZoneStatsPod
-{
-#define FOR_EACH_SIZE(macro) \
-    macro(Other,   NotLiveGCThing, gcHeapArenaAdmin) \
-    macro(Other,   NotLiveGCThing, unusedGCThings) \
-    macro(Other,   IsLiveGCThing,  lazyScriptsGCHeap) \
-    macro(Other,   NotLiveGCThing, lazyScriptsMallocHeap) \
-    macro(Other,   IsLiveGCThing,  jitCodesGCHeap) \
-    macro(Other,   IsLiveGCThing,  typeObjectsGCHeap) \
-    macro(Other,   NotLiveGCThing, typeObjectsMallocHeap) \
-    macro(Other,   NotLiveGCThing, typePool) \
-    macro(Strings, IsLiveGCThing,  stringsGCHeap) \
-    macro(Strings, NotLiveGCThing, stringsMallocHeap)
-
-    ZoneStatsPod()
-      : FOR_EACH_SIZE(ZERO_SIZE)
-        extra()
-    {}
-
-    void add(const ZoneStatsPod &other) {
-        FOR_EACH_SIZE(ADD_OTHER_SIZE)
-        // Do nothing with |extra|.
-    }
-
-    size_t sizeOfLiveGCThings() const {
-        size_t n = 0;
-        FOR_EACH_SIZE(ADD_SIZE_TO_N_IF_LIVE_GC_THING)
-        // Do nothing with |extra|.
-        return n;
-    }
-
-    void addToTabSizes(JS::TabSizes *sizes) const {
-        FOR_EACH_SIZE(ADD_TO_TAB_SIZES)
-        // Do nothing with |extra|.
-    }
-
-    FOR_EACH_SIZE(DECL_SIZE)
-    void *extra;    // This field can be used by embedders.
-
-#undef FOR_EACH_SIZE
-};
-
 } // namespace js
 
 namespace JS {
 
 // Data for tracking memory usage of things hanging off objects.
 struct ObjectsExtraSizes
 {
 #define FOR_EACH_SIZE(macro) \
@@ -235,70 +196,76 @@ struct GCSizes
 
 // This class holds information about the memory taken up by identical copies of
 // a particular string.  Multiple JSStrings may have their sizes aggregated
 // together into one StringInfo object.  Note that two strings with identical
 // chars will not be aggregated together if one is a short string and the other
 // is not.
 struct StringInfo
 {
+#define FOR_EACH_SIZE(macro) \
+    macro(Strings, IsLiveGCThing,  gcHeap) \
+    macro(Strings, NotLiveGCThing, mallocHeap) \
+
     StringInfo()
-      : numCopies(0),
-        gcHeap(0),
-        mallocHeap(0)
+      : FOR_EACH_SIZE(ZERO_SIZE)
+        numCopies(0)
     {}
 
-    StringInfo(size_t gcSize, size_t mallocSize)
-      : numCopies(1),
-        gcHeap(gcSize),
-        mallocHeap(mallocSize)
-    {}
+    void add(const StringInfo &other) {
+        FOR_EACH_SIZE(ADD_OTHER_SIZE);
+        numCopies++;
+    }
 
-    void add(size_t gcSize, size_t mallocSize) {
-        numCopies++;
-        gcHeap += gcSize;
-        mallocHeap += mallocSize;
+    void subtract(const StringInfo &other) {
+        FOR_EACH_SIZE(SUB_OTHER_SIZE);
+        numCopies--;
+    }
+
+    bool isNotable() const {
+        static const size_t NotabilityThreshold = 16 * 1024;
+        size_t n = 0;
+        FOR_EACH_SIZE(ADD_SIZE_TO_N)
+        return n >= NotabilityThreshold;
     }
 
-    void add(const StringInfo& info) {
-        numCopies += info.numCopies;
-        gcHeap += info.gcHeap;
-        mallocHeap += info.mallocHeap;
+    size_t sizeOfLiveGCThings() const {
+        size_t n = 0;
+        FOR_EACH_SIZE(ADD_SIZE_TO_N_IF_LIVE_GC_THING)
+        return n;
     }
 
+    void addToTabSizes(TabSizes *sizes) const {
+        FOR_EACH_SIZE(ADD_TO_TAB_SIZES)
+    }
+
+    FOR_EACH_SIZE(DECL_SIZE)
     uint32_t numCopies;     // How many copies of the string have we seen?
 
-    // These are all totals across all copies of the string we've seen.
-    size_t gcHeap;
-    size_t mallocHeap;
+#undef FOR_EACH_SIZE
 };
 
-// Holds data about a notable string (one which uses more than
-// NotableStringInfo::notableSize() bytes of memory), so we can report it
-// individually.
+// Holds data about a notable string (one which, counting all duplicates, uses
+// more than a certain amount of memory) so we can report it individually.
 //
-// Essentially the only difference between this class and StringInfo is that
-// NotableStringInfo holds a copy of the string's chars.
+// The only difference between this class and StringInfo is that
+// NotableStringInfo holds a copy of some or all of the string's chars.
 struct NotableStringInfo : public StringInfo
 {
+    static const size_t MAX_SAVED_CHARS = 1024;
+
     NotableStringInfo();
     NotableStringInfo(JSString *str, const StringInfo &info);
     NotableStringInfo(NotableStringInfo &&info);
     NotableStringInfo &operator=(NotableStringInfo &&info);
 
     ~NotableStringInfo() {
         js_free(buffer);
     }
 
-    // A string needs to take up this many bytes of storage before we consider
-    // it to be "notable".
-    static size_t notableSize() {
-        return js::MemoryReportingSundriesThreshold();
-    }
-
     char *buffer;
     size_t length;
 
   private:
     NotableStringInfo(const NotableStringInfo& info) MOZ_DELETE;
 };
 
 // These measurements relate directly to the JSRuntime, and not to zones and
@@ -326,73 +293,99 @@ struct RuntimeSizes
 
     FOR_EACH_SIZE(DECL_SIZE)
     CodeSizes code;
     GCSizes   gc;
 
 #undef FOR_EACH_SIZE
 };
 
-struct ZoneStats : js::ZoneStatsPod
+struct ZoneStats
 {
+#define FOR_EACH_SIZE(macro) \
+    macro(Other,   NotLiveGCThing, gcHeapArenaAdmin) \
+    macro(Other,   NotLiveGCThing, unusedGCThings) \
+    macro(Other,   IsLiveGCThing,  lazyScriptsGCHeap) \
+    macro(Other,   NotLiveGCThing, lazyScriptsMallocHeap) \
+    macro(Other,   IsLiveGCThing,  jitCodesGCHeap) \
+    macro(Other,   IsLiveGCThing,  typeObjectsGCHeap) \
+    macro(Other,   NotLiveGCThing, typeObjectsMallocHeap) \
+    macro(Other,   NotLiveGCThing, typePool) \
+
     ZoneStats()
-      : strings(nullptr)
+      : FOR_EACH_SIZE(ZERO_SIZE)
+        stringInfo(),
+        extra(),
+        allStrings(nullptr),
+        notableStrings(),
+        isTotals(true)
     {}
 
     ZoneStats(ZoneStats &&other)
-      : ZoneStatsPod(mozilla::Move(other)),
-        strings(other.strings),
-        notableStrings(mozilla::Move(other.notableStrings))
+      : FOR_EACH_SIZE(COPY_OTHER_SIZE)
+        stringInfo(mozilla::Move(other.stringInfo)),
+        extra(other.extra),
+        allStrings(other.allStrings),
+        notableStrings(mozilla::Move(other.notableStrings)),
+        isTotals(other.isTotals)
     {
-        other.strings = nullptr;
+        other.allStrings = nullptr;
+        MOZ_ASSERT(!other.isTotals);
+    }
+
+    ~ZoneStats() {
+        // |allStrings| is usually deleted and set to nullptr before this
+        // destructor runs. But there are failure cases due to OOMs that may
+        // prevent that, so it doesn't hurt to try again here.
+        js_delete(allStrings);
     }
 
     bool initStrings(JSRuntime *rt);
 
-    // Add |other|'s numbers to this object's numbers.  The strings data isn't
-    // touched.
-    void addIgnoringStrings(const ZoneStats &other) {
-        ZoneStatsPod::add(other);
-    }
-
-    // Add |other|'s strings data to this object's strings data.  (We don't do
-    // anything with notableStrings.)
-    void addStrings(const ZoneStats &other) {
-        for (StringsHashMap::Range r = other.strings->all(); !r.empty(); r.popFront()) {
-            StringsHashMap::AddPtr p = strings->lookupForAdd(r.front().key());
-            if (p) {
-                // We've seen this string before; add its size to our tally.
-                p->value().add(r.front().value());
-            } else {
-                // We haven't seen this string before; add it to the hashtable.
-                strings->add(p, r.front().key(), r.front().value());
-            }
-        }
+    void addSizes(const ZoneStats &other) {
+        MOZ_ASSERT(isTotals);
+        FOR_EACH_SIZE(ADD_OTHER_SIZE)
+        stringInfo.add(other.stringInfo);
     }
 
     size_t sizeOfLiveGCThings() const {
-        size_t n = ZoneStatsPod::sizeOfLiveGCThings();
-        for (size_t i = 0; i < notableStrings.length(); i++) {
-            const JS::NotableStringInfo& info = notableStrings[i];
-            n += info.gcHeap;
-        }
+        MOZ_ASSERT(isTotals);
+        size_t n = 0;
+        FOR_EACH_SIZE(ADD_SIZE_TO_N_IF_LIVE_GC_THING)
+        n += stringInfo.sizeOfLiveGCThings();
         return n;
     }
 
-    typedef js::HashMap<JSString*,
-                        StringInfo,
+    void addToTabSizes(JS::TabSizes *sizes) const {
+        MOZ_ASSERT(isTotals);
+        FOR_EACH_SIZE(ADD_TO_TAB_SIZES)
+        stringInfo.addToTabSizes(sizes);
+    }
+
+    // These string measurements are initially for all strings.  At the end,
+    // if the measurement granularity is FineGrained, we subtract the
+    // measurements of the notable script sources and move them into
+    // |notableStrings|.
+    FOR_EACH_SIZE(DECL_SIZE)
+    StringInfo stringInfo;
+    void *extra;    // This field can be used by embedders.
+
+    typedef js::HashMap<JSString*, StringInfo,
                         js::InefficientNonFlatteningStringHashPolicy,
                         js::SystemAllocPolicy> StringsHashMap;
 
-    // |strings| is only used transiently.  During the zone traversal it is
+    // |allStrings| is only used transiently.  During the zone traversal it is
     // filled with info about every string in the zone.  It's then used to fill
     // in |notableStrings| (which actually gets reported), and immediately
     // discarded afterwards.
-    StringsHashMap *strings;
+    StringsHashMap *allStrings;
     js::Vector<NotableStringInfo, 0, js::SystemAllocPolicy> notableStrings;
+    bool isTotals;
+
+#undef FOR_EACH_SIZE
 };
 
 struct CompartmentStats
 {
 #define FOR_EACH_SIZE(macro) \
     macro(Objects, IsLiveGCThing,  objectsGCHeapOrdinary) \
     macro(Objects, IsLiveGCThing,  objectsGCHeapFunction) \
     macro(Objects, IsLiveGCThing,  objectsGCHeapDenseArray) \
@@ -559,12 +552,14 @@ AddSizeOfTab(JSRuntime *rt, JS::HandleOb
              ObjectPrivateVisitor *opv, TabSizes *sizes);
 
 } // namespace JS
 
 #undef DECL_SIZE
 #undef ZERO_SIZE
 #undef COPY_OTHER_SIZE
 #undef ADD_OTHER_SIZE
+#undef SUB_OTHER_SIZE
+#undef ADD_SIZE_TO_N
 #undef ADD_SIZE_TO_N_IF_LIVE_GC_THING
 #undef ADD_TO_TAB_SIZES
 
 #endif /* js_MemoryMetrics_h */
--- a/js/src/vm/MemoryMetrics.cpp
+++ b/js/src/vm/MemoryMetrics.cpp
@@ -89,42 +89,43 @@ InefficientNonFlatteningStringHashPolicy
     return PodEqual(c1, c2, k->length());
 }
 
 } // namespace js
 
 namespace JS {
 
 NotableStringInfo::NotableStringInfo()
-  : buffer(0),
+  : StringInfo(),
+    buffer(0),
     length(0)
 {
 }
 
 NotableStringInfo::NotableStringInfo(JSString *str, const StringInfo &info)
   : StringInfo(info),
     length(str->length())
 {
-    size_t bufferSize = Min(str->length() + 1, size_t(4096));
+    size_t bufferSize = Min(str->length() + 1, size_t(MAX_SAVED_CHARS));
     buffer = js_pod_malloc<char>(bufferSize);
     if (!buffer) {
         MOZ_CRASH("oom");
     }
 
     const jschar* chars;
     ScopedJSFreePtr<jschar> ownedChars;
     if (str->hasPureChars()) {
         chars = str->pureChars();
     } else {
         if (!str->copyNonPureChars(/* tcx */ nullptr, ownedChars))
             MOZ_CRASH("oom");
         chars = ownedChars;
     }
 
-    // We might truncate |str| even if it's much shorter than 4096 chars, if
+    // We might truncate |str| even if it's much shorter than 1024 chars, if
     // |str| contains unicode chars.  Since this is just for a memory reporter,
     // we don't care.
     PutEscapedString(buffer, bufferSize, chars, str->length(), /* quote */ 0);
 }
 
 NotableStringInfo::NotableStringInfo(NotableStringInfo &&info)
   : StringInfo(Move(info)),
     length(info.length)
@@ -176,17 +177,18 @@ static void
 StatsZoneCallback(JSRuntime *rt, void *data, Zone *zone)
 {
     // Append a new CompartmentStats to the vector.
     RuntimeStats *rtStats = static_cast<StatsClosure *>(data)->rtStats;
 
     // CollectRuntimeStats reserves enough space.
     MOZ_ALWAYS_TRUE(rtStats->zoneStatsVector.growBy(1));
     ZoneStats &zStats = rtStats->zoneStatsVector.back();
-    zStats.initStrings(rt);
+    if (!zStats.initStrings(rt))
+        MOZ_CRASH("oom");
     rtStats->initExtraZoneStats(zone, &zStats);
     rtStats->currZoneStats = &zStats;
 
     zone->addSizeOfIncludingThis(rtStats->mallocSizeOf_, &zStats.typePool);
 }
 
 static void
 StatsCompartmentCallback(JSRuntime *rt, void *data, JSCompartment *compartment)
@@ -275,31 +277,32 @@ StatsCellCallback(JSRuntime *rt, void *d
                 cStats->objectsPrivate += opv->sizeOfIncludingThis(iface);
         }
         break;
       }
 
       case JSTRACE_STRING: {
         JSString *str = static_cast<JSString *>(thing);
 
-        size_t strCharsSize = str->sizeOfExcludingThis(rtStats->mallocSizeOf_);
+        JS::StringInfo info;
+        info.gcHeap = thingSize;
+        info.mallocHeap = str->sizeOfExcludingThis(rtStats->mallocSizeOf_);
+        info.numCopies = 1;
 
-        zStats->stringsGCHeap += thingSize;
-        zStats->stringsMallocHeap += strCharsSize;
+        zStats->stringInfo.add(info);
 
         if (granularity == FineGrained) {
-            ZoneStats::StringsHashMap::AddPtr p = zStats->strings->lookupForAdd(str);
+            ZoneStats::StringsHashMap::AddPtr p = zStats->allStrings->lookupForAdd(str);
             if (!p) {
-                JS::StringInfo info(thingSize, strCharsSize);
-                zStats->strings->add(p, str, info);
+                // Ignore failure -- we just won't record the string as notable.
+                (void)zStats->allStrings->add(p, str, info);
             } else {
-                p->value().add(thingSize, strCharsSize);
+                p->value().add(info);
             }
         }
-
         break;
       }
 
       case JSTRACE_SHAPE: {
         Shape *shape = static_cast<Shape *>(thing);
         CompartmentStats *cStats = GetCompartmentStats(shape->compartment());
         if (shape->inDictionary()) {
             cStats->shapesGCHeapDict += thingSize;
@@ -374,57 +377,58 @@ StatsCellCallback(JSRuntime *rt, void *d
       default:
         MOZ_ASSUME_UNREACHABLE("invalid traceKind");
     }
 
     // Yes, this is a subtraction:  see StatsArenaCallback() for details.
     zStats->unusedGCThings -= thingSize;
 }
 
-static void
+static bool
 FindNotableStrings(ZoneStats &zStats)
 {
     using namespace JS;
 
-    // You should only run FindNotableStrings once per ZoneStats object
-    // (although it's not going to break anything if you run it more than once,
-    // unless you add to |strings| in the meantime).
+    // We should only run FindNotableStrings once per ZoneStats object.
     MOZ_ASSERT(zStats.notableStrings.empty());
 
-    for (ZoneStats::StringsHashMap::Range r = zStats.strings->all(); !r.empty(); r.popFront()) {
+    for (ZoneStats::StringsHashMap::Range r = zStats.allStrings->all(); !r.empty(); r.popFront()) {
 
         JSString *str = r.front().key();
         StringInfo &info = r.front().value();
 
-        // If this string is too small, or if we can't grow the notableStrings
-        // vector, skip this string.
-        if (info.gcHeap + info.mallocHeap < NotableStringInfo::notableSize() ||
-            !zStats.notableStrings.growBy(1))
+        if (!info.isNotable())
             continue;
 
+        if (!zStats.notableStrings.growBy(1))
+            return false;
+
         zStats.notableStrings.back() = NotableStringInfo(str, info);
 
         // We're moving this string from a non-notable to a notable bucket, so
         // subtract it out of the non-notable tallies.
-        MOZ_ASSERT(zStats.stringsGCHeap >= info.gcHeap);
-        MOZ_ASSERT(zStats.stringsMallocHeap >= info.mallocHeap);
-        zStats.stringsGCHeap -= info.gcHeap;
-        zStats.stringsMallocHeap -= info.mallocHeap;
+        zStats.stringInfo.subtract(info);
     }
+    // Delete |allStrings| now, rather than waiting for zStats's destruction,
+    // to reduce peak memory consumption during reporting.
+    js_delete(zStats.allStrings);
+    zStats.allStrings = nullptr;
+    return true;
 }
 
 bool
 ZoneStats::initStrings(JSRuntime *rt)
 {
-    strings = rt->new_<StringsHashMap>();
-    if (!strings)
+    isTotals = false;
+    allStrings = rt->new_<StringsHashMap>();
+    if (!allStrings)
         return false;
-    if (!strings->init()) {
-        js_delete(strings);
-        strings = nullptr;
+    if (!allStrings->init()) {
+        js_delete(allStrings);
+        allStrings = nullptr;
         return false;
     }
     return true;
 }
 
 JS_PUBLIC_API(bool)
 JS::CollectRuntimeStats(JSRuntime *rt, RuntimeStats *rtStats, ObjectPrivateVisitor *opv)
 {
@@ -451,51 +455,27 @@ JS::CollectRuntimeStats(JSRuntime *rt, R
                                         StatsArenaCallback, StatsCellCallback<FineGrained>);
 
     // Take the "explicit/js/runtime/" measurements.
     rt->addSizeOfIncludingThis(rtStats->mallocSizeOf_, &rtStats->runtime);
 
     ZoneStatsVector &zs = rtStats->zoneStatsVector;
     ZoneStats &zTotals = rtStats->zTotals;
 
-    // For each zone:
-    // - sum everything except its strings data into zTotals, and
-    // - find its notable strings.
-    // Also, record which zone had the biggest |strings| hashtable -- to save
-    // time and memory, we will re-use that hashtable to find the notable
-    // strings for zTotals.
-    size_t iMax = 0;
-    for (size_t i = 0; i < zs.length(); i++) {
-        zTotals.addIgnoringStrings(zs[i]);
-        FindNotableStrings(zs[i]);
-        if (zs[i].strings->count() > zs[iMax].strings->count())
-            iMax = i;
-    }
+    // We don't look for notable strings for zTotals. So we first sum all the
+    // zones' measurements to get the totals. Then we find the notable strings
+    // within each zone.
+    for (size_t i = 0; i < zs.length(); i++)
+        zTotals.addSizes(zs[i]);
 
-    // Transfer the biggest strings table to zTotals.  We can do this because:
-    // (a) we've found the notable strings for zs[IMax], and so don't need it
-    //     any more for zs, and
-    // (b) zs[iMax].strings contains a subset of the values that will end up in
-    //     zTotals.strings.
-    MOZ_ASSERT(!zTotals.strings);
-    zTotals.strings = zs[iMax].strings;
-    zs[iMax].strings = nullptr;
+    for (size_t i = 0; i < zs.length(); i++)
+        if (!FindNotableStrings(zs[i]))
+            return false;
 
-    // Add the remaining strings hashtables to zTotals, and then get the
-    // notable strings for zTotals.
-    for (size_t i = 0; i < zs.length(); i++) {
-        if (i != iMax) {
-            zTotals.addStrings(zs[i]);
-            js_delete(zs[i].strings);
-            zs[i].strings = nullptr;
-        }
-    }
-    FindNotableStrings(zTotals);
-    js_delete(zTotals.strings);
-    zTotals.strings = nullptr;
+    MOZ_ASSERT(!zTotals.allStrings);
 
     for (size_t i = 0; i < rtStats->compartmentStatsVector.length(); i++) {
         CompartmentStats &cStats = rtStats->compartmentStatsVector[i];
         rtStats->cTotals.add(cStats);
     }
 
     rtStats->gcHeapGCThings = rtStats->zTotals.sizeOfLiveGCThings() +
                               rtStats->cTotals.sizeOfLiveGCThings();
@@ -593,17 +573,17 @@ AddSizeOfTab(JSRuntime *rt, HandleObject
     StatsClosure closure(&rtStats, opv);
     if (!closure.init())
         return false;
     IterateZoneCompartmentsArenasCells(rt, zone, &closure, StatsZoneCallback,
                                        StatsCompartmentCallback, StatsArenaCallback,
                                        StatsCellCallback<CoarseGrained>);
 
     JS_ASSERT(rtStats.zoneStatsVector.length() == 1);
-    rtStats.zTotals.add(rtStats.zoneStatsVector[0]);
+    rtStats.zTotals.addSizes(rtStats.zoneStatsVector[0]);
 
     for (size_t i = 0; i < rtStats.compartmentStatsVector.length(); i++) {
         CompartmentStats &cStats = rtStats.compartmentStatsVector[i];
         rtStats.cTotals.add(cStats);
     }
 
     for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next())
         comp->compartmentStats = nullptr;
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -1754,16 +1754,18 @@ static nsresult
 ReportZoneStats(const JS::ZoneStats &zStats,
                 const xpc::ZoneStatsExtras &extras,
                 nsIMemoryReporterCallback *cb,
                 nsISupports *closure, size_t *gcTotalOut = nullptr)
 {
     const nsAutoCString& pathPrefix = extras.pathPrefix;
     size_t gcTotal = 0, sundriesGCHeap = 0, sundriesMallocHeap = 0;
 
+    MOZ_ASSERT(!gcTotalOut == zStats.isTotals);
+
     ZCREPORT_GC_BYTES(pathPrefix + NS_LITERAL_CSTRING("gc-heap-arena-admin"),
         zStats.gcHeapArenaAdmin,
         "Bookkeeping information and alignment padding within GC arenas.");
 
     ZCREPORT_GC_BYTES(pathPrefix + NS_LITERAL_CSTRING("unused-gc-things"),
         zStats.unusedGCThings,
         "Empty GC thing cells within non-empty arenas.");
 
@@ -1797,84 +1799,94 @@ ReportZoneStats(const JS::ZoneStats &zSt
     #define MAYBE_INLINE \
         "The characters may be inline or on the malloc heap."
     #define MAYBE_OVERALLOCATED \
         "Sometimes over-allocated to simplify string concatenation."
 
     for (size_t i = 0; i < zStats.notableStrings.length(); i++) {
         const JS::NotableStringInfo& info = zStats.notableStrings[i];
 
+        MOZ_ASSERT(!zStats.isTotals);
+
         nsDependentCString notableString(info.buffer);
 
         // Viewing about:memory generates many notable strings which contain
         // "string(length=".  If we report these as notable, then we'll create
         // even more notable strings the next time we open about:memory (unless
         // there's a GC in the meantime), and so on ad infinitum.
         //
         // To avoid cluttering up about:memory like this, we stick notable
-        // strings which contain "strings/notable/string(length=" into their own
-        // bucket.
+        // strings which contain "string(length=" into their own bucket.
 #       define STRING_LENGTH "string(length="
         if (FindInReadable(NS_LITERAL_CSTRING(STRING_LENGTH), notableString)) {
             stringsNotableAboutMemoryGCHeap += info.gcHeap;
             stringsNotableAboutMemoryMallocHeap += info.mallocHeap;
             continue;
         }
 
         // Escape / to \ before we put notableString into the memory reporter
         // path, because we don't want any forward slashes in the string to
         // count as path separators.
         nsCString escapedString(notableString);
         escapedString.ReplaceSubstring("/", "\\");
 
         bool truncated = notableString.Length() < info.length;
 
         nsCString path = pathPrefix +
-            nsPrintfCString("strings/notable/" STRING_LENGTH "%d, copies=%d, \"%s\"%s)/",
+            nsPrintfCString("strings/" STRING_LENGTH "%d, copies=%d, \"%s\"%s)/",
                             info.length, info.numCopies, escapedString.get(),
                             truncated ? " (truncated)" : "");
 
-        REPORT_BYTES(path + NS_LITERAL_CSTRING("gc-heap"),
-            KIND_NONHEAP, info.gcHeap,
-            "A notable string, i.e. one whose copies together use a lot of "
-            "GC heap and malloc heap memory. " MAYBE_INLINE);
-        gcTotal += info.gcHeap;
+        if (info.gcHeap > 0) {
+            REPORT_GC_BYTES(path + NS_LITERAL_CSTRING("gc-heap"),
+                info.gcHeap,
+                "Strings. " MAYBE_INLINE);
+        }
 
         if (info.mallocHeap > 0) {
             REPORT_BYTES(path + NS_LITERAL_CSTRING("malloc-heap"),
                 KIND_HEAP, info.mallocHeap,
-                "Non-inline string characters of a notable string. "
-                MAYBE_OVERALLOCATED);
+                "Non-inline string characters. " MAYBE_OVERALLOCATED);
         }
     }
 
-    ZCREPORT_GC_BYTES(pathPrefix + NS_LITERAL_CSTRING("strings/non-notable/gc-heap"),
-        zStats.stringsGCHeap,
-        "Non-notable strings. " MAYBE_INLINE);
-
-    ZCREPORT_BYTES(pathPrefix + NS_LITERAL_CSTRING("strings/non-notable/malloc-heap"),
-        zStats.stringsMallocHeap,
-        "Non-inline string characters of non-notable strings. "
-        MAYBE_OVERALLOCATED);
+    nsCString nonNotablePath = pathPrefix;
+    nonNotablePath += zStats.isTotals
+                    ? NS_LITERAL_CSTRING("strings/")
+                    : NS_LITERAL_CSTRING("strings/string(<non-notable strings>)/");
+
+    if (zStats.stringInfo.gcHeap > 0) {
+        REPORT_GC_BYTES(nonNotablePath + NS_LITERAL_CSTRING("gc-heap"),
+            zStats.stringInfo.gcHeap,
+            "Strings. " MAYBE_INLINE);
+    }
+
+    if (zStats.stringInfo.mallocHeap > 0) {
+        REPORT_BYTES(nonNotablePath + NS_LITERAL_CSTRING("malloc-heap"),
+            KIND_HEAP, zStats.stringInfo.mallocHeap,
+            "Non-inline string characters. " MAYBE_OVERALLOCATED);
+    }
 
     if (stringsNotableAboutMemoryGCHeap > 0) {
-        ZCREPORT_GC_BYTES(pathPrefix + NS_LITERAL_CSTRING("strings/notable/about-memory/gc-heap"),
+        MOZ_ASSERT(!zStats.isTotals);
+        REPORT_GC_BYTES(pathPrefix + NS_LITERAL_CSTRING("strings/string(<about-memory>)/gc-heap"),
             stringsNotableAboutMemoryGCHeap,
-            "Notable strings that contain the characters '" STRING_LENGTH "', "
-            "which are probably from about:memory itself." MAYBE_INLINE
+            "Strings that contain the characters '" STRING_LENGTH "', which "
+            "are probably from about:memory itself." MAYBE_INLINE
             " We filter them out rather than display them, because displaying "
             "them would create even more such strings every time about:memory "
             "is refreshed.");
     }
 
     if (stringsNotableAboutMemoryMallocHeap > 0) {
-        ZCREPORT_BYTES(pathPrefix + NS_LITERAL_CSTRING("strings/notable/about-memory/malloc-heap"),
-            stringsNotableAboutMemoryMallocHeap,
-            "Non-inline string characters of notable strings that contain "
-            "the characters '" STRING_LENGTH "', which are probably from "
+        MOZ_ASSERT(!zStats.isTotals);
+        REPORT_BYTES(pathPrefix + NS_LITERAL_CSTRING("strings/string(<about-memory>)/malloc-heap"),
+            KIND_HEAP, stringsNotableAboutMemoryMallocHeap,
+            "Non-inline string characters of strings that contain the "
+            "characters '" STRING_LENGTH "', which are probably from "
             "about:memory itself. " MAYBE_OVERALLOCATED
             " We filter them out rather than display them, because displaying "
             "them would create even more such strings every time about:memory "
             "is refreshed.");
     }
 
     if (sundriesGCHeap > 0) {
         // We deliberately don't use ZCREPORT_GC_BYTES here.