[Blink-Worker] WorkerThread fires idleHandler only at the end of processing all tasks.

Source/core/workers/WorkerThread.cpp

 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 146 matching lines @@
         // ensure none of those are ran.
         if (!workerGlobalScope)
             return;
 
         if (m_isInstrumented)
             InspectorInstrumentation::willPerformExecutionContextTask(workerGlobalScope, m_task.get());
         if ((!workerGlobalScope->isClosing() && !m_workerThread.terminated()) || m_task->isCleanupTask())
             m_task->performTask(workerGlobalScope);
         if (m_isInstrumented)
             InspectorInstrumentation::didPerformExecutionContextTask(workerGlobalScope);
+
+        // If no more tasks are queued up after this, then queue up the idleHandler
+        // to trigger GC.
+        WorkerThread& workerThread = const_cast<WorkerThread&>(m_workerThread);
+        if (!workerThread.decrementAndReturnTaskCount() && !workerThread.isIdleHandlerTaskFiredOnce())

[jochen (gone - plz use gerrit), 2014/09/02 10:14:14]

maybe teach WorkerThreadTask that it's an idle task instead of always
decrementing the task count? then you can't get negative in
decrementAndReturnTaskCount()

[Mayur Kankanwadi, 2014/09/03 11:26:21]

Done.

+            workerThread.queueUpIdleHandlerNow();
     }
 
 private:
     WorkerThreadTask(const WorkerThread& workerThread, PassOwnPtr<ExecutionContextTask> task, bool isInstrumented)
         : m_workerThread(workerThread)
         , m_task(task)
         , m_isInstrumented(isInstrumented)
     {
         if (m_isInstrumented)
             m_isInstrumented = !m_task->taskNameForInstrumentation().isEmpty();
@@ skipping 23 matching lines @@
 
     WorkerThread* m_thread;
 };
 
 WorkerThread::WorkerThread(WorkerLoaderProxy& workerLoaderProxy, WorkerReportingProxy& workerReportingProxy, PassOwnPtrWillBeRawPtr<WorkerThreadStartupData> startupData)
     : m_terminated(false)
     , m_workerLoaderProxy(workerLoaderProxy)
     , m_workerReportingProxy(workerReportingProxy)
     , m_startupData(startupData)
     , m_shutdownEvent(adoptPtr(blink::Platform::current()->createWaitableEvent()))
+    , m_tasksCount(0)
+    , m_isIdleHandlerTaskFiredOnce(false)
+    , m_isIdleHandlerTask(false)
 {
     MutexLocker lock(threadSetMutex());
     workerThreads().add(this);
 }
 
 WorkerThread::~WorkerThread()
 {
     MutexLocker lock(threadSetMutex());
     ASSERT(workerThreads().contains(this));
     workerThreads().remove(this);
@@ skipping 175 matching lines @@
     InspectorInstrumentation::didKillAllExecutionContextTasks(m_workerGlobalScope.get());
     m_debuggerMessageQueue.kill();
     postTask(WorkerThreadShutdownStartTask::create());
 }
 
 bool WorkerThread::isCurrentThread() const
 {
     return m_thread && m_thread->isCurrentThread();
 }
 
