Bug 1479388 - Move most WeakMap inline method definitions into a separate header r=sfink
☠☠ backed out by 959675040740 ☠ ☠
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 31 Jul 2018 09:50:08 +0100
changeset 429363 e988c40d551901b8608c038d76d36dbb44409f8c
parent 429362 4d8ba2d6bd3567eb334c1321761c9217108b6fad
child 429364 a5cff647a86e5a16e793a5c6c13a9fbe0a9c9509
push id34362
push userbtara@mozilla.com
push dateTue, 31 Jul 2018 18:30:34 +0000
treeherdermozilla-central@5a5fb40fb922 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1479388
milestone63.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 1479388 - Move most WeakMap inline method definitions into a separate header r=sfink
js/src/builtin/WeakMapObject-inl.h
js/src/builtin/WeakMapObject.h
js/src/frontend/SharedContext.h
js/src/gc/WeakMap.cpp
js/src/gc/WeakMap.h
js/src/gc/WeakMapPtr.cpp
js/src/vm/Debugger-inl.h
js/src/vm/Debugger.h
js/src/vm/GlobalObject.h
--- a/js/src/builtin/WeakMapObject-inl.h
+++ b/js/src/builtin/WeakMapObject-inl.h
@@ -6,16 +6,18 @@
 
 #ifndef builtin_WeakMapObject_inl_h
 #define builtin_WeakMapObject_inl_h
 
 #include "builtin/WeakMapObject.h"
 
 #include "vm/ProxyObject.h"
 
