Bug 707051 - Change MarkChildren for shapes (r=igor)

js/src/jsapi.cpp

@@ -4180,7 +4180,7 @@ JS_NextProperty(JSContext *cx, JSObject *iterobj, jsid *idp)
             JS_ASSERT(shape->isEmptyShape());
             *idp = JSID_VOID;
         } else {
-            iterobj->setPrivate(const_cast<Shape *>(shape->previous()));
+            iterobj->setPrivate(const_cast<Shape *>(shape->previous().get()));
             *idp = shape->propid();
         }
     } else {

js/src/jsfriendapi.cpp

@@ -143,6 +143,12 @@ JS_WrapPropertyDescriptor(JSContext *cx, js::PropertyDescriptor *desc)
     return cx->compartment->wrap(cx, desc);
 }
 
+JS_FRIEND_API(void *)
+JS_TraceShapeChildrenAcyclic(JSTracer *trc, void *shape)
+{
+    return (void *)MarkShapeChildrenAcyclic(trc, (const Shape *)shape);
+}
+
 AutoPreserveCompartment::AutoPreserveCompartment(JSContext *cx
                                                  JS_GUARD_OBJECT_NOTIFIER_PARAM_NO_INIT)
   : cx(cx), oldCompartment(cx->compartment)

js/src/jsfriendapi.h

@@ -85,6 +85,13 @@ JS_GetCustomIteratorCount(JSContext *cx);
 extern JS_FRIEND_API(JSBool)
 JS_NondeterministicGetWeakMapKeys(JSContext *cx, JSObject *obj, JSObject **ret);
 
+/*
+ * Marks all the children of a shape except the parent, which avoids using
+ * unbounded stack space. Returns the parent.
+ */
+extern JS_FRIEND_API(void *)
+JS_TraceShapeChildrenAcyclic(JSTracer *trc, void *shape);
+
 enum {
     JS_TELEMETRY_GC_REASON,
     JS_TELEMETRY_GC_IS_COMPARTMENTAL,

js/src/jsgcmark.cpp

@@ -961,16 +961,36 @@ MarkChildren(JSTracer *trc, JSScript *script)
         script->types->trace(trc);
 }
 
+const Shape *
+MarkShapeChildrenAcyclic(JSTracer *trc, const Shape *shape)
+{
+    /*
+     * This function is used by the cycle collector to ensure that we use O(1)
+     * stack space when building the CC graph. It must avoid traversing through
+     * an unbounded number of shapes before reaching an object. (Objects are
+     * added to the CC graph, so reaching one terminates the recursion.)
+     *
+     * Traversing through shape->base() will use bounded space. All but one of
+     * the fields of BaseShape is an object, and objects terminate the
+     * recursion. An owned BaseShape may point to an unowned BaseShape, but
+     * unowned BaseShapes will not point to any other shapes. So the recursion
+     * is bounded.
+     */
+    MarkBaseShapeUnbarriered(trc, shape->base(), "base");
+    MarkIdUnbarriered(trc, shape->maybePropid(), "propid");
+    return shape->previous();
+}
+
 void
 MarkChildren(JSTracer *trc, const Shape *shape)
 {
-restart:
-    MarkBaseShapeUnbarriered(trc, shape->base(), "base");
-    MarkIdUnbarriered(trc, shape->maybePropid(), "propid");
-
-    shape = shape->previous();
-    if (shape)
-        goto restart;
+    /*
+     * We ignore the return value of MarkShapeChildrenAcyclic and use
+     * shape->previous() instead so that the return value has MarkablePtr type.
+     */
+    MarkShapeChildrenAcyclic(trc, shape);
+    if (shape->previous())
+        MarkShape(trc, shape->previous(), "parent");
 }
 
 void

js/src/jsgcmark.h

@@ -196,6 +196,13 @@ MarkChildren(JSTracer *trc, JSScript *script);
 void
 MarkChildren(JSTracer *trc, JSXML *xml);
 
+/*
+ * Marks all the children of a shape except the parent, which avoids using
+ * unbounded stack space. Returns the parent.
+ */
+const Shape *
+MarkShapeChildrenAcyclic(JSTracer *trc, const Shape *shape);
+
 /*
  * Use function overloading to decide which function should be called based on
  * the type of the object. The static type is used at compile time to link to

js/src/jsscope.h

@@ -592,7 +592,7 @@ struct Shape : public js::gc::Cell
         return !(flags & NON_NATIVE);
     }
 
-    const js::Shape *previous() const {
+    const HeapPtrShape &previous() const {
         return parent;
     }
 

js/xpconnect/src/nsXPConnect.cpp

@@ -765,6 +765,14 @@ struct TraversalTracer : public JSTracer
 static void
 NoteJSChild(JSTracer *trc, void *thing, JSGCTraceKind kind)
 {
+    /*
+     * This function needs to be careful to avoid stack overflow. Normally, when
+     * AddToCCKind is true, the recursion terminates immediately as we just add
+     * |thing| to the CC graph. So overflow is only possible when there are long
+     * chains of non-AddToCCKind GC things. Currently, this only can happen via
+     * shape parent pointers. The special JSTRACE_SHAPE case below handles
+     * parent pointers iteratively, rather than recursively, to avoid overflow.
+     */
     if (AddToCCKind(kind)) {
         TraversalTracer *tracer = static_cast<TraversalTracer*>(trc);
 
@@ -792,6 +800,10 @@ NoteJSChild(JSTracer *trc, void *thing, JSGCTraceKind kind)
         }
 #endif
         tracer->cb.NoteScriptChild(nsIProgrammingLanguage::JAVASCRIPT, thing);
+    } else if (kind == JSTRACE_SHAPE) {
+        do {
+            thing = JS_TraceShapeChildrenAcyclic(trc, thing);
+        } while (thing);
     } else if (kind != JSTRACE_STRING) {
         JS_TraceChildren(trc, thing, kind);
     }