Bug 982561 - Add zone edges for unmarked weakmap keys with delegates in a different zone r=terrence
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 16 May 2014 09:44:43 +0100
changeset 183496 36eba9f589835a35fc5eaac1a373be47ae032003
parent 183495 0ca489a88977865fbf8a3b56f20a0ec0450b16f8
child 183497 67f5286dda31cd67d55396c5973c46d301049910
push idunknown
push userunknown
push dateunknown
reviewersterrence
bugs982561
milestone32.0a1
Bug 982561 - Add zone edges for unmarked weakmap keys with delegates in a different zone r=terrence
js/src/gc/GCRuntime.h
js/src/gc/Zone.cpp
js/src/gc/Zone.h
js/src/jsgc.cpp
js/src/jsweakmap.cpp
js/src/jsweakmap.h
js/src/vm/Runtime.cpp
js/src/vm/WeakMapObject.h
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -134,16 +134,17 @@ class GCRuntime
     bool shouldPreserveJITCode(JSCompartment *comp, int64_t currentTime);
     bool drainMarkStack(SliceBudget &sliceBudget, gcstats::Phase phase);
     template <class CompartmentIterT> void markWeakReferences(gcstats::Phase phase);
     void markWeakReferencesInCurrentGroup(gcstats::Phase phase);
     template <class ZoneIterT, class CompartmentIterT> void markGrayReferences();
     void markGrayReferencesInCurrentGroup();
     void beginSweepPhase(bool lastGC);
     void findZoneGroups();
+    bool findZoneEdgesForWeakMaps();
     void getNextZoneGroup();
     void endMarkingZoneGroup();
     void beginSweepingZoneGroup();
     bool releaseObservedTypes();
     void endSweepingZoneGroup();
     bool sweepPhase(SliceBudget &sliceBudget);
     void endSweepPhase(JSGCInvocationKind gckind, bool lastGC);
     void sweepZones(FreeOp *fop, bool lastGC);
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -58,16 +58,21 @@ Zone::~Zone()
     if (this == rt->gc.systemZone)
         rt->gc.systemZone = nullptr;
 
 #ifdef JS_ION
     js_delete(jitZone_);
 #endif
 }
 
