From 5777657ab859934962882a5ad5fa9ddbe829e489 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Thu, 17 Aug 2023 09:51:17 +0000 Subject: [PATCH] Bug 1848090 - When parsing JSON, Stop allocating in the nursery after two nursery collections r=sfink The patch adds a nursery collection callback for the duration of parsing and counts the collections. After the second collection we switch to allocating everything in the tenured heap. This change made parsing the JSON data in the bug significantly faster. Differential Revision: https://phabricator.services.mozilla.com/D186362 --- js/src/util/StringBuffer.cpp | 12 ++++--- js/src/util/StringBuffer.h | 4 +-- js/src/vm/JSONParser.cpp | 76 ++++++++++++++++++++++++++++++++++++++------ js/src/vm/JSONParser.h | 20 +++++++----- js/src/vm/PlainObject.cpp | 20 +++++++----- js/src/vm/PlainObject.h | 9 +++--- 6 files changed, 105 insertions(+), 36 deletions(-) diff --git a/js/src/util/StringBuffer.cpp b/js/src/util/StringBuffer.cpp index d780c1a811c8..03d9a7279cc3 100644 --- a/js/src/util/StringBuffer.cpp +++ b/js/src/util/StringBuffer.cpp @@ -90,7 +90,8 @@ bool StringBuffer::append(const frontend::ParserAtomsTable& parserAtoms, } template -JSLinearString* StringBuffer::finishStringInternal(JSContext* cx) { +JSLinearString* StringBuffer::finishStringInternal(JSContext* cx, + gc::Heap heap) { size_t len = length(); if (JSAtom* staticStr = cx->staticStrings().lookup(begin(), len)) { @@ -109,7 +110,8 @@ JSLinearString* StringBuffer::finishStringInternal(JSContext* cx) { return nullptr; } - JSLinearString* str = NewStringDontDeflate(cx, std::move(buf), len); + JSLinearString* str = + NewStringDontDeflate(cx, std::move(buf), len, heap); if (!str) { return nullptr; } @@ -117,7 +119,7 @@ JSLinearString* StringBuffer::finishStringInternal(JSContext* cx) { return str; } -JSLinearString* JSStringBuilder::finishString() { +JSLinearString* JSStringBuilder::finishString(gc::Heap heap) { MOZ_ASSERT(maybeCx_); size_t len = length(); @@ -134,8 +136,8 @@ JSLinearString* JSStringBuilder::finishString() { static_assert(JSFatInlineString::MAX_LENGTH_LATIN1 < Latin1CharBuffer::InlineLength); - return isLatin1() ? finishStringInternal(maybeCx_) - : finishStringInternal(maybeCx_); + return isLatin1() ? finishStringInternal(maybeCx_, heap) + : finishStringInternal(maybeCx_, heap); } JSAtom* StringBuffer::finishAtom() { diff --git a/js/src/util/StringBuffer.h b/js/src/util/StringBuffer.h index 0e10b60df84c..23c1ff61ae73 100644 --- a/js/src/util/StringBuffer.h +++ b/js/src/util/StringBuffer.h @@ -178,7 +178,7 @@ class StringBuffer { [[nodiscard]] bool inflateChars(); template - JSLinearString* finishStringInternal(JSContext* cx); + JSLinearString* finishStringInternal(JSContext* cx, gc::Heap heap); public: explicit StringBuffer(JSContext* cx, @@ -390,7 +390,7 @@ class JSStringBuilder : public StringBuffer { * * Returns nullptr if string creation failed. */ - JSLinearString* finishString(); + JSLinearString* finishString(gc::Heap heap = gc::Heap::Default); }; inline bool StringBuffer::append(const char16_t* begin, const char16_t* end) { diff --git a/js/src/vm/JSONParser.cpp b/js/src/vm/JSONParser.cpp index 3206ff993b8e..ae3dc70251ea 100644 --- a/js/src/vm/JSONParser.cpp +++ b/js/src/vm/JSONParser.cpp @@ -585,7 +585,24 @@ void JSONTokenizer::getTextPosition( *line = row; } +JSONFullParseHandlerAnyChar::JSONFullParseHandlerAnyChar(JSContext* cx) + : cx(cx), freeElements(cx), freeProperties(cx) { + JS::AddGCNurseryCollectionCallback(cx, &NurseryCollectionCallback, this); +} + +JSONFullParseHandlerAnyChar::JSONFullParseHandlerAnyChar( + JSONFullParseHandlerAnyChar&& other) noexcept + : cx(other.cx), + v(other.v), + parseType(other.parseType), + freeElements(std::move(other.freeElements)), + freeProperties(std::move(other.freeProperties)) { + JS::AddGCNurseryCollectionCallback(cx, &NurseryCollectionCallback, this); +} + JSONFullParseHandlerAnyChar::~JSONFullParseHandlerAnyChar() { + JS::RemoveGCNurseryCollectionCallback(cx, &NurseryCollectionCallback, this); + for (size_t i = 0; i < freeElements.length(); i++) { js_delete(freeElements[i]); } @@ -595,6 +612,31 @@ JSONFullParseHandlerAnyChar::~JSONFullParseHandlerAnyChar() { } } +/* static */ +void JSONFullParseHandlerAnyChar::NurseryCollectionCallback( + JSContext* cx, JS::GCNurseryProgress progress, JS::GCReason reason, + void* data) { + auto* handler = static_cast(data); + + // Switch to allocating in the tenured heap if we trigger more than one + // nursery collection. + // + // JSON parsing allocates from the leaves of the tree upwards (unlike + // structured clone deserialization which works from the root + // downwards). Because of this it doesn't necessarily make sense to stop + // nursery allocation after the first collection as this doesn't doom the + // whole data structure to being tenured. We don't know ahead of time how big + // the resulting data structure will be but after two nursery collections then + // at least half of it will end up tenured. + + if (progress == JS::GCNurseryProgress::GC_NURSERY_COLLECTION_END) { + handler->nurseryCollectionCount++; + if (handler->nurseryCollectionCount == 2) { + handler->gcHeap = gc::Heap::Tenured; + } + } +} + void JSONFullParseHandlerAnyChar::trace(JSTracer* trc) { JS::TraceRoot(trc, &v, "JSONFullParseHandlerAnyChar current value"); } @@ -628,9 +670,13 @@ template template inline bool JSONFullParseHandler::setStringValue(CharPtr start, size_t length) { - JSLinearString* str = (ST == JSONStringType::PropertyName) - ? AtomizeChars(cx, start.get(), length) - : NewStringCopyN(cx, start.get(), length); + JSString* str; + if constexpr (ST == JSONStringType::PropertyName) { + str = AtomizeChars(cx, start.get(), length); + } else { + str = NewStringCopyN(cx, start.get(), length, gcHeap); + } + if (!str) { return false; } @@ -642,9 +688,13 @@ template template inline bool JSONFullParseHandler::setStringValue( StringBuilder& builder) { - JSLinearString* str = (ST == JSONStringType::PropertyName) - ? builder.buffer.finishAtom() - : builder.buffer.finishString(); + JSString* str; + if constexpr (ST == JSONStringType::PropertyName) { + str = builder.buffer.finishAtom(); + } else { + str = builder.buffer.finishString(gcHeap); + } + if (!str) { return false; } @@ -707,8 +757,12 @@ inline bool JSONFullParseHandlerAnyChar::finishObject( PropertyVector* properties) { MOZ_ASSERT(properties == &stack.back().properties()); - JSObject* obj = NewPlainObjectWithMaybeDuplicateKeys(cx, properties->begin(), - properties->length()); + NewObjectKind newKind = GenericObject; + if (gcHeap == gc::Heap::Tenured) { + newKind = TenuredObject; + } + JSObject* obj = NewPlainObjectWithMaybeDuplicateKeys( + cx, properties->begin(), properties->length(), newKind); if (!obj) { return false; } @@ -752,8 +806,12 @@ inline bool JSONFullParseHandlerAnyChar::finishArray( ElementVector* elements) { MOZ_ASSERT(elements == &stack.back().elements()); + NewObjectKind newKind = GenericObject; + if (gcHeap == gc::Heap::Tenured) { + newKind = TenuredObject; + } ArrayObject* obj = - NewDenseCopiedArray(cx, elements->length(), elements->begin()); + NewDenseCopiedArray(cx, elements->length(), elements->begin(), newKind); if (!obj) { return false; } diff --git a/js/src/vm/JSONParser.h b/js/src/vm/JSONParser.h index ea74097ff76d..f5a4c61bbe64 100644 --- a/js/src/vm/JSONParser.h +++ b/js/src/vm/JSONParser.h @@ -194,6 +194,12 @@ class MOZ_STACK_CLASS JSONFullParseHandlerAnyChar { ParseType parseType = ParseType::JSONParse; + // The number of nursery collections that have occurred. + size_t nurseryCollectionCount = 0; + + // The heap to use for allocating GC things. + gc::Heap gcHeap = gc::Heap::Default; + private: // Unused element and property vectors for previous in progress arrays and // objects. These vectors are not freed until the end of the parse to avoid @@ -202,17 +208,11 @@ class MOZ_STACK_CLASS JSONFullParseHandlerAnyChar { Vector freeProperties; public: - explicit JSONFullParseHandlerAnyChar(JSContext* cx) - : cx(cx), freeElements(cx), freeProperties(cx) {} + explicit JSONFullParseHandlerAnyChar(JSContext* cx); ~JSONFullParseHandlerAnyChar(); // Allow move construction for use with Rooted. - JSONFullParseHandlerAnyChar(JSONFullParseHandlerAnyChar&& other) noexcept - : cx(other.cx), - v(other.v), - parseType(other.parseType), - freeElements(std::move(other.freeElements)), - freeProperties(std::move(other.freeProperties)) {} + JSONFullParseHandlerAnyChar(JSONFullParseHandlerAnyChar&& other) noexcept; JSONFullParseHandlerAnyChar(const JSONFullParseHandlerAnyChar& other) = delete; @@ -271,6 +271,10 @@ class MOZ_STACK_CLASS JSONFullParseHandlerAnyChar { inline void freeStackEntry(StackEntry& entry); void trace(JSTracer* trc); + + static void NurseryCollectionCallback(JSContext* cx, + JS::GCNurseryProgress progress, + JS::GCReason reason, void* data); }; template diff --git a/js/src/vm/PlainObject.cpp b/js/src/vm/PlainObject.cpp index 265c204a05ea..8341636796c6 100644 --- a/js/src/vm/PlainObject.cpp +++ b/js/src/vm/PlainObject.cpp @@ -241,14 +241,15 @@ enum class KeysKind { UniqueNames, Unknown }; template static PlainObject* NewPlainObjectWithProperties(JSContext* cx, IdValuePair* properties, - size_t nproperties) { + size_t nproperties, + NewObjectKind newKind) { auto& cache = cx->realm()->newPlainObjectWithPropsCache; // If we recently created an object with these properties, we can use that // Shape directly. if (SharedShape* shape = cache.lookup(properties, nproperties)) { Rooted shapeRoot(cx, shape); - PlainObject* obj = PlainObject::createWithShape(cx, shapeRoot); + PlainObject* obj = PlainObject::createWithShape(cx, shapeRoot, newKind); if (!obj) { return nullptr; } @@ -260,7 +261,8 @@ static PlainObject* NewPlainObjectWithProperties(JSContext* cx, } gc::AllocKind allocKind = gc::GetGCObjectKind(nproperties); - Rooted obj(cx, NewPlainObjectWithAllocKind(cx, allocKind)); + Rooted obj(cx, + NewPlainObjectWithAllocKind(cx, allocKind, newKind)); if (!obj) { return nullptr; } @@ -321,14 +323,16 @@ static PlainObject* NewPlainObjectWithProperties(JSContext* cx, PlainObject* js::NewPlainObjectWithUniqueNames(JSContext* cx, IdValuePair* properties, - size_t nproperties) { - return NewPlainObjectWithProperties(cx, properties, - nproperties); + size_t nproperties, + NewObjectKind newKind) { + return NewPlainObjectWithProperties( + cx, properties, nproperties, newKind); } PlainObject* js::NewPlainObjectWithMaybeDuplicateKeys(JSContext* cx, IdValuePair* properties, - size_t nproperties) { + size_t nproperties, + NewObjectKind newKind) { return NewPlainObjectWithProperties(cx, properties, - nproperties); + nproperties, newKind); } diff --git a/js/src/vm/PlainObject.h b/js/src/vm/PlainObject.h index 86e086e954fe..5dcbe29a46b0 100644 --- a/js/src/vm/PlainObject.h +++ b/js/src/vm/PlainObject.h @@ -97,14 +97,15 @@ extern PlainObject* NewPlainObjectWithProtoAndAllocKind( // Create a plain object with the given properties. The list must not contain // duplicate keys or integer keys. -extern PlainObject* NewPlainObjectWithUniqueNames(JSContext* cx, - IdValuePair* properties, - size_t nproperties); +extern PlainObject* NewPlainObjectWithUniqueNames( + JSContext* cx, IdValuePair* properties, size_t nproperties, + NewObjectKind newKind = GenericObject); // Create a plain object with the given properties. The list may contain integer // keys or duplicate keys. extern PlainObject* NewPlainObjectWithMaybeDuplicateKeys( - JSContext* cx, IdValuePair* properties, size_t nproperties); + JSContext* cx, IdValuePair* properties, size_t nproperties, + NewObjectKind newKind = GenericObject); } // namespace js -- 2.11.4.GIT