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 183453 36eba9f589835a35fc5eaac1a373be47ae032003
parent 183452 0ca489a88977865fbf8a3b56f20a0ec0450b16f8
child 183454 67f5286dda31cd67d55396c5973c46d301049910
push id43560
push userjcoppeard@mozilla.com
push dateFri, 16 May 2014 09:09:59 +0000
treeherdermozilla-inbound@8475dbade7b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs982561
milestone32.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 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()); }
 };