Bug 1167452 - Switch weakmap marking code back from GCCellPtr to plain Cell* r=jonco
☠☠ backed out by c452cbbba791 ☠ ☠
authorSteve Fink <sfink@mozilla.com>
Wed, 29 May 2019 20:46:27 +0000
changeset 476124 1566746f179c509d193423c5b5b2328024007a1f
parent 476123 01cf1b9a681c044fcc5f5a303a6fbed0b9cf0f25
child 476125 d1b2e8a0682284abcafaf7e88cd34a9b299a89d2
push id36086
push usershindli@mozilla.com
push dateThu, 30 May 2019 03:47:55 +0000
treeherdermozilla-central@73c98da145a7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1167452
milestone69.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 1167452 - Switch weakmap marking code back from GCCellPtr to plain Cell* r=jonco Differential Revision: https://phabricator.services.mozilla.com/D31952
js/src/gc/GC.cpp
js/src/gc/GCMarker.h
js/src/gc/Marking.cpp
js/src/gc/WeakMap-inl.h
js/src/gc/WeakMap.h
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -4842,17 +4842,17 @@ void js::gc::MarkingValidator::nonIncrem
     }
   }
 
   WeakMapBase::restoreMarkedWeakMaps(markedWeakMaps);
 
   for (gc::WeakKeyTable::Range r = savedWeakKeys.all(); !r.empty();
        r.popFront()) {
     AutoEnterOOMUnsafeRegion oomUnsafe;
-    Zone* zone = gc::TenuredCell::fromPointer(r.front().key.asCell())->zone();
+    Zone* zone = gc::TenuredCell::fromPointer(r.front().key)->zone();
     if (!zone->gcWeakKeys().put(std::move(r.front().key),
                                 std::move(r.front().value))) {
       oomUnsafe.crash("restoring weak keys table for validator");
     }
   }
 
   gc->incrementalState = state;
 }
--- a/js/src/gc/GCMarker.h
+++ b/js/src/gc/GCMarker.h
@@ -20,37 +20,38 @@ class WeakMapBase;
 static const size_t NON_INCREMENTAL_MARK_STACK_BASE_CAPACITY = 4096;
 static const size_t INCREMENTAL_MARK_STACK_BASE_CAPACITY = 32768;
 
 namespace gc {
 
 struct Cell;
 
 struct WeakKeyTableHashPolicy {
-  typedef JS::GCCellPtr Lookup;
-  static HashNumber hash(const Lookup& v, const mozilla::HashCodeScrambler&) {
-    return mozilla::HashGeneric(v.asCell());
+  using Lookup = Cell*;
+  static HashNumber hash(const Lookup& v,
+                         const mozilla::HashCodeScrambler& hcs) {
+    return hcs.scramble(mozilla::HashGeneric(v));
   }
-  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; }
+  static bool match(Cell* const& k, const Lookup& l) { return k == l; }
+  static bool isEmpty(Cell* const& v) { return !v; }
+  static void makeEmpty(Cell** vp) { *vp = nullptr; }
 };
 
 struct WeakMarkable {
   WeakMapBase* weakmap;
-  JS::GCCellPtr key;
+  Cell* key;
 
-  WeakMarkable(WeakMapBase* weakmapArg, JS::GCCellPtr keyArg)
+  WeakMarkable(WeakMapBase* weakmapArg, Cell* keyArg)
       : weakmap(weakmapArg), key(keyArg) {}
 };
 
 using WeakEntryVector = Vector<WeakMarkable, 2, js::SystemAllocPolicy>;
 
 using WeakKeyTable =
-    OrderedHashMap<JS::GCCellPtr, WeakEntryVector, WeakKeyTableHashPolicy,
+    OrderedHashMap<Cell*, WeakEntryVector, WeakKeyTableHashPolicy,
                    js::SystemAllocPolicy>;
 
 /*
  * When the mark stack is full, the GC does not call js::TraceChildren to mark
  * the reachable "children" of the thing. Rather the thing is put aside and
  * js::TraceChildren is called later when the mark stack is empty.
  *
  * To implement such delayed marking of the children with minimal overhead for
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -628,17 +628,17 @@ void GCMarker::markImplicitEdgesHelper(T
   if (!isWeakMarkingTracer()) {
     return;
   }
 
   Zone* zone = gc::TenuredCell::fromPointer(markedThing)->zone();
   MOZ_ASSERT(zone->isGCMarking());
   MOZ_ASSERT(!zone->isGCSweeping());
 
-  auto p = zone->gcWeakKeys().get(JS::GCCellPtr(markedThing));
+  auto p = zone->gcWeakKeys().get(markedThing);
   if (!p) {
     return;
   }
   WeakEntryVector& markables = p->value;
 
   markEphemeronValues(markedThing, markables);
   markables.clear();  // If key address is reused, it should do nothing
 }
--- a/js/src/gc/WeakMap-inl.h
+++ b/js/src/gc/WeakMap-inl.h
@@ -60,23 +60,20 @@ WeakMap<K, V>::WeakMap(JSContext* cx, JS
 // is the key in the weakmap. These will probably be the same, but can be
 // different eg when markedCell is a delegate for origKey.
 //
 // This implementation does not use 'markedCell'; it looks up origKey and checks
 // the mark bits on everything it cares about, one of which will be
 // markedCell. But a subclass might use it to optimize the liveness check.
 template <class K, class V>
 void WeakMap<K, V>::markEntry(GCMarker* marker, gc::Cell* markedCell,
-                              JS::GCCellPtr origKey) {
+                              gc::Cell* origKey) {
   MOZ_ASSERT(marked);
 
-  // If this cast fails, then you're instantiating the WeakMap with a
-  // Lookup that can't be constructed from a Cell*. The WeakKeyTable
-  // mechanism is indexed with a GCCellPtr, so that won't work.
-  Ptr p = Base::lookup(static_cast<Lookup>(origKey.asCell()));
+  Ptr p = Base::lookup(static_cast<Lookup>(origKey));
   MOZ_ASSERT(p.found());
 
   K key(p->key());
   MOZ_ASSERT((markedCell == extractUnbarriered(key)) ||
              (markedCell == getDelegate(key)));
   if (marker->isMarked(&key)) {
     TraceEdge(marker, &p->value(), "ephemeron value");
   } else if (keyNeedsMark(marker, key)) {
@@ -126,29 +123,29 @@ void WeakMap<K, V>::trace(JSTracer* trc)
   // DoNotTraceWeakMaps).
   for (Range r = Base::all(); !r.empty(); r.popFront()) {
     TraceEdge(trc, &r.front().value(), "WeakMap entry value");
   }
 }
 
 template <class K, class V>
 /* static */ void WeakMap<K, V>::addWeakEntry(
-    GCMarker* marker, JS::GCCellPtr key, const gc::WeakMarkable& markable) {
-  Zone* zone = key.asCell()->asTenured().zone();
+    GCMarker* marker, gc::Cell* key, const gc::WeakMarkable& markable) {
+  Zone* zone = key->asTenured().zone();
 
   auto p = zone->gcWeakKeys().get(key);
   if (p) {
     gc::WeakEntryVector& weakEntries = p->value;
     if (!weakEntries.append(markable)) {
       marker->abortLinearWeakMarking();
     }
   } else {
     gc::WeakEntryVector weakEntries;
     MOZ_ALWAYS_TRUE(weakEntries.append(markable));
-    if (!zone->gcWeakKeys().put(JS::GCCellPtr(key), std::move(weakEntries))) {
+    if (!zone->gcWeakKeys().put(key, std::move(weakEntries))) {
       marker->abortLinearWeakMarking();
     }
   }
 }
 
 template <class K, class V>
 bool WeakMap<K, V>::markIteratively(GCMarker* marker) {
   MOZ_ASSERT(marked);
@@ -174,21 +171,21 @@ bool WeakMap<K, V>::markIteratively(GCMa
         TraceEdge(marker, &e.front().value(), "WeakMap entry value");
         markedAny = true;
       }
     } else if (marker->isWeakMarkingTracer()) {
       // Entry is not yet known to be live. Record this weakmap and
       // the lookup key in the list of weak keys. Also record the
       // delegate, if any, because marking the delegate also marks
       // the entry.
-      JS::GCCellPtr weakKey(extractUnbarriered(e.front().key()));
+      gc::Cell* weakKey = extractUnbarriered(e.front().key());
       gc::WeakMarkable markable(this, weakKey);
       addWeakEntry(marker, weakKey, markable);
       if (JSObject* delegate = getDelegate(e.front().key())) {
-        addWeakEntry(marker, JS::GCCellPtr(delegate), markable);
+        addWeakEntry(marker, delegate, markable);
       }
     }
   }
 
   return markedAny;
 }
 
 template <class K, class V>
