Bug 817218 - Move UnmarkGray to the JS engine (r=mccr8)
authorBill McCloskey <wmccloskey@mozilla.com>
Mon, 03 Dec 2012 11:19:23 -0800
changeset 123914 ce24dbcb88b3796a11b900134826c6f07ab82b07
parent 123913 57a020c3628ba569253ed90c9497eceda27290a0
child 123915 810a5eea090b2f16a094c552dcb1bce8ccf5ac0f
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs817218
milestone20.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 817218 - Move UnmarkGray to the JS engine (r=mccr8)
js/src/gc/Iteration.cpp
js/src/gc/Marking.cpp
js/src/jsapi.cpp
js/src/jscntxt.h
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
js/src/jsgc.cpp
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/src/xpcprivate.h
js/xpconnect/src/xpcpublic.h
--- a/js/src/gc/Iteration.cpp
+++ b/js/src/gc/Iteration.cpp
@@ -101,17 +101,17 @@ js::IterateCells(JSRuntime *rt, JSCompar
         for (CompartmentsIter c(rt); !c.done(); c.next()) {
             for (CellIterUnderGC i(c, thingKind); !i.done(); i.next())
                 cellCallback(rt, data, i.getCell(), traceKind, thingSize);
         }
     }
 }
 
 void
-js::IterateGrayObjects(JSCompartment *compartment, GCThingCallback *cellCallback, void *data)
+js::IterateGrayObjects(JSCompartment *compartment, GCThingCallback cellCallback, void *data)
 {
     JS_ASSERT(compartment);
     AutoPrepareForTracing prep(compartment->rt);
 
     for (size_t finalizeKind = 0; finalizeKind <= FINALIZE_OBJECT_LAST; finalizeKind++) {
         for (CellIterUnderGC i(compartment, AllocKind(finalizeKind)); !i.done(); i.next()) {
             Cell *cell = i.getCell();
             if (cell->isMarked(GRAY))
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -1483,8 +1483,110 @@ js::TraceChildren(JSTracer *trc, void *t
 void
 js::CallTracer(JSTracer *trc, void *thing, JSGCTraceKind kind)
 {
     JS_ASSERT(thing);
     void *tmp = thing;
     MarkKind(trc, &tmp, kind);
     JS_ASSERT(tmp == thing);
 }
+
+static void
+UnmarkGrayGCThing(void *thing)
+{
+    static_cast<js::gc::Cell *>(thing)->unmark(js::gc::GRAY);
+}
+
+struct UnmarkGrayTracer : public JSTracer
+{
+    UnmarkGrayTracer() : tracingShape(false), previousShape(NULL) {}
+    UnmarkGrayTracer(JSTracer *trc, bool tracingShape)
+        : tracingShape(tracingShape), previousShape(NULL)
+    {
+        JS_TracerInit(this, trc->runtime, trc->callback);
+    }
+
+    /* True iff we are tracing the immediate children of a shape. */
+    bool tracingShape;
+
+    /* If tracingShape, shape child or NULL. Otherwise, NULL. */
+    void *previousShape;
+};
+
+/*
+ * The GC and CC are run independently. Consequently, the following sequence of
+ * events can occur:
+ * 1. GC runs and marks an object gray.
+ * 2. Some JS code runs that creates a pointer from a JS root to the gray
+ *    object. If we re-ran a GC at this point, the object would now be black.
+ * 3. Now we run the CC. It may think it can collect the gray object, even
+ *    though it's reachable from the JS heap.
+ *
+ * To prevent this badness, we unmark the gray bit of an object when it is
+ * accessed by callers outside XPConnect. This would cause the object to go
+ * black in step 2 above. This must be done on everything reachable from the
+ * object being returned. The following code takes care of the recursive
+ * re-coloring.
+ */
+static void
+UnmarkGrayChildren(JSTracer *trc, void **thingp, JSGCTraceKind kind)
+{
+    void *thing = *thingp;
+    int stackDummy;
+    if (!JS_CHECK_STACK_SIZE(js::GetNativeStackLimit(trc->runtime), &stackDummy)) {
+        /*
+         * If we run out of stack, we take a more drastic measure: require that
+         * we GC again before the next CC.
+         */
+        trc->runtime->gcGrayBitsValid = false;
+        return;
+    }
+
+    if (!GCThingIsMarkedGray(thing))
+        return;
+
+    UnmarkGrayGCThing(thing);
+
+    /*
+     * Trace children of |thing|. If |thing| and its parent are both shapes,
+     * |thing| will get saved to mPreviousShape without being traced. The parent
+     * will later trace |thing|. This is done to avoid increasing the stack
+     * depth during shape tracing. It is safe to do because a shape can only
+     * have one child that is a shape.
+     */
+    UnmarkGrayTracer *tracer = static_cast<UnmarkGrayTracer *>(trc);
+    UnmarkGrayTracer childTracer(tracer, kind == JSTRACE_SHAPE);
+
+    if (kind != JSTRACE_SHAPE) {
+        JS_TraceChildren(&childTracer, thing, kind);
+        JS_ASSERT(!childTracer.previousShape);
+        return;
+    }
+
+    if (tracer->tracingShape) {
+        JS_ASSERT(!tracer->previousShape);
+        tracer->previousShape = thing;
+        return;
+    }
+
+    do {
+        JS_ASSERT(!GCThingIsMarkedGray(thing));
+        JS_TraceChildren(&childTracer, thing, JSTRACE_SHAPE);
+        thing = childTracer.previousShape;
+        childTracer.previousShape = NULL;
+    } while (thing);
+}
+
+JS_FRIEND_API(void)
+js::UnmarkGrayGCThingRecursively(void *thing, JSGCTraceKind kind)
+{
+    JS_ASSERT(kind != JSTRACE_SHAPE);
+
+    if (!GCThingIsMarkedGray(thing))
+        return;
+
+    UnmarkGrayGCThing(thing);
+
+    JSRuntime *rt = static_cast<Cell *>(thing)->compartment()->rt;
+    UnmarkGrayTracer trc;
+    JS_TracerInit(&trc, rt, UnmarkGrayChildren);
+    JS_TraceChildren(&trc, thing, kind);
+}
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -781,16 +781,17 @@ JSRuntime::JSRuntime(JSUseHelperThreads 
     gcHighFrequencyLowLimitBytes(100 * 1024 * 1024),
     gcHighFrequencyHighLimitBytes(500 * 1024 * 1024),
     gcHighFrequencyHeapGrowthMax(3.0),
     gcHighFrequencyHeapGrowthMin(1.5),
     gcLowFrequencyHeapGrowth(1.5),
     gcDynamicHeapGrowth(false),
     gcDynamicMarkSlice(false),
     gcShouldCleanUpEverything(false),
+    gcGrayBitsValid(false),
     gcIsNeeded(0),
     gcStats(thisFromCtor()),
     gcNumber(0),
     gcStartNumber(0),
     gcIsFull(false),
     gcTriggerReason(gcreason::NO_REASON),
     gcStrictCompartmentChecking(false),
     gcDisableStrictProxyCheckingCount(0),
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -645,16 +645,22 @@ struct JSRuntime : js::RuntimeFriendFiel
     double              gcLowFrequencyHeapGrowth;
     bool                gcDynamicHeapGrowth;
     bool                gcDynamicMarkSlice;
 
     /* During shutdown, the GC needs to clean up every possible object. */
     bool                gcShouldCleanUpEverything;
 
     /*
+     * The gray bits can become invalid if UnmarkGray overflows the stack. A
+     * full GC will reset this bit, since it fills in all the gray bits.
+     */
+    bool                gcGrayBitsValid;
+
+    /*
      * These flags must be kept separate so that a thread requesting a
      * compartment GC doesn't cancel another thread's concurrent request for a
      * full GC.
      */
     volatile uintptr_t  gcIsNeeded;
 
     js::gcstats::Statistics gcStats;
 
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -559,31 +559,31 @@ js::TraceWeakMaps(WeakMapTracer *trc)
 
 JS_FRIEND_API(bool)
 js::GCThingIsMarkedGray(void *thing)
 {
     JS_ASSERT(thing);
     return reinterpret_cast<gc::Cell *>(thing)->isMarked(gc::GRAY);
 }
 
+extern JS_FRIEND_API(bool)
+js::AreGCGrayBitsValid(JSRuntime *rt)
+{
+    return rt->gcGrayBitsValid;
+}
+
 JS_FRIEND_API(JSGCTraceKind)
 js::GCThingTraceKind(void *thing)
 {
     JS_ASSERT(thing);
     return gc::GetGCThingTraceKind(thing);
 }
 
 JS_FRIEND_API(void)
-js::UnmarkGrayGCThing(void *thing)
-{
-    static_cast<js::gc::Cell *>(thing)->unmark(js::gc::GRAY);
-}
-
-JS_FRIEND_API(void)
-js::VisitGrayWrapperTargets(JSCompartment *comp, GCThingCallback *callback, void *closure)
+js::VisitGrayWrapperTargets(JSCompartment *comp, GCThingCallback callback, void *closure)
 {
     for (JSCompartment::WrapperEnum e(comp); !e.empty(); e.popFront()) {
         gc::Cell *thing = e.front().key.wrapped;
         if (thing->isMarked(gc::GRAY))
             callback(closure, thing);
     }
 }
 
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -264,36 +264,43 @@ struct WeakMapTracer {
 };
 
 extern JS_FRIEND_API(void)
 TraceWeakMaps(WeakMapTracer *trc);
 
 extern JS_FRIEND_API(bool)
 GCThingIsMarkedGray(void *thing);
 
-JS_FRIEND_API(void)
-UnmarkGrayGCThing(void *thing);
+extern JS_FRIEND_API(bool)
+AreGCGrayBitsValid(JSRuntime *rt);
+
+/*
+ * Unsets the gray bit for anything reachable from |thing|. |kind| should not be
+ * JSTRACE_SHAPE. |thing| should be non-null.
+ */
+extern JS_FRIEND_API(void)
+UnmarkGrayGCThingRecursively(void *thing, JSGCTraceKind kind);
 
 typedef void
-(GCThingCallback)(void *closure, void *gcthing);
+(*GCThingCallback)(void *closure, void *gcthing);
 
 extern JS_FRIEND_API(void)
-VisitGrayWrapperTargets(JSCompartment *comp, GCThingCallback *callback, void *closure);
+VisitGrayWrapperTargets(JSCompartment *comp, GCThingCallback callback, void *closure);
 
 extern JS_FRIEND_API(JSObject *)
 GetWeakmapKeyDelegate(JSObject *key);
 
 JS_FRIEND_API(JSGCTraceKind)
 GCThingTraceKind(void *thing);
 
 /*
  * Invoke cellCallback on every gray JS_OBJECT in the given compartment.
  */
 extern JS_FRIEND_API(void)
-IterateGrayObjects(JSCompartment *compartment, GCThingCallback *cellCallback, void *data);
+IterateGrayObjects(JSCompartment *compartment, GCThingCallback cellCallback, void *data);
 
 /*
  * Shadow declarations of JS internal structures, for access by inline access
  * functions below. Do not use these structures in any other way. When adding
  * new fields for access by inline methods, make sure to add static asserts to
  * the original header file to ensure that offsets are consistent.
  */
 namespace shadow {
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3513,18 +3513,23 @@ EndSweepPhase(JSRuntime *rt, JSGCInvocat
 
         bool isFull = true;
         for (CompartmentsIter c(rt); !c.done(); c.next()) {
             if (!c->isCollecting()) {
                 rt->gcIsFull = false;
                 break;
             }
         }
+
         if (rt->gcFinalizeCallback)
             rt->gcFinalizeCallback(&fop, JSFINALIZE_COLLECTION_END, !isFull);
+
+        /* If we finished a full GC, then the gray bits are correct. */
+        if (isFull)
+            rt->gcGrayBitsValid = true;
     }
 
     /* Set up list of compartments for sweeping of background things. */
     JS_ASSERT(!rt->gcSweepingCompartments);
     for (GCCompartmentsIter c(rt); !c.done(); c.next())
         AddGraphNode(rt->gcSweepingCompartments, c.get());
 
     /* If not sweeping on background thread then we must do it here. */
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -74,17 +74,16 @@ const char XPC_XPCONNECT_CONTRACTID[]   
 /***************************************************************************/
 
 nsXPConnect::nsXPConnect()
     :   mRuntime(nullptr),
         mInterfaceInfoManager(do_GetService(NS_INTERFACEINFOMANAGER_SERVICE_CONTRACTID)),
         mDefaultSecurityManager(nullptr),
         mDefaultSecurityManagerFlags(0),
         mShuttingDown(false),
-        mNeedGCBeforeCC(true),
         mEventDepth(0),
         mCycleCollectionContext(nullptr)
 {
     mRuntime = XPCJSRuntime::newXPCJSRuntime(this);
 
     nsCycleCollector_registerJSRuntime(this);
 
     char* reportableEnv = PR_GetEnv("MOZ_REPORT_ALL_JS_EXCEPTIONS");
@@ -312,17 +311,17 @@ nsresult
 nsXPConnect::GetInfoForName(const char * name, nsIInterfaceInfo** info)
 {
     return FindInfo(NameTester, name, mInterfaceInfoManager, info);
 }
 
 bool
 nsXPConnect::NeedCollect()
 {
-    return !!mNeedGCBeforeCC;
+    return !js::AreGCGrayBitsValid(GetRuntime()->GetJSRuntime());
 }
 
 void
 nsXPConnect::Collect(uint32_t reason)
 {
     // We're dividing JS objects into 2 categories:
     //
     // 1. "real" roots, held by the JS engine itself or rooted through the root
@@ -585,108 +584,16 @@ nsXPConnect::GetParticipant()
 
 JSBool
 xpc_GCThingIsGrayCCThing(void *thing)
 {
     return AddToCCKind(js::GCThingTraceKind(thing)) &&
            xpc_IsGrayGCThing(thing);
 }
 
-struct UnmarkGrayTracer : public JSTracer
-{
-    UnmarkGrayTracer() : mTracingShape(false), mPreviousShape(nullptr) {}
-    UnmarkGrayTracer(JSTracer *trc, bool aTracingShape)
-        : mTracingShape(aTracingShape), mPreviousShape(nullptr)
-    {
-        JS_TracerInit(this, trc->runtime, trc->callback);
-    }
-    bool mTracingShape; // true iff we are tracing the immediate children of a shape
-    void *mPreviousShape; // If mTracingShape, shape child or NULL. Otherwise, NULL.
-};
-
-/*
- * The GC and CC are run independently. Consequently, the following sequence of
- * events can occur:
- * 1. GC runs and marks an object gray.
- * 2. Some JS code runs that creates a pointer from a JS root to the gray
- *    object. If we re-ran a GC at this point, the object would now be black.
- * 3. Now we run the CC. It may think it can collect the gray object, even
- *    though it's reachable from the JS heap.
- *
- * To prevent this badness, we unmark the gray bit of an object when it is
- * accessed by callers outside XPConnect. This would cause the object to go
- * black in step 2 above. This must be done on everything reachable from the
- * object being returned. The following code takes care of the recursive
- * re-coloring.
- */
-static void
-UnmarkGrayChildren(JSTracer *trc, void **thingp, JSGCTraceKind kind)
-{
-    void *thing = *thingp;
-    int stackDummy;
-    if (!JS_CHECK_STACK_SIZE(js::GetNativeStackLimit(trc->runtime), &stackDummy)) {
-        /*
-         * If we run out of stack, we take a more drastic measure: require that
-         * we GC again before the next CC.
-         */
-        nsXPConnect* xpc = nsXPConnect::GetXPConnect();
-        xpc->EnsureGCBeforeCC();
-        return;
-    }
-
-    if (!xpc_IsGrayGCThing(thing))
-        return;
-
-    js::UnmarkGrayGCThing(thing);
-
-    /*
-     * Trace children of |thing|. If |thing| and its parent are both shapes, |thing| will
-     * get saved to mPreviousShape without being traced. The parent will later
-     * trace |thing|. This is done to avoid increasing the stack depth during shape
-     * tracing. It is safe to do because a shape can only have one child that is a shape.
-     */
-    UnmarkGrayTracer *tracer = static_cast<UnmarkGrayTracer*>(trc);
-    UnmarkGrayTracer childTracer(tracer, kind == JSTRACE_SHAPE);
-
-    if (kind != JSTRACE_SHAPE) {
-        JS_TraceChildren(&childTracer, thing, kind);
-        MOZ_ASSERT(!childTracer.mPreviousShape);
-        return;
-    }
-
-    if (tracer->mTracingShape) {
-        MOZ_ASSERT(!tracer->mPreviousShape);
-        tracer->mPreviousShape = thing;
-        return;
-    }
-
-    do {
-        MOZ_ASSERT(!xpc_IsGrayGCThing(thing));
-        JS_TraceChildren(&childTracer, thing, JSTRACE_SHAPE);
-        thing = childTracer.mPreviousShape;
-        childTracer.mPreviousShape = nullptr;
-    } while (thing);
-}
-
-void
-xpc_UnmarkGrayGCThingRecursive(void *thing, JSGCTraceKind kind)
-{
-    MOZ_ASSERT(thing, "Don't pass me null!");
-    MOZ_ASSERT(kind != JSTRACE_SHAPE, "UnmarkGrayGCThingRecursive not intended for Shapes");
-
-    // Unmark.
-    js::UnmarkGrayGCThing(thing);
-
-    // Trace children.
-    UnmarkGrayTracer trc;
-    JSRuntime *rt = nsXPConnect::GetRuntimeInstance()->GetJSRuntime();
-    JS_TracerInit(&trc, rt, UnmarkGrayChildren);
-    JS_TraceChildren(&trc, thing, kind);
-}
-
 struct TraversalTracer : public JSTracer
 {
     TraversalTracer(nsCycleCollectionTraversalCallback &aCb) : cb(aCb)
     {
     }
     nsCycleCollectionTraversalCallback &cb;
 };
 
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -517,19 +517,16 @@ public:
     static void InitStatics() { gSelf = nullptr; gOnceAliveNowDead = false; }
     // Called by module code on dll shutdown.
     static void ReleaseXPConnectSingleton();
 
     virtual ~nsXPConnect();
 
     JSBool IsShuttingDown() const {return mShuttingDown;}
 
-    void EnsureGCBeforeCC() { mNeedGCBeforeCC = true; }
-    void ClearGCBeforeCC() { mNeedGCBeforeCC = false; }
-
     nsresult GetInfoForIID(const nsIID * aIID, nsIInterfaceInfo** info);
     nsresult GetInfoForName(const char * name, nsIInterfaceInfo** info);
 
     // nsCycleCollectionLanguageRuntime
     virtual bool NotifyLeaveMainThread();
     virtual void NotifyEnterCycleCollectionThread();
     virtual void NotifyLeaveCycleCollectionThread();
     virtual void NotifyEnterMainThread();
@@ -574,17 +571,16 @@ private:
     static nsXPConnect*      gSelf;
     static JSBool            gOnceAliveNowDead;
 
     XPCJSRuntime*            mRuntime;
     nsCOMPtr<nsIInterfaceInfoSuperManager> mInterfaceInfoManager;
     nsIXPCSecurityManager*   mDefaultSecurityManager;
     uint16_t                 mDefaultSecurityManagerFlags;
     JSBool                   mShuttingDown;
-    JSBool                   mNeedGCBeforeCC;
 
     // nsIThreadInternal doesn't remember which observers it called
     // OnProcessNextEvent on when it gets around to calling AfterProcessNextEvent.
     // So if XPConnect gets initialized mid-event (which can happen), we'll get
     // an 'after' notification without getting an 'on' notification. If we don't
     // watch out for this, we'll do an unmatched |pop| on the context stack.
     uint16_t                   mEventDepth;
     nsAutoPtr<XPCCallContext> mCycleCollectionContext;
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -131,26 +131,22 @@ xpc_IsGrayGCThing(void *thing)
     return js::GCThingIsMarkedGray(thing);
 }
 
 // The cycle collector only cares about some kinds of GCthings that are
 // reachable from an XPConnect root. Implemented in nsXPConnect.cpp.
 extern JSBool
 xpc_GCThingIsGrayCCThing(void *thing);
 
-// Implemented in nsXPConnect.cpp.
-extern void
-xpc_UnmarkGrayGCThingRecursive(void *thing, JSGCTraceKind kind);
-
 // Unmark gray for known-nonnull cases
 MOZ_ALWAYS_INLINE void
 xpc_UnmarkNonNullGrayObject(JSObject *obj)
 {
     if (xpc_IsGrayGCThing(obj))
-        xpc_UnmarkGrayGCThingRecursive(obj, JSTRACE_OBJECT);
+        js::UnmarkGrayGCThingRecursively(obj, JSTRACE_OBJECT);
     else if (js::IsIncrementalBarrierNeededOnObject(obj))
         js::IncrementalReferenceBarrier(obj);
 }
 
 // Remove the gray color from the given JSObject and any other objects that can
 // be reached through it.
 MOZ_ALWAYS_INLINE JSObject *
 xpc_UnmarkGrayObject(JSObject *obj)
@@ -160,17 +156,17 @@ xpc_UnmarkGrayObject(JSObject *obj)
     return obj;
 }
 
 inline JSScript *
 xpc_UnmarkGrayScript(JSScript *script)
 {
     if (script) {
         if (xpc_IsGrayGCThing(script))
-            xpc_UnmarkGrayGCThingRecursive(script, JSTRACE_SCRIPT);
+            js::UnmarkGrayGCThingRecursively(script, JSTRACE_SCRIPT);
         else if (js::IsIncrementalBarrierNeededOnScript(script))
             js::IncrementalReferenceBarrier(script);
     }
     return script;
 }
 
 inline JSContext *
 xpc_UnmarkGrayContext(JSContext *cx)