Refactor TimeBase to post tasks. Workers to use real 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 657ab336614bda55f2398b61fc25b02510d19dd9..5c6e02d1a6986998a2d7adb1da4c23d1188ef796 100644
--- a/Source/core/workers/WorkerThread.cpp
+++ b/Source/core/workers/WorkerThread.cpp
@@ -38,9 +38,7 @@
 #include "core/workers/WorkerClients.h"
 #include "core/workers/WorkerReportingProxy.h"
 #include "core/workers/WorkerThreadStartupData.h"
-#include "platform/PlatformThreadData.h"
 #include "platform/Task.h"
-#include "platform/ThreadTimers.h"
 #include "platform/heap/SafePoint.h"
 #include "platform/heap/ThreadState.h"
 #include "platform/weborigin/KURL.h"
@@ -54,8 +52,6 @@
 namespace blink {
 
 namespace {
-const int64_t kShortIdleHandlerDelayMs = 1000;
-const int64_t kLongIdleHandlerDelayMs = 10*1000;
 
 class MicrotaskRunner : public WebThread::TaskObserver {
 public:
@@ -131,69 +127,6 @@ private:
     bool m_taskCanceled;
 };
 
-class WorkerSharedTimer : public SharedTimer {
-public:
-    explicit WorkerSharedTimer(WorkerThread* workerThread)
-        : m_workerThread(workerThread)
-        , m_running(false)
-    { }
-
-    typedef void (*SharedTimerFunction)();
-    virtual void setFiredFunction(SharedTimerFunction func)
-    {
-        m_sharedTimerFunction = func;
-    }
-
-    virtual void setFireInterval(double interval)
-    {
-        ASSERT(m_sharedTimerFunction);
-
-        // See BlinkPlatformImpl::setSharedTimerFireInterval for explanation of
-        // why ceil is used in the interval calculation.
-        int64_t delay = static_cast<int64_t>(ceil(interval * 1000));
-
-        if (delay < 0) {
-            delay = 0;
-        }
-
-        m_running = true;
-
-        if (m_lastQueuedTask.get())
-            m_lastQueuedTask->cancelTask();
-
-        // Now queue the task as a cancellable one.
-        OwnPtr<WorkerThreadCancelableTask> task = WorkerThreadCancelableTask::create(bind(&WorkerSharedTimer::OnTimeout, this));
-        m_lastQueuedTask = task->createWeakPtr();
-        m_workerThread->postDelayedTask(FROM_HERE, task.release(), delay);
-    }
-
-    virtual void stop()
-    {
-        m_running = false;
-        m_lastQueuedTask = nullptr;
-    }
-
-private:
-    void OnTimeout()
-    {
-        ASSERT(m_workerThread->workerGlobalScope());
-
-        m_lastQueuedTask = nullptr;
-
-        if (m_sharedTimerFunction && m_running && !m_workerThread->workerGlobalScope()->isClosing())
-            m_sharedTimerFunction();
-    }
-
-    WorkerThread* m_workerThread;
-    SharedTimerFunction m_sharedTimerFunction;
-    bool m_running;
-
-    // The task to run OnTimeout, if any. While OnTimeout resets
-    // m_lastQueuedTask, this must be a weak pointer because the
-    // worker runloop may delete the task as it is shutting down.
-    WeakPtr<WorkerThreadCancelableTask> m_lastQueuedTask;
-};
-
 class WorkerThreadTask : public blink::WebThread::Task {
     WTF_MAKE_NONCOPYABLE(WorkerThreadTask); WTF_MAKE_FAST_ALLOCATED(WorkerThreadTask);
 public:
@@ -263,6 +196,8 @@ WorkerThread::WorkerThread(PassRefPtr<WorkerLoaderProxy> workerLoaderProxy, Work
     , m_isolate(nullptr)
     , m_shutdownEvent(adoptPtr(blink::Platform::current()->createWaitableEvent()))
     , m_terminationEvent(adoptPtr(blink::Platform::current()->createWaitableEvent()))
+    , m_minQuietPeriodMsBeforeDoingIdleGc(300ul)
+    , m_dontStartIdleTaskYet(false)
 {
     MutexLocker lock(threadSetMutex());
     workerThreads().add(this);
@@ -298,6 +233,62 @@ PlatformThreadId WorkerThread::platformThreadId() const
     return m_thread->platformThread().threadId();
 }
 
+// TODO(scheduler-dev): Ideally the WorkerScheduler should determine Quiescence rather than doing it here.

[Sami, 2015/04/09 10:52:29]

nit: I think we should use real usernames for TODOs since I don't see anyone
else using aliases like this.

[alex clarke (OOO till 29th), 2015/04/10 15:29:35]

Acknowledged.

+class WorkerThreadWaitUntilQuiescenceTask : public WebThread::Task, public WebThread::IdleTask  {
+public:
+    explicit WorkerThreadWaitUntilQuiescenceTask(WorkerThread* thread) : m_thread(thread) { }
+
+    ~WorkerThreadWaitUntilQuiescenceTask() override
+    {
+    }
+
+    void run() override
+    {
+        m_thread->maybeStartIdleTask();
+    }
+
+    void run(double) override
+    {
+        m_thread->dontStartIdleTaskYet();
+    }
+
+private:
+    RawPtr<WorkerThread> m_thread;
+};
+
+class WorkerThreadIdleTask : public WebThread::IdleTask {
+public:
+    explicit WorkerThreadIdleTask(WorkerThread* thread)
+        : m_thread(thread) { }
+
+    ~WorkerThreadIdleTask() override { }
+
+    void run(double deadlineSeconds) override
+    {
+        m_thread->idleTask(deadlineSeconds);
+    }
+
+private:
+    RawPtr<WorkerThread> m_thread;
+};
+
+class WorkerThreadWakeupIdleTask : public WebThread::IdleTask {
+public:
+    explicit WorkerThreadWakeupIdleTask(WorkerThread* thread)
+        : m_thread(thread) { }
+
+    ~WorkerThreadWakeupIdleTask() override { }
+
+    void run(double) override
+    {
+        m_thread->dontStartIdleTaskYet();
+        m_thread->maybeStartIdleTask();
+    }
+
+private:
+    RawPtr<WorkerThread> m_thread;
+};
+
 void WorkerThread::initialize()
 {
     KURL scriptURL = m_startupData->m_scriptURL;
@@ -324,8 +315,6 @@ void WorkerThread::initialize()
         m_isolate = initializeIsolate();
         m_workerGlobalScope = createWorkerGlobalScope(m_startupData.release());
         m_workerGlobalScope->scriptLoaded(sourceCode.length(), cachedMetaData.get() ? cachedMetaData->size() : 0);
-
-        PlatformThreadData::current().threadTimers().setSharedTimer(adoptPtr(new WorkerSharedTimer(this)));
     }
 
     // The corresponding call to stopRunLoop() is in ~WorkerScriptController().
@@ -347,7 +336,8 @@ void WorkerThread::initialize()
 
     postInitialize();
 
-    postDelayedTask(FROM_HERE, createSameThreadTask(&WorkerThread::idleHandler, this), kShortIdleHandlerDelayMs);
+    dontStartIdleTaskYet();
+    maybeStartIdleTask();

[Sami, 2015/04/09 10:52:29]

This reads a little poorly ("don't do idle ... oh wait I might have changed my
mind so maybe do it after all"). Should we add something like
startPollingForQuietPeriod?

[alex clarke (OOO till 29th), 2015/04/10 15:29:36]

Acknowledged.

 }
 
 PassOwnPtr<WebThreadSupportingGC> WorkerThread::createWebThreadSupportingGC()
@@ -380,9 +370,6 @@ void WorkerThread::cleanup()
     workerReportingProxy().workerThreadTerminated();
 
     m_terminationEvent->signal();
-
-    // Clean up PlatformThreadData before WTF::WTFThreadData goes away!
-    PlatformThreadData::current().destroy();
 }
 
 class WorkerThreadShutdownFinishTask : public ExecutionContextTask {
@@ -417,7 +404,6 @@ public:
     {
         WorkerGlobalScope* workerGlobalScope = toWorkerGlobalScope(context);
         workerGlobalScope->stopActiveDOMObjects();
-        PlatformThreadData::current().threadTimers().setSharedTimer(nullptr);
 
         // Event listeners would keep DOMWrapperWorld objects alive for too long. Also, they have references to JS objects,
         // which become dangling once Heap is destroyed.
@@ -509,21 +495,42 @@ bool WorkerThread::isCurrentThread() const
     return m_thread && m_thread->isCurrentThread();
 }
 
-void WorkerThread::idleHandler()
+void WorkerThread::dontStartIdleTaskYet()
 {
-    ASSERT(m_workerGlobalScope.get());
-    int64_t delay = kLongIdleHandlerDelayMs;
+    m_dontStartIdleTaskYet = true;
+}
 
-    // 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;
+void WorkerThread::maybeStartIdleTask()
+{
+    if (m_dontStartIdleTaskYet) {
+        // We don't want to be too quick to enter idle mode (where we start doing more GC) because
+        // that might cause the worker thread to be unresponsive to messages.  To avoid this we
+        // check every |m_minQuietPeriodMsBeforeDoingIdleGc| if any tasks have been run.  If they havent we can start doing idle GCs.

[Sami, 2015/04/09 10:52:29]

s/havent/haven't/

[alex clarke (OOO till 29th), 2015/04/10 15:29:36]

Acknowledged.

+        // NOTE we use the IdleTaskAfterWakeup call to work out if something has run.  It's execution

[Sami, 2015/04/09 10:52:29]

nit: Its

[alex clarke (OOO till 29th), 2015/04/10 15:29:35]

Acknowledged.

+        // only happens if some other task gets posted.
+        m_thread->postDelayedTask(FROM_HERE, new WorkerThreadWaitUntilQuiescenceTask(this), m_minQuietPeriodMsBeforeDoingIdleGc);
+        m_thread->postIdleTaskAfterWakeup(FROM_HERE, new WorkerThreadWaitUntilQuiescenceTask(this));
+        m_dontStartIdleTaskYet = false;
+        return;
     }
 
-    postDelayedTask(FROM_HERE, createSameThreadTask(&WorkerThread::idleHandler, this), delay);
+    m_thread->postIdleTask(FROM_HERE, new WorkerThreadIdleTask(this));
+}
+
+void WorkerThread::idleTask(double deadlineSeconds)
+{
+    if (doIdleGc(deadlineSeconds))
+        m_thread->postIdleTaskAfterWakeup(FROM_HERE, new WorkerThreadWakeupIdleTask(this));
+    else
+        m_thread->postIdleTask(FROM_HERE, new WorkerThreadIdleTask(this));
+}
+
+bool WorkerThread::doIdleGc(double deadlineSeconds)
+{
+    if (deadlineSeconds > Platform::current()->monotonicallyIncreasingTime())
+        return isolate()->IdleNotificationDeadline(deadlineSeconds);
+
+    return false;
 }
 
 void WorkerThread::postTask(const WebTraceLocation& location, PassOwnPtr<ExecutionContextTask> task)