mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1593399 - Rework how mark colors are handled in weakmap marking r=jonco 

Differential Revision: https://phabricator.services.mozilla.com/D51492

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Steve Fink 2019-11-12 19:54:06 +00:00
Родитель 63db4cb9ba
Коммит b3059609b0
13 изменённых файлов: 350 добавлений и 235 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

2
js/public/HeapAPI.h
Просмотреть файл

@ -379,8 +379,6 @@ class JS_FRIEND_API GCCellPtr {
return uintptr_t(p) | (uintptr_t(traceKind) & OutOfLineTraceKindMask);
}
bool mayBeOwnedByOtherRuntimeSlow() const;
JS::TraceKind outOfLineKind() const;
uintptr_t ptr;

61
js/src/gc/Cell.h
Просмотреть файл

@ -52,6 +52,54 @@ class TenuredCell;
extern bool CurrentThreadIsGCMarking();
#endif
// Like gc::MarkColor but allows the possibility of the cell being unmarked.
//
// This class mimics an enum class, but supports operator overloading.
class CellColor {
public:
enum Color { White = 0, Gray = 1, Black = 2 };
CellColor() : color(White) {}
MOZ_IMPLICIT CellColor(MarkColor markColor)
: color(markColor == MarkColor::Black ? Black : Gray) {}
MOZ_IMPLICIT constexpr CellColor(Color c) : color(c) {}
MarkColor asMarkColor() const {
MOZ_ASSERT(color != White);
return color == Black ? MarkColor::Black : MarkColor::Gray;
}
// Implement a total ordering for CellColor, with white being 'least marked'
// and black being 'most marked'.
bool operator<(const CellColor other) const { return color < other.color; }
bool operator>(const CellColor other) const { return color > other.color; }
bool operator<=(const CellColor other) const { return color <= other.color; }
bool operator>=(const CellColor other) const { return color >= other.color; }
bool operator!=(const CellColor other) const { return color != other.color; }
bool operator==(const CellColor other) const { return color == other.color; }
explicit operator bool() const { return color != White; }
#if defined(JS_GC_ZEAL) || defined(DEBUG)
const char* name() const {
switch (color) {
case CellColor::White:
return "white";
case CellColor::Black:
return "black";
case CellColor::Gray:
return "gray";
default:
MOZ_CRASH("Unexpected cell color");
}
}
#endif
private:
Color color;
};
// [SMDOC] GC Cell
//
// A GC cell is the base class for all GC things. All types allocated on the GC
@ -90,6 +138,12 @@ struct alignas(gc::CellAlignBytes) Cell {
MOZ_ALWAYS_INLINE bool isMarked(gc::MarkColor color) const;
MOZ_ALWAYS_INLINE bool isMarkedAtLeast(gc::MarkColor color) const;
MOZ_ALWAYS_INLINE CellColor color() const {
return isMarkedBlack()
? CellColor::Black
: isMarkedGray() ? CellColor::Gray : CellColor::White;
}
inline JSRuntime* runtimeFromMainThread() const;
// Note: Unrestricted access to the runtime of a GC thing from an arbitrary
@ -169,6 +223,13 @@ class TenuredCell : public Cell {
MOZ_ALWAYS_INLINE bool isMarkedBlack() const;
MOZ_ALWAYS_INLINE bool isMarkedGray() const;
// Same as Cell::color, but skips nursery checks.
MOZ_ALWAYS_INLINE CellColor color() const {
return isMarkedBlack()
? CellColor::Black
: isMarkedGray() ? CellColor::Gray : CellColor::White;
}
// The return value indicates if the cell went from unmarked to marked.
MOZ_ALWAYS_INLINE bool markIfUnmarked(
MarkColor color = MarkColor::Black) const;

2
js/src/gc/GC.cpp
Просмотреть файл

@ -4300,7 +4300,7 @@ void js::gc::MarkingValidator::nonIncrementalMark(AutoGCSession& session) {
* collecting.
*/
WeakMapSet markedWeakMaps;
WeakMapColors markedWeakMaps;
/*
* For saving, smush all of the keys into one big table and split them back

18
js/src/gc/GCMarker.h
Просмотреть файл

@ -284,21 +284,6 @@ class GCMarker : public JSTracer {
// must be empty when this is called.
void setMainStackColor(gc::MarkColor newColor);
// Return whether a cell is marked relative to the current marking color. If
// the cell is black then this returns true, but if it's gray it will return
// false if the mark color is black.
template <typename T>
bool isMarked(T* thingp) {
return color == gc::MarkColor::Black ? gc::IsMarkedBlack(runtime(), thingp)
: gc::IsMarked(runtime(), thingp);
}
template <typename T>
bool isMarkedUnbarriered(T* thingp) {
return color == gc::MarkColor::Black
? gc::IsMarkedBlackUnbarriered(runtime(), thingp)
: gc::IsMarkedUnbarriered(runtime(), thingp);
}
void enterWeakMarkingMode();
void leaveWeakMarkingMode();
void abortLinearWeakMarking() {
@ -513,6 +498,9 @@ class MOZ_RAII AutoSetMarkColor {
marker_.setMarkColor(newColor);
}
AutoSetMarkColor(GCMarker& marker, CellColor newColor)
: AutoSetMarkColor(marker, newColor.asMarkColor()) {}
~AutoSetMarkColor() { marker_.setMarkColor(initialColor_); }
};

13
js/src/gc/Marking.cpp
Просмотреть файл

@ -12,6 +12,7 @@
#include "mozilla/ReentrancyGuard.h"
#include "mozilla/ScopeExit.h"
#include "mozilla/TypeTraits.h"
#include "mozilla/Unused.h"
#include <algorithm>
@ -41,6 +42,7 @@
#include "gc/GC-inl.h"
#include "gc/Nursery-inl.h"
#include "gc/PrivateIterators-inl.h"
#include "gc/WeakMap-inl.h"
#include "gc/Zone-inl.h"
#include "vm/GeckoProfiler-inl.h"
#include "vm/NativeObject-inl.h"
@ -654,12 +656,7 @@ void GCMarker::markEphemeronValues(gc::Cell* markedCell,
DebugOnly<size_t> initialLen = values.length();
for (const auto& markable : values) {
if (color == gc::MarkColor::Black &&
markable.weakmap->markColor == gc::MarkColor::Gray) {
continue;
}
markable.weakmap->markEntry(this, markedCell, markable.key);
markable.weakmap->markKey(this, markedCell, markable.key);
}
// The vector should not be appended to during iteration because the key is
@ -2607,8 +2604,8 @@ void GCMarker::enterWeakMarkingMode() {
for (SweepGroupZonesIter zone(runtime()); !zone.done(); zone.next()) {
for (WeakMapBase* m : zone->gcWeakMapList()) {
if (m->marked) {
(void)m->markEntries(this);
if (m->mapColor) {
mozilla::Unused << m->markEntries(this);
}
}
}

48
js/src/gc/Verifier.cpp
Просмотреть файл

@ -10,6 +10,8 @@
#include "mozilla/IntegerPrintfMacros.h"
#include "mozilla/Sprintf.h"
#include <algorithm>
#ifdef MOZ_VALGRIND
# include <valgrind/memcheck.h>
#endif
@ -453,23 +455,6 @@ void js::gc::GCRuntime::finishVerifier() {
#if defined(JS_GC_ZEAL) || defined(DEBUG)
static const char* CellColorName(CellColor color) {
switch (color) {
case CellColor::White:
return "white";
case CellColor::Black:
return "black";
case CellColor::Gray:
return "gray";
default:
MOZ_CRASH("Unexpected cell color");
}
}
static const char* GetCellColorName(Cell* cell) {
return CellColorName(GetCellColor(cell));
}
class HeapCheckTracerBase : public JS::CallbackTracer {
public:
explicit HeapCheckTracerBase(JSRuntime* rt, WeakMapTraceKind weakTraceKind);
@ -587,7 +572,7 @@ void HeapCheckTracerBase::dumpCellInfo(Cell* cell) {
JSObject* obj =
kind == JS::TraceKind::Object ? static_cast<JSObject*>(cell) : nullptr;
fprintf(stderr, "%s %s", GetCellColorName(cell), GCTraceKindToAscii(kind));
fprintf(stderr, "%s %s", cell->color().name(), GCTraceKindToAscii(kind));
if (obj) {
fprintf(stderr, " %s", obj->getClass()->name);
}
@ -769,31 +754,26 @@ bool js::gc::CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key,
Zone* valueZone = GetCellZoneFromAnyThread(value);
MOZ_ASSERT(valueZone == zone || valueZone->isAtomsZone());
CellColor mapColor =
map->markColor == MarkColor::Black ? CellColor::Black : CellColor::Gray;
if (object && GetCellColor(object) != mapColor) {
if (object && object->color() != map->mapColor) {
fprintf(stderr, "WeakMap object is marked differently to the map\n");
fprintf(stderr, "(map %p is %s, object %p is %s)\n", map,
CellColorName(mapColor), object,
CellColorName(GetCellColor(object)));
map->mapColor.name(), object, object->color().name());
ok = false;
}
CellColor keyColor = GetCellColor(key);
// Values belonging to other runtimes or in uncollected zones are treated as
// black.
CellColor valueColor = CellColor::Black;
if (value->runtimeFromAnyThread() == zone->runtimeFromAnyThread() &&
valueZone->isGCMarking()) {
valueColor = GetCellColor(value);
valueColor = value->color();
}
if (valueColor < ExpectedWeakMapValueColor(mapColor, keyColor)) {
if (valueColor < std::min(map->mapColor, key->color())) {
fprintf(stderr, "WeakMap value is less marked than map and key\n");
fprintf(stderr, "(map %p is %s, key %p is %s, value %p is %s)\n", map,
CellColorName(mapColor), key, CellColorName(keyColor), value,
CellColorName(valueColor));
map->mapColor.name(), key, key->color().name(), value,
valueColor.name());
ok = false;
}
@ -812,17 +792,17 @@ bool js::gc::CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key,
CellColor delegateColor;
if (delegate->zone()->isGCMarking() || delegate->zone()->isGCSweeping()) {
delegateColor = GetCellColor(delegate);
delegateColor = delegate->color();
} else {
// IsMarked() assumes cells in uncollected zones are marked.
delegateColor = CellColor::Black;
}
if (keyColor < ExpectedWeakMapKeyColor(mapColor, delegateColor)) {
fprintf(stderr, "WeakMap key is less marked than map and delegate\n");
if (key->color() < std::min(map->mapColor, delegateColor)) {
fprintf(stderr, "WeakMap key is less marked than map or delegate\n");
fprintf(stderr, "(map %p is %s, delegate %p is %s, key %p is %s)\n", map,
CellColorName(mapColor), delegate, CellColorName(delegateColor),
key, CellColorName(keyColor));
map->mapColor.name(), delegate, delegateColor.name(), key,
key->color().name());
ok = false;
}

35
js/src/gc/Verifier.h
Просмотреть файл

@ -18,47 +18,12 @@
namespace js {
namespace gc {
// Like gc::MarkColor but allows the possibility of the cell being
// unmarked.
enum class CellColor : uint8_t {
White = 0,
Gray = uint8_t(MarkColor::Gray),
Black = uint8_t(MarkColor::Black)
};
static constexpr CellColor AllCellColors[] = {CellColor::White, CellColor::Gray,
CellColor::Black};
static constexpr CellColor MarkedCellColors[] = {CellColor::Gray,
CellColor::Black};
inline CellColor GetCellColor(Cell* cell) {
if (cell->isMarkedBlack()) {
return CellColor::Black;
}
if (cell->isMarkedGray()) {
return CellColor::Gray;
}
return CellColor::White;
}
inline CellColor ExpectedWeakMapValueColor(CellColor keyColor,
CellColor mapColor) {
return std::min(keyColor, mapColor);
}
inline CellColor ExpectedWeakMapKeyColor(CellColor mapColor,
CellColor delegateColor) {
return std::min(mapColor, delegateColor);
}
inline CellColor ExpectedKeyAndDelegateColor(CellColor keyColor,
CellColor delegateColor) {
return std::max(keyColor, delegateColor);
}
} // namespace gc
} // namespace js

211
js/src/gc/WeakMap-inl.h
Просмотреть файл

@ -9,38 +9,78 @@
#include "gc/WeakMap.h"
#include "mozilla/Unused.h"
#include <algorithm>
#include "gc/Zone.h"
#include "js/TraceKind.h"
#include "vm/JSContext.h"
namespace js {
namespace gc {
// Specializations for barriered types.
template <typename T>
inline Cell* ToMarkable(WriteBarriered<T>* thingp) {
return ToMarkable(thingp->get());
}
namespace detail {
template <typename T>
static T extractUnbarriered(const WriteBarriered<T>& v) {
static T ExtractUnbarriered(const WriteBarriered<T>& v) {
return v.get();
}
template <typename T>
static T* extractUnbarriered(T* v) {
static T* ExtractUnbarriered(T* v) {
return v;
}
inline /* static */ JSObject* WeakMapBase::getDelegate(JSObject* key) {
if (!IsWrapper(key)) {
return nullptr;
// Return the effective cell color given the current marking state.
// This must be kept in sync with ShouldMark in Marking.cpp.
template <typename T>
static CellColor GetEffectiveColor(JSRuntime* rt, const T& item) {
Cell* cell = ToMarkable(item);
if (!cell->isTenured()) {
return CellColor::Black;
}
return UncheckedUnwrapWithoutExpose(key);
const TenuredCell& t = cell->asTenured();
if (rt != t.runtimeFromAnyThread()) {
return CellColor::Black;
}
if (!t.zoneFromAnyThread()->shouldMarkInZone()) {
return CellColor::Black;
}
return cell->color();
}
inline /* static */ JSObject* WeakMapBase::getDelegate(JSScript* script) {
// Only objects have delegates, so default to returning nullptr. Note that some
// compilation units will only ever use the object version.
static MOZ_MAYBE_UNUSED JSObject* GetDelegateInternal(gc::Cell* key) {
return nullptr;
}
inline /* static */ JSObject* WeakMapBase::getDelegate(LazyScript* script) {
return nullptr;
static JSObject* GetDelegateInternal(JSObject* key) {
JSObject* delegate = UncheckedUnwrapWithoutExpose(key);
return (key == delegate) ? nullptr : delegate;
}
// Use a helper function to do overload resolution to handle cases like
// Heap<ObjectSubclass*>: find everything that is convertible to JSObject* (and
// avoid calling barriers).
template <typename T>
static inline JSObject* GetDelegate(const T& key) {
return GetDelegateInternal(ExtractUnbarriered(key));
}
template <>
inline JSObject* GetDelegate(gc::Cell* const&) = delete;
} /* namespace detail */
} /* namespace gc */
template <class K, class V>
WeakMap<K, V>::WeakMap(JSContext* cx, JSObject* memOf)
: Base(cx->zone()), WeakMapBase(memOf, cx->zone()) {
@ -56,37 +96,64 @@ WeakMap<K, V>::WeakMap(JSContext* cx, JSObject* memOf)
zone()->gcWeakMapList().insertFront(this);
if (zone()->wasGCStarted()) {
marked = true;
markColor = gc::MarkColor::Black;
mapColor = CellColor::Black;
}
}
// 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.
// is the key in the weakmap. In the absence of delegates, these will be the
// same, but when a delegate is marked then origKey will be its wrapper.
// `markedCell` is only used for an assertion.
template <class K, class V>
void WeakMap<K, V>::markEntry(GCMarker* marker, gc::Cell* markedCell,
gc::Cell* origKey) {
MOZ_ASSERT(marked);
void WeakMap<K, V>::markKey(GCMarker* marker, gc::Cell* markedCell,
gc::Cell* origKey) {
MOZ_ASSERT(mapColor);
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)) {
TraceEdge(marker, &p->value(), "WeakMap ephemeron value");
TraceEdge(marker, &key, "proxy-preserved WeakMap ephemeron key");
MOZ_ASSERT(key == p->key()); // No moving
gc::Cell* oldKey = gc::ToMarkable(p->key());
MOZ_ASSERT((markedCell == oldKey) ||
(markedCell == gc::detail::GetDelegate(p->key())));
markEntry(marker, p->mutableKey(), p->value());
MOZ_ASSERT(oldKey == gc::ToMarkable(p->key()), "no moving GC");
}
// If the entry is live, ensure its key and value are marked. Also make sure
// the key is at least as marked as the delegate, so it cannot get discarded
// and then recreated by rewrapping the delegate.
template <class K, class V>
bool WeakMap<K, V>::markEntry(GCMarker* marker, K& key, V& value) {
bool marked = false;
JSRuntime* rt = zone()->runtimeFromAnyThread();
CellColor keyColor = gc::detail::GetEffectiveColor(rt, key);
JSObject* delegate = gc::detail::GetDelegate(key);
if (delegate) {
CellColor delegateColor = gc::detail::GetEffectiveColor(rt, delegate);
if (keyColor < delegateColor) {
gc::AutoSetMarkColor autoColor(*marker, delegateColor);
TraceEdge(marker, &key, "proxy-preserved WeakMap entry key");
MOZ_ASSERT(key->color() >= delegateColor);
marked = true;
keyColor = delegateColor;
}
}
key.unsafeSet(nullptr); // Prevent destructor from running barriers.
if (keyColor) {
gc::Cell* cellValue = gc::ToMarkable(&value);
if (cellValue) {
gc::AutoSetMarkColor autoColor(*marker, std::min(mapColor, keyColor));
CellColor valueColor = gc::detail::GetEffectiveColor(rt, cellValue);
if (valueColor < marker->markColor()) {
TraceEdge(marker, &value, "WeakMap entry value");
MOZ_ASSERT(cellValue->color() >= std::min(mapColor, keyColor));
marked = true;
}
}
}
return marked;
}
template <class K, class V>
@ -99,17 +166,13 @@ void WeakMap<K, V>::trace(JSTracer* trc) {
MOZ_ASSERT(trc->weakMapAction() == ExpandWeakMaps);
auto marker = GCMarker::fromTracer(trc);
// Don't change the map color from black to gray. This can happen when a
// barrier pushes the map object onto the black mark stack when it's already
// present on the gray mark stack, which is marked later.
if (marked && markColor == gc::MarkColor::Black &&
marker->markColor() == gc::MarkColor::Gray) {
return;
// Don't downgrade the map color from black to gray. This can happen when a
// barrier pushes the map object onto the black mark stack when it's
// already present on the gray mark stack, which is marked later.
if (mapColor < marker->markColor()) {
mapColor = marker->markColor();
mozilla::Unused << markEntries(marker);
}
marked = true;
markColor = marker->markColor();
(void)markEntries(marker);
return;
}
@ -154,38 +217,32 @@ template <class K, class V>
template <class K, class V>
bool WeakMap<K, V>::markEntries(GCMarker* marker) {
MOZ_ASSERT(marked);
if (marker->markColor() == gc::MarkColor::Black &&
markColor == gc::MarkColor::Gray) {
return false;
}
MOZ_ASSERT(mapColor);
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 = marker->isMarked(&e.front().mutableKey());
if (!keyIsMarked && keyNeedsMark(marker, e.front().key())) {
TraceEdge(marker, &e.front().mutableKey(),
"proxy-preserved WeakMap entry key");
keyIsMarked = true;
if (markEntry(marker, e.front().mutableKey(), e.front().value())) {
markedAny = true;
}
if (keyIsMarked) {
if (!marker->isMarked(&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.
gc::Cell* weakKey = extractUnbarriered(e.front().key());
JSRuntime* rt = zone()->runtimeFromAnyThread();
CellColor keyColor =
gc::detail::GetEffectiveColor(rt, e.front().key().get());
// Changes in the map's mark color will be handled in this code, but
// changes in the key's mark color are handled through the weak keys table.
// So we only need to populate the table if the key is less marked than the
// map, to catch later updates in the key's mark color.
if (keyColor < mapColor) {
MOZ_ASSERT(marker->weakMapAction() == ExpandWeakMaps);
// The final color of the key is not yet known. Record this weakmap and
// the lookup key in the list of weak keys. If the key has a delegate,
// then the lookup key is the delegate (because marking the key will end
// up marking the delegate and thereby mark the entry.)
gc::Cell* weakKey = gc::detail::ExtractUnbarriered(e.front().key());
gc::WeakMarkable markable(this, weakKey);
addWeakEntry(marker, weakKey, markable);
if (JSObject* delegate = getDelegate(e.front().key())) {
if (JSObject* delegate = gc::detail::GetDelegate(e.front().key())) {
addWeakEntry(marker, delegate, markable);
}
}
@ -194,28 +251,6 @@ bool WeakMap<K, V>::markEntries(GCMarker* marker) {
return markedAny;
}
template <class K, class V>
inline bool WeakMap<K, V>::keyNeedsMark(GCMarker* marker, 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 && marker->isMarkedUnbarriered(&delegate);
}
template <class K, class V>
inline bool WeakMap<K, V>::keyNeedsMark(GCMarker* marker,
JSScript* script) const {
return false;
}
template <class K, class V>
inline bool WeakMap<K, V>::keyNeedsMark(GCMarker* marker,
LazyScript* script) const {
return false;
}
template <class K, class V>
void WeakMap<K, V>::sweep() {
/* Remove all entries whose keys remain unmarked. */
@ -251,7 +286,7 @@ void WeakMap<K, V>::assertEntriesNotAboutToBeFinalized() {
for (Range r = Base::all(); !r.empty(); r.popFront()) {
K k(r.front().key());
MOZ_ASSERT(!gc::IsAboutToBeFinalized(&k));
JSObject* delegate = getDelegate(k);
JSObject* delegate = gc::detail::GetDelegate(k);
if (delegate) {
MOZ_ASSERT(!gc::IsAboutToBeFinalizedUnbarriered(&delegate),
"weakmap marking depends on a key tracing its delegate");

29
js/src/gc/WeakMap.cpp
Просмотреть файл

@ -23,7 +23,7 @@ using namespace js;
using namespace js::gc;
WeakMapBase::WeakMapBase(JSObject* memOf, Zone* zone)
: memberOf(memOf), zone_(zone), marked(false), markColor(MarkColor::Black) {
: memberOf(memOf), zone_(zone), mapColor(CellColor::White) {
MOZ_ASSERT_IF(memberOf, memberOf->compartment()->zone() == zone);
}
@ -33,7 +33,7 @@ WeakMapBase::~WeakMapBase() {
void WeakMapBase::unmarkZone(JS::Zone* zone) {
for (WeakMapBase* m : zone->gcWeakMapList()) {
m->marked = false;
m->mapColor = CellColor::White;
}
}
@ -52,7 +52,7 @@ bool WeakMapBase::checkMarkingForZone(JS::Zone* zone) {
bool ok = true;
for (WeakMapBase* m : zone->gcWeakMapList()) {
if (m->marked && !m->checkMarking()) {
if (m->mapColor && !m->checkMarking()) {
ok = false;
}
}
@ -64,7 +64,7 @@ bool WeakMapBase::checkMarkingForZone(JS::Zone* zone) {
bool WeakMapBase::markZoneIteratively(JS::Zone* zone, GCMarker* marker) {
bool markedAny = false;
for (WeakMapBase* m : zone->gcWeakMapList()) {
if (m->marked && m->markEntries(marker)) {
if (m->mapColor && m->markEntries(marker)) {
markedAny = true;
}
}
@ -83,7 +83,7 @@ bool WeakMapBase::findSweepGroupEdgesForZone(JS::Zone* zone) {
void WeakMapBase::sweepZone(JS::Zone* zone) {
for (WeakMapBase* m = zone->gcWeakMapList().getFirst(); m;) {
WeakMapBase* next = m->getNext();
if (m->marked) {
if (m->mapColor) {
m->sweep();
} else {
m->clearAndCompact();
@ -94,7 +94,7 @@ void WeakMapBase::sweepZone(JS::Zone* zone) {
#ifdef DEBUG
for (WeakMapBase* m : zone->gcWeakMapList()) {
MOZ_ASSERT(m->isInList() && m->marked);
MOZ_ASSERT(m->isInList() && m->mapColor);
}
#endif
}
@ -111,21 +111,22 @@ void WeakMapBase::traceAllMappings(WeakMapTracer* tracer) {
}
bool WeakMapBase::saveZoneMarkedWeakMaps(JS::Zone* zone,
WeakMapSet& markedWeakMaps) {
WeakMapColors& markedWeakMaps) {
for (WeakMapBase* m : zone->gcWeakMapList()) {
if (m->marked && !markedWeakMaps.put(m)) {
if (m->mapColor && !markedWeakMaps.put(m, m->mapColor)) {
return false;
}
}
return true;
}
void WeakMapBase::restoreMarkedWeakMaps(WeakMapSet& markedWeakMaps) {
for (WeakMapSet::Range r = markedWeakMaps.all(); !r.empty(); r.popFront()) {
WeakMapBase* map = r.front();
void WeakMapBase::restoreMarkedWeakMaps(WeakMapColors& markedWeakMaps) {
for (WeakMapColors::Range r = markedWeakMaps.all(); !r.empty();
r.popFront()) {
WeakMapBase* map = r.front().key();
MOZ_ASSERT(map->zone()->isGCMarking());
MOZ_ASSERT(!map->marked);
map->marked = true;
MOZ_ASSERT(map->mapColor == CellColor::White);
map->mapColor = r.front().value();
}
}
@ -146,7 +147,7 @@ bool ObjectValueWeakMap::findSweepGroupEdges() {
if (key->asTenured().isMarkedBlack()) {
continue;
}
JSObject* delegate = getDelegate(key);
JSObject* delegate = gc::detail::GetDelegate(key);
if (!delegate) {
continue;
}

44
js/src/gc/WeakMap.h
Просмотреть файл

@ -47,8 +47,8 @@ bool CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key, Cell* value);
// the implicit edges stored in the map) and of removing (sweeping) table
// entries when collection is complete.
typedef HashSet<WeakMapBase*, DefaultHasher<WeakMapBase*>, SystemAllocPolicy>
WeakMapSet;
using WeakMapColors = HashMap<WeakMapBase*, js::gc::CellColor,
DefaultHasher<WeakMapBase*>, SystemAllocPolicy>;
// Common base class for all WeakMap specializations, used for calling
// subclasses' GC-related methods.
@ -56,6 +56,8 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
friend class js::GCMarker;
public:
using CellColor = js::gc::CellColor;
WeakMapBase(JSObject* memOf, JS::Zone* zone);
virtual ~WeakMapBase();
@ -88,19 +90,15 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
// Save information about which weak maps are marked for a zone.
static bool saveZoneMarkedWeakMaps(JS::Zone* zone,
WeakMapSet& markedWeakMaps);
WeakMapColors& markedWeakMaps);
// Restore information about which weak maps are marked for many zones.
static void restoreMarkedWeakMaps(WeakMapSet& markedWeakMaps);
static void restoreMarkedWeakMaps(WeakMapColors& markedWeakMaps);
#if defined(JS_GC_ZEAL) || defined(DEBUG)
static bool checkMarkingForZone(JS::Zone* zone);
#endif
static JSObject* getDelegate(JSObject* key);
static JSObject* getDelegate(JSScript* script);
static JSObject* getDelegate(LazyScript* script);
protected:
// Instance member functions called by the above. Instantiations of WeakMap
// override these with definitions appropriate for their Key and Value types.
@ -112,8 +110,7 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
// 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,
gc::Cell* l) = 0;
virtual void markKey(GCMarker* marker, gc::Cell* markedCell, gc::Cell* l) = 0;
virtual bool markEntries(GCMarker* marker) = 0;
@ -132,10 +129,21 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
// Whether this object has been marked during garbage collection and which
// color it was marked.
bool marked;
gc::MarkColor markColor;
gc::CellColor mapColor;
};
namespace detail {
template <typename T>
struct RemoveBarrier {};
template <typename T>
struct RemoveBarrier<js::HeapPtr<T>> {
using Type = T;
};
} // namespace detail
template <class Key, class Value>
class WeakMap
: private HashMap<Key, Value, MovableCellHasher<Key>, ZoneAllocPolicy>,
@ -161,6 +169,8 @@ class WeakMap
// Resolve ambiguity with LinkedListElement<>::remove.
using Base::remove;
using UnbarrieredKey = typename detail::RemoveBarrier<Key>::Type;
explicit WeakMap(JSContext* cx, JSObject* memOf = nullptr);
// Add a read barrier to prevent an incorrectly gray value from escaping the
@ -211,8 +221,10 @@ class WeakMap
}
#endif
void markEntry(GCMarker* marker, gc::Cell* markedCell,
gc::Cell* origKey) override;
void markKey(GCMarker* marker, gc::Cell* markedCell,
gc::Cell* origKey) override;
bool markEntry(GCMarker* marker, Key& key, Value& value);
void trace(JSTracer* trc) override;
@ -242,10 +254,6 @@ class WeakMap
JS::ExposeObjectToActiveJS(obj);
}
bool keyNeedsMark(GCMarker* marker, JSObject* key) const;
bool keyNeedsMark(GCMarker* marker, JSScript* script) const;
bool keyNeedsMark(GCMarker* marker, LazyScript* script) const;
bool findSweepGroupEdges() override {
// This is overridden by ObjectValueWeakMap and DebuggerWeakMap.
return true;

2
js/src/gc/Zone.cpp
Просмотреть файл

@ -228,7 +228,7 @@ void Zone::sweepWeakKeysAfterMinorGC() {
// If the key has a delegate, then it will map to a WeakKeyEntryVector
// containing the key that needs to be updated.
JSObject* delegate = WeakMapBase::getDelegate(key->as<JSObject>());
JSObject* delegate = gc::detail::GetDelegate(key->as<JSObject>());
if (!delegate) {
continue;
}

81
js/src/jit-test/tests/gc/weak-marking-03.js
Просмотреть файл

@ -412,6 +412,7 @@ function grayMapKey() {
"key is now marked black");
finishgc(); // Finish the GC
assertEq(getMarks().join("/"), "gray/black/gray",
"at end: black/gray => gray");
@ -483,3 +484,83 @@ function grayKeyMap() {
if (this.enqueueMark)
runtest(grayKeyMap);
// Cause a key to be marked black *during gray marking*, by first marking a
// delegate black, then marking the map and key gray. When the key is scanned,
// it should be seen to be a CCW of a black delegate and so should itself be
// marked black.
//
// Note that this is currently buggy -- the key will be marked because the
// delegate is marked, but the color won't be taken into account. So the key
// will be marked gray (or rather, it will see that it's already gray.) The bad
// behavior this would cause is if:
//
// 1. You wrap an object in a CCW and use it as a weakmap key to some
// information.
// 2. You keep a strong reference to the object (in its compartment).
// 3. The only references to the CCW are gray, and are in fact part of a cycle.
// 4. The CC runs and discards the CCW.
// 5. You look up the object in the weakmap again. This creates a new wrapper
// to use as a key. It is not in the weakmap, so the information you stored
// before is not found. (It may have even been collected, if you had no
// other references to it.)
//
function blackDuringGray() {
const g = newGlobal({newCompartment: true});
const vals = {};
vals.map = new WeakMap();
vals.key = g.eval("Object.create(null)");
vals.val = Object.create(null);
vals.map.set(vals.key, vals.val);
g.delegate = vals.key;
addMarkObservers([vals.map, vals.key]);
g.addMarkObservers([vals.key]);
addMarkObservers([vals.val]);
// Mark observers: map, key, delegate, value
gc();
g.enqueueMark(vals.key); // Mark the delegate black
enqueueMark("yield"); // checkpoint 1
// Mark the map gray. This will scan through all entries, find our key, and
// mark it black because its delegate is black.
enqueueMark("set-color-gray");
enqueueMark(vals.map); // Mark the map gray
vals.map = null;
vals.val = null;
vals.key = null;
g.delegate = null;
const showmarks = () => {
print("[map,key,delegate,value] marked " + JSON.stringify(getMarks()));
};
print("Starting incremental GC");
startgc(100000);
// Checkpoint 1, after marking delegate black
showmarks();
var marks = getMarks();
assertEq(marks[0], "unmarked", "map is not marked yet");
assertEq(marks[1], "unmarked", "key is not marked yet");
assertEq(marks[2], "black", "delegate is black");
assertEq(marks[3], "unmarked", "values is not marked yet");
finishgc();
showmarks();
marks = getMarks();
assertEq(marks[0], "gray", "map is gray");
assertEq(marks[1], "black", "key inherits black even though in gray marking");
assertEq(marks[2], "black", "delegate is still black");
assertEq(marks[3], "gray", "gray map + black key => gray value");
clearMarkQueue();
clearMarkObservers();
grayRoot().length = 0;
g.eval("grayRoot().length = 0");
}
if (this.enqueueMark)
runtest(blackDuringGray);

39
js/src/jsapi-tests/testGCGrayMarking.cpp
Просмотреть файл

@ -5,6 +5,8 @@
* 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 <algorithm>
#include "gc/Heap.h"
#include "gc/Verifier.h"
#include "gc/WeakMap.h"
@ -155,7 +157,7 @@ bool TestMarking() {
return true;
}
static const CellColor DontMark = CellColor::White;
static constexpr CellColor DontMark = CellColor::White;
enum MarkKeyOrDelegate : bool { MarkKey = true, MarkDelegate = false };
@ -163,8 +165,7 @@ bool TestJSWeakMaps() {
for (auto keyOrDelegateColor : MarkedCellColors) {
for (auto mapColor : MarkedCellColors) {
for (auto markKeyOrDelegate : {MarkKey, MarkDelegate}) {
CellColor expected =
ExpectedWeakMapValueColor(keyOrDelegateColor, mapColor);
CellColor expected = std::min(keyOrDelegateColor, mapColor);
CHECK(TestJSWeakMap(markKeyOrDelegate, keyOrDelegateColor, mapColor,
expected));
#ifdef JS_GC_ZEAL
@ -186,10 +187,10 @@ bool TestInternalWeakMaps() {
continue;
}
CellColor keyOrDelegateColor =
ExpectedKeyAndDelegateColor(keyMarkColor, delegateMarkColor);
CellColor expected =
ExpectedWeakMapValueColor(keyOrDelegateColor, CellColor::Black);
// The map is black. The delegate marks its key via wrapper preservation.
// The key maps its delegate and the value. Thus, all three end up the
// maximum of the key and delegate colors.
CellColor expected = std::max(keyMarkColor, delegateMarkColor);
CHECK(TestInternalWeakMap(keyMarkColor, delegateMarkColor, expected));
#ifdef JS_GC_ZEAL
@ -240,9 +241,9 @@ bool TestJSWeakMap(MarkKeyOrDelegate markKey, CellColor weakMapMarkColor,
ClearGrayRoots();
CHECK(GetCellColor(weakMap) == weakMapMarkColor);
CHECK(GetCellColor(keyOrDelegate) == keyOrDelegateMarkColor);
CHECK(GetCellColor(value) == expectedValueColor);
CHECK(weakMap->color() == weakMapMarkColor);
CHECK(keyOrDelegate->color() == keyOrDelegateMarkColor);
CHECK(value->color() == expectedValueColor);
}
return true;
@ -296,9 +297,9 @@ bool TestJSWeakMapWithGrayUnmarking(MarkKeyOrDelegate markKey,
ClearGrayRoots();
CHECK(GetCellColor(weakMap) == weakMapMarkColor);
CHECK(GetCellColor(keyOrDelegate) == keyOrDelegateMarkColor);
CHECK(GetCellColor(value) == expectedValueColor);
CHECK(weakMap->color() == weakMapMarkColor);
CHECK(keyOrDelegate->color() == keyOrDelegateMarkColor);
CHECK(value->color() == expectedValueColor);
}
JS_UnsetGCZeal(cx, uint8_t(ZealMode::YieldWhileGrayMarking));
@ -369,9 +370,9 @@ bool TestInternalWeakMap(CellColor keyMarkColor, CellColor delegateMarkColor,
ClearGrayRoots();
CHECK(GetCellColor(key) == expectedColor);
CHECK(GetCellColor(delegate) == expectedColor);
CHECK(GetCellColor(value) == expectedColor);
CHECK(key->color() == expectedColor);
CHECK(delegate->color() == expectedColor);
CHECK(value->color() == expectedColor);
}
return true;
@ -421,9 +422,9 @@ bool TestInternalWeakMapWithGrayUnmarking(CellColor keyMarkColor,
ClearGrayRoots();
CHECK(GetCellColor(key) == expectedColor);
CHECK(GetCellColor(delegate) == expectedColor);
CHECK(GetCellColor(value) == expectedColor);
CHECK(key->color() == expectedColor);
CHECK(delegate->color() == expectedColor);
CHECK(value->color() == expectedColor);
}
JS_UnsetGCZeal(cx, uint8_t(ZealMode::YieldWhileGrayMarking));