Bug 1216744 - Move weakKeys to the zone, r=terrence
authorSteve Fink <sfink@mozilla.com>
Fri, 16 Oct 2015 17:33:41 -0700
changeset 305313 b3a42d2881e5c12d98a96f2f0bfe8a32fb093c9c
parent 305312 01ae6c388605ece6feea6d96cc31e3bd27e9a1d7
child 305314 f6d251d7b4f7e58299effdbea37e555843bb3ca8
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1216744
milestone44.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 1216744 - Move weakKeys to the zone, r=terrence
js/src/gc/Marking.cpp
js/src/gc/Marking.h
js/src/gc/Zone.cpp
js/src/gc/Zone.h
js/src/jsgc.cpp
js/src/jsweakmap.h
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -23,16 +23,17 @@
 #include "vm/ArrayObject.h"
 #include "vm/ScopeObject.h"
 #include "vm/Shape.h"
 #include "vm/Symbol.h"
 #include "vm/TypedArrayObject.h"
 #include "vm/UnboxedObject.h"
 
 #include "jscompartmentinlines.h"
+#include "jsgcinlines.h"
 #include "jsobjinlines.h"
 
 #include "gc/Nursery-inl.h"
 #include "vm/String-inl.h"
 #include "vm/UnboxedObject-inl.h"
 
 using namespace js;
 using namespace js::gc;
@@ -615,20 +616,21 @@ GCMarker::markEphemeronValues(gc::Cell* 
 
 template <typename T>
 void
 GCMarker::markImplicitEdgesHelper(T markedThing)
 {
     if (!isWeakMarkingTracer())
         return;
 
-    MOZ_ASSERT(gc::TenuredCell::fromPointer(markedThing)->zone()->isGCMarking());
-    MOZ_ASSERT(!gc::TenuredCell::fromPointer(markedThing)->zone()->isGCSweeping());
+    Zone* zone = gc::TenuredCell::fromPointer(markedThing)->zone();
+    MOZ_ASSERT(zone->isGCMarking());
+    MOZ_ASSERT(!zone->isGCSweeping());
 
-    auto p = weakKeys.get(JS::GCCellPtr(markedThing));
+    auto p = zone->gcWeakKeys.get(JS::GCCellPtr(markedThing));
     if (!p)
         return;
     WeakEntryVector& markables = p->value;
 
     markEphemeronValues(markedThing, markables);
     markables.clear(); // If key address is reused, it should do nothing
 }
 
@@ -1705,17 +1707,17 @@ GCMarker::GCMarker(JSRuntime* rt)
     started(false),
     strictCompartmentChecking(false)
 {
 }
 
 bool
 GCMarker::init(JSGCMode gcMode)
 {
-    return stack.init(gcMode) && weakKeys.init();
+    return stack.init(gcMode);
 }
 
 void
 GCMarker::start()
 {
     MOZ_ASSERT(!started);
     started = true;
     color = BLACK;
@@ -1733,17 +1735,18 @@ GCMarker::stop()
     MOZ_ASSERT(started);
     started = false;
 
     MOZ_ASSERT(!unmarkedArenaStackTop);
     MOZ_ASSERT(markLaterArenas == 0);
 
     /* Free non-ballast stack memory. */
     stack.reset();
-    weakKeys.clear();
+    for (GCZonesIter zone(runtime()); !zone.done(); zone.next())
+        zone->gcWeakKeys.clear();
 }
 
 void
 GCMarker::reset()
 {
     color = BLACK;
 
     stack.reset();
@@ -1780,16 +1783,29 @@ GCMarker::enterWeakMarkingMode()
                 if (m->marked)
                     (void) m->traceEntries(this);
             }
         }
     }
 }
 
 void
