Implement ThreadPool.

runtime/vm/thread_pool.h

+// Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#ifndef VM_THREAD_POOL_H_
+#define VM_THREAD_POOL_H_
+
+#include "vm/thread.h"
+
+namespace dart {
+
+class ThreadPool {
+ private:
+  class Worker;

[siva, 2012/03/06 05:07:00]

Why is this forward declaration needed? I don't see any reference to it before
the actual declaration below.

[turnidge, 2012/03/06 22:34:01]

I used to have a task->worker pointer, but I don't anymore.  Removed.

+
+ public:
+  // Subclasses of Task are able to run on a ThreadPool.
+  class Task {
+   protected:
+    Task();
+
+   public:
+    virtual ~Task();
+
+    // Override this to provide task-specific behavior.
+    virtual void Run() = 0;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Task);
+  };
+
+  ThreadPool();
+
+  // Shuts down this thread pool.  Causes workers to terminate
+  // themselves when they are active again.
+  ~ThreadPool();
+
+  // Runs a task on the thread pool.
+  void Run(Task* task);
+
+  // Some simple stats.
+  uint64_t workers_started() const { return workers_started_; }
+  uint64_t workers_stopped() const { return workers_stopped_; }

[siva, 2012/03/06 05:07:00]

workers_active() and workers_idle() would also be useful stats.

[turnidge, 2012/03/06 22:34:01]

Added workers_running() and workers_idle().

+
+ private:
+  class Worker {
+   public:
+    explicit Worker(ThreadPool* pool);
+    ~Worker();
+
+    // Runs a task on the worker.
+    void Run(Task* task);
+
+    // Main loop for a worker.
+    void Loop();
+
+    // Causes worker to terminate eventually.
+    void Shutdown();
+
+   private:
+    friend class ThreadPool;
+
+    Monitor* monitor_;

[siva, 2012/03/06 05:07:00]

Why not use a value object for the monitor?

[turnidge, 2012/03/06 22:34:01]

Done.

+    ThreadPool* pool_;
+    Task* task_;
+    bool done_;
+
+    // Fields owned by ThreadPool.
+    Worker* next_;  // Protected by ThreadPool::mutex_
+    Worker* prev_;  // Protected by ThreadPool::mutex_

[siva, 2012/03/06 05:07:00]

Do we need a doubly linked list for this? a singly linked list maybe sufficient.
Most frequent operations are going to be add to head and remove from head.

[turnidge, 2012/03/06 22:34:01]

Changed the code to use two singly-linked lists.

+
+    DISALLOW_COPY_AND_ASSIGN(Worker);
+  };
+
+  void Shutdown();
+
+  // List primitives.
+  void RemoveWorkerFromList(Worker* worker, Worker** head);
+  void AddWorkerToList(Worker* worker, Worker** head);
+
+  // Worker operations.
+  Worker* GetIdleWorker();
+  void SetIdle(Worker* worker);
+  bool RemoveIdleWorker(Worker* worker);
+  void ShutdownWorkerList(Worker* head);
+
+  // The main entry point for new worker threads.
+  static void Main(uword args);
+
+  Mutex* mutex_;

[siva, 2012/03/06 05:07:00]

why not use a valueobject for the mutex?

[turnidge, 2012/03/06 22:34:01]

Done.

+  bool done_;

[siva, 2012/03/06 05:07:00]

would 'bool shutting_down_;' be more readable?
Using done_ both in the threadpool and the worker class makes it hard to read.

[turnidge, 2012/03/06 22:34:01]

Done.

+  Worker* idle_workers_;
+  Worker* running_workers_;
+  uint64_t workers_started_;
+  uint64_t workers_stopped_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThreadPool);
+};
+
+}  // namespace dart
+
+#endif  // VM_THREAD_POOL_H_