--- a/js/src/gc/WeakMap.h
+++ b/js/src/gc/WeakMap.h
@@ -111,17 +111,17 @@ class WeakMapBase : public mozilla::Link
   virtual bool findZoneEdges() = 0;
   virtual void sweep() = 0;
   virtual void traceMappings(WeakMapTracer* tracer) = 0;
   virtual void clearAndCompact() = 0;
 
   // Any weakmap key types that want to participate in the non-iterative
   // ephemeron marking must override this method.
   virtual void markEntry(GCMarker* marker, gc::Cell* markedCell,
-                         JS::GCCellPtr l) = 0;
+                         gc::Cell* l) = 0;
 
   virtual bool markIteratively(GCMarker* marker) = 0;
 
 #ifdef JS_GC_ZEAL
   virtual bool checkMarking() const = 0;
   virtual bool allowKeysInOtherZones() const { return false; }
   friend bool gc::CheckWeakMapEntryMarking(const WeakMapBase*, gc::Cell*,
                                            gc::Cell*);
@@ -195,22 +195,22 @@ class WeakMap
   MOZ_MUST_USE bool relookupOrAdd(AddPtr& ptr, KeyInput&& key,
                                   ValueInput&& value) {
     MOZ_ASSERT(key);
     return Base::relookupOrAdd(ptr, std::forward<KeyInput>(key),
                                std::forward<ValueInput>(value));
   }
 
   void markEntry(GCMarker* marker, gc::Cell* markedCell,
-                 JS::GCCellPtr origKey) override;
+                 gc::Cell* origKey) override;
 
   void trace(JSTracer* trc) override;
 
  protected:
-  static void addWeakEntry(GCMarker* marker, JS::GCCellPtr key,
+  static void addWeakEntry(GCMarker* marker, gc::Cell* key,
                            const gc::WeakMarkable& markable);
 
   bool markIteratively(GCMarker* marker) override;
 
   /**
    * If a wrapper is used as a key in a weakmap, the garbage collector should
    * keep that object around longer than it otherwise would. We want to avoid
    * collecting the wrapper (and removing the weakmap entry) as long as the