Schedule garbage collection on worker threads using idle tasks

Source/core/workers/WorkerThread.cpp

Index: Source/core/workers/WorkerThread.cpp
diff --git a/Source/core/workers/WorkerThread.cpp b/Source/core/workers/WorkerThread.cpp
index 5842471ba2fab4c37a5111ef05079c1ab08d6367..1293544be91344bb4fd1c23e47e37f91d117a970 100644
--- a/Source/core/workers/WorkerThread.cpp
+++ b/Source/core/workers/WorkerThread.cpp
@@ -46,6 +46,7 @@
 #include "platform/heap/ThreadState.h"
 #include "platform/weborigin/KURL.h"
 #include "public/platform/Platform.h"
+#include "public/platform/WebScheduler.h"
 #include "public/platform/WebThread.h"
 #include "public/platform/WebWaitableEvent.h"
 #include "wtf/Noncopyable.h"
@@ -55,8 +56,7 @@
 namespace blink {
 
 namespace {
-const int64_t kShortIdleHandlerDelayMs = 1000;
-const int64_t kLongIdleHandlerDelayMs = 10*1000;
+const double kLongIdlePeriodSecs = 1.0;
 
 } // namespace
 
@@ -267,6 +267,7 @@ WorkerThread::WorkerThread(PassRefPtr<WorkerLoaderProxy> workerLoaderProxy, Work
     , m_terminated(false)
     , m_workerLoaderProxy(workerLoaderProxy)
     , m_workerReportingProxy(workerReportingProxy)
+    , m_webScheduler(nullptr)
     , m_startupData(startupData)
     , m_isolate(nullptr)
     , m_shutdownEvent(adoptPtr(Platform::current()->createWaitableEvent()))
@@ -306,6 +307,24 @@ PlatformThreadId WorkerThread::platformThreadId()
     return backingThread().platformThread().threadId();
 }
 
+// TODO(alexclarke): Use base::Bind instead of this class when the repo's merge. Unfortunately we
+// can't use WTF::bind because the type-erasure for member function pointers with parameters is broken.

[kinuko, 2015/05/13 06:10:52]

Could you give a bit more details about what's broken in WTF::bind? (This one
looks fine, but arbitrarily mixing up base::Bind and WTF::bind in the same code
base could be dangerous and I want to discourage doing so until we have some
guideline. Context: go/thread-safe-blink)

[Sami, 2015/05/13 17:54:19]

This part was written by Alex so I needed to reverse engineer it a bit. I think
the problem was that WTF::bind doesn't derive unbound variables automatically
like base::Bind, e.g.:

void func(int x) { ... }

(*WTF::bind(&func))(123)       // doesn't work because bind returns a
Function<void()>, not Function<void(int)>.

(*WTF::bind<int>(&func))(123)  // works.

I've now taken out this wrapper and replaced it with a bind<double>.

[kinuko, 2015/05/14 06:07:05]

Ah interesting. Thanks for the detail!

+class WorkerThreadIdleTask : public WebThread::IdleTask {
+public:
+    explicit WorkerThreadIdleTask(WorkerThread* thread)
+        : m_thread(thread) { }
+
+    ~WorkerThreadIdleTask() override { }
+
+    void run(double deadlineSeconds) override
+    {
+        m_thread->performIdleWork(deadlineSeconds);
+    }
+
+private:
+    RawPtr<WorkerThread> m_thread; // NOT OWNED
+};
+
 void WorkerThread::initialize()
 {
     KURL scriptURL = m_startupData->m_scriptURL;
@@ -314,6 +333,7 @@ void WorkerThread::initialize()
     OwnPtr<Vector<char>> cachedMetaData = m_startupData->m_cachedMetaData.release();
     V8CacheOptions v8CacheOptions = m_startupData->m_v8CacheOptions;
 
+    m_webScheduler = backingThread().platformThread().scheduler();
     {
         MutexLocker lock(m_threadStateMutex);
 
@@ -355,7 +375,7 @@ void WorkerThread::initialize()
 
     postInitialize();
 
-    postDelayedTask(FROM_HERE, createSameThreadTask(&WorkerThread::idleHandler, this), kShortIdleHandlerDelayMs);
+    m_webScheduler->postIdleTaskAfterWakeup(FROM_HERE, new WorkerThreadIdleTask(this));
 }
 
 void WorkerThread::shutdown()
@@ -474,21 +494,27 @@ bool WorkerThread::isCurrentThread()
     return m_started && backingThread().isCurrentThread();
 }
 
-void WorkerThread::idleHandler()
+void WorkerThread::performIdleWork(double deadlineSeconds)
 {
-    ASSERT(m_workerGlobalScope.get());
-    int64_t delay = kLongIdleHandlerDelayMs;
+    double gcDeadlineSeconds = deadlineSeconds;
 
-    // Do a script engine idle notification if the next event is distant enough.
-    const double kMinIdleTimespan = 0.3;
-    const double nextFireTime = PlatformThreadData::current().threadTimers().nextFireTime();
-    if (nextFireTime == 0.0 || nextFireTime > currentTime() + kMinIdleTimespan) {
-        bool hasMoreWork = !isolate()->IdleNotificationDeadline(Platform::current()->monotonicallyIncreasingTime() + 1.0);
-        if (hasMoreWork)
-            delay = kShortIdleHandlerDelayMs;
-    }
+    // The V8 GC does some GC steps (e.g. compaction) only when the idle notification is ~1s.
+    // TODO(rmcilroy): Refactor so extending the deadline like this this isn't needed.
+    if (m_webScheduler->canExceedIdleDeadlineIfRequired())
+        gcDeadlineSeconds = Platform::current()->monotonicallyIncreasingTime() + kLongIdlePeriodSecs;
 
-    postDelayedTask(FROM_HERE, createSameThreadTask(&WorkerThread::idleHandler, this), delay);
+    if (doIdleGc(gcDeadlineSeconds))
+        m_webScheduler->postIdleTaskAfterWakeup(FROM_HERE, new WorkerThreadIdleTask(this));
+    else
+        m_webScheduler->postIdleTask(FROM_HERE, new WorkerThreadIdleTask(this));
+}
+
+bool WorkerThread::doIdleGc(double deadlineSeconds)
+{
+    bool gcFinished = false;
+    if (deadlineSeconds > Platform::current()->monotonicallyIncreasingTime())
+        gcFinished = isolate()->IdleNotificationDeadline(deadlineSeconds);
+    return gcFinished;
 }
 
 void WorkerThread::postTask(const WebTraceLocation& location, PassOwnPtr<ExecutionContextTask> task)