+bool Zone::init()
+{
+    return gcZoneGroupEdges.init();
+}
+
 void
 Zone::setNeedsBarrier(bool needs, ShouldUpdateIon updateIon)
 {
 #ifdef JS_ION
     if (updateIon == UpdateIon && needs != ionUsingBarriers_) {
         jit::ToggleBarriers(this, needs);
         ionUsingBarriers_ = needs;
     }
--- a/js/src/gc/Zone.h
+++ b/js/src/gc/Zone.h
@@ -267,22 +267,33 @@ struct Zone : public JS::shadow::Zone,
      * This should be a bool, but Atomic only supports 32-bit and pointer-sized
      * types.
      */
     mozilla::Atomic<uint32_t, mozilla::ReleaseAcquire> gcMallocGCTriggered;
 
     /* This compartment's gray roots. */
     js::Vector<js::GrayRoot, 0, js::SystemAllocPolicy> gcGrayRoots;
 
+    /*
+     * A set of edges from this zone to other zones.
+     *
+     * This is used during GC while calculating zone groups to record edges that
+     * can't be determined by examining this zone by itself.
+     */
+    typedef js::HashSet<Zone *, js::DefaultHasher<Zone *>, js::SystemAllocPolicy> ZoneSet;
+    ZoneSet gcZoneGroupEdges;
+
     /* Per-zone data for use by an embedder. */
     void *data;
 
     Zone(JSRuntime *rt);
     ~Zone();
 
+    bool init();
+
     void findOutgoingEdges(js::gc::ComponentFinder<JS::Zone> &finder);
 
     void discardJitCode(js::FreeOp *fop);
 
     void addSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf,
                                 size_t *typePool,
                                 size_t *baselineStubsOptimized);
 
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3437,23 +3437,48 @@ Zone::findOutgoingEdges(ComponentFinder<
      * compartment, and these aren't in the cross compartment map.
      */
     JSRuntime *rt = runtimeFromMainThread();
     if (rt->atomsCompartment()->zone()->isGCMarking())
         finder.addEdgeTo(rt->atomsCompartment()->zone());
 
     for (CompartmentsInZoneIter comp(this); !comp.done(); comp.next())
         comp->findOutgoingEdges(finder);
+
+    for (ZoneSet::Range r = gcZoneGroupEdges.all(); !r.empty(); r.popFront())
+        finder.addEdgeTo(r.front());
+    gcZoneGroupEdges.clear();
+}
+
+bool
+GCRuntime::findZoneEdgesForWeakMaps()
+{
+    /*
+     * Weakmaps which have keys with delegates in a different zone introduce the
+     * need for zone edges from the delegate's zone to the weakmap zone.
+     *
+     * Since the edges point into and not away from the zone the weakmap is in
+     * we must find these edges in advance and store them in a set on the Zone.
+     * If we run out of memory, we fall back to sweeping everything in one
+     * group.
+     */
+
+    for (GCCompartmentsIter comp(rt); !comp.done(); comp.next()) {
+        if (!WeakMapBase::findZoneEdgesForCompartment(comp))
+            return false;
+    }
+
+    return true;
 }
 
 void
 GCRuntime::findZoneGroups()
 {
     ComponentFinder<Zone> finder(rt->mainThread.nativeStackLimit[StackForSystemCode]);
-    if (!isIncremental)
+    if (!isIncremental || !findZoneEdgesForWeakMaps())
         finder.useOneComponent();
 
     for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
         JS_ASSERT(zone->isGCMarking());
         finder.addNode(zone);
     }
     zoneGroups = finder.getResultsList();
     currentZoneGroup = zoneGroups;
