Bug 635873 - Fix shape marking during per-compartment GCs (r=gal,a=dmandelin)
authorBill McCloskey <wmccloskey@mozilla.com>
Tue, 22 Feb 2011 12:45:18 -0800
changeset 63083 0c398bad4c9935f55fd29087b0f09630ecd5fae1
parent 62972 a8926981d45261fcf234feb51f30f580bb644c51
child 63084 4586c38a6a6e196e15b86e3b8905798d6a3b4d26
push id19025
push userrsayre@mozilla.com
push dateFri, 25 Feb 2011 18:00:56 +0000
treeherdermozilla-central@0f1f5cc97ed5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgal, dmandelin
bugs635873
milestone2.0b12pre
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 635873 - Fix shape marking during per-compartment GCs (r=gal,a=dmandelin)
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jsgc.cpp
js/src/jspropertytree.cpp
js/src/jspropertytree.h
js/src/jsscope.cpp
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -458,22 +458,22 @@ JSCompartment::markCrossCompartment(JSTr
         MarkValue(trc, e.front().key, "cross-compartment wrapper");
 }
 
 void
 JSCompartment::mark(JSTracer *trc)
 {
     if (IS_GC_MARKING_TRACER(trc)) {
         JSRuntime *rt = trc->context->runtime;
-        if (rt->gcCurrentCompartment != NULL && rt->gcCurrentCompartment != this)
+
+        if (rt->gcCurrentCompartment && rt->gcCurrentCompartment != this)
             return;
-        
+
         if (marked)
             return;
-        
         marked = true;
     }
 
     if (emptyArgumentsShape)
         emptyArgumentsShape->trace(trc);
     if (emptyBlockShape)
         emptyBlockShape->trace(trc);
     if (emptyCallShape)
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -444,17 +444,17 @@ struct JS_FRIEND_API(JSCompartment) {
 
     js::ToSourceCache            toSourceCache;
 
     JSCompartment(JSRuntime *rt);
     ~JSCompartment();
 
     bool init();
 
-    /* Mark cross-compartment pointers. */
+    /* Mark cross-compartment wrappers. */
     void markCrossCompartment(JSTracer *trc);
 
     /* Mark this compartment's local roots. */
     void mark(JSTracer *trc);
 
     bool wrap(JSContext *cx, js::Value *vp);
     bool wrap(JSContext *cx, JSString **strp);
     bool wrap(JSContext *cx, JSObject **objp);
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2184,19 +2184,22 @@ SweepCompartments(JSContext *cx, JSGCInv
     JSCompartment **end = rt->compartments.end();
     JSCompartment **write = read;
     JS_ASSERT(rt->compartments.length() >= 1);
     JS_ASSERT(*rt->compartments.begin() == rt->atomsCompartment);
 
     while (read < end) {
         JSCompartment *compartment = *read++;
 
-        /* Unmarked compartments containing marked objects don't get deleted, except LAST_CONTEXT GC is performed. */
-        if ((!compartment->isMarked() && compartment->arenaListsAreEmpty())
-            || gckind == GC_LAST_CONTEXT)
+        /*
+         * Unmarked compartments containing marked objects don't get deleted,
+         * except when LAST_CONTEXT GC is performed.
+         */
+        if ((!compartment->isMarked() && compartment->arenaListsAreEmpty()) ||
+            gckind == GC_LAST_CONTEXT)
         {
             JS_ASSERT(compartment->freeLists.isEmpty());
             if (callback)
                 (void) callback(cx, compartment, JSCOMPARTMENT_DESTROY);
             if (compartment->principals)
                 JSPRINCIPALS_DROP(cx, compartment->principals);
             js_delete(compartment);
             continue;
@@ -2351,23 +2354,25 @@ MarkAndSweepCompartment(JSContext *cx, J
     comp->sweep(cx, 0);
 
     comp->finalizeObjectArenaLists(cx);
     TIMESTAMP(sweepObjectEnd);
 
     comp->finalizeStringArenaLists(cx);
     TIMESTAMP(sweepStringEnd);
 
-#ifdef DEBUG
-    /* Make sure that we didn't mark a Shape in another compartment. */
+    /*
+     * Unmark all shapes. Even a per-compartment GC can mark shapes in other
+     * compartments, and we need to clear these bits. See bug 635873. This will
+     * be fixed in bug 569422.
+     */
     for (JSCompartment **c = rt->compartments.begin(); c != rt->compartments.end(); ++c)
-        JS_ASSERT_IF(*c != comp, (*c)->propertyTree.checkShapesAllUnmarked(cx));
+        (*c)->propertyTree.unmarkShapes(cx);
 
     PropertyTree::dumpShapes(cx);
-#endif
 
     /*
      * Destroy arenas after we finished the sweeping so finalizers can safely
      * use js_IsAboutToBeFinalized().
      */
     ExpireGCChunks(rt);
     TIMESTAMP(sweepDestroyEnd);
 
@@ -2716,17 +2721,17 @@ GCUntilDone(JSContext *cx, JSCompartment
     for (JSCompartment **c = rt->compartments.begin(); c != rt->compartments.end(); ++c)
         JS_ASSERT(!(*c)->isMarked());
 
     /*
      * We should not be depending on cx->compartment in the GC, so set it to
      * NULL to look for violations.
      */
     SwitchToCompartment(cx, (JSCompartment *)NULL);
-    
+
     JS_ASSERT(!rt->gcCurrentCompartment);
     rt->gcCurrentCompartment = comp;
 
     METER(rt->gcStats.poke++);
 
     bool firstRun = true;
     rt->gcMarkAndSweep = true;
 #ifdef JS_THREADSAFE
--- a/js/src/jspropertytree.cpp
+++ b/js/src/jspropertytree.cpp
@@ -618,35 +618,32 @@ js::PropertyTree::sweepShapes(JSContext 
 
 #undef RATE
 
         fflush(logfp);
     }
 #endif /* DEBUG */
 }
 
-bool
-js::PropertyTree::checkShapesAllUnmarked(JSContext *cx)
+void
+js::PropertyTree::unmarkShapes(JSContext *cx)
 {
     JSArena **ap = &arenaPool.first.next;
     while (JSArena *a = *ap) {
         Shape *limit = (Shape *) a->avail;
 
         for (Shape *shape = (Shape *) a->base; shape < limit; shape++) {
             /* If the id is null, shape is already on the freelist. */
             if (JSID_IS_VOID(shape->id))
                 continue;
 
-            if (shape->marked())
-                return false;
+            shape->clearMark();
         }
         ap = &a->next;
     }
-
-    return true;
 }
 
 void
 js::PropertyTree::dumpShapes(JSContext *cx)
 {
 #ifdef DEBUG
     JSRuntime *rt = cx->runtime;
 
--- a/js/src/jspropertytree.h
+++ b/js/src/jspropertytree.h
@@ -123,17 +123,17 @@ class PropertyTree
     void finish();
 
     js::Shape *newShapeUnchecked();
     js::Shape *newShape(JSContext *cx);
     js::Shape *getChild(JSContext *cx, js::Shape *parent, const js::Shape &child);
 
     void orphanChildren(js::Shape *shape);
     void sweepShapes(JSContext *cx);
-    bool checkShapesAllUnmarked(JSContext *cx);
+    void unmarkShapes(JSContext *cx);
 
     static void dumpShapes(JSContext *cx);
 #ifdef DEBUG
     static void meter(JSBasicStats *bs, js::Shape *node);
 #endif
 };
 
 } /* namespace js */
--- a/js/src/jsscope.cpp
+++ b/js/src/jsscope.cpp
@@ -1478,21 +1478,16 @@ PrintPropertyMethod(JSTracer *trc, char 
     if (n < bufsize)
         JS_snprintf(buf + n, bufsize - n, " method");
 }
 #endif
 
 void
 Shape::trace(JSTracer *trc) const
 {
-#ifdef DEBUG
-    JSRuntime *rt = trc->context->runtime;
-    JS_ASSERT_IF(rt->gcCurrentCompartment, compartment == rt->gcCurrentCompartment);
-#endif
-
     if (IS_GC_MARKING_TRACER(trc))
         mark();
 
     MarkId(trc, id, "id");
 
     if (attrs & (JSPROP_GETTER | JSPROP_SETTER)) {
         if ((attrs & JSPROP_GETTER) && rawGetter) {
             JS_SET_TRACING_DETAILS(trc, PrintPropertyGetterOrSetter, this, 0);