Bug 1568923 - String deduplication fixups from review r=jonco

Differential Revision: https://phabricator.services.mozilla.com/D79583
This commit is contained in:
Steve Fink 2020-06-26 02:17:04 +00:00
parent 6a240f86aa
commit 4d9132ebfd
9 changed files with 125 additions and 137 deletions

View File

@ -609,6 +609,14 @@ class CellHeaderWithLengthAndFlags {
#endif
}
bool operator==(const CellHeaderWithLengthAndFlags& other) const {
#if JS_BITS_PER_WORD == 32
return length_ == other.length_ && flagsField() == other.flagsField();
#else
return header() == other.header();
#endif
}
// Sub classes can store temporary data in the flags word. This is not GC safe
// and users must ensure flags/length are never checked (including by asserts)
// while this data is stored. Use of this method is strongly discouraged!

View File

@ -114,24 +114,13 @@ inline RelocatedCellHeader::RelocatedCellHeader(Cell* location,
header_ = ptr | flags | FORWARD_BIT;
}
inline RelocationOverlay::RelocationOverlay(Cell* dst, uintptr_t flags)
: header_(dst, flags) {}
inline RelocationOverlay::RelocationOverlay(Cell* dst) : header_(dst, 0) {}
/* static */
inline RelocationOverlay* RelocationOverlay::forwardCell(Cell* src, Cell* dst) {
MOZ_ASSERT(!src->isForwarded());
MOZ_ASSERT(!dst->isForwarded());
// Preserve old flags because nursery may check them before checking
// if this is a forwarded Cell.
//
// This is pretty terrible and we should find a better way to implement
// Cell::getTraceKind() that doesn't rely on this behavior.
//
// The copied over flags are only used for nursery Cells, when the Cell is
// tenured, these bits are never read and hence may contain any content.
uintptr_t flags = reinterpret_cast<CellHeader*>(dst)->flags();
return new (src) RelocationOverlay(dst, flags);
return new (src) RelocationOverlay(dst);
}
inline bool IsAboutToBeFinalizedDuringMinorSweep(Cell** cellp) {

View File

@ -3107,6 +3107,69 @@ static inline void TraceWholeCell(TenuringTracer& mover, JSObject* object) {
mover.traceObject(object);
}
// Non-deduplicatable marking is necessary because of the following 2 reasons:
//
// 1. Tenured string chars cannot be updated:
//
// If any of the tenured string's bases were deduplicated during tenuring,
// the tenured string's chars pointer would need to be adjusted. This would
// then require updating any other tenured strings that are dependent on the
// first tenured string, and we have no way to find them without scanning
// the entire tenured heap.
//
// 2. Tenured string cannot store its nursery base or base's chars:
//
// Tenured strings have no place to stash a pointer to their nursery base or
// its chars. You need to be able to traverse any dependent string's chain
// of bases up to a nursery "root base" that points to the malloced chars
// that the dependent strings started out pointing to, so that you can
// calculate the offset of any dependent string and update the ptr+offset if
// the root base gets deduplicated to a different allocation. Tenured
// strings in this base chain will stop you from reaching the nursery
// version of the root base; you can only get to the tenured version, and it
// has no place to store the original chars pointer.
static inline void PreventDeduplicationOfReachableStrings(JSString* str) {
MOZ_ASSERT(str->isTenured());
MOZ_ASSERT(!str->isForwarded());
JSLinearString* baseOrRelocOverlay = str->nurseryBaseOrRelocOverlay();
// Walk along the chain of dependent strings' base string pointers
// to mark them all non-deduplicatable.
while (true) {
// baseOrRelocOverlay can be one of the three cases:
// 1. forwarded nursery string:
// The forwarded string still retains the flag that can tell whether
// this string is a dependent string with a base. Its
// StringRelocationOverlay holds a saved pointer to its base in the
// nursery.
// 2. not yet forwarded nursery string:
// Retrieve the base field directly from the string.
// 3. tenured string:
// The nursery base chain ends here, so stop traversing.
if (baseOrRelocOverlay->isForwarded()) {
JSLinearString* tenuredBase = Forwarded(baseOrRelocOverlay);
if (!tenuredBase->hasBase()) {
break;
}
baseOrRelocOverlay = StringRelocationOverlay::fromCell(baseOrRelocOverlay)
->savedNurseryBaseOrRelocOverlay();
} else {
JSLinearString* base = baseOrRelocOverlay;
if (base->isTenured()) {
break;
}
if (base->isDeduplicatable()) {
base->setNonDeduplicatable();
}
if (!base->hasBase()) {
break;
}
baseOrRelocOverlay = base->nurseryBaseOrRelocOverlay();
}
}
}
static inline void TraceWholeCell(TenuringTracer& mover, JSString* str) {
MOZ_ASSERT_IF(str->storeBuffer(),
str->storeBuffer()->markingNondeduplicatable);
@ -3114,62 +3177,8 @@ static inline void TraceWholeCell(TenuringTracer& mover, JSString* str) {
// Mark all strings reachable from the tenured string `str` as
// non-deduplicatable. These strings are the bases of the tenured dependent
// string.
//
// Non-deduplicatable marking is necessary because of the following 2 reasons:
//
// 1. Tenured string chars cannot be updated:
// If any of the tenured string's bases were deduplicated during tenuring,
// the tenured string's chars pointer would need to be adjusted, as would
// any other dependent tenured string whose base is the tenured string in
// question. We have no way to find all affected dependent strings.
//
// 2. Tenured string cannot store its nursery base:
// Since the string is already tenured, it does not have a string
// relocation overlay in which to store its nursery base. Once the base
// chain enters the tenured heap, there is no way to recover the nursery
// address of a tenured base.
if (str->hasBase()) {
MOZ_ASSERT(str->isTenured());
MOZ_ASSERT(!str->isForwarded());
JSLinearString* baseOrRelocOverlay = str->nurseryBaseOrRelocOverlay();
// Walk along the chain of dependent strings' base string pointers
// to mark them all non-deduplicatable.
while (true) {
// baseOrRelocOverlay can be one of the three cases:
// 1. forwarded nursery string:
// The forwarded string still retains the flag that can tell whether
// this string is a dependent string with a base. Its
// StringRelocationOverlay holds a saved pointer to its base in the
// nursery.
// 2. not yet forwarded nursery string:
// Retrieve the base field directly from the string.
// 3. tenured string:
// The nursery base chain ends here, so stop traversing.
if (baseOrRelocOverlay->isForwarded()) {
JSLinearString* tenuredBase =
Forwarded(reinterpret_cast<JSLinearString*>(baseOrRelocOverlay));
if (!tenuredBase->hasBase()) {
break;
}
baseOrRelocOverlay =
StringRelocationOverlay::fromCell(baseOrRelocOverlay)
->savedNurseryBaseOrRelocOverlay();
} else {
JSLinearString* base = baseOrRelocOverlay;
if (base->isTenured()) {
break;
}
if (base->isDeduplicatable()) {
base->setNonDeduplicatable();
}
if (!base->hasBase()) {
break;
}
baseOrRelocOverlay = base->nurseryBaseOrRelocOverlay();
}
}
PreventDeduplicationOfReachableStrings(str);
}
str->traceChildren(&mover);
@ -3584,18 +3593,17 @@ JSString* js::TenuringTracer::moveToTenured(JSString* src) {
// 1. Its length is smaller than MAX_DEDUPLICATABLE_STRING_LENGTH:
// Hashing a long string can affect performance.
// 2. It is linear:
// Deduplicating every node in would end up doing O(n^2) hashing work.
// Deduplicating every node in it would end up doing O(n^2) hashing work.
// 3. It is deduplicatable:
// This is defined by the NON_DEDUP_BIT in JSString flag.
// The JSString NON_DEDUP_BIT flag is unset.
// 4. It matches an entry in stringDeDupSet.
if (src->length() < MAX_DEDUPLICATABLE_STRING_LENGTH && src->isLinear() &&
src->isDeduplicatable() && nursery().stringDeDupSet.isSome()) {
if (auto p = nursery().stringDeDupSet->lookup(src)) {
// Deduplicate to the looked-up string!
dst = *p;
StringRelocationOverlay::forwardCell(src, dst);
gcprobes::PromoteToTenured(src, dst);
return dst;
}
@ -3609,13 +3617,11 @@ JSString* js::TenuringTracer::moveToTenured(JSString* src) {
}
} else {
dst = allocTenuredString(src, zone, dstKind);
dst->clearNonDeduplicatable();
}
auto overlay = StringRelocationOverlay::forwardCell(src, dst);
// Note that dst is guaranteed to have been created during this
// minor GC, so the cleared bit can only have been set during this
// minor GC as well (as opposed to by eg AutoStableStringChars).
dst->clearNonDeduplicatable();
auto* overlay = StringRelocationOverlay::forwardCell(src, dst);
MOZ_ASSERT(dst->isDeduplicatable());
// The base root might be deduplicated, so the non-inlined chars might no
// longer be valid. Insert the overlay into this list to relocate it later.
insertIntoStringFixupList(overlay);

View File

@ -581,26 +581,25 @@ class Nursery {
}
static MOZ_ALWAYS_INLINE bool match(const Key& key, const Lookup& lookup) {
if (key->length() != lookup->length() || key->zone() != lookup->zone() ||
key->flags() != lookup->flags() ||
if (!key->sameLengthAndFlags(*lookup) ||
key->asTenured().zone() != lookup->zone() ||
key->asTenured().getAllocKind() != lookup->getAllocKind()) {
return false;
}
JS::AutoCheckCannotGC nogc;
if (key->asLinear().hasLatin1Chars() &&
lookup->asLinear().hasLatin1Chars()) {
if (key->asLinear().hasLatin1Chars()) {
MOZ_ASSERT(lookup->asLinear().hasLatin1Chars());
return mozilla::ArrayEqual(key->asLinear().latin1Chars(nogc),
lookup->asLinear().latin1Chars(nogc),
lookup->length());
} else if (key->asLinear().hasTwoByteChars() &&
lookup->asLinear().hasTwoByteChars()) {
} else {
MOZ_ASSERT(key->asLinear().hasTwoByteChars());
MOZ_ASSERT(lookup->asLinear().hasTwoByteChars());
return EqualChars(key->asLinear().twoByteChars(nogc),
lookup->asLinear().twoByteChars(nogc),
lookup->length());
} else {
return false;
}
}
};

View File

@ -46,7 +46,7 @@ class RelocationOverlay : public Cell {
/* A list entry to track all relocated things. */
RelocationOverlay* next_;
RelocationOverlay(Cell* dst, uintptr_t flags);
explicit RelocationOverlay(Cell* dst);
public:
static const RelocationOverlay* fromCell(const Cell* cell) {

View File

@ -1088,17 +1088,13 @@ bool js::gc::CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key,
bool GCRuntime::isPointerWithinTenuredCell(void* ptr, JS::TraceKind traceKind) {
AutoLockGC lock(this);
for (auto chunk = allNonEmptyChunks(lock); !chunk.done(); chunk.next()) {
if (chunk->isNurseryChunk()) {
continue;
}
MOZ_ASSERT(!chunk->isNurseryChunk());
if (ptr >= &chunk->arenas[0] && ptr < &chunk->arenas[ArenasPerChunk]) {
auto* arena = reinterpret_cast<Arena*>(uintptr_t(ptr) & ~ArenaMask);
if (!arena->allocated()) {
return false;
}
MOZ_ASSERT(!chunk->isNurseryChunk());
return MapAllocToTraceKind(arena->getAllocKind()) == traceKind;
}
}

View File

@ -59,6 +59,10 @@ using FinalizationRecordVector = GCVector<HeapPtrObject, 1, ZoneAllocPolicy>;
} // namespace gc
// If two different nursery strings are wrapped into the same zone, and have
// the same contents, then deduplication may make them duplicates.
// `DuplicatesPossible` will allow this and map both wrappers to the same (now
// tenured) source string.
using StringWrapperMap =
NurseryAwareHashMap<JSString*, JSString*, DefaultHasher<JSString*>,
ZoneAllocPolicy, DuplicatesPossible>;

View File

@ -1345,16 +1345,6 @@ bool AutoStableStringChars::init(JSContext* cx, JSString* s) {
MOZ_ASSERT(state_ == Uninitialized);
auto _atexit = mozilla::MakeScopeExit([this] {
// Mark the string non-deduplicatable if init() succeeds (s_ is not
// nullptr). This class can be nested, but only the outermost one for a
// given string would do this.
if (s_ && !s_->isAtom() && s_->isDeduplicatable()) {
s_->setNonDeduplicatable();
preventedDeduplication_ = true;
}
});
// If the chars are inline then we need to copy them since they may be moved
// by a compacting GC.
if (baseIsInline(linearString)) {
@ -1370,6 +1360,13 @@ bool AutoStableStringChars::init(JSContext* cx, JSString* s) {
twoByteChars_ = linearString->rawTwoByteChars();
}
// Mark the string non-deduplicatable. This class can be nested, but only the
// outermost one for a given string would do this.
if (!linearString->isTenured() && linearString->isDeduplicatable()) {
linearString->setNonDeduplicatable();
preventedDeduplication_ = true;
}
s_ = linearString;
return true;
}
@ -1382,16 +1379,6 @@ bool AutoStableStringChars::initTwoByte(JSContext* cx, JSString* s) {
MOZ_ASSERT(state_ == Uninitialized);
auto _atexit = mozilla::MakeScopeExit([this] {
// Mark the string non-deduplicatable if init() succeeds (s_ is not
// nullptr). This class can be nested, but only the outermost one for a
// given string would do this.
if (s_ && !s_->isAtom() && s_->isDeduplicatable()) {
s_->setNonDeduplicatable();
preventedDeduplication_ = true;
}
});
if (linearString->hasLatin1Chars()) {
return copyAndInflateLatin1Chars(cx, linearString);
}
@ -1404,6 +1391,14 @@ bool AutoStableStringChars::initTwoByte(JSContext* cx, JSString* s) {
state_ = TwoByte;
twoByteChars_ = linearString->rawTwoByteChars();
// Mark the string non-deduplicatable. This class can be nested, but only the
// outermost one for a given string would do this.
if (!linearString->isTenured() && linearString->isDeduplicatable()) {
linearString->setNonDeduplicatable();
preventedDeduplication_ = true;
}
s_ = linearString;
return true;
}

View File

@ -321,6 +321,10 @@ class JSString : public js::gc::Cell {
return offsetof(JSString, header_) + Header::offsetOfLength();
}
bool sameLengthAndFlags(const JSString& other) const {
return header_ == other.header_;
}
static void staticAsserts() {
static_assert(JSString::MAX_LENGTH < UINT32_MAX,
"Length must fit in 32 bits");
@ -541,7 +545,7 @@ class JSString : public js::gc::Cell {
// or the return value can be the actual base when it is not forwarded.
inline JSLinearString* nurseryBaseOrRelocOverlay() const;
inline bool canBeRootBase() const;
inline bool canOwnDependentChars() const;
inline void setBase(JSLinearString* newBase);
@ -1787,10 +1791,9 @@ inline JSLinearString* JSString::nurseryBaseOrRelocOverlay() const {
return d.s.u3.base;
}
inline bool JSString::canBeRootBase() const {
// A root base is a base that does not have bases (is not a dependent
// string). It serves as the owner of the dependent string chars. A base must
// be linear and non-inline.
inline bool JSString::canOwnDependentChars() const {
// A string that could own the malloced chars used by another (dependent)
// string. It will not have a base and must be linear and non-inline.
return isLinear() && !isInline() && !hasBase();
}
@ -1964,8 +1967,9 @@ class StringRelocationOverlay : public RelocationOverlay {
};
public:
inline StringRelocationOverlay(Cell* dst, uintptr_t flags)
: RelocationOverlay(dst, flags) {}
explicit StringRelocationOverlay(Cell* dst) : RelocationOverlay(dst) {
static_assert(sizeof(JSString) >= sizeof(StringRelocationOverlay));
}
static const StringRelocationOverlay* fromCell(const Cell* cell) {
return static_cast<const StringRelocationOverlay*>(cell);
@ -2007,20 +2011,7 @@ class StringRelocationOverlay : public RelocationOverlay {
MOZ_ASSERT(!dst->isForwarded());
JS::AutoCheckCannotGC nogc;
// Preserve old flags because the nursery may check them before checking if
// this is a forwarded Cell.
//
// This is pretty terrible and we should find a better way to implement
// Cell::getTraceKind() that doesn't rely on this behavior.
//
// The copied over flags are only used for nursery Cells. For tenured
// cells, these bits are never read and hence may contain any content.
StringRelocationOverlay* overlay = nullptr;
auto set_overlay = [&] {
uintptr_t flags = reinterpret_cast<CellHeader*>(dst)->flags();
overlay = new (src) StringRelocationOverlay(dst, flags);
};
StringRelocationOverlay* overlay;
// Initialize the overlay, and remember the nursery base string if there is
// one, or nursery non-inlined chars if it can be the root base of other
@ -2033,20 +2024,20 @@ class StringRelocationOverlay : public RelocationOverlay {
// remembered in overlays.
if (src->hasBase()) {
auto nurseryBaseOrRelocOverlay = src->nurseryBaseOrRelocOverlay();
set_overlay();
overlay = new (src) StringRelocationOverlay(dst);
overlay->nurseryBaseOrRelocOverlay = nurseryBaseOrRelocOverlay;
} else if (src->canBeRootBase()) {
} else if (src->canOwnDependentChars()) {
if (src->hasTwoByteChars()) {
auto nurseryCharsTwoByte = src->asLinear().twoByteChars(nogc);
set_overlay();
overlay = new (src) StringRelocationOverlay(dst);
overlay->nurseryCharsTwoByte = nurseryCharsTwoByte;
} else {
auto nurseryCharsLatin1 = src->asLinear().latin1Chars(nogc);
set_overlay();
overlay = new (src) StringRelocationOverlay(dst);
overlay->nurseryCharsLatin1 = nurseryCharsLatin1;
}
} else {
set_overlay();
overlay = new (src) StringRelocationOverlay(dst);
}
return overlay;