+#include "gc/WeakMap-inl.h"
+
 namespace js {
 
 static bool
 TryPreserveReflector(JSContext* cx, HandleObject obj)
 {
     if (obj->getClass()->isWrappedNative() ||
         obj->getClass()->isDOMClass() ||
         (obj->is<ProxyObject>() &&
--- a/js/src/builtin/WeakMapObject.h
+++ b/js/src/builtin/WeakMapObject.h
@@ -3,17 +3,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef builtin_WeakMapObject_h
 #define builtin_WeakMapObject_h
 
 #include "gc/WeakMap.h"
-#include "vm/JSObject.h"
+#include "vm/NativeObject.h"
 
 namespace js {
 
 class GlobalObject;
 
 // Abstract base class for WeakMapObject and WeakSetObject.
 class WeakCollectionObject : public NativeObject
 {
--- a/js/src/frontend/SharedContext.h
+++ b/js/src/frontend/SharedContext.h
@@ -9,16 +9,17 @@
 
 #include "jspubtd.h"
 #include "jstypes.h"
 
 #include "builtin/ModuleObject.h"
 #include "ds/InlineTable.h"
 #include "frontend/ParseNode.h"
 #include "frontend/TokenStream.h"
+#include "gc/Zone.h"
 #include "vm/BytecodeUtil.h"
 #include "vm/EnvironmentObject.h"
 #include "vm/JSAtom.h"
 #include "vm/JSScript.h"
 
 namespace js {
 namespace frontend {
 
--- a/js/src/gc/WeakMap.cpp
+++ b/js/src/gc/WeakMap.cpp
@@ -1,15 +1,15 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "gc/WeakMap.h"
+#include "gc/WeakMap-inl.h"
 
 #include <string.h>
 
 #include "jsapi.h"
 #include "jsfriendapi.h"
 
 #include "gc/PublicIterators.h"
 #include "js/Wrapper.h"
--- a/js/src/gc/WeakMap.h
+++ b/js/src/gc/WeakMap.h
@@ -3,30 +3,34 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef gc_WeakMap_h
 #define gc_WeakMap_h
 
 #include "mozilla/LinkedList.h"
-#include "mozilla/Move.h"
 
-#include "jsfriendapi.h"
-
+#include "gc/Barrier.h"
 #include "gc/DeletePolicy.h"
-#include "gc/StoreBuffer.h"
 #include "js/HashTable.h"
-#include "vm/JSObject.h"
-#include "vm/Realm.h"
+
+namespace JS {
+class Zone;
+} // namespace JS
 
 namespace js {
 
 class GCMarker;
 class WeakMapBase;
+struct WeakMapTracer;
+
+namespace gc {
+struct WeakMarkable;
+} // namespace gc
 
 // A subclass template of js::HashMap whose keys and values may be garbage-collected. When
 // a key is collected, the table entry disappears, dropping its reference to the value.
 //
 // More precisely:
 //
 //     A WeakMap entry is live if and only if both the WeakMap and the entry's key
 //     are live. An entry holds a strong reference to its value.
@@ -44,17 +48,17 @@ typedef HashSet<WeakMapBase*, DefaultHas
 class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase>
 {
     friend class js::GCMarker;
 
   public:
     WeakMapBase(JSObject* memOf, JS::Zone* zone);
     virtual ~WeakMapBase();
 
-    Zone* zone() const { return zone_; }
+    JS::Zone* zone() const { return zone_; }
 
     // Garbage collector entry points.
 
     // Unmark all weak maps in a zone.
     static void unmarkZone(JS::Zone* zone);
 
     // Mark all the weakmaps in a zone.
     static void traceZone(JS::Zone* zone, JSTracer* tracer);
@@ -102,51 +106,33 @@ class WeakMapBase : public mozilla::Link
 
     // Zone containing this weak map.
     JS::Zone* zone_;
 
     // Whether this object has been traced during garbage collection.
     bool marked;
 };
 
-template <typename T>
-static T extractUnbarriered(const WriteBarrieredBase<T>& v)
-{
-    return v.get();
-}
-template <typename T>
-static T* extractUnbarriered(T* v)
-{
-    return v;
-}
-
 template <class Key, class Value,
           class HashPolicy = DefaultHasher<Key> >
 class WeakMap : public HashMap<Key, Value, HashPolicy, ZoneAllocPolicy>,
                 public WeakMapBase
 {
   public:
     typedef HashMap<Key, Value, HashPolicy, ZoneAllocPolicy> Base;
     typedef typename Base::Enum Enum;
     typedef typename Base::Lookup Lookup;
     typedef typename Base::Entry Entry;
     typedef typename Base::Range Range;
     typedef typename Base::Ptr Ptr;
     typedef typename Base::AddPtr AddPtr;
 
-    explicit WeakMap(JSContext* cx, JSObject* memOf = nullptr)
-        : Base(cx->zone()), WeakMapBase(memOf, cx->zone()) { }
+    explicit WeakMap(JSContext* cx, JSObject* memOf = nullptr);
 
-    bool init(uint32_t len = 16) {
-        if (!Base::init(len))
-            return false;
-        zone()->gcWeakMapList().insertFront(this);
-        marked = JS::IsIncrementalGCInProgress(TlsContext.get());
-        return true;
-    }
+    bool init(uint32_t len = 16);
 
     // Overwritten to add a read barrier to prevent an incorrectly gray value
     // from escaping the weak map. See the UnmarkGrayTracer::onChild comment in
     // gc/Marking.cpp.
     Ptr lookup(const Lookup& l) const {
         Ptr p = Base::lookup(l);
         if (p)
             exposeGCThingToActiveJS(p->value());
@@ -165,214 +151,56 @@ class WeakMap : public HashMap<Key, Valu
         if (p)
             exposeGCThingToActiveJS(p->value());
         return p;
     }
 
     // Resolve ambiguity with LinkedListElement<>::remove.
     using Base::remove;
 
-    // Trace a WeakMap entry based on 'markedCell' getting marked, where
-    // 'origKey' 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.
-    void markEntry(GCMarker* marker, gc::Cell* markedCell, JS::GCCellPtr origKey) override
-    {
-        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()));
-        MOZ_ASSERT(p.found());
-
-        Key key(p->key());
-        MOZ_ASSERT((markedCell == extractUnbarriered(key)) || (markedCell == getDelegate(key)));
-        if (gc::IsMarked(marker->runtime(), &key)) {
-            TraceEdge(marker, &p->value(), "ephemeron value");
-        } else if (keyNeedsMark(key)) {
-            TraceEdge(marker, &p->value(), "WeakMap ephemeron value");
-            TraceEdge(marker, &key, "proxy-preserved WeakMap ephemeron key");
-            MOZ_ASSERT(key == p->key()); // No moving
-        }
-        key.unsafeSet(nullptr); // Prevent destructor from running barriers.
-    }
+    void markEntry(GCMarker* marker, gc::Cell* markedCell, JS::GCCellPtr origKey) override;
 
-    void trace(JSTracer* trc) override {
-        MOZ_ASSERT_IF(JS::RuntimeHeapIsBusy(), isInList());
-
-        TraceNullableEdge(trc, &memberOf, "WeakMap owner");
-
-        if (!Base::initialized())
-            return;
-
-        if (trc->isMarkingTracer()) {
-            MOZ_ASSERT(trc->weakMapAction() == ExpandWeakMaps);
-            marked = true;
-            (void) markIteratively(GCMarker::fromTracer(trc));
-            return;
-        }
-
-        if (trc->weakMapAction() == DoNotTraceWeakMaps)
-            return;
-
-        // Trace keys only if weakMapAction() says to.
-        if (trc->weakMapAction() == TraceWeakMapKeysValues) {
-            for (Enum e(*this); !e.empty(); e.popFront())
-                TraceEdge(trc, &e.front().mutableKey(), "WeakMap entry key");
-        }
-
-        // Always trace all values (unless weakMapAction() is
-        // DoNotTraceWeakMaps).
-        for (Range r = Base::all(); !r.empty(); r.popFront())
-            TraceEdge(trc, &r.front().value(), "WeakMap entry value");
-    }
+    void trace(JSTracer* trc) override;
 
   protected:
-    static void addWeakEntry(GCMarker* marker, JS::GCCellPtr key, gc::WeakMarkable markable)
-    {
-        Zone* zone = key.asCell()->asTenured().zone();
-
-        auto p = zone->gcWeakKeys().get(key);
-        if (p) {
-            gc::WeakEntryVector& weakEntries = p->value;
-            if (!weakEntries.append(std::move(markable)))
-                marker->abortLinearWeakMarking();
-        } else {
-            gc::WeakEntryVector weakEntries;
-            MOZ_ALWAYS_TRUE(weakEntries.append(std::move(markable)));
-            if (!zone->gcWeakKeys().put(JS::GCCellPtr(key), std::move(weakEntries)))
-                marker->abortLinearWeakMarking();
-        }
-    }
-
-    bool markIteratively(GCMarker* marker) override {
-        MOZ_ASSERT(marked);
-
-        bool markedAny = false;
-
-        for (Enum e(*this); !e.empty(); e.popFront()) {
-            // If the entry is live, ensure its key and value are marked.
-            bool keyIsMarked = gc::IsMarked(marker->runtime(), &e.front().mutableKey());
-            if (!keyIsMarked && keyNeedsMark(e.front().key())) {
-                TraceEdge(marker, &e.front().mutableKey(), "proxy-preserved WeakMap entry key");
-                keyIsMarked = true;
-                markedAny = true;
-            }
+    static void addWeakEntry(GCMarker* marker, JS::GCCellPtr key,
+                             const gc::WeakMarkable& markable);
 
-            if (keyIsMarked) {
-                if (!gc::IsMarked(marker->runtime(), &e.front().value())) {
-                    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::WeakMarkable markable(this, weakKey);
-                addWeakEntry(marker, weakKey, markable);
-                if (JSObject* delegate = getDelegate(e.front().key()))
-                    addWeakEntry(marker, JS::GCCellPtr(delegate), markable);
-            }
-        }
+    bool markIteratively(GCMarker* marker) override;
 
-        return markedAny;
-    }
-
-    JSObject* getDelegate(JSObject* key) const {
-        JSWeakmapKeyDelegateOp op = key->getClass()->extWeakmapKeyDelegateOp();
-        if (!op)
-            return nullptr;
-
-        JSObject* obj = op(key);
-        if (!obj)
-            return nullptr;
-
-        MOZ_ASSERT(obj->runtimeFromMainThread() == zone()->runtimeFromMainThread());
-        return obj;
-    }
-
-    JSObject* getDelegate(JSScript* script) const {
-        return nullptr;
-    }
-    JSObject* getDelegate(LazyScript* script) const {
-        return nullptr;
-    }
+    JSObject* getDelegate(JSObject* key) const;
+    JSObject* getDelegate(JSScript* script) const;
+    JSObject* getDelegate(LazyScript* script) const;
 
   private:
     void exposeGCThingToActiveJS(const JS::Value& v) const { JS::ExposeValueToActiveJS(v); }
     void exposeGCThingToActiveJS(JSObject* obj) const { JS::ExposeObjectToActiveJS(obj); }
 
-    bool keyNeedsMark(JSObject* key) const {
-        JSObject* delegate = getDelegate(key);
-        /*
-         * Check if the delegate is marked with any color to properly handle
-         * gray marking when the key's delegate is black and the map is gray.
-         */
-        return delegate && gc::IsMarkedUnbarriered(zone()->runtimeFromMainThread(), &delegate);
-    }
-
-    bool keyNeedsMark(JSScript* script) const {
-        return false;
-    }
-    bool keyNeedsMark(LazyScript* script) const {
-        return false;
-    }
+    bool keyNeedsMark(JSObject* key) const;
+    bool keyNeedsMark(JSScript* script) const;
+    bool keyNeedsMark(LazyScript* script) const;
 
     bool findZoneEdges() override {
         // This is overridden by ObjectValueMap.
         return true;
     }
 
-    void sweep() override {
-        /* Remove all entries whose keys remain unmarked. */
-        for (Enum e(*this); !e.empty(); e.popFront()) {
-            if (gc::IsAboutToBeFinalized(&e.front().mutableKey()))
-                e.removeFront();
-        }
-        /*
-         * Once we've swept, all remaining edges should stay within the
-         * known-live part of the graph.
-         */
-        assertEntriesNotAboutToBeFinalized();
-    }
+    void sweep() override;
 
     void finish() override {
         Base::finish();
     }
 
     /* memberOf can be nullptr, which means that the map is not part of a JSObject. */
-    void traceMappings(WeakMapTracer* tracer) override {
-        for (Range r = Base::all(); !r.empty(); r.popFront()) {
-            gc::Cell* key = gc::ToMarkable(r.front().key());
-            gc::Cell* value = gc::ToMarkable(r.front().value());
-            if (key && value) {
-                tracer->trace(memberOf,
-                              JS::GCCellPtr(r.front().key().get()),
-                              JS::GCCellPtr(r.front().value().get()));
-            }
-        }
-    }
+    void traceMappings(WeakMapTracer* tracer) override;
 
   protected:
-    void assertEntriesNotAboutToBeFinalized() {
 #if DEBUG
-        for (Range r = Base::all(); !r.empty(); r.popFront()) {
-            Key k(r.front().key());
-            MOZ_ASSERT(!gc::IsAboutToBeFinalized(&k));
-            MOZ_ASSERT(!gc::IsAboutToBeFinalized(&r.front().value()));
-            MOZ_ASSERT(k == r.front().key());
-        }
+    void assertEntriesNotAboutToBeFinalized();
 #endif
-    }
 };
 
 
 class ObjectValueMap : public WeakMap<HeapPtr<JSObject*>, HeapPtr<Value>,
                                       MovableCellHasher<HeapPtr<JSObject*>>>
 {
   public:
     ObjectValueMap(JSContext* cx, JSObject* obj)
--- a/js/src/gc/WeakMapPtr.cpp
+++ b/js/src/gc/WeakMapPtr.cpp
@@ -1,17 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "js/WeakMapPtr.h"
 
-#include "gc/WeakMap.h"
+#include "gc/WeakMap-inl.h"
 
 //
 // Machinery for the externally-linkable JS::WeakMapPtr, which wraps js::WeakMap
 // for a few public data types.
 //
 
 using namespace js;
 
--- a/js/src/vm/Debugger-inl.h
+++ b/js/src/vm/Debugger-inl.h
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef vm_Debugger_inl_h
 #define vm_Debugger_inl_h
 
 #include "vm/Debugger.h"
 
+#include "gc/WeakMap-inl.h"
 #include "vm/Stack-inl.h"
 
 /* static */ inline bool
 js::Debugger::onLeaveFrame(JSContext* cx, AbstractFramePtr frame, jsbytecode* pc, bool ok)
 {
     MOZ_ASSERT_IF(frame.isInterpreterFrame(), frame.asInterpreterFrame() == cx->interpreterFrame());
     MOZ_ASSERT_IF(frame.hasScript() && frame.script()->isDebuggee(), frame.isDebuggee());
     /* Traps must be cleared from eval frames, see slowPathOnLeaveFrame. */
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -214,17 +214,19 @@ class DebuggerWeakMap : private WeakMap<
     void sweep() override {
         MOZ_ASSERT(CurrentThreadIsPerformingGC());
         for (Enum e(*static_cast<Base*>(this)); !e.empty(); e.popFront()) {
             if (gc::IsAboutToBeFinalized(&e.front().mutableKey())) {
                 decZoneCount(e.front().key()->zoneFromAnyThread());
                 e.removeFront();
             }
         }
+#ifdef DEBUG
         Base::assertEntriesNotAboutToBeFinalized();
+#endif
     }
 
     MOZ_MUST_USE bool incZoneCount(JS::Zone* zone) {
         CountMap::Ptr p = zoneCounts.lookupWithDefault(zone, 0);
         if (!p)
             return false;
         ++p->value();
         return true;
--- a/js/src/vm/GlobalObject.h
+++ b/js/src/vm/GlobalObject.h
@@ -11,16 +11,17 @@
 #include "jsnum.h"
 
 #include "builtin/Array.h"
 #include "builtin/Boolean.h"
 #include "js/Vector.h"
 #include "vm/ArrayBufferObject.h"
 #include "vm/ErrorObject.h"
 #include "vm/JSFunction.h"
+#include "vm/Realm.h"
 #include "vm/RegExpStatics.h"
 #include "vm/Runtime.h"
 
 namespace js {
 
 class Debugger;
 class TypedObjectModuleObject;
 class LexicalEnvironmentObject;