From e0ec59962ba2413709f86ce400c460466cbc7beb Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Tue, 31 Jul 2018 09:50:08 +0100 Subject: [PATCH] Bug 1479388 - Move most WeakMap inline method definitions into a separate header r=sfink --- js/src/builtin/WeakMapObject-inl.h | 2 + js/src/builtin/WeakMapObject.h | 2 +- js/src/frontend/SharedContext.h | 1 + js/src/gc/WeakMap.cpp | 2 +- js/src/gc/WeakMap.h | 226 ++++------------------------- js/src/gc/WeakMapPtr.cpp | 2 +- js/src/vm/Debugger-inl.h | 1 + js/src/vm/Debugger.h | 2 + js/src/vm/GlobalObject.h | 1 + 9 files changed, 37 insertions(+), 202 deletions(-) diff --git a/js/src/builtin/WeakMapObject-inl.h b/js/src/builtin/WeakMapObject-inl.h index f249a6bd14e0..be63310c78a2 100644 --- a/js/src/builtin/WeakMapObject-inl.h +++ b/js/src/builtin/WeakMapObject-inl.h @@ -11,6 +11,8 @@ #include "vm/ProxyObject.h" +#include "gc/WeakMap-inl.h" + namespace js { static bool diff --git a/js/src/builtin/WeakMapObject.h b/js/src/builtin/WeakMapObject.h index a3f365f057d3..2a1511926ec1 100644 --- a/js/src/builtin/WeakMapObject.h +++ b/js/src/builtin/WeakMapObject.h @@ -8,7 +8,7 @@ #define builtin_WeakMapObject_h #include "gc/WeakMap.h" -#include "vm/JSObject.h" +#include "vm/NativeObject.h" namespace js { diff --git a/js/src/frontend/SharedContext.h b/js/src/frontend/SharedContext.h index 636ebfd366e7..41f442d1f1e5 100644 --- a/js/src/frontend/SharedContext.h +++ b/js/src/frontend/SharedContext.h @@ -14,6 +14,7 @@ #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" diff --git a/js/src/gc/WeakMap.cpp b/js/src/gc/WeakMap.cpp index 22060f65be78..79e9b426014e 100644 --- a/js/src/gc/WeakMap.cpp +++ b/js/src/gc/WeakMap.cpp @@ -4,7 +4,7 @@ * 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 diff --git a/js/src/gc/WeakMap.h b/js/src/gc/WeakMap.h index 15991d968476..1ec7c145c4e0 100644 --- a/js/src/gc/WeakMap.h +++ b/js/src/gc/WeakMap.h @@ -8,20 +8,24 @@ #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. @@ -49,7 +53,7 @@ class WeakMapBase : public mozilla::LinkedListElement WeakMapBase(JSObject* memOf, JS::Zone* zone); virtual ~WeakMapBase(); - Zone* zone() const { return zone_; } + JS::Zone* zone() const { return zone_; } // Garbage collector entry points. @@ -107,17 +111,6 @@ class WeakMapBase : public mozilla::LinkedListElement bool marked; }; -template -static T extractUnbarriered(const WriteBarrieredBase& v) -{ - return v.get(); -} -template -static T* extractUnbarriered(T* v) -{ - return v; -} - template > class WeakMap : public HashMap, @@ -132,16 +125,9 @@ class WeakMap : public HashMap, 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 @@ -170,204 +156,46 @@ class WeakMap : public HashMap, // 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); + void markEntry(GCMarker* marker, gc::Cell* markedCell, JS::GCCellPtr origKey) override; - // 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(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 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(); + static void addWeakEntry(GCMarker* marker, JS::GCCellPtr key, + const gc::WeakMarkable& markable); - 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; - 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; - } - - 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); - } - } - - 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 - } }; diff --git a/js/src/gc/WeakMapPtr.cpp b/js/src/gc/WeakMapPtr.cpp index 8e7fa22a1abf..5421a66d22e4 100644 --- a/js/src/gc/WeakMapPtr.cpp +++ b/js/src/gc/WeakMapPtr.cpp @@ -6,7 +6,7 @@ #include "js/WeakMapPtr.h" -#include "gc/WeakMap.h" +#include "gc/WeakMap-inl.h" // // Machinery for the externally-linkable JS::WeakMapPtr, which wraps js::WeakMap diff --git a/js/src/vm/Debugger-inl.h b/js/src/vm/Debugger-inl.h index 7b57908b229b..bc4a338cb3c7 100644 --- a/js/src/vm/Debugger-inl.h +++ b/js/src/vm/Debugger-inl.h @@ -9,6 +9,7 @@ #include "vm/Debugger.h" +#include "gc/WeakMap-inl.h" #include "vm/Stack-inl.h" /* static */ inline bool diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index 6eb92f066111..3ecd5ee7b342 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -219,7 +219,9 @@ class DebuggerWeakMap : private WeakMap, HeapPtr