From e0c479c2e8bdc4473d872db99d4c3dcefa81e87d Mon Sep 17 00:00:00 2001 From: Yusuke Suzuki Date: Wed, 29 Nov 2023 19:27:18 -0800 Subject: [PATCH] [JSC] Use synchronous GCActivityCallback GC with RunLoopObserver https://bugs.webkit.org/show_bug.cgi?id=265515 rdar://118930139 Reviewed by Wenson Hsieh. Now we can schedule GC only when we are idle from GCActivityCallback. So we do not need to run async version. This patch changes two things. 1. We set up RunLoopObserver and run GC when RunLoop gets idle state after GCActivityCallback detects GC opportunities. 2. We use sync GC instead of async GC since we now run this only when we are idle. We do not need to run async version. * Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp: (JSC::EdenGCActivityCallback::EdenGCActivityCallback): (JSC::EdenGCActivityCallback::doCollection): * Source/JavaScriptCore/heap/EdenGCActivityCallback.h: (JSC::EdenGCActivityCallback::tryCreate): * Source/JavaScriptCore/heap/FullGCActivityCallback.cpp: (JSC::FullGCActivityCallback::FullGCActivityCallback): (JSC::FullGCActivityCallback::doCollection): * Source/JavaScriptCore/heap/FullGCActivityCallback.h: (JSC::FullGCActivityCallback::tryCreate): * Source/JavaScriptCore/heap/GCActivityCallback.cpp: (JSC::GCActivityCallback::GCActivityCallback): * Source/JavaScriptCore/heap/GCActivityCallback.h: * Source/JavaScriptCore/heap/Synchronousness.h: * Source/WebCore/page/OpportunisticTaskScheduler.cpp: (WebCore::OpportunisticTaskScheduler::FullGCActivityCallback::FullGCActivityCallback): (WebCore::OpportunisticTaskScheduler::FullGCActivityCallback::doCollection): (WebCore::OpportunisticTaskScheduler::EdenGCActivityCallback::EdenGCActivityCallback): (WebCore::OpportunisticTaskScheduler::EdenGCActivityCallback::doCollection): * Source/WebCore/page/OpportunisticTaskScheduler.h: Canonical link: https://commits.webkit.org/271324@main --- .../heap/EdenGCActivityCallback.cpp | 6 ++-- .../heap/EdenGCActivityCallback.h | 6 ++-- .../heap/FullGCActivityCallback.cpp | 6 ++-- .../heap/FullGCActivityCallback.h | 6 ++-- .../heap/GCActivityCallback.cpp | 7 ++-- .../JavaScriptCore/heap/GCActivityCallback.h | 6 ++-- Source/JavaScriptCore/heap/Synchronousness.h | 2 +- .../page/OpportunisticTaskScheduler.cpp | 36 ++++++++++++++++++- .../WebCore/page/OpportunisticTaskScheduler.h | 12 +++---- 9 files changed, 62 insertions(+), 25 deletions(-) diff --git a/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp b/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp index f95f5b05119e..bc7b31fb6dd3 100644 --- a/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp +++ b/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp @@ -30,8 +30,8 @@ namespace JSC { -EdenGCActivityCallback::EdenGCActivityCallback(Heap& heap) - : GCActivityCallback(heap) +EdenGCActivityCallback::EdenGCActivityCallback(Heap& heap, Synchronousness synchronousness) + : GCActivityCallback(heap, synchronousness) { } @@ -40,7 +40,7 @@ EdenGCActivityCallback::~EdenGCActivityCallback() = default; void EdenGCActivityCallback::doCollection(VM& vm) { setDidGCRecently(false); - vm.heap.collectAsync(CollectionScope::Eden); + vm.heap.collect(m_synchronousness, CollectionScope::Eden); } Seconds EdenGCActivityCallback::lastGCLength(Heap& heap) diff --git a/Source/JavaScriptCore/heap/EdenGCActivityCallback.h b/Source/JavaScriptCore/heap/EdenGCActivityCallback.h index ce7ca5a0985a..1f0b9ae24739 100644 --- a/Source/JavaScriptCore/heap/EdenGCActivityCallback.h +++ b/Source/JavaScriptCore/heap/EdenGCActivityCallback.h @@ -31,14 +31,14 @@ namespace JSC { class EdenGCActivityCallback : public GCActivityCallback { public: - static RefPtr tryCreate(Heap& heap) + static RefPtr tryCreate(Heap& heap, Synchronousness synchronousness = Synchronousness::Async) { - return s_shouldCreateGCTimer ? adoptRef(new EdenGCActivityCallback(heap)) : nullptr; + return s_shouldCreateGCTimer ? adoptRef(new EdenGCActivityCallback(heap, synchronousness)) : nullptr; } JS_EXPORT_PRIVATE void doCollection(VM&) override; - JS_EXPORT_PRIVATE EdenGCActivityCallback(Heap&); + JS_EXPORT_PRIVATE EdenGCActivityCallback(Heap&, Synchronousness); JS_EXPORT_PRIVATE ~EdenGCActivityCallback(); private: diff --git a/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp b/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp index 59bf936d1f88..f5545ef32c32 100644 --- a/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp +++ b/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp @@ -31,8 +31,8 @@ namespace JSC { -FullGCActivityCallback::FullGCActivityCallback(Heap& heap) - : GCActivityCallback(heap) +FullGCActivityCallback::FullGCActivityCallback(Heap& heap, Synchronousness synchronousness) + : GCActivityCallback(heap, synchronousness) { } @@ -52,7 +52,7 @@ void FullGCActivityCallback::doCollection(VM& vm) } #endif - heap.collectAsync(CollectionScope::Full); + heap.collect(m_synchronousness, CollectionScope::Full); } Seconds FullGCActivityCallback::lastGCLength(Heap& heap) diff --git a/Source/JavaScriptCore/heap/FullGCActivityCallback.h b/Source/JavaScriptCore/heap/FullGCActivityCallback.h index eb49dad063d4..d15a3236f3f2 100644 --- a/Source/JavaScriptCore/heap/FullGCActivityCallback.h +++ b/Source/JavaScriptCore/heap/FullGCActivityCallback.h @@ -31,14 +31,14 @@ namespace JSC { class FullGCActivityCallback : public GCActivityCallback { public: - static RefPtr tryCreate(Heap& heap) + static RefPtr tryCreate(Heap& heap, Synchronousness synchronousness = Synchronousness::Async) { - return s_shouldCreateGCTimer ? adoptRef(new FullGCActivityCallback(heap)) : nullptr; + return s_shouldCreateGCTimer ? adoptRef(new FullGCActivityCallback(heap, synchronousness)) : nullptr; } JS_EXPORT_PRIVATE void doCollection(VM&) override; - JS_EXPORT_PRIVATE FullGCActivityCallback(Heap&); + JS_EXPORT_PRIVATE FullGCActivityCallback(Heap&, Synchronousness); JS_EXPORT_PRIVATE ~FullGCActivityCallback(); private: diff --git a/Source/JavaScriptCore/heap/GCActivityCallback.cpp b/Source/JavaScriptCore/heap/GCActivityCallback.cpp index b6527959b3b4..39ed766a2596 100644 --- a/Source/JavaScriptCore/heap/GCActivityCallback.cpp +++ b/Source/JavaScriptCore/heap/GCActivityCallback.cpp @@ -38,13 +38,14 @@ bool GCActivityCallback::s_shouldCreateGCTimer = true; const double timerSlop = 2.0; // Fudge factor to avoid performance cost of resetting timer. -GCActivityCallback::GCActivityCallback(Heap& heap) - : GCActivityCallback(heap.vm()) +GCActivityCallback::GCActivityCallback(Heap& heap, Synchronousness synchronousness) + : GCActivityCallback(heap.vm(), synchronousness) { } -GCActivityCallback::GCActivityCallback(VM& vm) +GCActivityCallback::GCActivityCallback(VM& vm, Synchronousness synchronousness) : Base(vm) + , m_synchronousness(synchronousness) { } diff --git a/Source/JavaScriptCore/heap/GCActivityCallback.h b/Source/JavaScriptCore/heap/GCActivityCallback.h index 787004049f76..37b7f287702b 100644 --- a/Source/JavaScriptCore/heap/GCActivityCallback.h +++ b/Source/JavaScriptCore/heap/GCActivityCallback.h @@ -29,6 +29,7 @@ #pragma once #include "JSRunLoopTimer.h" +#include "Synchronousness.h" #include namespace JSC { @@ -40,7 +41,7 @@ class GCActivityCallback : public JSRunLoopTimer { public: using Base = JSRunLoopTimer; - JS_EXPORT_PRIVATE GCActivityCallback(Heap&); + JS_EXPORT_PRIVATE GCActivityCallback(Heap&, Synchronousness); JS_EXPORT_PRIVATE ~GCActivityCallback(); JS_EXPORT_PRIVATE void doWork(VM&) override; @@ -63,8 +64,9 @@ class GCActivityCallback : public JSRunLoopTimer { virtual double deathRate(Heap&) = 0; JS_EXPORT_PRIVATE void scheduleTimer(Seconds); - GCActivityCallback(VM&); + GCActivityCallback(VM&, Synchronousness); + Synchronousness m_synchronousness { Synchronousness::Async }; bool m_enabled { true }; bool m_didGCRecently { false }; Seconds m_delay { s_decade }; diff --git a/Source/JavaScriptCore/heap/Synchronousness.h b/Source/JavaScriptCore/heap/Synchronousness.h index 2c941d91fed2..bbc39ebc5aa4 100644 --- a/Source/JavaScriptCore/heap/Synchronousness.h +++ b/Source/JavaScriptCore/heap/Synchronousness.h @@ -27,7 +27,7 @@ namespace JSC { -enum Synchronousness { +enum Synchronousness : uint8_t { Async, Sync }; diff --git a/Source/WebCore/page/OpportunisticTaskScheduler.cpp b/Source/WebCore/page/OpportunisticTaskScheduler.cpp index 557b083e140b..3c55e4238c61 100644 --- a/Source/WebCore/page/OpportunisticTaskScheduler.cpp +++ b/Source/WebCore/page/OpportunisticTaskScheduler.cpp @@ -173,6 +173,18 @@ static bool isBusyForTimerBasedGC() return opportunisticSweepingAndGarbageCollectionEnabled && isVisibleAndActive && hasPendingTasks; } +OpportunisticTaskScheduler::FullGCActivityCallback::FullGCActivityCallback(JSC::Heap& heap) + : Base(heap, JSC::Synchronousness::Sync) + , m_vm(heap.vm()) + , m_runLoopObserver(makeUnique(RunLoopObserver::WellKnownOrder::PostRenderingUpdate, [this] { + JSC::JSLockHolder locker(m_vm); + m_version = 0; + m_deferCount = 0; + Base::doCollection(m_vm); + }, RunLoopObserver::Type::OneShot)) +{ +} + // We would like to keep FullGCActivityCallback::doCollection and EdenGCActivityCallback::doCollection separate // since we would like to encode more and more different heuristics for them. void OpportunisticTaskScheduler::FullGCActivityCallback::doCollection(JSC::VM& vm) @@ -194,13 +206,30 @@ void OpportunisticTaskScheduler::FullGCActivityCallback::doCollection(JSC::VM& v setTimeUntilFire(delay); return; } + + m_runLoopObserver->invalidate(); + m_runLoopObserver->schedule(); + return; } + JSC::JSLockHolder locker(m_vm); m_version = 0; m_deferCount = 0; Base::doCollection(vm); } +OpportunisticTaskScheduler::EdenGCActivityCallback::EdenGCActivityCallback(JSC::Heap& heap) + : Base(heap, JSC::Synchronousness::Sync) + , m_vm(heap.vm()) + , m_runLoopObserver(makeUnique(RunLoopObserver::WellKnownOrder::PostRenderingUpdate, [this] { + JSC::JSLockHolder locker(m_vm); + m_version = 0; + m_deferCount = 0; + Base::doCollection(m_vm); + }, RunLoopObserver::Type::OneShot)) +{ +} + void OpportunisticTaskScheduler::EdenGCActivityCallback::doCollection(JSC::VM& vm) { constexpr Seconds delay { 10_ms }; @@ -220,11 +249,16 @@ void OpportunisticTaskScheduler::EdenGCActivityCallback::doCollection(JSC::VM& v setTimeUntilFire(delay); return; } + + m_runLoopObserver->invalidate(); + m_runLoopObserver->schedule(); + return; } + JSC::JSLockHolder locker(m_vm); m_version = 0; m_deferCount = 0; - Base::doCollection(vm); + Base::doCollection(m_vm); } } // namespace WebCore diff --git a/Source/WebCore/page/OpportunisticTaskScheduler.h b/Source/WebCore/page/OpportunisticTaskScheduler.h index 62e8ece678b5..e70ca27b74e5 100644 --- a/Source/WebCore/page/OpportunisticTaskScheduler.h +++ b/Source/WebCore/page/OpportunisticTaskScheduler.h @@ -82,10 +82,10 @@ class OpportunisticTaskScheduler final : public RefCounted m_runLoopObserver; JSC::HeapVersion m_version { 0 }; unsigned m_deferCount { 0 }; }; @@ -102,10 +102,10 @@ class OpportunisticTaskScheduler final : public RefCounted m_runLoopObserver; JSC::HeapVersion m_version { 0 }; unsigned m_deferCount { 0 }; };