From 92174d55de4735986844355534bb0579b98c0bb9 Mon Sep 17 00:00:00 2001 From: Tomasz Karczewski Date: Mon, 12 Aug 2024 12:41:14 +0200 Subject: [PATCH] ARRISEOS-45892 limit conservative scan range (#410) * ARRISEOS-45892 limit conservative scan range Turns out that conservative GC root scanning is scanning too deep into the stack & this results in GC being prevented (for more details see the ticket & upstream issue: https://github.com/WebPlatformForEmbedded/WPEWebKit/issues/1363 This workaround introduces 'stack guards' that prevent this in particular cases, that is - if there is gloop main loop routine in the stack (see RunLoopGLib), the GC root search will not go deeper that this frame. This is enough to make GC work when invoked synchronously, via VM::shrinkFootprintWhenIdle; therefore for the workaround to work, wpe plugin needs to call that (another part of the workaround is implemented in WebKitBrowser plugin) * ARRISEOS-45892 limit conservative scan range Small correction: immediate garbageCollectNow is still useful, since under heavy load shrinkFootprintWhenIdle might be executed quite late. --- .../heap/MachineStackMarker.cpp | 3 +- Source/WTF/wtf/CMakeLists.txt | 2 + .../WTF/wtf/ConservativeScanStackGuards.cpp | 6 ++ Source/WTF/wtf/ConservativeScanStackGuards.h | 65 +++++++++++++++++++ Source/WTF/wtf/glib/RunLoopGLib.cpp | 3 + Source/WebKit/WebProcess/WebProcess.cpp | 4 ++ 6 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 Source/WTF/wtf/ConservativeScanStackGuards.cpp create mode 100644 Source/WTF/wtf/ConservativeScanStackGuards.h diff --git a/Source/JavaScriptCore/heap/MachineStackMarker.cpp b/Source/JavaScriptCore/heap/MachineStackMarker.cpp index 99a70fdf3efcd..c914f07c29286 100644 --- a/Source/JavaScriptCore/heap/MachineStackMarker.cpp +++ b/Source/JavaScriptCore/heap/MachineStackMarker.cpp @@ -27,6 +27,7 @@ #include #include #include +#include namespace JSC { @@ -44,7 +45,7 @@ void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoot conservativeRoots.add(registersBegin, registersEnd, jitStubRoutines, codeBlocks); } - conservativeRoots.add(currentThreadState.stackTop, currentThreadState.stackOrigin, jitStubRoutines, codeBlocks); + conservativeRoots.add(currentThreadState.stackTop, WTF::ConservativeScanStackGuards::updatedStackOrigin(currentThreadState.stackTop, currentThreadState.stackOrigin), jitStubRoutines, codeBlocks); } static inline int osRedZoneAdjustment() diff --git a/Source/WTF/wtf/CMakeLists.txt b/Source/WTF/wtf/CMakeLists.txt index 2be4241815acd..a403345c50e19 100644 --- a/Source/WTF/wtf/CMakeLists.txt +++ b/Source/WTF/wtf/CMakeLists.txt @@ -46,6 +46,7 @@ set(WTF_PUBLIC_HEADERS ConcurrentPtrHashSet.h ConcurrentVector.h Condition.h + ConservativeScanStackGuards.h CountingLock.h CrossThreadCopier.h CrossThreadQueue.h @@ -420,6 +421,7 @@ set(WTF_SOURCES CompilationThread.cpp ConcurrentBuffer.cpp ConcurrentPtrHashSet.cpp + ConservativeScanStackGuards.cpp CountingLock.cpp CrossThreadCopier.cpp CrossThreadTaskHandler.cpp diff --git a/Source/WTF/wtf/ConservativeScanStackGuards.cpp b/Source/WTF/wtf/ConservativeScanStackGuards.cpp new file mode 100644 index 0000000000000..7acf5ad146fdd --- /dev/null +++ b/Source/WTF/wtf/ConservativeScanStackGuards.cpp @@ -0,0 +1,6 @@ +#include "ConservativeScanStackGuards.h" + +namespace WTF { + std::atomic_bool ConservativeScanStackGuards::active {true}; + thread_local std::deque ConservativeScanStackGuards::guard_ptr_stack;; +} \ No newline at end of file diff --git a/Source/WTF/wtf/ConservativeScanStackGuards.h b/Source/WTF/wtf/ConservativeScanStackGuards.h new file mode 100644 index 0000000000000..1216474ce0dfb --- /dev/null +++ b/Source/WTF/wtf/ConservativeScanStackGuards.h @@ -0,0 +1,65 @@ +#pragma once + +#include "Logging.h" + +#include +#include + +namespace WTF { + class ConservativeScanStackGuards { + public: + struct ConservativeScanStackGuard { + ConservativeScanStackGuard() { + ConservativeScanStackGuards::setStackGuard(this); + } + ~ConservativeScanStackGuard() { + ConservativeScanStackGuards::resetStackGuard(this); + } + + private: + // Prevent heap allocation + void *operator new (size_t); + void *operator new[] (size_t); + void operator delete (void *); + void operator delete[] (void*); + }; + + friend struct ConservativeScanStackGuard; + + static void* updatedStackOrigin(void *stackTop, void *stackOrigin) { + if (!active.load() || guard_ptr_stack.empty()) { + return stackOrigin; + } + + void *ret = stackOrigin; + + void *guard_ptr = guard_ptr_stack.back(); + + if (guard_ptr > stackTop && guard_ptr < stackOrigin) { + WTFLogAlways("ConservativeScanStackGuards: guard IN RANGE: stackTop: %p guard: %p stackOrigin: %p; correcting stackOrigin\n", stackTop, guard_ptr, stackOrigin); + ret = guard_ptr; + } + return ret; + } + + static void setActive(bool act) { + active.store(act); + } + + private: + + static thread_local std::deque guard_ptr_stack; + static std::atomic_bool active; + + static void setStackGuard(void *ptr) { + guard_ptr_stack.push_back(ptr); + } + + static void resetStackGuard(void *ptr) { + if (ptr != guard_ptr_stack.back()) { + WTFLogAlways("ConservativeScanStackGuards::resetStackGuard expected %p, had: %p", guard_ptr_stack.back(), ptr); + } + guard_ptr_stack.pop_back(); + } + }; +} \ No newline at end of file diff --git a/Source/WTF/wtf/glib/RunLoopGLib.cpp b/Source/WTF/wtf/glib/RunLoopGLib.cpp index 027aa10964f12..dc839b89d6273 100644 --- a/Source/WTF/wtf/glib/RunLoopGLib.cpp +++ b/Source/WTF/wtf/glib/RunLoopGLib.cpp @@ -31,6 +31,8 @@ #include #include +#include "wtf/ConservativeScanStackGuards.h" + namespace WTF { typedef struct { @@ -103,6 +105,7 @@ void RunLoop::run() ASSERT(!runLoop.m_mainLoops.isEmpty()); GMainLoop* innermostLoop = runLoop.m_mainLoops[0].get(); + ConservativeScanStackGuards::ConservativeScanStackGuard guard; if (!g_main_loop_is_running(innermostLoop)) { g_main_context_push_thread_default(mainContext); g_main_loop_run(innermostLoop); diff --git a/Source/WebKit/WebProcess/WebProcess.cpp b/Source/WebKit/WebProcess/WebProcess.cpp index b4ecfda7693bc..9c0a4fd46e1e9 100644 --- a/Source/WebKit/WebProcess/WebProcess.cpp +++ b/Source/WebKit/WebProcess/WebProcess.cpp @@ -1042,6 +1042,10 @@ void WebProcess::isJITEnabled(CompletionHandler&& completionHandler) void WebProcess::garbageCollectJavaScriptObjects() { + { + JSLockHolder lock(commonVM()); + commonVM().shrinkFootprintWhenIdle(); + } GCController::singleton().garbageCollectNow(); }