From a615c25b00cffd816bf920888c02fde43942e4d1 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Mon, 18 Sep 2023 12:10:33 +0000 Subject: [PATCH] Bug 1853305 - Part 2: Use AutoSelectGCHeap in JSON parsing r=sfink This replaces the current logic with use of the new class. I had to change some of the JSONParser construction because I got warnings about RAII classes being used as temporaries. This should construct the parser directly in the Rooted now rather than copying it. Differential Revision: https://phabricator.services.mozilla.com/D188334 --- js/src/builtin/Eval.cpp | 3 +-- js/src/builtin/JSON.cpp | 5 ++--- js/src/vm/JSONParser.cpp | 47 ++++++++++++++--------------------------------- js/src/vm/JSONParser.h | 11 ++--------- 4 files changed, 19 insertions(+), 47 deletions(-) diff --git a/js/src/builtin/Eval.cpp b/js/src/builtin/Eval.cpp index bbfd949b0945..0cff2486ad4f 100644 --- a/js/src/builtin/Eval.cpp +++ b/js/src/builtin/Eval.cpp @@ -181,8 +181,7 @@ static EvalJSONResult ParseEvalStringAsJSON( chars.begin().get() + 1U, len - 2); Rooted> parser( - cx, JSONParser(cx, jsonChars, - JSONParser::ParseType::AttemptForEval)); + cx, cx, jsonChars, JSONParser::ParseType::AttemptForEval); if (!parser.parse(rval)) { return EvalJSONResult::Failure; } diff --git a/js/src/builtin/JSON.cpp b/js/src/builtin/JSON.cpp index cae9ffce3593..c65ed732cd6b 100644 --- a/js/src/builtin/JSON.cpp +++ b/js/src/builtin/JSON.cpp @@ -1834,9 +1834,8 @@ static bool Revive(JSContext* cx, HandleValue reviver, MutableHandleValue vp) { template bool ParseJSON(JSContext* cx, const mozilla::Range chars, MutableHandleValue vp) { - Rooted> parser( - cx, - JSONParser(cx, chars, JSONParser::ParseType::JSONParse)); + Rooted> parser(cx, cx, chars, + JSONParser::ParseType::JSONParse); return parser.parse(vp); } diff --git a/js/src/vm/JSONParser.cpp b/js/src/vm/JSONParser.cpp index ae3dc70251ea..8bad8268a8e0 100644 --- a/js/src/vm/JSONParser.cpp +++ b/js/src/vm/JSONParser.cpp @@ -585,24 +585,30 @@ void JSONTokenizer::getTextPosition( *line = row; } +// JSONFullParseHandlerAnyChar uses an AutoSelectGCHeap to 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. + JSONFullParseHandlerAnyChar::JSONFullParseHandlerAnyChar(JSContext* cx) - : cx(cx), freeElements(cx), freeProperties(cx) { - JS::AddGCNurseryCollectionCallback(cx, &NurseryCollectionCallback, this); -} + : cx(cx), gcHeap(cx, 1), freeElements(cx), freeProperties(cx) {} JSONFullParseHandlerAnyChar::JSONFullParseHandlerAnyChar( JSONFullParseHandlerAnyChar&& other) noexcept : cx(other.cx), v(other.v), parseType(other.parseType), + gcHeap(cx, 1), freeElements(std::move(other.freeElements)), - freeProperties(std::move(other.freeProperties)) { - JS::AddGCNurseryCollectionCallback(cx, &NurseryCollectionCallback, this); -} + freeProperties(std::move(other.freeProperties)) {} JSONFullParseHandlerAnyChar::~JSONFullParseHandlerAnyChar() { - JS::RemoveGCNurseryCollectionCallback(cx, &NurseryCollectionCallback, this); - for (size_t i = 0; i < freeElements.length(); i++) { js_delete(freeElements[i]); } @@ -612,31 +618,6 @@ 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"); } diff --git a/js/src/vm/JSONParser.h b/js/src/vm/JSONParser.h index f5a4c61bbe64..3acd70a90f6f 100644 --- a/js/src/vm/JSONParser.h +++ b/js/src/vm/JSONParser.h @@ -17,6 +17,7 @@ #include // std::move #include "ds/IdValuePair.h" // IdValuePair +#include "gc/GC.h" // AutoSelectGCHeap #include "js/GCVector.h" // JS::GCVector #include "js/RootingAPI.h" // JS::Handle, JS::MutableHandle, MutableWrappedPtrOperations #include "js/Value.h" // JS::Value, JS::BooleanValue, JS::NullValue @@ -194,11 +195,7 @@ 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; + AutoSelectGCHeap gcHeap; private: // Unused element and property vectors for previous in progress arrays and @@ -271,10 +268,6 @@ 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 -- 2.11.4.GIT