Bug 978387 - Mark the ArrayBufferObject view list weakly during minor collections; r=sfink
authorTerrence Cole <terrence@mozilla.com>
Mon, 03 Mar 2014 18:41:58 -0800
changeset 189069 b5899e48b7fbefda79b6d6a06f8ace2ec3f1c5e9
parent 189068 61ae64c9aa0b04612af26e5eade1fee55dbe16de
child 189070 330b86147644082482c6fc7b91454c325b98cc8a
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs978387
milestone30.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 978387 - Mark the ArrayBufferObject view list weakly during minor collections; r=sfink
js/src/gc/Marking.cpp
js/src/gc/Marking.h
js/src/gc/Nursery.cpp
js/src/gc/StoreBuffer.cpp
js/src/vm/ArrayBufferObject.cpp
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -361,19 +361,29 @@ IsAboutToBeFinalized(T **thingp)
      * compartment group.  Rather than do the extra check, we just assert that
      * it's not necessary.
      */
     JS_ASSERT(!(*thingp)->arenaHeader()->allocatedDuringIncremental);
 
     return !(*thingp)->isMarked();
 }
 
+template <typename T>
+T *
+UpdateIfRelocated(JSRuntime *rt, T **thingp)
+{
+    JS_ASSERT(thingp);
+    if (*thingp && rt->isHeapMinorCollecting())
+        IsAboutToBeFinalized<T>(thingp);
+    return *thingp;
+}
+
 #define DeclMarkerImpl(base, type)                                                                \
 void                                                                                              \