+GCMarker::leaveWeakMarkingMode()
+{
+    MOZ_ASSERT_IF(weakMapAction() == ExpandWeakMaps && !linearWeakMarkingDisabled_,
+                  tag_ == TracerKindTag::WeakMarking);
+    tag_ = TracerKindTag::Marking;
+
+    // Table is expensive to maintain when not in weak marking mode, so we'll
+    // rebuild it upon entry rather than allow it to contain stale data.
+    for (GCZonesIter zone(runtime()); !zone.done(); zone.next())
+        zone->gcWeakKeys.clear();
+}
+
+void
 GCMarker::markDelayedChildren(ArenaHeader* aheader)
 {
     if (aheader->markOverflow) {
         bool always = aheader->allocatedDuringIncremental;
         aheader->markOverflow = 0;
 
         for (ArenaCellIterUnderGC i(aheader); !i.done(); i.next()) {
             TenuredCell* t = i.getCell();
--- a/js/src/gc/Marking.h
+++ b/js/src/gc/Marking.h
@@ -25,16 +25,17 @@
 class JSLinearString;
 class JSRope;
 namespace js {
 class BaseShape;
 class GCMarker;
 class LazyScript;
 class NativeObject;
 class ObjectGroup;
+class WeakMapBase;
 namespace gc {
 struct ArenaHeader;
 } // namespace gc
 namespace jit {
 class JitCode;
 } // namespace jit
 
 static const size_t NON_INCREMENTAL_MARK_STACK_BASE_CAPACITY = 4096;
@@ -132,18 +133,16 @@ class MarkStack
     /* Grow the stack, ensuring there is space for at least count elements. */
     bool enlarge(unsigned count);
 
     void setGCMode(JSGCMode gcMode);
 
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 };
 
-class WeakMapBase;
-
 namespace gc {
 
 struct WeakKeyTableHashPolicy {
     typedef JS::GCCellPtr Lookup;
     static HashNumber hash(const Lookup& v) { return mozilla::HashGeneric(v.asCell()); }
     static bool match(const JS::GCCellPtr& k, const Lookup& l) { return k == l; }
     static bool isEmpty(const JS::GCCellPtr& v) { return !v; }
     static void makeEmpty(JS::GCCellPtr* vp) { *vp = nullptr; }
@@ -152,22 +151,22 @@ struct WeakKeyTableHashPolicy {
 struct WeakMarkable {
     WeakMapBase* weakmap;
     JS::GCCellPtr key;
 
     WeakMarkable(WeakMapBase* weakmapArg, JS::GCCellPtr keyArg)
       : weakmap(weakmapArg), key(keyArg) {}
 };
 
-typedef Vector<WeakMarkable, 2, js::SystemAllocPolicy> WeakEntryVector;
+using WeakEntryVector = Vector<WeakMarkable, 2, js::SystemAllocPolicy>;
 
-typedef OrderedHashMap<JS::GCCellPtr,
-                       WeakEntryVector,
-                       WeakKeyTableHashPolicy,
-                       js::SystemAllocPolicy> WeakKeyTable;
+using WeakKeyTable = OrderedHashMap<JS::GCCellPtr,
+                                    WeakEntryVector,
+                                    WeakKeyTableHashPolicy,
+                                    js::SystemAllocPolicy>;
 
 } /* namespace gc */
 
 class GCMarker : public JSTracer
 {
   public:
     explicit GCMarker(JSRuntime* rt);
     bool init(JSGCMode gcMode);
@@ -204,27 +203,17 @@ class GCMarker : public JSTracer
     void setMarkColorBlack() {
         MOZ_ASSERT(isDrained());
         MOZ_ASSERT(color == gc::GRAY);
         color = gc::BLACK;
     }
     uint32_t markColor() const { return color; }
 
     void enterWeakMarkingMode();
-
-    void leaveWeakMarkingMode() {
-        MOZ_ASSERT_IF(weakMapAction() == ExpandWeakMaps && !linearWeakMarkingDisabled_, tag_ == TracerKindTag::WeakMarking);
-        tag_ = TracerKindTag::Marking;
-
-        // Table is expensive to maintain when not in weak marking mode, so
-        // we'll rebuild it upon entry rather than allow it to contain stale
-        // data.
-        weakKeys.clear();
-    }
-
+    void leaveWeakMarkingMode();
     void abortLinearWeakMarking() {
         leaveWeakMarkingMode();
         linearWeakMarkingDisabled_ = true;
     }
 
     void delayMarkingArena(gc::ArenaHeader* aheader);
     void delayMarkingChildren(const void* thing);
     void markDelayedChildren(gc::ArenaHeader* aheader);
@@ -244,22 +233,16 @@ class GCMarker : public JSTracer
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
 #ifdef DEBUG
     bool shouldCheckCompartments() { return strictCompartmentChecking; }
 #endif
 
     void markEphemeronValues(gc::Cell* markedCell, gc::WeakEntryVector& entry);
 
-    /*
-     * Mapping from not yet marked keys to a vector of all values that the key
-     * maps to in any live weak map.
-     */
-    gc::WeakKeyTable weakKeys;
-
   private:
 #ifdef DEBUG
     void checkZone(void* p);
 #else
     void checkZone(void* p) {}
 #endif
 
     /*
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -60,17 +60,17 @@ Zone::~Zone()
 
     js_delete(debuggers);
     js_delete(jitZone_);
 }
 
 bool Zone::init(bool isSystemArg)
 {
     isSystem = isSystemArg;
-    return uniqueIds_.init() && gcZoneGroupEdges.init();
+    return uniqueIds_.init() && gcZoneGroupEdges.init() && gcWeakKeys.init();
 }
 
 void
 Zone::setNeedsIncrementalBarrier(bool needs, ShouldUpdateJit updateJit)
 {
     if (updateJit == UpdateJit && needs != jitUsingBarriers_) {
         jit::ToggleBarriers(this, needs);
         jitUsingBarriers_ = needs;
--- a/js/src/gc/Zone.h
+++ b/js/src/gc/Zone.h
@@ -294,16 +294,22 @@ struct Zone : public JS::shadow::Zone,
     typedef js::Vector<js::gc::Cell*, 0, js::SystemAllocPolicy> GrayRootVector;
     GrayRootVector gcGrayRoots;
 
     // This zone's weak edges found via graph traversal during marking,
     // preserved for re-scanning during sweeping.
     using WeakEdges = js::Vector<js::gc::TenuredCell**, 0, js::SystemAllocPolicy>;
     WeakEdges gcWeakRefs;
 
+    /*
+     * Mapping from not yet marked keys to a vector of all values that the key
+     * maps to in any live weak map.
+     */
+    js::gc::WeakKeyTable gcWeakKeys;
+
     // 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.
     ZoneSet gcZoneGroupEdges;
 
     // Malloc counter to measure memory pressure for GC scheduling. It runs from
     // gcMaxMallocBytes down to zero. This counter should be used only when it's
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -4223,39 +4223,43 @@ js::gc::MarkingValidator::nonIncremental
      * Temporarily clear the weakmaps' mark flags for the compartments we are
      * collecting.
      */
 
     WeakMapSet markedWeakMaps;
     if (!markedWeakMaps.init())
         return;
 
+    /*
+     * For saving, smush all of the keys into one big table and split them back
+     * up into per-zone tables when restoring.
+     */
+    gc::WeakKeyTable savedWeakKeys;
+    if (!savedWeakKeys.init())
+        return;
+
     for (GCZonesIter zone(runtime); !zone.done(); zone.next()) {
         if (!WeakMapBase::saveZoneMarkedWeakMaps(zone, markedWeakMaps))
             return;
-    }
-
-    gc::WeakKeyTable savedWeakKeys;
-    if (!savedWeakKeys.init())
-        return;
-
-    for (gc::WeakKeyTable::Range r = gc->marker.weakKeys.all(); !r.empty(); r.popFront()) {
-        AutoEnterOOMUnsafeRegion oomUnsafe;
-        if (!savedWeakKeys.put(Move(r.front().key), Move(r.front().value)))
-            oomUnsafe.crash("saving weak keys table for validator");
+
+        for (gc::WeakKeyTable::Range r = zone->gcWeakKeys.all(); !r.empty(); r.popFront()) {
+            AutoEnterOOMUnsafeRegion oomUnsafe;
+            if (!savedWeakKeys.put(Move(r.front().key), Move(r.front().value)))
+                oomUnsafe.crash("saving weak keys table for validator");
+        }
+
+        zone->gcWeakKeys.clear();
     }
 
     /*
      * After this point, the function should run to completion, so we shouldn't
      * do anything fallible.
      */
     initialized = true;
 
-    gc->marker.weakKeys.clear();
-
     /* Re-do all the marking, but non-incrementally. */
     js::gc::State state = gc->incrementalState;
     gc->incrementalState = MARK_ROOTS;
 
     {
         gcstats::AutoPhase ap(gc->stats, gcstats::PHASE_MARK);
 
         {
@@ -4306,24 +4310,27 @@ js::gc::MarkingValidator::nonIncremental
 
     /* Take a copy of the non-incremental mark state and restore the original. */
     for (auto chunk = gc->allNonEmptyChunks(); !chunk.done(); chunk.next()) {
         ChunkBitmap* bitmap = &chunk->bitmap;
         ChunkBitmap* entry = map.lookup(chunk)->value();
         Swap(*entry, *bitmap);
     }
 
-    for (GCZonesIter zone(runtime); !zone.done(); zone.next())
+    for (GCZonesIter zone(runtime); !zone.done(); zone.next()) {
         WeakMapBase::unmarkZone(zone);
+        zone->gcWeakKeys.clear();
+    }
+
     WeakMapBase::restoreMarkedWeakMaps(markedWeakMaps);
 
-    gc->marker.weakKeys.clear();
     for (gc::WeakKeyTable::Range r = savedWeakKeys.all(); !r.empty(); r.popFront()) {
         AutoEnterOOMUnsafeRegion oomUnsafe;
-        if (!gc->marker.weakKeys.put(Move(r.front().key), Move(r.front().value)))
+        Zone* zone = gc::TenuredCell::fromPointer(r.front().key.asCell())->zone();
+        if (!zone->gcWeakKeys.put(Move(r.front().key), Move(r.front().value)))
             oomUnsafe.crash("restoring weak keys table for validator");
     }
 
     gc->incrementalState = state;
 }
 
 void
 js::gc::MarkingValidator::validate()
@@ -4982,35 +4989,27 @@ GCRuntime::beginSweepingZoneGroup()
         if (rt->sweepZoneCallback)
             rt->sweepZoneCallback(zone);
 
         zone->gcLastZoneGroupIndex = zoneGroupIndex;
     }
 
     validateIncrementalMarking();
 
-    /* Clear out this zone group's keys from the weakKeys table, to prevent later accesses. */
-    for (WeakKeyTable::Range r = marker.weakKeys.all(); !r.empty(); ) {
-        auto key(r.front().key);
-        r.popFront();
-        if (gc::TenuredCell::fromPointer(key.asCell())->zone()->isGCSweeping()) {
-            bool found;
-            marker.weakKeys.remove(key, &found);
-            MOZ_ASSERT(found);
-        }
-    }
-
-    /* Clear all weakrefs that point to unmarked things. */
     for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) {
+        /* Clear all weakrefs that point to unmarked things. */
         for (auto edge : zone->gcWeakRefs) {
             /* Edges may be present multiple times, so may already be nulled. */
             if (*edge && IsAboutToBeFinalizedDuringSweep(**edge))
                 *edge = nullptr;
         }
         zone->gcWeakRefs.clear();
+
+        /* No need to look up any more weakmap keys from this zone group. */
+        zone->gcWeakKeys.clear();
     }
 
     FreeOp fop(rt);
     SweepAtomsTask sweepAtomsTask(rt);
     SweepInnerViewsTask sweepInnerViewsTask(rt);
     SweepCCWrappersTask sweepCCWrappersTask(rt);
     SweepBaseShapesTask sweepBaseShapesTask(rt);
     SweepInitialShapesTask sweepInitialShapesTask(rt);
--- a/js/src/jsweakmap.h
+++ b/js/src/jsweakmap.h
@@ -226,26 +226,27 @@ class WeakMap : public HashMap<Key, Valu
         MOZ_ASSERT(trc->weakMapAction() == ExpandWeakMaps);
         (void) traceEntries(trc);
     }
 
   protected:
     static void addWeakEntry(JSTracer* trc, JS::GCCellPtr key, gc::WeakMarkable markable)
     {
         GCMarker& marker = *static_cast<GCMarker*>(trc);
+        Zone* zone = key.asCell()->asTenured().zone();
 
-        auto p = marker.weakKeys.get(key);
+        auto p = zone->gcWeakKeys.get(key);
         if (p) {
             gc::WeakEntryVector& weakEntries = p->value;
             if (!weakEntries.append(Move(markable)))
                 marker.abortLinearWeakMarking();
         } else {
             gc::WeakEntryVector weakEntries;
             MOZ_ALWAYS_TRUE(weakEntries.append(Move(markable)));
-            if (!marker.weakKeys.put(JS::GCCellPtr(key), Move(weakEntries)))
+            if (!zone->gcWeakKeys.put(JS::GCCellPtr(key), Move(weakEntries)))
                 marker.abortLinearWeakMarking();
         }
     }
 
     bool traceEntries(JSTracer* trc) override {
         MOZ_ASSERT(marked);
 
         bool markedAny = false;