@@ -5017,16 +5042,19 @@ js::NewCompartment(JSContext *cx, Zone *
     ScopedJSDeletePtr<Zone> zoneHolder;
     if (!zone) {
         zone = cx->new_<Zone>(rt);
         if (!zone)
             return nullptr;
 
         zoneHolder.reset(zone);
 
+        if (!zone->init())
+            return nullptr;
+
         zone->setGCLastBytes(8192, GC_NORMAL);
 
         const JSPrincipals *trusted = rt->trustedPrincipals();
         zone->isSystem = principals && principals == trusted;
     }
 
     ScopedJSDeletePtr<JSCompartment> compartment(cx->new_<JSCompartment>(zone, options));
     if (!compartment || !compartment->init(cx))
--- a/js/src/jsweakmap.cpp
+++ b/js/src/jsweakmap.cpp
@@ -15,16 +15,17 @@
 #include "jswrapper.h"
 
 #include "vm/GlobalObject.h"
 #include "vm/WeakMapObject.h"
 
 #include "jsobjinlines.h"
 
 using namespace js;
+using namespace js::gc;
 
 WeakMapBase::WeakMapBase(JSObject *memOf, JSCompartment *c)
   : memberOf(memOf),
     compartment(c),
     next(WeakMapNotInList),
     marked(false)
 {
     JS_ASSERT_IF(memberOf, memberOf->compartment() == c);
@@ -73,16 +74,26 @@ WeakMapBase::markCompartmentIteratively(
     bool markedAny = false;
     for (WeakMapBase *m = c->gcWeakMapList; m; m = m->next) {
         if (m->marked && m->markIteratively(tracer))
             markedAny = true;
     }
     return markedAny;
 }
 
+bool
+WeakMapBase::findZoneEdgesForCompartment(JSCompartment *c)
+{
+    for (WeakMapBase *m = c->gcWeakMapList; m; m = m->next) {
+        if (!m->findZoneEdges())
+            return false;
+    }
+    return true;
+}
+
 void
 WeakMapBase::sweepCompartment(JSCompartment *c)
 {
     WeakMapBase **tailPtr = &c->gcWeakMapList;
     for (WeakMapBase *m = c->gcWeakMapList, *next; m; m = next) {
         next = m->next;
         if (m->marked) {
             m->sweep();
@@ -141,16 +152,44 @@ WeakMapBase::removeWeakMapFromList(WeakM
         if (*p == weakmap) {
             *p = (*p)->next;
             weakmap->next = WeakMapNotInList;
             break;
         }
     }
 }
 
+bool
+ObjectValueMap::findZoneEdges()
+{
+    /*
+     * For unmarked weakmap keys with delegates in a different zone, add a zone
+     * edge to ensure that the delegate zone does finish marking after the key
+     * zone.
+     */
+    Zone *mapZone = compartment->zone();
+    for (Range r = all(); !r.empty(); r.popFront()) {
+        JSObject *key = r.front().key();
+        if (key->isMarked(BLACK) && !key->isMarked(GRAY))
+            continue;
+        JSWeakmapKeyDelegateOp op = key->getClass()->ext.weakmapKeyDelegateOp;
+        if (!op)
+            continue;
+        JSObject *delegate = op(key);
+        if (!delegate)
+            continue;
+        Zone *delegateZone = delegate->zone();
+        if (delegateZone == mapZone)
+            continue;
+        if (!delegateZone->gcZoneGroupEdges.put(key->zone()))
+            return false;
+    }
+    return true;
+}
+
 static JSObject *
 GetKeyArg(JSContext *cx, CallArgs &args)
 {
     if (args[0].isPrimitive()) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_NOT_NONNULL_OBJECT);
         return nullptr;
     }
     return &args[0].toObject();
@@ -314,17 +353,17 @@ WeakMapPostWriteBarrier(JSRuntime *rt, O
      * WeakMap's multiple inheritace, We need to do this in two stages, first to
      * the HashMap base class and then to the unbarriered version.
      */
     ObjectValueMap::Base *baseHashMap = static_cast<ObjectValueMap::Base *>(weakMap);
 
     typedef HashMap<JSObject *, Value> UnbarrieredMap;
     UnbarrieredMap *unbarrieredMap = reinterpret_cast<UnbarrieredMap *>(baseHashMap);
 
-    typedef gc::HashKeyRef<UnbarrieredMap, JSObject *> Ref;
+    typedef HashKeyRef<UnbarrieredMap, JSObject *> Ref;
     if (key && IsInsideNursery(rt, key))
         rt->gc.storeBuffer.putGeneric(Ref((unbarrieredMap), key));
 #endif
 }
 
 MOZ_ALWAYS_INLINE bool
 SetWeakMapEntryInternal(JSContext *cx, Handle<WeakMapObject*> mapObj,
                         HandleObject key, HandleValue value)
@@ -401,17 +440,17 @@ JS_NondeterministicGetWeakMapKeys(JSCont
         return true;
     }
     RootedObject arr(cx, NewDenseEmptyArray(cx));
     if (!arr)
         return false;
     ObjectValueMap *map = obj->as<WeakMapObject>().getMap();
     if (map) {
         // Prevent GC from mutating the weakmap while iterating.
-        gc::AutoSuppressGC suppress(cx);
+        AutoSuppressGC suppress(cx);
         for (ObjectValueMap::Base::Range r = map->all(); !r.empty(); r.popFront()) {
             RootedObject key(cx, r.front().key());
             if (!cx->compartment()->wrap(cx, &key))
                 return false;
             if (!NewbornArrayPush(cx, arr, ObjectValue(*key)))
                 return false;
         }
     }
--- a/js/src/jsweakmap.h
+++ b/js/src/jsweakmap.h
@@ -50,16 +50,19 @@ class WeakMapBase {
     static void unmarkCompartment(JSCompartment *c);
 
     // Check all weak maps in a compartment that have been marked as live in this garbage
     // collection, and mark the values of all entries that have become strong references
     // to them. Return true if we marked any new values, indicating that we need to make
     // another pass. In other words, mark my marked maps' marked members' mid-collection.
     static bool markCompartmentIteratively(JSCompartment *c, JSTracer *tracer);
 
+    // Add zone edges for weakmaps with key delegates in a different zone.
+    static bool findZoneEdgesForCompartment(JSCompartment *c);
+
     // Sweep the weak maps in a compartment, removing dead weak maps and removing
     // entries of live weak maps whose keys are dead.
     static void sweepCompartment(JSCompartment *c);
 
     // Trace all delayed weak map bindings. Used by the cycle collector.
     static void traceAllMappings(WeakMapTracer *tracer);
 
     bool isInList() { return next != WeakMapNotInList; }
@@ -74,16 +77,17 @@ class WeakMapBase {
     static void removeWeakMapFromList(WeakMapBase *weakmap);
 
   protected:
     // Instance member functions called by the above. Instantiations of WeakMap override
     // these with definitions appropriate for their Key and Value types.
     virtual void nonMarkingTraceKeys(JSTracer *tracer) = 0;
     virtual void nonMarkingTraceValues(JSTracer *tracer) = 0;
     virtual bool markIteratively(JSTracer *tracer) = 0;
+    virtual bool findZoneEdges() = 0;
     virtual void sweep() = 0;
     virtual void traceMappings(WeakMapTracer *tracer) = 0;
     virtual void finish() = 0;
 
     // Object that this weak map is part of, if any.
     JSObject *memberOf;
 
     // Compartment that this weak map is part of.
@@ -177,16 +181,21 @@ class WeakMap : public HashMap<Key, Valu
                     entryMoved(e, key);
                 markedAny = true;
             }
             key.unsafeSet(nullptr);
         }
         return markedAny;
     }
 
+    bool findZoneEdges() {
+        // This is overridden by ObjectValueMap.
+        return true;
+    }
+
     void sweep() {
         /* Remove all entries whose keys remain unmarked. */
         for (Enum e(*this); !e.empty(); e.popFront()) {
             Key k(e.front().key());
             if (gc::IsAboutToBeFinalized(&k))
                 e.removeFront();
             else if (k != e.front().key())
                 entryMoved(e, k);
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -292,17 +292,17 @@ JSRuntime::init(uint32_t maxbytes)
     if (!gc.init(maxbytes))
         return false;
 
     const char *size = getenv("JSGC_MARK_STACK_LIMIT");
     if (size)
         SetMarkStackLimit(this, atoi(size));
 
     ScopedJSDeletePtr<Zone> atomsZone(new_<Zone>(this));
-    if (!atomsZone)
+    if (!atomsZone || !atomsZone->init())
         return false;
 
     JS::CompartmentOptions options;
     ScopedJSDeletePtr<JSCompartment> atomsCompartment(new_<JSCompartment>(atomsZone.get(), options));
     if (!atomsCompartment || !atomsCompartment->init(nullptr))
         return false;
 
     gc.zones.append(atomsZone.get());
--- a/js/src/vm/WeakMapObject.h
+++ b/js/src/vm/WeakMapObject.h
@@ -7,17 +7,24 @@
 #ifndef vm_WeakMapObject_h
 #define vm_WeakMapObject_h
 
 #include "jsobj.h"
 #include "jsweakmap.h"
 
 namespace js {
 
-typedef WeakMap<PreBarrieredObject, RelocatableValue> ObjectValueMap;
+class ObjectValueMap : public WeakMap<PreBarrieredObject, RelocatableValue>
+{
+  public:
+    ObjectValueMap(JSContext *cx, JSObject *obj)
+      : WeakMap<PreBarrieredObject, RelocatableValue>(cx, obj) {}
+
+    virtual bool findZoneEdges();
+};
 
 class WeakMapObject : public JSObject
 {
   public:
     static const Class class_;
 
     ObjectValueMap *getMap() { return static_cast<ObjectValueMap*>(getPrivate()); }
 };