Bug 1194418 - Use only JS::ubi::* interfaces in census analyses. r=sfink
authorNick Fitzgerald <fitzgen@gmail.com>
Mon, 17 Aug 2015 15:01:00 -0400
changeset 291025 9f979aa382a916bd2beb516c16c3c796d325a8fe
parent 291024 defee685fedd563607313025d6bf9f2817c18f75
child 291026 f786a3ae04103553ea724bf9271c562a6fb10b0c
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1194418
milestone43.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 1194418 - Use only JS::ubi::* interfaces in census analyses. r=sfink In order to run a census analysis on anything other than the live heap graph (the notable example being offline heap snapshots) then the census analysis cannot unwrap |ubi::Node|s into their live heap thing referents.
js/src/vm/DebuggerMemory.cpp
js/src/vm/SavedFrame.h
js/src/vm/SavedStacks.cpp
--- a/js/src/vm/DebuggerMemory.cpp
+++ b/js/src/vm/DebuggerMemory.cpp
@@ -1142,31 +1142,31 @@ class ByUbinodeType : public CountType {
         return true;
     }
 };
 
 
 // A count type that categorizes nodes by the JS stack under which they were
 // allocated.
 class ByAllocationStack : public CountType {
-    typedef HashMap<SavedFrame*, CountBasePtr, DefaultHasher<SavedFrame*>,
+    typedef HashMap<JS::ubi::StackFrame, CountBasePtr, DefaultHasher<JS::ubi::StackFrame>,
                     SystemAllocPolicy> Table;
     typedef Table::Entry Entry;
 
     struct Count : public CountBase {
-        // NOTE: You may look up entries in this table by SavedFrame key only
-        // during traversal, NOT ONCE TRAVERSAL IS COMPLETE. Once traversal is
-        // complete, you may only iterate over it.
+        // NOTE: You may look up entries in this table by JS::ubi::StackFrame
+        // key only during traversal, NOT ONCE TRAVERSAL IS COMPLETE. Once
+        // traversal is complete, you may only iterate over it.
         //
-        // In this hash table, keys are JSObjects, and we use JSObject identity
-        // (that is, address identity) as key identity. The normal way to
-        // support such a table is to make the trace function notice keys that
-        // have moved and re-key them in the table. However, our trace function
-        // does *not* rehash; the first GC may render the hash table
-        // unsearchable.
+        // In this hash table, keys are JSObjects (with some indirection), and
+        // we use JSObject identity (that is, address identity) as key
+        // identity. The normal way to support such a table is to make the trace
+        // function notice keys that have moved and re-key them in the
+        // table. However, our trace function does *not* rehash; the first GC
+        // may render the hash table unsearchable.
         //
         // This is as it should be:
         //
         // First, the heap traversal phase needs lookups by key to work. But no
         // GC may ever occur during a traversal; this is enforced by the
         // JS::ubi::BreadthFirst template. So the traceCount function doesn't
         // need to do anything to help traversal; it never even runs then.
         //
@@ -1211,51 +1211,45 @@ class ByAllocationStack : public CountTy
     }
 
     void traceCount(CountBase& countBase, JSTracer* trc) override {
         Count& count= static_cast<Count&>(countBase);
         for (Table::Range r = count.table.all(); !r.empty(); r.popFront()) {
             // Trace our child Counts.
             r.front().value()->trace(trc);
 
-            // Trace the SavedFrame that is this entry's key. Do not re-key if
+            // Trace the StackFrame that is this entry's key. Do not re-key if
             // it has moved; see comments for ByAllocationStack::Count::table.
-            SavedFrame** keyPtr = const_cast<SavedFrame**>(&r.front().key());
-            TraceRoot(trc, keyPtr, "Debugger.Memory.prototype.census byAllocationStack count key");
+            const JS::ubi::StackFrame* key = &r.front().key();
+            auto& k = *const_cast<JS::ubi::StackFrame*>(key);
+            k.trace(trc);
         }
         count.noStack->trace(trc);
     }
 
     void destructCount(CountBase& countBase) override {
         Count& count = static_cast<Count&>(countBase);
         count.~Count();
     }
 
     bool count(CountBase& countBase, const Node& node) override {
         Count& count = static_cast<Count&>(countBase);
         count.total_++;
 
-        SavedFrame* allocationStack = nullptr;
-        if (node.is<JSObject>()) {
-            JSObject* metadata = GetObjectMetadata(node.as<JSObject>());
-            if (metadata && metadata->is<SavedFrame>())
-                allocationStack = &metadata->as<SavedFrame>();
-        }
-        // If any other types had allocation site data, we could retrieve it
-        // here.
-
         // If we do have an allocation stack for this node, include it in the
         // count for that stack.
-        if (allocationStack) {
-            Table::AddPtr p = count.table.lookupForAdd(allocationStack);
+        if (node.hasAllocationStack()) {
+            auto allocationStack = node.allocationStack();
+            auto p = count.table.lookupForAdd(allocationStack);
             if (!p) {
                 CountBasePtr stackCount(entryType->makeCount());
                 if (!stackCount || !count.table.add(p, allocationStack, Move(stackCount)))
                     return false;
             }
+            MOZ_ASSERT(p);
             return p->value()->count(node);
         }
 
         // Otherwise, count it in the "no stack" category.
         return count.noStack->count(node);
     }
 
     bool report(CountBase& countBase, MutableHandleValue report) override {
@@ -1278,28 +1272,32 @@ class ByAllocationStack : public CountTy
         qsort(entries.begin(), entries.length(), sizeof(*entries.begin()), compareEntries<Entry>);
 
         // Now build the result by iterating over the sorted vector.
         Rooted<MapObject*> map(cx, MapObject::create(cx));
         if (!map)
             return false;
         for (Entry** entryPtr = entries.begin(); entryPtr < entries.end(); entryPtr++) {
             Entry& entry = **entryPtr;
+            MOZ_ASSERT(entry.key());
 
-            MOZ_ASSERT(entry.key());
-            RootedValue stack(cx, ObjectValue(*entry.key()));
-            if (!cx->compartment()->wrap(cx, &stack))
+            RootedObject stack(cx);
+            if (!entry.key().constructSavedFrameStack(cx, &stack) ||
+                !cx->compartment()->wrap(cx, &stack))
+            {
                 return false;
+            }
+            RootedValue stackVal(cx, ObjectValue(*stack));
 
             CountBasePtr& stackCount = entry.value();
             RootedValue stackReport(cx);
             if (!stackCount->report(&stackReport))
                 return false;
 
-            if (!MapObject::set(cx, map, stack, stackReport))
+            if (!MapObject::set(cx, map, stackVal, stackReport))
                 return false;
         }
 
         if (count.noStack->total_ > 0) {
             RootedValue noStackReport(cx);
             if (!count.noStack->report(&noStackReport))
                 return false;
             RootedValue noStack(cx, StringValue(cx->names().noStack));
--- a/js/src/vm/SavedFrame.h
+++ b/js/src/vm/SavedFrame.h
@@ -135,19 +135,21 @@ class ConcreteStackFrame<SavedFrame> : p
     }
 
     AtomOrTwoByteChars functionDisplayName() const override {
         auto name = get().getFunctionDisplayName();
         return AtomOrTwoByteChars(name);
     }
 
     void trace(JSTracer* trc) override {
-        JSObject* obj = &get();
-        js::TraceManuallyBarrieredEdge(trc, &obj, "ConcreteStackFrame<SavedFrame>::ptr");
-        ptr = obj;
+        JSObject* prev = &get();
+        JSObject* next = prev;
+        js::TraceRoot(trc, &next, "ConcreteStackFrame<SavedFrame>::ptr");
+        if (next != prev)
+            ptr = next;
     }
 
     bool isSelfHosted() const override { return get().isSelfHosted(); }
 
     bool isSystem() const override;
 
     bool constructSavedFrameStack(JSContext* cx,
                                  MutableHandleObject outSavedFrameStack) const override;
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -147,17 +147,18 @@ struct SavedFrame::Lookup {
         asyncCause(asyncCause),
         parent(parent),
         principals(principals),
         framePtr(framePtr),
         pc(pc),
         activation(activation)
     {
         MOZ_ASSERT(source);
-        MOZ_ASSERT(activation);
+        MOZ_ASSERT_IF(framePtr.isSome(), pc);
+        MOZ_ASSERT_IF(framePtr.isSome(), activation);
     }
 
     explicit Lookup(SavedFrame& savedFrame)
       : source(savedFrame.getSource()),
         line(savedFrame.getLine()),
         column(savedFrame.getColumn()),
         functionDisplayName(savedFrame.getFunctionDisplayName()),
         asyncCause(savedFrame.getAsyncCause()),