From 113dc2399c95926b9b62ff5ed7d0afb0a31fe78c Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Fri, 17 Sep 2021 10:00:11 +0000 Subject: [PATCH] Bug 1536061 - Change the gray root trace hook to allow gray roots to be marked incrementally r=sfink,mccr8 This adds a slice budget parameter and boolean return value to indicate whether tracing has finished. Differential Revision: https://phabricator.services.mozilla.com/D125558 --- js/public/GCAPI.h | 13 +++++++++++++ js/src/gc/GC.cpp | 2 +- js/src/gc/GCRuntime.h | 4 ++-- js/src/gc/RootMarking.cpp | 5 +++-- js/src/jsapi-tests/testGCGrayMarking.cpp | 3 ++- js/src/jsfriendapi.cpp | 3 ++- js/src/jsfriendapi.h | 2 +- js/src/shell/js.cpp | 4 +++- xpcom/base/CycleCollectedJSRuntime.cpp | 6 +++++- xpcom/base/CycleCollectedJSRuntime.h | 7 ++++++- 10 files changed, 38 insertions(+), 11 deletions(-) diff --git a/js/public/GCAPI.h b/js/public/GCAPI.h index 6bbb9981972b..4669d64e417c 100644 --- a/js/public/GCAPI.h +++ b/js/public/GCAPI.h @@ -26,6 +26,7 @@ namespace js { namespace gc { class GCRuntime; } // namespace gc +class JS_PUBLIC_API SliceBudget; namespace gcstats { struct Statistics; } // namespace gcstats @@ -434,6 +435,18 @@ typedef enum JSGCParamKey { */ typedef void (*JSTraceDataOp)(JSTracer* trc, void* data); +/* + * Trace hook used to trace gray roots incrementally. + * + * This should return whether tracing is finished. It will be called repeatedly + * in subsequent GC slices until it returns true. + * + * While tracing this should check the budget and return false if it has been + * exceeded. When passed an unlimited budget it should always return true. + */ +typedef bool (*JSGrayRootsTracer)(JSTracer* trc, js::SliceBudget& budget, + void* data); + typedef enum JSGCStatus { JSGC_BEGIN, JSGC_END } JSGCStatus; typedef void (*JSObjectsTenuredCallback)(JSContext* cx, void* data); diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp index 9d4a962bf656..4114da516ea5 100644 --- a/js/src/gc/GC.cpp +++ b/js/src/gc/GC.cpp @@ -1395,7 +1395,7 @@ void GCRuntime::removeBlackRootsTracer(JSTraceDataOp traceOp, void* data) { } } -void GCRuntime::setGrayRootsTracer(JSTraceDataOp traceOp, void* data) { +void GCRuntime::setGrayRootsTracer(JSGrayRootsTracer traceOp, void* data) { AssertHeapIsIdle(); grayRootTracer.ref() = {traceOp, data}; } diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h index a5771945a491..37b0e512be77 100644 --- a/js/src/gc/GCRuntime.h +++ b/js/src/gc/GCRuntime.h @@ -447,7 +447,7 @@ class GCRuntime { bool initSweepActions(); - void setGrayRootsTracer(JSTraceDataOp traceOp, void* data); + void setGrayRootsTracer(JSGrayRootsTracer traceOp, void* data); [[nodiscard]] bool addBlackRootsTracer(JSTraceDataOp traceOp, void* data); void removeBlackRootsTracer(JSTraceDataOp traceOp, void* data); void clearBlackAndGrayRootTracers(); @@ -1191,7 +1191,7 @@ class GCRuntime { * collector. */ MainThreadData> blackRootTracers; - MainThreadOrGCTaskData> grayRootTracer; + MainThreadOrGCTaskData> grayRootTracer; /* Always preserve JIT code during GCs, for testing. */ MainThreadData alwaysPreserveCode; diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp index 68a93718b722..61e8e7362a6d 100644 --- a/js/src/gc/RootMarking.cpp +++ b/js/src/gc/RootMarking.cpp @@ -414,8 +414,9 @@ void GCRuntime::traceEmbeddingGrayRoots(JSTracer* trc) { JS::AutoSuppressGCAnalysis nogc; const auto& callback = grayRootTracer.ref(); - if (JSTraceDataOp op = callback.op) { - (*op)(trc, callback.data); + if (JSGrayRootsTracer op = callback.op) { + SliceBudget budget = SliceBudget::unlimited(); + MOZ_ALWAYS_TRUE((*op)(trc, budget, callback.data)); } } diff --git a/js/src/jsapi-tests/testGCGrayMarking.cpp b/js/src/jsapi-tests/testGCGrayMarking.cpp index bdab2bed1511..2b9a6f09ba49 100644 --- a/js/src/jsapi-tests/testGCGrayMarking.cpp +++ b/js/src/jsapi-tests/testGCGrayMarking.cpp @@ -661,10 +661,11 @@ void RemoveGrayRootTracer() { JS_SetGrayGCRootsTracer(cx, nullptr, nullptr); } -static void TraceGrayRoots(JSTracer* trc, void* data) { +static bool TraceGrayRoots(JSTracer* trc, SliceBudget& budget, void* data) { auto grayRoots = static_cast(data); TraceEdge(trc, &grayRoots->grayRoot1, "gray root 1"); TraceEdge(trc, &grayRoots->grayRoot2, "gray root 2"); + return true; } JSObject* AllocPlainObject() { diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp index 88776b3ee430..5496b6b253fc 100644 --- a/js/src/jsfriendapi.cpp +++ b/js/src/jsfriendapi.cpp @@ -67,7 +67,8 @@ JS::RootingContext::RootingContext() : realm_(nullptr), zone_(nullptr) { #endif } -JS_PUBLIC_API void JS_SetGrayGCRootsTracer(JSContext* cx, JSTraceDataOp traceOp, +JS_PUBLIC_API void JS_SetGrayGCRootsTracer(JSContext* cx, + JSGrayRootsTracer traceOp, void* data) { cx->runtime()->gc.setGrayRootsTracer(traceOp, data); } diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index a8c1927f06a8..5f59ba5c641e 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -31,7 +31,7 @@ class JSJitInfo; * required. */ extern JS_PUBLIC_API void JS_SetGrayGCRootsTracer(JSContext* cx, - JSTraceDataOp traceOp, + JSGrayRootsTracer traceOp, void* data); extern JS_PUBLIC_API JSObject* JS_FindCompilationScope(JSContext* cx, diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 8ec0c615b8e6..686dde215a6d 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -787,7 +787,7 @@ ShellContext* js::shell::GetShellContext(JSContext* cx) { return sc; } -static void TraceGrayRoots(JSTracer* trc, void* data) { +static bool TraceGrayRoots(JSTracer* trc, SliceBudget& budget, void* data) { JSRuntime* rt = trc->runtime(); for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) { for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) { @@ -798,6 +798,8 @@ static void TraceGrayRoots(JSTracer* trc, void* data) { } } } + + return true; } static mozilla::UniqueFreePtr GetLine(FILE* file, const char* prompt) { diff --git a/xpcom/base/CycleCollectedJSRuntime.cpp b/xpcom/base/CycleCollectedJSRuntime.cpp index 7949ead695a7..62514e9274d5 100644 --- a/xpcom/base/CycleCollectedJSRuntime.cpp +++ b/xpcom/base/CycleCollectedJSRuntime.cpp @@ -984,7 +984,9 @@ void CycleCollectedJSRuntime::TraceBlackJS(JSTracer* aTracer, void* aData) { } /* static */ -void CycleCollectedJSRuntime::TraceGrayJS(JSTracer* aTracer, void* aData) { +bool CycleCollectedJSRuntime::TraceGrayJS(JSTracer* aTracer, + js::SliceBudget& budget, + void* aData) { CycleCollectedJSRuntime* self = static_cast(aData); // Mark these roots as gray so the CC can walk them later. @@ -996,6 +998,8 @@ void CycleCollectedJSRuntime::TraceGrayJS(JSTracer* aTracer, void* aData) { } self->TraceNativeGrayRoots(aTracer, which); + + return true; } /* static */ diff --git a/xpcom/base/CycleCollectedJSRuntime.h b/xpcom/base/CycleCollectedJSRuntime.h index 1ff15ba9219d..8c67add89e77 100644 --- a/xpcom/base/CycleCollectedJSRuntime.h +++ b/xpcom/base/CycleCollectedJSRuntime.h @@ -270,7 +270,12 @@ class CycleCollectedJSRuntime { void TraverseNativeRoots(nsCycleCollectionNoteRootCallback& aCb); static void TraceBlackJS(JSTracer* aTracer, void* aData); - static void TraceGrayJS(JSTracer* aTracer, void* aData); + + // Trace gray JS roots until budget is exceeded and return whether we + // finished. + static bool TraceGrayJS(JSTracer* aTracer, js::SliceBudget& budget, + void* aData); + static void GCCallback(JSContext* aContext, JSGCStatus aStatus, JS::GCReason aReason, void* aData); static void GCSliceCallback(JSContext* aContext, JS::GCProgress aProgress,