Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: src/Scheduler.h

Issue 29370876: Issue #4692, #3595 - Stop sleeping in the implementation of `SetTimeout`
Patch Set: Created Jan. 9, 2017, 1:22 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/JsEngineInternal.h ('k') | src/Scheduler.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/Scheduler.h
===================================================================
--- a/src/Scheduler.h
+++ b/src/Scheduler.h
@@ -26,46 +26,6 @@
#include <queue>
#include <thread>
-namespace AdblockPlus
-{
- /*
- * Adapter class for heap allocated function objects
- *
- * The legacy behavior of the original threading regime combined task life
- * cycle and execution. Previously, tasks were allocated by the user and
- * deleted at the end of execution internally. This adapter class allows
- * separation of these concerns. It allows the legacy allocation behavior
- * without burdening the new scheduler with any concern over task life cycle.
- */
- template<class T>
- class HeapFunction
- {
- std::shared_ptr<T> body;
-
- public:
- HeapFunction(std::shared_ptr<T> body)
- :body(body)
- {}
-
- void operator()()
- {
- if (body)
- {
- body->operator()();
- }
- }
- };
-
- /*
- * Utility function uses type inference to simplify expressions at point of use.
- */
- template<class T>
- HeapFunction<T> MakeHeapFunction(std::shared_ptr<T> body)
- {
- return HeapFunction<T>(body);
- }
-}
-
/**
* Interface class for scheduled tasks.
*/
@@ -84,14 +44,6 @@
};
/**
- * Execute a task immediately in detached thread that's used only for this task.
- *
- * The present version is nothing more than a rewrite of the legacy behavior,
- * which was to create a new thread for each task.
- */
-void StartImmediatelyInSingleUseDetachedThread(std::function<void()> task);
-
-/**
* Active object to run arbitrary function objects.
*
* Operations execute in a thread owned by this instance.
@@ -196,19 +148,53 @@
* but there's no interface for this.
* - No support for delayed execution to support JavaScript `SetTimeout`.
*/
-template<class Worker>
+template<class Worker, class UserData>
class SchedulerT
{
/**
+ * A task as scheduled for execution.
+ *
+ * This class is a combination of two kinds of data:
+ * 1. An external object containing the body of the task.
+ * 2. All the internal elements required to run the task.
+ */
+ template<class UserData>
+ struct ScheduledTask
+ {
+ /**
+ * The external task.
+ * This member comes to us from our Run() method.
+ */
+ std::shared_ptr<TaskFunctionInterface> body;
+ /**
+ * The only internal data at present is the worker thread.
+ */
+ Worker worker;
+ /**
+ * Data associated with this task by the user of the scheduler
+ */
+ std::shared_ptr<UserData> userData;
+ /**
+ * Member worker is default-constructed
+ * because VS2012 insists on copy constructors when it doesn't need them.
+ */
+ ScheduledTask(std::shared_ptr<TaskFunctionInterface>&& body, std::shared_ptr<UserData>&& ud)
+ : body(std::forward<std::shared_ptr<TaskFunctionInterface>>(body)),
+ userData(std::forward<std::shared_ptr<UserData>>(ud))
+ {}
+ };
+
+ /**
* Shared mutex for all synchronization
*/
std::mutex m;
typedef std::unique_lock<std::mutex> UniqueLockType;
+ typedef std::list<ScheduledTask<UserData>> TaskListType;
/**
* A list containing an entry for each running task.
*/
- std::list<Worker> taskList;
+ TaskListType taskList;
/**
* Condition variable notified when a running task is removed from its list
@@ -221,13 +207,20 @@
OperationRunner monitor;
/**
+ * State flag.
+ *
+ * We only have two states: normal and terminating.
+ */
+ bool isNormal;
+
+ /**
* Execution wrapper for a task.
* @param task
* An iterator to the task within the task list.
*/
- void WorkerMain(std::function<void()> body, typename std::list<Worker>::iterator task)
+ void WorkerMain(typename TaskListType::iterator task)
{
- body();
+ (*task->body)();
monitor.Run([this, task]() { JoinAndRemove(task); });
}
@@ -238,9 +231,9 @@
*
* This is the only function that may remove tasks from the running task list.
*/
- void JoinAndRemove(typename std::list<Worker>::iterator task)
+ void JoinAndRemove(typename TaskListType::iterator task)
{
- task->Join();
+ task->worker.Join();
{
UniqueLockType ul(m);
taskList.erase(task);
@@ -250,6 +243,7 @@
public:
SchedulerT()
+ : isNormal(true)
{}
~SchedulerT()
@@ -259,21 +253,46 @@
// We call it in the destructor to ensure all tasks have completed.
// If we do not, we risk destruction of a joinable thread,
// which would generate a call to `std::terminate()`.
+ ShutDown();
JoinAll();
}
/**
+ * Interrupt all pending tasks so that they'll terminate soon.
+ * Execution may proceed after a call to Interrupt(),
+ * but it does indicate that a task should terminate as soon as it can.
+ */
+ void ShutDown()
+ {
+ UniqueLockType ul(m);
+ // Ensure the task list does not grow during shutdown.
+ isNormal = false;
+
+ for (auto& it : taskList)
+ {
+ it.body->Interrupt();
+ }
+ }
+
+ /**
* Construct a `Worker` and run a task in it immediately.
*
* This is the only function that may add tasks to the running task list.
*/
- void Run(std::function<void()> body)
+ void Run(std::shared_ptr<TaskFunctionInterface>&& body, std::shared_ptr<UserData>&& ud)
{
UniqueLockType ul(m);
+ if (!isNormal)
+ {
+ // We don't run new tasks if we're in a terminating state
+ return;
+ }
// Note that the interface to `Worker` uses a constructor and not a factory
// Here we construct in place, avoiding both move and copy.
- auto tt = taskList.emplace(taskList.begin());
- tt->Run([this, body, tt]() { WorkerMain(body, tt); });
+ auto tt = taskList.emplace(taskList.begin(),
+ std::forward<std::shared_ptr<TaskFunctionInterface>>(body),
+ std::forward<std::shared_ptr<UserData>>(ud));
+ tt->worker.Run([this, tt]() { WorkerMain(tt); });
}
/**
« no previous file with comments | « src/JsEngineInternal.h ('k') | src/Scheduler.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld