Bug 862606 - Shift around some marking for brain transplants (r=bhackett)
authorBill McCloskey <wmccloskey@mozilla.com>
Mon, 22 Apr 2013 14:04:10 -0700
changeset 140468 ec44739db921d9060558817d5a93722cbb07dd0b
parent 140467 fdaa0659fe13283cd81955309fefd7e1b4889f13
child 140469 6ac63545817c72c74000acf91f59c18b0bf1694f
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs862606
milestone23.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 862606 - Shift around some marking for brain transplants (r=bhackett)
js/src/jsapi.cpp
js/src/jscntxt.h
js/src/jscompartment.cpp
js/src/jsgc.cpp
js/src/jsgc.h
js/src/jsobj.cpp
js/src/jswrapper.cpp
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -809,17 +809,19 @@ JSRuntime::JSRuntime(JSUseHelperThreads 
     gcGrayBitsValid(false),
     gcIsNeeded(0),
     gcStats(thisFromCtor()),
     gcNumber(0),
     gcStartNumber(0),
     gcIsFull(false),
     gcTriggerReason(JS::gcreason::NO_REASON),
     gcStrictCompartmentChecking(false),
+#ifdef DEBUG
     gcDisableStrictProxyCheckingCount(0),
+#endif
     gcIncrementalState(gc::NO_INCREMENTAL),
     gcLastMarkSlice(false),
     gcSweepOnBackgroundThread(false),
     gcFoundBlackGrayEdges(false),
     gcSweepingZones(NULL),
     gcZoneGroupIndex(0),
     gcZoneGroups(NULL),
     gcCurrentZoneGroup(NULL),
@@ -1628,16 +1630,17 @@ JS_TransplantObject(JSContext *cx, JSObj
     RootedObject origobj(cx, origobjArg);
     RootedObject target(cx, targetArg);
     AssertHeapIsIdle(cx);
     JS_ASSERT(origobj != target);
     JS_ASSERT(!IsCrossCompartmentWrapper(origobj));
     JS_ASSERT(!IsCrossCompartmentWrapper(target));
 
     AutoMaybeTouchDeadZones agc(cx);
+    AutoDisableProxyCheck adpc(cx->runtime);
 
     JSCompartment *destination = target->compartment();
     RootedValue origv(cx, ObjectValue(*origobj));
     RootedObject newIdentity(cx);
 
     if (origobj->compartment() == destination) {
         // If the original object is in the same compartment as the
         // destination, then we know that we won't find a wrapper in the
@@ -1701,16 +1704,17 @@ js_TransplantObjectWithWrapper(JSContext
                                JSObject *targetwrapperArg)
 {
     RootedObject origobj(cx, origobjArg);
     RootedObject origwrapper(cx, origwrapperArg);
     RootedObject targetobj(cx, targetobjArg);
     RootedObject targetwrapper(cx, targetwrapperArg);
 
     AutoMaybeTouchDeadZones agc(cx);
+    AutoDisableProxyCheck adpc(cx->runtime);
 
     AssertHeapIsIdle(cx);
     JS_ASSERT(!IsCrossCompartmentWrapper(origobj));
     JS_ASSERT(!IsCrossCompartmentWrapper(origwrapper));
     JS_ASSERT(!IsCrossCompartmentWrapper(targetobj));
     JS_ASSERT(!IsCrossCompartmentWrapper(targetwrapper));
 
     RootedObject newWrapper(cx);
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -951,23 +951,27 @@ struct JSRuntime : public JS::shadow::Ru
     JS::gcreason::Reason gcTriggerReason;
 
     /*
      * If this is true, all marked objects must belong to a compartment being
      * GCed. This is used to look for compartment bugs.
      */
     bool                gcStrictCompartmentChecking;
 
+#ifdef DEBUG
     /*
      * If this is 0, all cross-compartment proxies must be registered in the
      * wrapper map. This checking must be disabled temporarily while creating
      * new wrappers. When non-zero, this records the recursion depth of wrapper
      * creation.
      */
     uintptr_t           gcDisableStrictProxyCheckingCount;
+#else
+    uintptr_t           unused1;
+#endif
 
     /*
      * The current incremental GC phase. This is also used internally in
      * non-incremental GC.
      */
     js::gc::State       gcIncrementalState;
 
     /* Indicates that the last incremental slice exhausted the mark stack. */
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -208,25 +208,17 @@ JSCompartment::wrap(JSContext *cx, Mutab
     JS_ASSERT_IF(existingArg, existingArg->compartment() == cx->compartment);
     JS_ASSERT_IF(existingArg, vp.isObject());
     JS_ASSERT_IF(existingArg, IsDeadProxyObject(existingArg));
 
     unsigned flags = 0;
 
     JS_CHECK_CHROME_RECURSION(cx, return false);
 
-#ifdef DEBUG
-    struct AutoDisableProxyCheck {
-        JSRuntime *runtime;
-        AutoDisableProxyCheck(JSRuntime *rt) : runtime(rt) {
-            runtime->gcDisableStrictProxyCheckingCount++;
-        }
-        ~AutoDisableProxyCheck() { runtime->gcDisableStrictProxyCheckingCount--; }
-    } adpc(rt);
-#endif
+    AutoDisableProxyCheck adpc(rt);
 
     /* Only GC things have to be wrapped or copied. */
     if (!vp.isMarkable())
         return true;
 
     if (vp.isString()) {
         RawString str = vp.toString();
 
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -5107,8 +5107,16 @@ AutoSuppressGC::AutoSuppressGC(JSContext
     suppressGC_++;
 }
 
 AutoSuppressGC::AutoSuppressGC(JSCompartment *comp)
   : suppressGC_(comp->rt->mainThread.suppressGC)
 {
     suppressGC_++;
 }
+
+#ifdef DEBUG
+AutoDisableProxyCheck::AutoDisableProxyCheck(JSRuntime *rt)
+  : count(rt->gcDisableStrictProxyCheckingCount)
+{
+    count++;
+}
+#endif
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -1280,14 +1280,33 @@ class AutoSuppressGC
     ~AutoSuppressGC()
     {
         suppressGC_--;
     }
 };
 
 } /* namespace gc */
 
+#ifdef DEBUG
+/* Use this to avoid assertions when manipulating the wrapper map. */
+struct AutoDisableProxyCheck
+{
+    uintptr_t &count;
+
+    AutoDisableProxyCheck(JSRuntime *rt);
+
+    ~AutoDisableProxyCheck() {
+        count--;
+    }
+};
+#else
+struct AutoDisableProxyCheck
+{
+    AutoDisableProxyCheck(JSRuntime *rt) {}
+};
+#endif
+
 void
 PurgeJITCaches(JS::Zone *zone);
 
 } /* namespace js */
 
 #endif /* jsgc_h___ */
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1768,40 +1768,33 @@ struct JSObject::TradeGutsReserved {
             js_free(newbslots);
     }
 };
 
 bool
 JSObject::ReserveForTradeGuts(JSContext *cx, JSObject *aArg, JSObject *bArg,
                               TradeGutsReserved &reserved)
 {
+    /*
+     * Avoid GC in here to avoid confusing the tracing code with our
+     * intermediate state.
+     */
+    AutoSuppressGC suppress(cx);
+
     RootedObject a(cx, aArg);
     RootedObject b(cx, bArg);
     JS_ASSERT(a->compartment() == b->compartment());
     AutoCompartment ac(cx, a);
 
     /*
      * When performing multiple swaps between objects which may have different
      * numbers of fixed slots, we reserve all space ahead of time so that the
      * swaps can be performed infallibly.
      */
 
-#ifdef JSGC_INCREMENTAL
-    /*
-     * We need a write barrier here. If |a| was marked and |b| was not, then
-     * after the swap, |b|'s guts would never be marked. The write barrier
-     * solves this.
-     */
-    JS::Zone *zone = a->zone();
-    if (zone->needsBarrier()) {
-        MarkChildren(zone->barrierTracer(), a);
-        MarkChildren(zone->barrierTracer(), b);
-    }
-#endif
-
     /*
      * Swap prototypes and classes on the two objects, so that TradeGuts can
      * preserve the types of the two objects.
      */
     Class *aClass = a->getClass();
     Class *bClass = b->getClass();
     Rooted<TaggedProto> aProto(cx, a->getTaggedProto());
     Rooted<TaggedProto> bProto(cx, b->getTaggedProto());
@@ -2010,16 +2003,34 @@ JSObject::TradeGuts(JSContext *cx, JSObj
     types::TypeObject::writeBarrierPost(a->type_, &a->type_);
     types::TypeObject::writeBarrierPost(b->type_, &b->type_);
 #endif
 
     if (a->inDictionaryMode())
         a->lastProperty()->listp = &a->shape_;
     if (b->inDictionaryMode())
         b->lastProperty()->listp = &b->shape_;
+
+#ifdef JSGC_INCREMENTAL
+    /*
+     * We need a write barrier here. If |a| was marked and |b| was not, then
+     * after the swap, |b|'s guts would never be marked. The write barrier
+     * solves this.
+     *
+     * Normally write barriers happen before the write. However, that's not
+     * necessary here because nothing is being destroyed. We're just swapping.
+     * We don't do the barrier before TradeGuts because ReserveForTradeGuts
+     * makes changes to the objects that might confuse the tracing code.
+     */
+    JS::Zone *zone = a->zone();
+    if (zone->needsBarrier()) {
+        MarkChildren(zone->barrierTracer(), a);
+        MarkChildren(zone->barrierTracer(), b);
+    }
+#endif
 }
 
 /* Use this method with extreme caution. It trades the guts of two objects. */
 bool
 JSObject::swap(JSContext *cx, HandleObject a, HandleObject b)
 {
     AutoMarkInDeadZone adc1(a->zone());
     AutoMarkInDeadZone adc2(b->zone());
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -948,16 +948,18 @@ js::RemapWrapper(JSContext *cx, JSObject
     RootedObject newTarget(cx, newTargetArg);
     JS_ASSERT(IsCrossCompartmentWrapper(wobj));
     JS_ASSERT(!IsCrossCompartmentWrapper(newTarget));
     JSObject *origTarget = Wrapper::wrappedObject(wobj);
     JS_ASSERT(origTarget);
     Value origv = ObjectValue(*origTarget);
     JSCompartment *wcompartment = wobj->compartment();
 
+    AutoDisableProxyCheck adpc(cx->runtime);
+
     // If we're mapping to a different target (as opposed to just recomputing
     // for the same target), we must not have an existing wrapper for the new
     // target, otherwise this will break.
     JS_ASSERT_IF(origTarget != newTarget,
                  !wcompartment->lookupWrapper(ObjectValue(*newTarget)));
 
     // The old value should still be in the cross-compartment wrapper map, and
     // the lookup should return wobj.