+bool WorkerThread::isIdleHandlerTaskFiredOnce()
+{
+    MutexLocker lock(m_taskCounterMutex);
+    return m_isIdleHandlerTaskFiredOnce;
+}
+
+void WorkerThread::queueUpIdleHandlerNow()
+{
+    m_isIdleHandlerTask = m_isIdleHandlerTaskFiredOnce = true;
+    postTask(createSameThreadTask(&WorkerThread::idleHandler, this));
+    m_isIdleHandlerTask = false;

[jochen (gone - plz use gerrit), 2014/09/02 10:14:14]

instead of using m_isIdleHandlerTask, this method should just directly create
and post the task on m_thread.

[Mayur Kankanwadi, 2014/09/03 11:26:21]

Done.

+}
+
 void WorkerThread::idleHandler()
 {
     ASSERT(m_workerGlobalScope.get());
     int64 delay = kLongIdleHandlerDelayMs;
 
     // Do a script engine idle notification if the next event is distant enough.
     const double kMinIdleTimespan = 0.3;
+    // This is set to true when idleNotification returns false.
+    // idleNotification returns false only when more GC is required. It returns
+    // true if no more GC is required and that is when callGCAgain will be set to false.
+    bool callGCAgain = false;
     if (m_sharedTimer->nextFireTime() == 0.0 || m_sharedTimer->nextFireTime() > currentTime() + kMinIdleTimespan) {
-        bool hasMoreWork = !m_workerGlobalScope->idleNotification();
-        if (hasMoreWork)
+        bool callGCAgain = !m_workerGlobalScope->idleNotification();

[Mayur Kankanwadi, 2014/09/03 11:26:21]

This(bool callGCAgain) was an extra definition, with a good scope for subtle
bugs.
Removed this as well.

+        if (callGCAgain)
             delay = kShortIdleHandlerDelayMs;
     }
+    if (callGCAgain) {
+        postDelayedTask(createSameThreadTask(&WorkerThread::idleHandler, this), delay);
+    }
+}
 
-    postDelayedTask(createSameThreadTask(&WorkerThread::idleHandler, this), delay);
+void WorkerThread::decrementTaskCount()

[jochen (gone - plz use gerrit), 2014/09/02 10:14:14]

this method isn't used anywhere?

[Mayur Kankanwadi, 2014/09/03 11:26:22]

True.Removed it.

+{
+    MutexLocker lock(m_taskCounterMutex);
+    if (!m_tasksCount)
+        return;
+    --m_tasksCount;
+}
+
+unsigned WorkerThread::decrementAndReturnTaskCount()
+{
+    MutexLocker lock(m_taskCounterMutex);
+    if (!m_tasksCount)

[jochen (gone - plz use gerrit), 2014/09/02 10:14:14]

see my comment about not invoking this for idle tasks

[Mayur Kankanwadi, 2014/09/03 11:26:21]

Done.

+        return 0;
+    --m_tasksCount;
+    return m_tasksCount;
+}
+
+unsigned WorkerThread::taskCount()

[jochen (gone - plz use gerrit), 2014/09/02 10:14:14]

not used anywhere?

[Mayur Kankanwadi, 2014/09/03 11:26:22]

True.Removed it.

+{
+    MutexLocker lock(m_taskCounterMutex);
+    return m_tasksCount;
 }
 
 void WorkerThread::postTask(PassOwnPtr<ExecutionContextTask> task)
 {
+    if (!m_isIdleHandlerTask) {
+        MutexLocker lock(m_taskCounterMutex);
+        ++m_tasksCount;
+        m_isIdleHandlerTaskFiredOnce = false;
+    }
     m_thread->postTask(WorkerThreadTask::create(*this, task, true).leakPtr());
 }
 
 void WorkerThread::postDelayedTask(PassOwnPtr<ExecutionContextTask> task, long long delayMs)
 {
+    if (!m_isIdleHandlerTask) {
+        MutexLocker lock(m_taskCounterMutex);
+        ++m_tasksCount;
+        m_isIdleHandlerTaskFiredOnce = false;
+    }
     m_thread->postDelayedTask(WorkerThreadTask::create(*this, task, true).leakPtr(), delayMs);
 }
 
 void WorkerThread::postDebuggerTask(PassOwnPtr<ExecutionContextTask> task)
 {
     m_debuggerMessageQueue.append(WorkerThreadTask::create(*this, task, false));
     postTask(RunDebuggerQueueTask::create(this));
 }
 
 MessageQueueWaitResult WorkerThread::runDebuggerTask(WaitMode waitMode)
@@ skipping 28 matching lines @@
     InspectorInstrumentation::didLeaveNestedRunLoop(m_workerGlobalScope.get());
 }
 
 void WorkerThread::setWorkerInspectorController(WorkerInspectorController* workerInspectorController)
 {
     MutexLocker locker(m_workerInspectorControllerMutex);
     m_workerInspectorController = workerInspectorController;
 }
 
 } // namespace blink