Bug 1262203 - Skip shape table tracing where possible r=terrence
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 12 Apr 2016 09:44:11 +0100
changeset 330663 acd02b077759bff2cbe13b1898610f72595d13b7
parent 330662 061ad34e7f0f8e87c114f7dd74fb287638a73851
child 330664 fd676a6ad20bc83c19f363d9e05f0af9e98f10d8
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1262203
milestone48.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 1262203 - Skip shape table tracing where possible r=terrence
js/src/gc/Marking.cpp
js/src/jit-test/tests/gc/bug-1259306.js
js/src/vm/Shape.cpp
js/src/vm/Shape.h
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -894,34 +894,40 @@ namespace js {
 template <>
 void
 GCMarker::traverse(AccessorShape* thing) {
     MOZ_CRASH("AccessorShape must be marked as a Shape");
 }
 } // namespace js
 
 template <typename S, typename T>
-void
-js::GCMarker::traverseEdge(S source, T* target)
+static void
+CheckTraversedEdge(S source, T* target)
 {
     // Atoms and Symbols do not have or mark their internal pointers, respectively.
     MOZ_ASSERT(!ThingIsPermanentAtomOrWellKnownSymbol(source));
 
     // The Zones must match, unless the target is an atom.
     MOZ_ASSERT_IF(!ThingIsPermanentAtomOrWellKnownSymbol(target),
                   target->zone()->isAtomsZone() || target->zone() == source->zone());
 
     // Atoms and Symbols do not have access to a compartment pointer, or we'd need
     // to adjust the subsequent check to catch that case.
     MOZ_ASSERT_IF(ThingIsPermanentAtomOrWellKnownSymbol(target), !target->maybeCompartment());
     MOZ_ASSERT_IF(target->zoneFromAnyThread()->isAtomsZone(), !target->maybeCompartment());
     // If we have access to a compartment pointer for both things, they must match.
     MOZ_ASSERT_IF(source->maybeCompartment() && target->maybeCompartment(),
                   source->maybeCompartment() == target->maybeCompartment());
-
+}
+
+template <typename S, typename T>
+void
+js::GCMarker::traverseEdge(S source, T* target)
+{
+    CheckTraversedEdge(source, target);
     traverse(target);
 }
 
 template <typename V, typename S> struct TraverseEdgeFunctor : public VoidDefaultAdaptor<V> {
     template <typename T> void operator()(T t, GCMarker* gcmarker, S s) {
         return gcmarker->traverseEdge(s, t);
     }
 };
@@ -1015,17 +1021,24 @@ Shape::traceChildren(JSTracer* trc)
     if (hasSetterObject())
         TraceManuallyBarrieredEdge(trc, &asAccessorShape().setterObj, "setter");
 }
 inline void
 js::GCMarker::eagerlyMarkChildren(Shape* shape)
 {
     MOZ_ASSERT(shape->isMarked(this->markColor()));
     do {
-        traverseEdge(shape, shape->base());
+        // Special case: if a base shape has a shape table then all its pointers
+        // must point to this shape or an anscestor.  Since these pointers will
+        // be traced by this loop they do not need to be traced here as well.
+        BaseShape* base = shape->base();
+        CheckTraversedEdge(shape, base);
+        if (mark(base))
+            base->traceChildrenSkipShapeTable(this);
+
         traverseEdge(shape, shape->propidRef().get());
 
         // When triggered between slices on belhalf of a barrier, these
         // objects may reside in the nursery, so require an extra check.
         // FIXME: Bug 1157967 - remove the isTenured checks.
         if (shape->hasGetterObject() && shape->getterObject()->isTenured())
             traverseEdge(shape, shape->getterObject());
         if (shape->hasSetterObject() && shape->setterObject()->isTenured())
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-1259306.js
@@ -0,0 +1,15 @@
+if (!('oomTest' in this))
+    quit();
+
+oomTest(() => {
+    var lfGlobal = newGlobal();
+    var lfVarx = `
+        gczeal(8, 1);
+        try {
+            (5).replace(r, () => {});
+        } catch (x) {}
+        gczeal(0);
+    `;
+    lfGlobal.offThreadCompileScript(lfVarx);
+    lfGlobal.runOffThreadScript();
+});
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -1327,28 +1327,39 @@ BaseShape::assertConsistency()
         MOZ_ASSERT(getObjectFlags() == unowned->getObjectFlags());
     }
 #endif
 }
 
 void
 BaseShape::traceChildren(JSTracer* trc)
 {
+    traceChildrenSkipShapeTable(trc);
+    traceShapeTable(trc);
+}
+
+void
+BaseShape::traceChildrenSkipShapeTable(JSTracer* trc)
+{
     assertConsistency();
 
     if (trc->isMarkingTracer())
         compartment()->mark();
 
     if (isOwned())
         TraceEdge(trc, &unowned_, "base");
 
     JSObject* global = compartment()->unsafeUnbarrieredMaybeGlobal();
     if (global)
         TraceManuallyBarrieredEdge(trc, &global, "global");
+}
 
+void
+BaseShape::traceShapeTable(JSTracer* trc)
+{
     if (hasTable())
         table().trace(trc);
 }
 
 void
 JSCompartment::sweepBaseShapeTable()
 {
     baseShapes.sweep();
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -449,26 +449,29 @@ class BaseShape : public gc::TenuredCell
     void assertConsistency();
 
     /* For JIT usage */
     static inline size_t offsetOfFlags() { return offsetof(BaseShape, flags); }
 
     static const JS::TraceKind TraceKind = JS::TraceKind::BaseShape;
 
     void traceChildren(JSTracer* trc);
+    void traceChildrenSkipShapeTable(JSTracer* trc);
 
     void fixupAfterMovingGC() {}
 
   private:
     static void staticAsserts() {
         JS_STATIC_ASSERT(offsetof(BaseShape, clasp_) == offsetof(js::shadow::BaseShape, clasp_));
         static_assert(sizeof(BaseShape) % gc::CellSize == 0,
                       "Things inheriting from gc::Cell must have a size that's "
                       "a multiple of gc::CellSize");
     }
+
+    void traceShapeTable(JSTracer* trc);
 };
 
 class UnownedBaseShape : public BaseShape {};
 
 UnownedBaseShape*
 BaseShape::unowned()
 {
     return isOwned() ? baseUnowned() : toUnowned();