Bug 658041 - Stack based marking for JSRopes. r=igor

js/src/jit-test/tests/basic/testBug614653.js

@@ -1,8 +1,8 @@
 /* Bug 614653 - This test .2 seconds with the fix, 20 minutes without. */
-for (var i = 0; i < 100; ++i) {
+for (var i = 0; i < 10; ++i) {
     var arr = [];
     var s = "abcdefghijklmnop";
-    for (var i = 0; i < 50000; ++i) {
+    for (var j = 0; j < 50000; ++j) {
         s = "<" + s + ">";
         arr.push(s);
     }

js/src/jit-test/tests/basic/testRopeMarking.js

@@ -0,0 +1,13 @@
+for (var i = 0; i < 10; ++i) {
+    var arr = [];
+    var s = "abcdefghijklmnop";
+    for (var j = 0; j < 5000; ++j) {
+        s = "<" + s + ">";
+        arr.push(s);
+    }
+    gc();
+    for (var j = 0; j < 5000; ++j) {
+        arr[j].search("a");
+    }
+    gc();
+}

js/src/jscntxt.h

@@ -427,6 +427,7 @@ struct JSRuntime {
 
     /* Pre-allocated space for the GC mark stacks. Pointer type ensures alignment. */
     void                *gcMarkStackObjs[js::OBJECT_MARK_STACK_SIZE / sizeof(void *)];
+    void                *gcMarkStackRopes[js::ROPES_MARK_STACK_SIZE / sizeof(void *)];
     void                *gcMarkStackXMLs[js::XML_MARK_STACK_SIZE / sizeof(void *)];
     void                *gcMarkStackLarges[js::LARGE_MARK_STACK_SIZE / sizeof(void *)];
 

js/src/jsgc.cpp

@@ -1514,6 +1514,7 @@ GCMarker::GCMarker(JSContext *cx)
   : color(0),
     unmarkedArenaStackTop(MarkingDelay::stackBottom()),
     objStack(cx->runtime->gcMarkStackObjs, sizeof(cx->runtime->gcMarkStackObjs)),
+    ropeStack(cx->runtime->gcMarkStackRopes, sizeof(cx->runtime->gcMarkStackRopes)),
     xmlStack(cx->runtime->gcMarkStackXMLs, sizeof(cx->runtime->gcMarkStackXMLs)),
     largeStack(cx->runtime->gcMarkStackLarges, sizeof(cx->runtime->gcMarkStackLarges))
 {

js/src/jsgc.h

@@ -1185,6 +1185,7 @@ struct LargeMarkItem
 };
 
 static const size_t OBJECT_MARK_STACK_SIZE = 32768 * sizeof(JSObject *);
+static const size_t ROPES_MARK_STACK_SIZE = 1024 * sizeof(JSString *);
 static const size_t XML_MARK_STACK_SIZE = 1024 * sizeof(JSXML *);
 static const size_t LARGE_MARK_STACK_SIZE = 64 * sizeof(LargeMarkItem);
 
@@ -1212,6 +1213,7 @@ struct GCMarker : public JSTracer {
 #endif
 
     MarkStack<JSObject *> objStack;
+    MarkStack<JSRope *> ropeStack;
     MarkStack<JSXML *> xmlStack;
     MarkStack<LargeMarkItem> largeStack;
 
@@ -1234,7 +1236,10 @@ struct GCMarker : public JSTracer {
     void markDelayedChildren();
 
     bool isMarkStackEmpty() {
-        return objStack.isEmpty() && xmlStack.isEmpty() && largeStack.isEmpty();
+        return objStack.isEmpty() &&
+               ropeStack.isEmpty() &&
+               xmlStack.isEmpty() &&
+               largeStack.isEmpty();
     }
 
     JS_FRIEND_API(void) drainMarkStack();
@@ -1244,6 +1249,11 @@ struct GCMarker : public JSTracer {
             delayMarkingChildren(obj);
     }
 
+    void pushRope(JSRope *rope) {
+        if (!ropeStack.push(rope))
+            delayMarkingChildren(rope);
+    }
+
     void pushXML(JSXML *xml) {
         if (!xmlStack.push(xml))
             delayMarkingChildren(xml);

js/src/jsgcmark.cpp

@@ -527,6 +527,34 @@ restart:
         goto restart;
 }
 
+static inline void
+ScanRope(GCMarker *gcmarker, JSRope *rope)
+{
+    JS_ASSERT_IF(gcmarker->context->runtime->gcCurrentCompartment,
+                 rope->compartment() == gcmarker->context->runtime->gcCurrentCompartment
+                 || rope->compartment() == gcmarker->context->runtime->atomsCompartment);
+    JS_ASSERT(rope->isMarked());
+
+    JSString *leftChild = NULL;
+    do {
+        JSString *rightChild = rope->rightChild();
+
+        if (rightChild->isRope()) {
+            if (rightChild->markIfUnmarked())
+                gcmarker->pushRope(&rightChild->asRope());
+        } else {
+            rightChild->asLinear().mark(gcmarker);
+        }
+        leftChild = rope->leftChild();
+
+        if (leftChild->isLinear()) {
+            leftChild->asLinear().mark(gcmarker);
+            return;
+        }
+        rope = &leftChild->asRope();
+    } while (leftChild->markIfUnmarked());
+}
+
 static inline void
 PushMarkStack(GCMarker *gcmarker, JSString *str)
 {
@@ -534,7 +562,13 @@ PushMarkStack(GCMarker *gcmarker, JSString *str)
                  str->compartment() == gcmarker->context->runtime->gcCurrentCompartment
                  || str->compartment() == gcmarker->context->runtime->atomsCompartment);
 
-    str->mark(gcmarker);
+    if (str->isLinear()) {
+        str->asLinear().mark(gcmarker);
+    } else {
+        JS_ASSERT(str->isRope());
+        if (str->markIfUnmarked())
+            ScanRope(gcmarker, &str->asRope());
+    }
 }
 
 static const uintN LARGE_OBJECT_CHUNK_SIZE = 2048;
@@ -721,6 +755,9 @@ void
 GCMarker::drainMarkStack()
 {
     while (!isMarkStackEmpty()) {
+        while (!ropeStack.isEmpty())
+            ScanRope(this, ropeStack.pop());
+
         while (!objStack.isEmpty())
             ScanObject(this, objStack.pop());
 

js/src/jsstr.cpp

@@ -111,26 +111,6 @@ JSString::isExternal() const
     return is_external;
 }
 
-static JS_ALWAYS_INLINE JSString *
-Tag(JSRope *str)
-{
-    JS_ASSERT(!(size_t(str) & 1));
-    return (JSString *)(size_t(str) | 1);
-}
-
-static JS_ALWAYS_INLINE bool
-Tagged(JSString *str)
-{
-    return (size_t(str) & 1) != 0;
-}
-
-static JS_ALWAYS_INLINE JSRope *
-Untag(JSString *str)
-{
-    JS_ASSERT((size_t(str) & 1) == 1);
-    return (JSRope *)(size_t(str) & ~size_t(1));
-}
-
 void
 JSLinearString::mark(JSTracer *)
 {
@@ -139,64 +119,6 @@ JSLinearString::mark(JSTracer *)
         str = str->asDependent().base();
 }
 
-void
-JSString::mark(JSTracer *trc)
-{
-    if (isLinear()) {
-        asLinear().mark(trc);
-        return;
-    }
-
-    /*
-     * This function must not fail, so a simple stack-based traversal must not
-     * be used (since it may oom if the stack grows large). Instead, strings
-     * are temporarily mutated to embed parent pointers as they are traversed.
-     * This algorithm is homomorphic to JSString::flatten.
-     */
-    JSRope *str = &asRope();
-    JSRope *parent = NULL;
-    first_visit_node: {
-        if (!str->markIfUnmarked())
-            goto finish_node;
-        JS_ASSERT(!Tagged(str->d.u1.left) && !Tagged(str->d.s.u2.right));
-        JSString *left = str->d.u1.left;
-        if (left->isRope()) {
-            str->d.u1.left = Tag(parent);
-            parent = str;
-            str = &left->asRope();
-            goto first_visit_node;
-        }
-        left->asLinear().mark(trc);
-    }
-    visit_right_child: {
-        JSString *right = str->d.s.u2.right;
-        if (right->isRope()) {
-            str->d.s.u2.right = Tag(parent);
-            parent = str;
-            str = &right->asRope();
-            goto first_visit_node;
-        }
-        right->asLinear().mark(trc);
-    }
-    finish_node: {
-        if (!parent)
-            return;
-        if (Tagged(parent->d.u1.left)) {
-            JS_ASSERT(!Tagged(parent->d.s.u2.right));
-            JSRope *nextParent = Untag(parent->d.u1.left);
-            parent->d.u1.left = str;
-            str = parent;
-            parent = nextParent;
-            goto visit_right_child;
-        }
-        JSRope *nextParent = Untag(parent->d.s.u2.right);
-        parent->d.s.u2.right = str;
-        str = parent;
-        parent = nextParent;
-        goto finish_node;
-    }
-}
-
 static JS_ALWAYS_INLINE size_t
 RopeCapacityFor(size_t length)
 {

js/src/jsstr.h

@@ -371,10 +371,6 @@ class JSString : public js::gc::Cell
 
     inline void finalize(JSContext *cx);
 
-    /* Called during GC for any string. */
-
-    void mark(JSTracer *trc);
-
     /* Offsets for direct field from jit code. */
 
     static size_t offsetOfLengthAndFlags() {
@@ -413,9 +409,10 @@ JS_STATIC_ASSERT(sizeof(JSRope) == sizeof(JSString));
 class JSLinearString : public JSString
 {
     friend class JSString;
-    void mark(JSTracer *trc);
 
   public:
+    void mark(JSTracer *trc);
+
     JS_ALWAYS_INLINE
     const jschar *chars() const {
         JS_ASSERT(isLinear());