-Mark##base(JSTracer *trc, BarrieredPtr<type> *thing, const char *name)                         \
+Mark##base(JSTracer *trc, BarrieredPtr<type> *thing, const char *name)                            \
 {                                                                                                 \
     Mark<type>(trc, thing, name);                                                                 \
 }                                                                                                 \
                                                                                                   \
 void                                                                                              \
 Mark##base##Root(JSTracer *trc, type **thingp, const char *name)                                  \
 {                                                                                                 \
     MarkRoot<type>(trc, thingp, name);                                                            \
@@ -404,33 +414,46 @@ Mark##base##RootRange(JSTracer *trc, siz
                                                                                                   \
 bool                                                                                              \
 Is##base##Marked(type **thingp)                                                                   \
 {                                                                                                 \
     return IsMarked<type>(thingp);                                                                \
 }                                                                                                 \
                                                                                                   \
 bool                                                                                              \
-Is##base##Marked(BarrieredPtr<type> *thingp)                                                   \
+Is##base##Marked(BarrieredPtr<type> *thingp)                                                      \
 {                                                                                                 \
     return IsMarked<type>(thingp->unsafeGet());                                                   \
 }                                                                                                 \
                                                                                                   \
 bool                                                                                              \
 Is##base##AboutToBeFinalized(type **thingp)                                                       \
 {                                                                                                 \
     return IsAboutToBeFinalized<type>(thingp);                                                    \
 }                                                                                                 \
                                                                                                   \
 bool                                                                                              \
-Is##base##AboutToBeFinalized(BarrieredPtr<type> *thingp)                                       \
+Is##base##AboutToBeFinalized(BarrieredPtr<type> *thingp)                                          \
 {                                                                                                 \
     return IsAboutToBeFinalized<type>(thingp->unsafeGet());                                       \
+}                                                                                                 \
+                                                                                                  \
+type *                                                                                            \
+Update##base##IfRelocated(JSRuntime *rt, BarrieredPtr<type> *thingp)                              \
+{                                                                                                 \
+    return UpdateIfRelocated<type>(rt, thingp->unsafeGet());                                      \
+}                                                                                                 \
+                                                                                                  \
+type *                                                                                            \
+Update##base##IfRelocated(JSRuntime *rt, type **thingp)                                           \
+{                                                                                                 \
+    return UpdateIfRelocated<type>(rt, thingp);                                                   \
 }
 
+
 DeclMarkerImpl(BaseShape, BaseShape)
 DeclMarkerImpl(BaseShape, UnownedBaseShape)
 DeclMarkerImpl(JitCode, jit::JitCode)
 DeclMarkerImpl(Object, ArgumentsObject)
 DeclMarkerImpl(Object, ArrayBufferObject)
 DeclMarkerImpl(Object, ArrayBufferViewObject)
 DeclMarkerImpl(Object, SharedArrayBufferObject)
 DeclMarkerImpl(Object, DebugScopeObject)
--- a/js/src/gc/Marking.h
+++ b/js/src/gc/Marking.h
@@ -75,27 +75,35 @@ namespace gc {
  *     indicates whether the object will will be finialized in the current group
  *     of compartments being swept.  Note that this will return false for any
  *     object not in the group of compartments currently being swept, as even if
  *     it is unmarked it may still become marked before it is swept.
  *
  * IsObjectMarked(JSObject **thing);
  *     This function is indended to be used in rare cases in code used to mark
  *     GC things.  It indicates whether the object is currently marked.
+ *
+ * UpdateObjectIfRelocated(JSObject **thingp);
+ *     In some circumstances -- e.g. optional weak marking -- it is necessary
+ *     to look at the pointer before marking it strongly or weakly. In these
+ *     cases, the following must be called to update the pointer before use.
  */
+
 #define DeclMarker(base, type)                                                                    \
-void Mark##base(JSTracer *trc, BarrieredPtr<type> *thing, const char *name);                   \
+void Mark##base(JSTracer *trc, BarrieredPtr<type> *thing, const char *name);                      \
 void Mark##base##Root(JSTracer *trc, type **thingp, const char *name);                            \
 void Mark##base##Unbarriered(JSTracer *trc, type **thingp, const char *name);                     \
 void Mark##base##Range(JSTracer *trc, size_t len, HeapPtr<type> *thing, const char *name);        \
 void Mark##base##RootRange(JSTracer *trc, size_t len, type **thing, const char *name);            \
 bool Is##base##Marked(type **thingp);                                                             \
-bool Is##base##Marked(BarrieredPtr<type> *thingp);                                             \
+bool Is##base##Marked(BarrieredPtr<type> *thingp);                                                \
 bool Is##base##AboutToBeFinalized(type **thingp);                                                 \
-bool Is##base##AboutToBeFinalized(BarrieredPtr<type> *thingp);                                 \
+bool Is##base##AboutToBeFinalized(BarrieredPtr<type> *thingp);                                    \
+type *Update##base##IfRelocated(JSRuntime *rt, BarrieredPtr<type> *thingp);                       \
+type *Update##base##IfRelocated(JSRuntime *rt, type **thingp);
 
 DeclMarker(BaseShape, BaseShape)
 DeclMarker(BaseShape, UnownedBaseShape)
 DeclMarker(JitCode, jit::JitCode)
 DeclMarker(Object, ArgumentsObject)
 DeclMarker(Object, ArrayBufferObject)
 DeclMarker(Object, ArrayBufferViewObject)
 DeclMarker(Object, SharedArrayBufferObject)
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -717,20 +717,20 @@ js::Nursery::collect(JSRuntime *rt, JS::
 
     if (isEmpty())
         return;
 
     TIME_START(total);
 
     AutoStopVerifyingBarriers av(rt, false);
 
-    /* Move objects pointed to by roots from the nursery to the major heap. */
+    // Move objects pointed to by roots from the nursery to the major heap.
     MinorCollectionTracer trc(rt, this);
 
-    /* Mark the store buffer. This must happen first. */
+    // Mark the store buffer. This must happen first.
     StoreBuffer &sb = rt->gcStoreBuffer;
     TIME_START(markValues);
     sb.markValues(&trc);
     TIME_END(markValues);
 
     TIME_START(markCells);
     sb.markCells(&trc);
     TIME_END(markCells);
@@ -766,110 +766,115 @@ js::Nursery::collect(JSRuntime *rt, JS::
     TIME_START(markDebugger);
     Debugger::markAll(&trc);
     TIME_END(markDebugger);
 
     TIME_START(clearNewObjectCache);
     rt->newObjectCache.clearNurseryObjects(rt);
     TIME_END(clearNewObjectCache);
 
-    /*
-     * Most of the work is done here. This loop iterates over objects that have
-     * been moved to the major heap. If these objects have any outgoing pointers
-     * to the nursery, then those nursery objects get moved as well, until no
-     * objects are left to move. That is, we iterate to a fixed point.
-     */
+    // Most of the work is done here. This loop iterates over objects that have
+    // been moved to the major heap. If these objects have any outgoing pointers
+    // to the nursery, then those nursery objects get moved as well, until no
+    // objects are left to move. That is, we iterate to a fixed point.
     TIME_START(collectToFP);
     TenureCountCache tenureCounts;
     collectToFixedPoint(&trc, tenureCounts);
     TIME_END(collectToFP);
 
+    // Update the array buffer object's view lists.
+    TIME_START(sweepArrayBufferViewList);
+    for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next()) {
+        if (c->gcLiveArrayBuffers)
+            ArrayBufferObject::sweep(c);
+    }
+    TIME_END(sweepArrayBufferViewList);
+
+    // Update any slot or element pointers whose destination has been tenured.
     TIME_START(updateJitActivations);
 #ifdef JS_ION
-    /* Update any slot or element pointers whose destination has been tenured. */
     js::jit::UpdateJitActivationsForMinorGC(rt, &trc);
 #endif
     TIME_END(updateJitActivations);
 
-    /* Resize the nursery. */
+    // Resize the nursery.
     TIME_START(resize);
     double promotionRate = trc.tenuredSize / double(allocationEnd() - start());
     if (promotionRate > 0.05)
         growAllocableSpace();
     else if (promotionRate < 0.01)
         shrinkAllocableSpace();
     TIME_END(resize);
 
-    TIME_START(pretenure);
     // If we are promoting the nursery, or exhausted the store buffer with
     // pointers to nursery things, which will force a collection well before
     // the nursery is full, look for object types that are getting promoted
     // excessively and try to pretenure them.
+    TIME_START(pretenure);
     if (pretenureTypes && (promotionRate > 0.8 || reason == JS::gcreason::FULL_STORE_BUFFER)) {
         for (size_t i = 0; i < ArrayLength(tenureCounts.entries); i++) {
             const TenureCount &entry = tenureCounts.entries[i];
             if (entry.count >= 3000)
                 pretenureTypes->append(entry.type); // ignore alloc failure
         }
     }
     TIME_END(pretenure);
 
-    /* Sweep. */
+    // Sweep.
     TIME_START(freeHugeSlots);
     freeHugeSlots(rt);
     TIME_END(freeHugeSlots);
 
     TIME_START(sweep);
     sweep(rt);
     TIME_END(sweep);
 
     TIME_START(clearStoreBuffer);
     rt->gcStoreBuffer.clear();
     TIME_END(clearStoreBuffer);
 
-    /*
-     * We ignore gcMaxBytes when allocating for minor collection. However, if we
-     * overflowed, we disable the nursery. The next time we allocate, we'll fail
-     * because gcBytes >= gcMaxBytes.
-     */
+    // We ignore gcMaxBytes when allocating for minor collection. However, if we
+    // overflowed, we disable the nursery. The next time we allocate, we'll fail
+    // because gcBytes >= gcMaxBytes.
     if (rt->gcBytes >= rt->gcMaxBytes)
         disable();
 
     TIME_END(total);
 
 #ifdef PROFILE_NURSERY
     int64_t totalTime = TIME_TOTAL(total);
 
     if (totalTime >= GCReportThreshold) {
         static bool printedHeader = false;
         if (!printedHeader) {
             fprintf(stderr,
-                    "MinorGC: Reason               PRate  Size Time   mkVals mkClls mkSlts mkWCll mkRVal mkRCll mkGnrc ckTbls mkRntm mkDbgr clrNOC collct updtIn resize pretnr frSlts clrSB  sweep\n");
+                    "MinorGC: Reason               PRate  Size Time   mkVals mkClls mkSlts mkWCll mkRVal mkRCll mkGnrc ckTbls mkRntm mkDbgr clrNOC collct swpABO updtIn resize pretnr frSlts clrSB  sweep\n");
             printedHeader = true;
         }
 
 #define FMT " %6" PRIu64
         fprintf(stderr,
-                "MinorGC: %20s %5.1f%% %4d" FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT "\n",
+                "MinorGC: %20s %5.1f%% %4d" FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT FMT "\n",
                 js::gcstats::ExplainReason(reason),
                 promotionRate * 100,
                 numActiveChunks_,
                 totalTime,
                 TIME_TOTAL(markValues),
                 TIME_TOTAL(markCells),
                 TIME_TOTAL(markSlots),
                 TIME_TOTAL(markWholeCells),
                 TIME_TOTAL(markRelocatableValues),
                 TIME_TOTAL(markRelocatableCells),
                 TIME_TOTAL(markGenericEntries),
                 TIME_TOTAL(checkHashTables),
                 TIME_TOTAL(markRuntime),
                 TIME_TOTAL(markDebugger),
                 TIME_TOTAL(clearNewObjectCache),
                 TIME_TOTAL(collectToFP),
+                TIME_TOTAL(sweepArrayBufferViewList),
                 TIME_TOTAL(updateJitActivations),
                 TIME_TOTAL(resize),
                 TIME_TOTAL(pretenure),
                 TIME_TOTAL(freeHugeSlots),
                 TIME_TOTAL(clearStoreBuffer),
                 TIME_TOTAL(sweep));
 #undef FMT
     }
--- a/js/src/gc/StoreBuffer.cpp
+++ b/js/src/gc/StoreBuffer.cpp
@@ -95,17 +95,17 @@ StoreBuffer::MonoTypeBuffer<T>::handleOv
         if (isAboutToOverflow())
             owner->setAboutToOverflow();
     } else {
          /*
           * A minor GC has already been triggered, so there's no point
           * compacting unless the buffer is totally full.
           */
         if (storage_->availableInCurrentChunk() < sizeof(T))
-            compact(owner);
+            maybeCompact(owner);
     }
 }
 
 template <typename T>
 void
 StoreBuffer::MonoTypeBuffer<T>::compactRemoveDuplicates(StoreBuffer *owner)
 {
     EdgeSet duplicates;
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -816,92 +816,81 @@ ArrayBufferObject::obj_trace(JSTracer *t
      */
     JSObject *delegate = static_cast<JSObject*>(obj->getPrivate());
     if (delegate) {
         JS_SET_TRACING_LOCATION(trc, &obj->privateRef(obj->numFixedSlots()));
         MarkObjectUnbarriered(trc, &delegate, "arraybuffer.delegate");
         obj->setPrivateUnbarriered(delegate);
     }
 
+    if (!IS_GC_MARKING_TRACER(trc) && !trc->runtime->isHeapMinorCollecting())
+        return;
+
     // ArrayBufferObjects need to maintain a list of possibly-weak pointers to
     // their views. The straightforward way to update the weak pointers would
     // be in the views' finalizers, but giving views finalizers means they
     // cannot be swept in the background. This results in a very high
     // performance cost.  Instead, ArrayBufferObjects with a single view hold a
     // strong pointer to the view. This can entrain garbage when the single
     // view becomes otherwise unreachable while the buffer is still live, but
     // this is expected to be rare. ArrayBufferObjects with 0-1 views are
     // expected to be by far the most common cases. ArrayBufferObjects with
     // multiple views are collected into a linked list during collection, and
     // then swept to prune out their dead views.
 
     ArrayBufferObject &buffer = AsArrayBuffer(obj);
-    ArrayBufferViewObject *viewsHead = GetViewList(&buffer);
+    ArrayBufferViewObject *viewsHead = UpdateObjectIfRelocated(trc->runtime,
+                                                               &GetViewListRef(&buffer));
     if (!viewsHead)
         return;
 
-    // During minor collections, mark weak pointers on the buffer strongly.
-    if (trc->runtime->isHeapMinorCollecting()) {
-        MarkObject(trc, &GetViewListRef(&buffer), "arraybuffer.viewlist");
-        ArrayBufferViewObject *prior = GetViewList(&buffer);
-        for (ArrayBufferViewObject *view = prior->nextView();
-             view;
-             prior = view, view = view->nextView())
-        {
-            MarkObjectUnbarriered(trc, &view, "arraybuffer.views");
-            prior->setNextView(view);
-        }
-        return;
-    }
-
+    viewsHead = UpdateObjectIfRelocated(trc->runtime, &GetViewListRef(&buffer));
     ArrayBufferViewObject *firstView = viewsHead;
     if (firstView->nextView() == nullptr) {
         // Single view: mark it, but only if we're actually doing a GC pass
         // right now. Otherwise, the tracing pass for barrier verification will
         // fail if we add another view and the pointer becomes weak.
-        if (IS_GC_MARKING_TRACER(trc))
-            MarkObject(trc, &GetViewListRef(&buffer), "arraybuffer.singleview");
+        MarkObject(trc, &GetViewListRef(&buffer), "arraybuffer.singleview");
     } else {
         // Multiple views: do not mark, but append buffer to list.
-        if (IS_GC_MARKING_TRACER(trc)) {
-            // obj_trace may be called multiple times before sweep(), so avoid
-            // adding this buffer to the list multiple times.
-            if (firstView->bufferLink() == UNSET_BUFFER_LINK) {
-                JS_ASSERT(obj->compartment() == firstView->compartment());
-                ArrayBufferObject **bufList = &obj->compartment()->gcLiveArrayBuffers;
-                firstView->setBufferLink(*bufList);
-                *bufList = &AsArrayBuffer(obj);
-            } else {
+        // obj_trace may be called multiple times before sweep(), so avoid
+        // adding this buffer to the list multiple times.
+        if (firstView->bufferLink() == UNSET_BUFFER_LINK) {
+            JS_ASSERT(obj->compartment() == firstView->compartment());
+            ArrayBufferObject **bufList = &obj->compartment()->gcLiveArrayBuffers;
+            firstView->setBufferLink(*bufList);
+            *bufList = &AsArrayBuffer(obj);
+        } else {
 #ifdef DEBUG
-                bool found = false;
-                for (ArrayBufferObject *p = obj->compartment()->gcLiveArrayBuffers;
-                     p;
-                     p = GetViewList(p)->bufferLink())
+            bool found = false;
+            for (ArrayBufferObject *p = obj->compartment()->gcLiveArrayBuffers;
+                 p;
+                 p = GetViewList(p)->bufferLink())
+            {
+                if (p == obj)
                 {
-                    if (p == obj)
-                    {
-                        JS_ASSERT(!found);
-                        found = true;
-                    }
+                    JS_ASSERT(!found);
+                    found = true;
                 }
+            }
 #endif
-            }
         }
     }
 }
 
-void
+/* static */ void
 ArrayBufferObject::sweep(JSCompartment *compartment)
 {
+    JSRuntime *rt = compartment->runtimeFromMainThread();
     ArrayBufferObject *buffer = compartment->gcLiveArrayBuffers;
     JS_ASSERT(buffer != UNSET_BUFFER_LINK);
     compartment->gcLiveArrayBuffers = nullptr;
 
     while (buffer) {
-        ArrayBufferViewObject *viewsHead = GetViewList(buffer);
+        ArrayBufferViewObject *viewsHead = UpdateObjectIfRelocated(rt, &GetViewListRef(buffer));
         JS_ASSERT(viewsHead);
 
         ArrayBufferObject *nextBuffer = viewsHead->bufferLink();
         JS_ASSERT(nextBuffer != UNSET_BUFFER_LINK);
         viewsHead->setBufferLink(UNSET_BUFFER_LINK);
 
         // Rebuild the list of views of the ArrayBufferObject, discarding dead
         // views.  If there is only one view, it will have already been marked.
@@ -909,17 +898,17 @@ ArrayBufferObject::sweep(JSCompartment *
         ArrayBufferViewObject *view = viewsHead;
         while (view) {
             JS_ASSERT(buffer->compartment() == view->compartment());
             ArrayBufferViewObject *nextView = view->nextView();
             if (!IsObjectAboutToBeFinalized(&view)) {
                 view->setNextView(prevLiveView);
                 prevLiveView = view;
             }
-            view = nextView;
+            view = UpdateObjectIfRelocated(rt, &nextView);
         }
         SetViewList(buffer, prevLiveView);
 
         buffer = nextBuffer;
     }
 }
 
 void