Bug 1189490 - Part 2: Stop using mozilla::LinkedList for the allocations and tenure promotions logs and use js::TraceableFifo instead; r=terrence

js/src/vm/Debugger.cpp

@@ -357,15 +357,15 @@ Debugger::Debugger(JSContext* cx, NativeObject* dbg)
   : object(dbg),
     uncaughtExceptionHook(nullptr),
     enabled(true),
+    allowUnobservedAsmJS(false),
     observedGCs(cx),
+    tenurePromotionsLog(cx),
     trackingTenurePromotions(false),
-    tenurePromotionsLogLength(0),
     maxTenurePromotionsLogLength(DEFAULT_MAX_LOG_LENGTH),
     tenurePromotionsLogOverflowed(false),
-    allowUnobservedAsmJS(false),
+    allocationsLog(cx),
     trackingAllocationSites(false),
     allocationSamplingProbability(1.0),
-    allocationsLogLength(0),
     maxAllocationsLogLength(DEFAULT_MAX_LOG_LENGTH),
     allocationsLogOverflowed(false),
     frames(cx->runtime()),
@@ -390,8 +390,8 @@ Debugger::Debugger(JSContext* cx, NativeObject* dbg)
 Debugger::~Debugger()
 {
     MOZ_ASSERT_IF(debuggees.initialized(), debuggees.empty());
-    emptyAllocationsLog();
+    allocationsLog.clear();
-    emptyTenurePromotionsLog();
+    tenurePromotionsLog.clear();
 
     /*
      * Since the inactive state for this link is a singleton cycle, it's always
@@ -1701,7 +1701,7 @@ Debugger::isDebuggee(const JSCompartment* compartment) const
     return compartment->isDebuggee() && debuggees.has(compartment->maybeGlobal());
 }
 
-Debugger::TenurePromotionsEntry::TenurePromotionsEntry(JSRuntime* rt, JSObject& obj, double when)
+Debugger::TenurePromotionsLogEntry::TenurePromotionsLogEntry(JSRuntime* rt, JSObject& obj, double when)
   : className(obj.getClass()->name),
     when(when),
     frame(getObjectAllocationSite(obj)),
@@ -1712,43 +1712,17 @@ Debugger::TenurePromotionsEntry::TenurePromotionsEntry(JSRuntime* rt, JSObject&
 void
 Debugger::logTenurePromotion(JSRuntime* rt, JSObject& obj, double when)
 {
-    auto* entry = js_new<TenurePromotionsEntry>(rt, obj, when);
+    if (!tenurePromotionsLog.emplaceBack(rt, obj, when))
-    if (!entry)
         CrashAtUnhandlableOOM("Debugger::logTenurePromotion");
 
-    tenurePromotionsLog.insertBack(entry);
+    if (tenurePromotionsLog.length() > maxTenurePromotionsLogLength) {
-    if (tenurePromotionsLogLength >= maxTenurePromotionsLogLength) {
+        if (!tenurePromotionsLog.popFront())
-        js_delete(tenurePromotionsLog.popFirst());
+            CrashAtUnhandlableOOM("Debugger::logTenurePromotion");
+        MOZ_ASSERT(tenurePromotionsLog.length() == maxTenurePromotionsLogLength);
         tenurePromotionsLogOverflowed = true;
-    } else {
-        tenurePromotionsLogLength++;
     }
 }
 
-/* static */ Debugger::AllocationSite*
-Debugger::AllocationSite::create(JSContext* cx, HandleObject frame, double when, HandleObject obj)
-{
-    assertSameCompartment(cx, frame);
-
-    RootedAtom ctorName(cx);
-    {
-        AutoCompartment ac(cx, obj);
-        if (!obj->constructorDisplayAtom(cx, &ctorName))
-            return nullptr;
-    }
-
-    AllocationSite* allocSite = cx->new_<AllocationSite>(frame, when);
-    if (!allocSite)
-        return nullptr;
-
-    allocSite->className = obj->getClass()->name;
-    allocSite->ctorName = ctorName.get();
-    allocSite->size = JS::ubi::Node(obj.get()).size(cx->runtime()->debuggerMallocSizeOf);
-
-    return allocSite;
-}
-
-
 bool
 Debugger::appendAllocationSite(JSContext* cx, HandleObject obj, HandleSavedFrame frame,
                                double when)
@@ -1760,38 +1734,34 @@ Debugger::appendAllocationSite(JSContext* cx, HandleObject obj, HandleSavedFrame
     if (!cx->compartment()->wrap(cx, &wrappedFrame))
         return false;
 
-    AllocationSite* allocSite = AllocationSite::create(cx, wrappedFrame, when, obj);
+    RootedAtom ctorName(cx);
-    if (!allocSite)
+    {
+        AutoCompartment ac(cx, obj);
+        if (!obj->constructorDisplayAtom(cx, &ctorName))
+            return false;
+    }
+
+    auto className = obj->getClass()->name;
+    auto size = JS::ubi::Node(obj.get()).size(cx->runtime()->debuggerMallocSizeOf);
+
+    if (!allocationsLog.emplaceBack(wrappedFrame, when, className, ctorName, size))
+    {
+        ReportOutOfMemory(cx);
         return false;
+    }
 
-    allocationsLog.insertBack(allocSite);
+    if (allocationsLog.length() > maxAllocationsLogLength) {
-
+        if (!allocationsLog.popFront()) {
-    if (allocationsLogLength >= maxAllocationsLogLength) {
+            ReportOutOfMemory(cx);
-        js_delete(allocationsLog.popFirst());
+            return false;
+        }
+        MOZ_ASSERT(allocationsLog.length() == maxAllocationsLogLength);
         allocationsLogOverflowed = true;
-    } else {
-        allocationsLogLength++;
     }
 
     return true;
 }
 
-void
-Debugger::emptyAllocationsLog()
-{
-    while (!allocationsLog.isEmpty())
-        js_delete(allocationsLog.popFirst());
-    allocationsLogLength = 0;
-}
-
-void
-Debugger::emptyTenurePromotionsLog()
-{
-    while (!tenurePromotionsLog.isEmpty())
-        js_delete(tenurePromotionsLog.popFirst());
-    tenurePromotionsLogLength = 0;
-}
-
 JSTrapStatus
 Debugger::firePromiseHook(JSContext* cx, Hook hook, HandleObject promise, MutableHandleValue vp)
 {
@@ -2332,7 +2302,7 @@ Debugger::removeAllocationsTrackingForAllDebuggees()
     for (WeakGlobalObjectSet::Range r = debuggees.all(); !r.empty(); r.popFront()) {
         Debugger::removeAllocationsTracking(*r.front().get());
     }
-    emptyAllocationsLog();
+    allocationsLog.clear();
 }
 
 
@@ -2351,19 +2321,7 @@ Debugger::markCrossCompartmentEdges(JSTracer* trc)
     // `Debugger::logTenurePromotion`, we can't hold onto CCWs inside the log,
     // and instead have unwrapped cross-compartment edges. We need to be sure to
     // mark those here.
-    traceTenurePromotionsLog(trc);
+    TenurePromotionsLog::trace(&tenurePromotionsLog, trc);
-}
-
-/*
- * Trace every entry in the promoted to tenured heap log.
- */
-void
-Debugger::traceTenurePromotionsLog(JSTracer* trc)
-{
-    for (TenurePromotionsEntry* e = tenurePromotionsLog.getFirst(); e; e = e->getNext()) {
-        if (e->frame)
-            TraceEdge(trc, &e->frame, "Debugger::tenurePromotionsLog SavedFrame");
-    }
 }
 
 /*
@@ -2544,17 +2502,8 @@ Debugger::trace(JSTracer* trc)
         TraceEdge(trc, &frameobj, "live Debugger.Frame");
     }
 
-    /*
+    AllocationsLog::trace(&allocationsLog, trc);
-     * Mark every allocation site in our allocation log.
+    TenurePromotionsLog::trace(&tenurePromotionsLog, trc);
-     */
-    for (AllocationSite* s = allocationsLog.getFirst(); s; s = s->getNext()) {
-        if (s->frame)
-            TraceEdge(trc, &s->frame, "allocation log SavedFrame");
-        if (s->ctorName)
-            TraceEdge(trc, &s->ctorName, "allocation log constructor name");
-    }
-
-    traceTenurePromotionsLog(trc);
 
     /* Trace the weak map from JSScript instances to Debugger.Script objects. */
     scripts.trace(trc);

js/src/vm/Debugger.h

@@ -24,6 +24,7 @@
 #include "js/HashTable.h"
 #include "vm/GlobalObject.h"
 #include "vm/SavedStacks.h"
+#include "js/TraceableFifo.h"
 
 enum JSTrapStatus {
     JSTRAP_ERROR,
@@ -283,63 +284,72 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
     void logTenurePromotion(JSRuntime* rt, JSObject& obj, double when);
     static JSObject* getObjectAllocationSite(JSObject& obj);
 
-  private:
+    struct TenurePromotionsLogEntry : public JS::Traceable
-    HeapPtrNativeObject object;         /* The Debugger object. Strong reference. */
-    WeakGlobalObjectSet debuggees;      /* Debuggee globals. Cross-compartment weak references. */
-    JS::ZoneSet debuggeeZones; /* Set of zones that we have debuggees in. */
-    js::HeapPtrObject uncaughtExceptionHook; /* Strong reference. */
-    bool enabled;
-    JSCList breakpoints;                /* Circular list of all js::Breakpoints in this debugger */
-
-    // The set of GC numbers for which one or more of this Debugger's observed
-    // debuggees participated in.
-    js::HashSet<uint64_t> observedGCs;
-
-    struct TenurePromotionsEntry : public mozilla::LinkedListElement<TenurePromotionsEntry>
     {
-        TenurePromotionsEntry(JSRuntime* rt, JSObject& obj, double when);
+        TenurePromotionsLogEntry(JSRuntime* rt, JSObject& obj, double when);
 
         const char* className;
         double when;
         RelocatablePtrObject frame;
         size_t size;
+
+        static void trace(TenurePromotionsLogEntry* e, JSTracer* trc) {
+            if (e->frame)
+                TraceEdge(trc, &e->frame, "Debugger::TenurePromotionsLogEntry::frame");
+        }
     };
 
-    using TenurePromotionsLog = mozilla::LinkedList<TenurePromotionsEntry>;
+    struct AllocationsLogEntry : public JS::Traceable
-    TenurePromotionsLog tenurePromotionsLog;
-    bool trackingTenurePromotions;
-    size_t tenurePromotionsLogLength;
-    size_t maxTenurePromotionsLogLength;
-    bool tenurePromotionsLogOverflowed;
-
-    struct AllocationSite : public mozilla::LinkedListElement<AllocationSite>
     {
-        AllocationSite(HandleObject frame, double when)
+        AllocationsLogEntry(HandleObject frame, double when, const char* className,
+                            HandleAtom ctorName, size_t size)
             : frame(frame),
               when(when),
-              className(nullptr),
+              className(className),
-              ctorName(nullptr),
+              ctorName(ctorName),
-              size(0)
+              size(size)
         {
             MOZ_ASSERT_IF(frame, UncheckedUnwrap(frame)->is<SavedFrame>());
         };
 
-        static AllocationSite* create(JSContext* cx, HandleObject frame, double when,
-                                      HandleObject obj);
-
         RelocatablePtrObject frame;
         double when;
         const char* className;
         RelocatablePtrAtom ctorName;
         size_t size;
-    };
-    typedef mozilla::LinkedList<AllocationSite> AllocationSiteList;
 
+        static void trace(AllocationsLogEntry* e, JSTracer* trc) {
+            if (e->frame)
+                TraceEdge(trc, &e->frame, "Debugger::AllocationsLogEntry::frame");
+            if (e->ctorName)
+                TraceEdge(trc, &e->ctorName, "Debugger::AllocationsLogEntry::ctorName");
+        }
+    };
+
+  private:
+    HeapPtrNativeObject object;         /* The Debugger object. Strong reference. */
+    WeakGlobalObjectSet debuggees;      /* Debuggee globals. Cross-compartment weak references. */
+    JS::ZoneSet debuggeeZones; /* Set of zones that we have debuggees in. */
+    js::HeapPtrObject uncaughtExceptionHook; /* Strong reference. */
+    bool enabled;
     bool allowUnobservedAsmJS;
+    JSCList breakpoints;                /* Circular list of all js::Breakpoints in this debugger */
+
+    // The set of GC numbers for which one or more of this Debugger's observed
+    // debuggees participated in.
+    js::HashSet<uint64_t> observedGCs;
+
+    using TenurePromotionsLog = js::TraceableFifo<TenurePromotionsLogEntry>;
+    TenurePromotionsLog tenurePromotionsLog;
+    bool trackingTenurePromotions;
+    size_t maxTenurePromotionsLogLength;
+    bool tenurePromotionsLogOverflowed;
+
+    using AllocationsLog = js::TraceableFifo<AllocationsLogEntry>;
+
+    AllocationsLog allocationsLog;
     bool trackingAllocationSites;
     double allocationSamplingProbability;
-    AllocationSiteList allocationsLog;
-    size_t allocationsLogLength;
     size_t maxAllocationsLogLength;
     bool allocationsLogOverflowed;
 
@@ -347,8 +357,6 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
 
     bool appendAllocationSite(JSContext* cx, HandleObject obj, HandleSavedFrame frame,
                               double when);
-    void emptyAllocationsLog();
-    void emptyTenurePromotionsLog();
 
     /*
      * Recompute the set of debuggee zones based on the set of debuggee globals.
@@ -503,7 +511,6 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
     void trace(JSTracer* trc);
     static void finalize(FreeOp* fop, JSObject* obj);
     void markCrossCompartmentEdges(JSTracer* tracer);
-    void traceTenurePromotionsLog(JSTracer* trc);
 
     static const Class jsclass;
 
@@ -920,6 +927,20 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
     Debugger & operator=(const Debugger&) = delete;
 };
 
+template<>
+struct DefaultTracer<Debugger::TenurePromotionsLogEntry> {
+    static void trace(JSTracer* trc, Debugger::TenurePromotionsLogEntry* e, const char*) {
+        Debugger::TenurePromotionsLogEntry::trace(e, trc);
+    }
+};
+
+template<>
+struct DefaultTracer<Debugger::AllocationsLogEntry> {
+    static void trace(JSTracer* trc, Debugger::AllocationsLogEntry* e, const char*) {
+        Debugger::AllocationsLogEntry::trace(e, trc);
+    }
+};
+
 class BreakpointSite {
     friend class Breakpoint;
     friend struct ::JSCompartment;

js/src/vm/DebuggerMemory.cpp

@@ -185,7 +185,7 @@ DebuggerMemory::drainAllocationsLog(JSContext* cx, unsigned argc, Value* vp)
         return false;
     }
 
-    size_t length = dbg->allocationsLogLength;
+    size_t length = dbg->allocationsLog.length();
 
     RootedArrayObject result(cx, NewDenseFullyAllocatedArray(cx, length));
     if (!result)
@@ -197,21 +197,21 @@ DebuggerMemory::drainAllocationsLog(JSContext* cx, unsigned argc, Value* vp)
         if (!obj)
             return false;
 
-        // Don't pop the AllocationSite yet. The queue's links are followed by
+        // Don't pop the AllocationsLogEntry yet. The queue's links are followed
-        // the GC to find the AllocationSite, but are not barriered, so we must
+        // by the GC to find the AllocationsLogEntry, but are not barriered, so
-        // edit them with great care. Use the queue entry in place, and then
+        // we must edit them with great care. Use the queue entry in place, and
-        // pop and delete together.
+        // then pop and delete together.
-        Debugger::AllocationSite* allocSite = dbg->allocationsLog.getFirst();
+        Debugger::AllocationsLogEntry& entry = dbg->allocationsLog.front();
 
-        RootedValue frame(cx, ObjectOrNullValue(allocSite->frame));
+        RootedValue frame(cx, ObjectOrNullValue(entry.frame));
         if (!DefineProperty(cx, obj, cx->names().frame, frame))
             return false;
 
-        RootedValue timestampValue(cx, NumberValue(allocSite->when));
+        RootedValue timestampValue(cx, NumberValue(entry.when));
         if (!DefineProperty(cx, obj, cx->names().timestamp, timestampValue))
             return false;
 
-        RootedString className(cx, Atomize(cx, allocSite->className, strlen(allocSite->className)));
+        RootedString className(cx, Atomize(cx, entry.className, strlen(entry.className)));
         if (!className)
             return false;
         RootedValue classNameValue(cx, StringValue(className));
@@ -219,26 +219,27 @@ DebuggerMemory::drainAllocationsLog(JSContext* cx, unsigned argc, Value* vp)
             return false;
 
         RootedValue ctorName(cx, NullValue());
-        if (allocSite->ctorName)
+        if (entry.ctorName)
-            ctorName.setString(allocSite->ctorName);
+            ctorName.setString(entry.ctorName);
         if (!DefineProperty(cx, obj, cx->names().constructor, ctorName))
             return false;
 
-        RootedValue size(cx, NumberValue(allocSite->size));
+        RootedValue size(cx, NumberValue(entry.size));
         if (!DefineProperty(cx, obj, cx->names().size, size))
             return false;
 
         result->setDenseElement(i, ObjectValue(*obj));
 
-        // Pop the front queue entry, and delete it immediately, so that
+        // Pop the front queue entry, and delete it immediately, so that the GC
-        // the GC sees the AllocationSite's RelocatablePtr barriers run
+        // sees the AllocationsLogEntry's RelocatablePtr barriers run atomically
-        // atomically with the change to the graph (the queue link).
+        // with the change to the graph (the queeue link).
-        MOZ_ALWAYS_TRUE(dbg->allocationsLog.popFirst() == allocSite);
+        if (!dbg->allocationsLog.popFront()) {
-        js_delete(allocSite);
+            ReportOutOfMemory(cx);
+            return false;
+        }
     }
 
     dbg->allocationsLogOverflowed = false;
-    dbg->allocationsLogLength = 0;
     args.rval().setObject(*result);
     return true;
 }
@@ -272,9 +273,11 @@ DebuggerMemory::setMaxAllocationsLogLength(JSContext* cx, unsigned argc, Value*
     Debugger* dbg = memory->getDebugger();
     dbg->maxAllocationsLogLength = max;
 
-    while (dbg->allocationsLogLength > dbg->maxAllocationsLogLength) {
+    while (dbg->allocationsLog.length() > dbg->maxAllocationsLogLength) {
-        js_delete(dbg->allocationsLog.getFirst());
+        if (!dbg->allocationsLog.popFront()) {
-        dbg->allocationsLogLength--;
+            ReportOutOfMemory(cx);
+            return false;
+        }
     }
 
     args.rval().setUndefined();
@@ -352,7 +355,7 @@ DebuggerMemory::drainTenurePromotionsLog(JSContext* cx, unsigned argc, Value* vp
         return false;
     }
 
-    size_t length = dbg->tenurePromotionsLogLength;
+    size_t length = dbg->tenurePromotionsLog.length();
 
     RootedArrayObject result(cx, NewDenseFullyAllocatedArray(cx, length));
     if (!result)
@@ -368,41 +371,42 @@ DebuggerMemory::drainTenurePromotionsLog(JSContext* cx, unsigned argc, Value* vp
         // followed by the GC to find the TenurePromotionsEntry, but are not
         // barriered, so we must edit them with great care. Use the queue entry
         // in place, and then pop and delete together.
-        auto* entry = dbg->tenurePromotionsLog.getFirst();
+        auto& entry = dbg->tenurePromotionsLog.front();
 
-        RootedValue frame(cx, ObjectOrNullValue(entry->frame));
+        RootedValue frame(cx, ObjectOrNullValue(entry.frame));
         if (!cx->compartment()->wrap(cx, &frame) ||
             !DefineProperty(cx, obj, cx->names().frame, frame))
         {
             return false;
         }
 
-        RootedValue timestampValue(cx, NumberValue(entry->when));
+        RootedValue timestampValue(cx, NumberValue(entry.when));
         if (!DefineProperty(cx, obj, cx->names().timestamp, timestampValue))
             return false;
 
-        RootedString className(cx, Atomize(cx, entry->className, strlen(entry->className)));
+        RootedString className(cx, Atomize(cx, entry.className, strlen(entry.className)));
         if (!className)
             return false;
         RootedValue classNameValue(cx, StringValue(className));
         if (!DefineProperty(cx, obj, cx->names().class_, classNameValue))
             return false;
 
-        RootedValue sizeValue(cx, NumberValue(entry->size));
+        RootedValue sizeValue(cx, NumberValue(entry.size));
         if (!DefineProperty(cx, obj, cx->names().size, sizeValue))
             return false;
 
         result->setDenseElement(i, ObjectValue(*obj));
 
-        // Pop the front queue entry, and delete it immediately, so that
+        // Pop the front queue entry, and delete it immediately, so that the GC
-        // the GC sees the TenurePromotionsEntry's RelocatablePtr barriers run
+        // sees the TenurePromotionsEntry's RelocatablePtr barriers run
         // atomically with the change to the graph (the queue link).
-        MOZ_ALWAYS_TRUE(dbg->tenurePromotionsLog.popFirst() == entry);
+        if (!dbg->tenurePromotionsLog.popFront()) {
-        js_delete(entry);
+            ReportOutOfMemory(cx);
+            return false;
+        }
     }
 
     dbg->tenurePromotionsLogOverflowed = false;
-    dbg->tenurePromotionsLogLength = 0;
     args.rval().setObject(*result);
     return true;
 }
@@ -436,9 +440,11 @@ DebuggerMemory::setMaxTenurePromotionsLogLength(JSContext* cx, unsigned argc, Va
     Debugger* dbg = memory->getDebugger();
     dbg->maxTenurePromotionsLogLength = max;
 
-    while (dbg->tenurePromotionsLogLength > dbg->maxAllocationsLogLength) {
+    while (dbg->tenurePromotionsLog.length() > dbg->maxAllocationsLogLength) {
-        js_delete(dbg->tenurePromotionsLog.getFirst());
+        if (!dbg->tenurePromotionsLog.popFront()) {
-        dbg->tenurePromotionsLogLength--;
+            ReportOutOfMemory(cx);
+            return false;
+        }
     }
 
     args.rval().setUndefined();