Skip to content

Commit

Permalink
ARRISEOS-45892 limit conservative scan range (#410)
Browse files Browse the repository at this point in the history
* 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:
WebPlatformForEmbedded#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.
  • Loading branch information
tomasz-karczewski-red authored and varadharajan-v committed Aug 21, 2024
1 parent 849a7cb commit 92174d5
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 1 deletion.
3 changes: 2 additions & 1 deletion Source/JavaScriptCore/heap/MachineStackMarker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <wtf/BitVector.h>
#include <wtf/PageBlock.h>
#include <wtf/StdLibExtras.h>
#include <wtf/ConservativeScanStackGuards.h>

namespace JSC {

Expand All @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions Source/WTF/wtf/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ set(WTF_PUBLIC_HEADERS
ConcurrentPtrHashSet.h
ConcurrentVector.h
Condition.h
ConservativeScanStackGuards.h
CountingLock.h
CrossThreadCopier.h
CrossThreadQueue.h
Expand Down Expand Up @@ -420,6 +421,7 @@ set(WTF_SOURCES
CompilationThread.cpp
ConcurrentBuffer.cpp
ConcurrentPtrHashSet.cpp
ConservativeScanStackGuards.cpp
CountingLock.cpp
CrossThreadCopier.cpp
CrossThreadTaskHandler.cpp
Expand Down
6 changes: 6 additions & 0 deletions Source/WTF/wtf/ConservativeScanStackGuards.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "ConservativeScanStackGuards.h"

namespace WTF {
std::atomic_bool ConservativeScanStackGuards::active {true};
thread_local std::deque<void*> ConservativeScanStackGuards::guard_ptr_stack;;
}
65 changes: 65 additions & 0 deletions Source/WTF/wtf/ConservativeScanStackGuards.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#pragma once

#include "Logging.h"

#include <atomic>
#include <deque>

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<void*> 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();
}
};
}
3 changes: 3 additions & 0 deletions Source/WTF/wtf/glib/RunLoopGLib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include <wtf/MainThread.h>
#include <wtf/glib/RunLoopSourcePriority.h>

#include "wtf/ConservativeScanStackGuards.h"

namespace WTF {

typedef struct {
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions Source/WebKit/WebProcess/WebProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,10 @@ void WebProcess::isJITEnabled(CompletionHandler<void(bool)>&& completionHandler)

void WebProcess::garbageCollectJavaScriptObjects()
{
{
JSLockHolder lock(commonVM());
commonVM().shrinkFootprintWhenIdle();
}
GCController::singleton().garbageCollectNow();
}

Expand Down

0 comments on commit 92174d5

Please sign in to comment.