[vm] Use std::unique_ptr for ThreadPool::Task
Updates #37244.
Change-Id: I32a5180a17fe43be5e18367d784cf756dffc6aeb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106009
Commit-Queue: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
diff --git a/runtime/lib/isolate.cc b/runtime/lib/isolate.cc
index 88d50b8..436e895 100644
--- a/runtime/lib/isolate.cc
+++ b/runtime/lib/isolate.cc
@@ -245,14 +245,7 @@
// Since this is a call to Isolate.spawn, copy the parent isolate's code.
state->isolate_flags()->copy_parent_code = true;
- ThreadPool::Task* spawn_task =
- new SpawnIsolateTask(isolate, std::move(state));
-
- if (!Dart::thread_pool()->Run(spawn_task)) {
- // Running on the thread pool failed. Clean up everything.
- delete spawn_task;
- spawn_task = NULL;
- }
+ Dart::thread_pool()->Run<SpawnIsolateTask>(isolate, std::move(state));
return Object::null();
}
}
@@ -373,14 +366,7 @@
// Since this is a call to Isolate.spawnUri, don't copy the parent's code.
state->isolate_flags()->copy_parent_code = false;
- ThreadPool::Task* spawn_task =
- new SpawnIsolateTask(isolate, std::move(state));
-
- if (!Dart::thread_pool()->Run(spawn_task)) {
- // Running on the thread pool failed. Clean up everything.
- delete spawn_task;
- spawn_task = NULL;
- }
+ Dart::thread_pool()->Run<SpawnIsolateTask>(isolate, std::move(state));
return Object::null();
}
diff --git a/runtime/vm/compiler/jit/compiler.cc b/runtime/vm/compiler/jit/compiler.cc
index e5b7b35..1a5213c 100644
--- a/runtime/vm/compiler/jit/compiler.cc
+++ b/runtime/vm/compiler/jit/compiler.cc
@@ -1389,8 +1389,7 @@
if (running_ || !done_) return;
running_ = true;
done_ = false;
- bool task_started =
- Dart::thread_pool()->Run(new BackgroundCompilerTask(this));
+ bool task_started = Dart::thread_pool()->Run<BackgroundCompilerTask>(this);
if (!task_started) {
running_ = false;
done_ = true;
diff --git a/runtime/vm/heap/compactor.cc b/runtime/vm/heap/compactor.cc
index 06ac415..45f566e 100644
--- a/runtime/vm/heap/compactor.cc
+++ b/runtime/vm/heap/compactor.cc
@@ -234,9 +234,9 @@
intptr_t next_forwarding_task = 0;
for (intptr_t task_index = 0; task_index < num_tasks; task_index++) {
- Dart::thread_pool()->Run(new CompactorTask(
+ Dart::thread_pool()->Run<CompactorTask>(
thread()->isolate(), this, &barrier, &next_forwarding_task,
- heads[task_index], &tails[task_index], freelist));
+ heads[task_index], &tails[task_index], freelist);
}
// Plan pages.
diff --git a/runtime/vm/heap/marker.cc b/runtime/vm/heap/marker.cc
index ec644c0..01149d5 100644
--- a/runtime/vm/heap/marker.cc
+++ b/runtime/vm/heap/marker.cc
@@ -804,8 +804,8 @@
&deferred_marking_stack_);
// Begin marking on a helper thread.
- bool result = Dart::thread_pool()->Run(
- new ConcurrentMarkTask(this, isolate_, page_space, visitors_[i]));
+ bool result = Dart::thread_pool()->Run<ConcurrentMarkTask>(
+ this, isolate_, page_space, visitors_[i]);
ASSERT(result);
}
@@ -862,8 +862,8 @@
isolate_, page_space, &marking_stack_, &deferred_marking_stack_);
}
- bool result = Dart::thread_pool()->Run(new ParallelMarkTask(
- this, isolate_, &marking_stack_, &barrier, visitor, &num_busy));
+ bool result = Dart::thread_pool()->Run<ParallelMarkTask>(
+ this, isolate_, &marking_stack_, &barrier, visitor, &num_busy);
ASSERT(result);
}
bool more_to_mark = false;
diff --git a/runtime/vm/heap/sweeper.cc b/runtime/vm/heap/sweeper.cc
index b1e385f..1d267ac 100644
--- a/runtime/vm/heap/sweeper.cc
+++ b/runtime/vm/heap/sweeper.cc
@@ -182,8 +182,8 @@
HeapPage* first,
HeapPage* last,
FreeList* freelist) {
- bool result = Dart::thread_pool()->Run(new ConcurrentSweeperTask(
- isolate, isolate->heap()->old_space(), first, last, freelist));
+ bool result = Dart::thread_pool()->Run<ConcurrentSweeperTask>(
+ isolate, isolate->heap()->old_space(), first, last, freelist);
ASSERT(result);
}
diff --git a/runtime/vm/isolate_test.cc b/runtime/vm/isolate_test.cc
index b019598c..2b0f84f 100644
--- a/runtime/vm/isolate_test.cc
+++ b/runtime/vm/isolate_test.cc
@@ -130,7 +130,7 @@
isolate->heap()->barrier_done());
// Start all tasks. They will busy-wait until interrupted in the first round.
for (intptr_t task = 0; task < InterruptChecker::kTaskCount; task++) {
- Dart::thread_pool()->Run(new InterruptChecker(thread, &barrier));
+ Dart::thread_pool()->Run<InterruptChecker>(thread, &barrier);
}
// Wait for all tasks to get ready for the first round.
barrier.Sync();
diff --git a/runtime/vm/kernel_isolate.cc b/runtime/vm/kernel_isolate.cc
index abf5036..e1b8c7c 100644
--- a/runtime/vm/kernel_isolate.cc
+++ b/runtime/vm/kernel_isolate.cc
@@ -232,7 +232,7 @@
KernelIsolate::InitializingFailed();
return;
}
- bool task_started = Dart::thread_pool()->Run(new RunKernelTask());
+ bool task_started = Dart::thread_pool()->Run<RunKernelTask>();
ASSERT(task_started);
}
diff --git a/runtime/vm/message_handler.cc b/runtime/vm/message_handler.cc
index 06fd9e2..122fb58 100644
--- a/runtime/vm/message_handler.cc
+++ b/runtime/vm/message_handler.cc
@@ -114,7 +114,7 @@
start_callback_ = start_callback;
end_callback_ = end_callback;
callback_data_ = data;
- task_running_ = pool_->Run(new MessageHandlerTask(this));
+ task_running_ = pool_->Run<MessageHandlerTask>(this);
ASSERT(task_running_);
}
@@ -157,7 +157,7 @@
if ((pool_ != NULL) && !task_running_) {
ASSERT(!delete_me_);
- task_running_ = pool_->Run(new MessageHandlerTask(this));
+ task_running_ = pool_->Run<MessageHandlerTask>(this);
ASSERT(task_running_);
}
}
diff --git a/runtime/vm/service_isolate.cc b/runtime/vm/service_isolate.cc
index 8a223ee..d76e551 100644
--- a/runtime/vm/service_isolate.cc
+++ b/runtime/vm/service_isolate.cc
@@ -483,7 +483,7 @@
ServiceIsolate::InitializingFailed();
return;
}
- bool task_started = Dart::thread_pool()->Run(new RunServiceTask());
+ bool task_started = Dart::thread_pool()->Run<RunServiceTask>();
ASSERT(task_started);
}
diff --git a/runtime/vm/thread_barrier_test.cc b/runtime/vm/thread_barrier_test.cc
index 42bc53b..c937eeb2 100644
--- a/runtime/vm/thread_barrier_test.cc
+++ b/runtime/vm/thread_barrier_test.cc
@@ -45,7 +45,7 @@
{
ThreadBarrier barrier(kNumTasks + 1, monitor, monitor_done);
for (intptr_t i = 0; i < kNumTasks; ++i) {
- Dart::thread_pool()->Run(new FuzzTask(kNumRounds, &barrier, i + 1));
+ Dart::thread_pool()->Run<FuzzTask>(kNumRounds, &barrier, i + 1);
}
for (intptr_t i = 0; i < kNumRounds; ++i) {
barrier.Sync();
diff --git a/runtime/vm/thread_pool.cc b/runtime/vm/thread_pool.cc
index 3a085ae..6f43097 100644
--- a/runtime/vm/thread_pool.cc
+++ b/runtime/vm/thread_pool.cc
@@ -30,7 +30,7 @@
Shutdown();
}
-bool ThreadPool::Run(Task* task) {
+bool ThreadPool::RunImpl(std::unique_ptr<Task> task) {
Worker* worker = NULL;
bool new_worker = false;
{
@@ -63,7 +63,7 @@
// Release ThreadPool::mutex_ before calling Worker functions.
ASSERT(worker != NULL);
- worker->SetTask(task);
+ worker->SetTask(std::move(task));
if (new_worker) {
// Call StartThread after we've assigned the first task.
worker->StartThread();
@@ -314,7 +314,7 @@
ThreadPool::Worker::Worker(ThreadPool* pool)
: pool_(pool),
- task_(NULL),
+ task_(nullptr),
id_(OSThread::kInvalidThreadId),
done_(false),
owned_(false),
@@ -332,7 +332,7 @@
// Must call SetTask before StartThread.
{ // NOLINT
MonitorLocker ml(&monitor_);
- ASSERT(task_ != NULL);
+ ASSERT(task_ != nullptr);
}
#endif
int result = OSThread::Start("Dart ThreadPool Worker", &Worker::Main,
@@ -342,10 +342,10 @@
}
}
-void ThreadPool::Worker::SetTask(Task* task) {
+void ThreadPool::Worker::SetTask(std::unique_ptr<Task> task) {
MonitorLocker ml(&monitor_);
- ASSERT(task_ == NULL);
- task_ = task;
+ ASSERT(task_ == nullptr);
+ task_ = std::move(task);
ml.Notify();
}
@@ -372,18 +372,17 @@
MonitorLocker ml(&monitor_);
int64_t idle_start;
while (true) {
- ASSERT(task_ != NULL);
- Task* task = task_;
- task_ = NULL;
+ ASSERT(task_ != nullptr);
+ std::unique_ptr<Task> task = std::move(task_);
// Release monitor while handling the task.
ml.Exit();
task->Run();
ASSERT(Isolate::Current() == NULL);
- delete task;
+ task.reset();
ml.Enter();
- ASSERT(task_ == NULL);
+ ASSERT(task_ == nullptr);
if (IsDone()) {
return false;
}
@@ -392,7 +391,7 @@
idle_start = OS::GetCurrentMonotonicMicros();
while (true) {
Monitor::WaitResult result = ml.WaitMicros(ComputeTimeout(idle_start));
- if (task_ != NULL) {
+ if (task_ != nullptr) {
// We've found a task. Process it, regardless of whether the
// worker is done_.
break;
diff --git a/runtime/vm/thread_pool.h b/runtime/vm/thread_pool.h
index 6fbe0d7..5a1ff54 100644
--- a/runtime/vm/thread_pool.h
+++ b/runtime/vm/thread_pool.h
@@ -5,6 +5,9 @@
#ifndef RUNTIME_VM_THREAD_POOL_H_
#define RUNTIME_VM_THREAD_POOL_H_
+#include <memory>
+#include <utility>
+
#include "vm/allocation.h"
#include "vm/globals.h"
#include "vm/os_thread.h"
@@ -35,7 +38,10 @@
~ThreadPool();
// Runs a task on the thread pool.
- bool Run(Task* task);
+ template <typename T, typename... Args>
+ bool Run(Args&&... args) {
+ return RunImpl(std::unique_ptr<Task>(new T(std::forward<Args>(args)...)));
+ }
// Some simple stats.
uint64_t workers_running() const { return count_running_; }
@@ -49,7 +55,7 @@
explicit Worker(ThreadPool* pool);
// Sets a task on the worker.
- void SetTask(Task* task);
+ void SetTask(std::unique_ptr<Task> task);
// Starts the thread for the worker. This should only be called
// after a task has been set by the initial call to SetTask().
@@ -76,7 +82,7 @@
// Fields owned by Worker.
Monitor monitor_;
ThreadPool* pool_;
- Task* task_;
+ std::unique_ptr<Task> task_;
ThreadId id_;
bool done_;
@@ -110,6 +116,7 @@
DISALLOW_COPY_AND_ASSIGN(JoinList);
};
+ bool RunImpl(std::unique_ptr<Task> task);
void Shutdown();
// Expensive. Use only in assertions.
diff --git a/runtime/vm/thread_pool_test.cc b/runtime/vm/thread_pool_test.cc
index 1706a99..ab197f5 100644
--- a/runtime/vm/thread_pool_test.cc
+++ b/runtime/vm/thread_pool_test.cc
@@ -44,7 +44,7 @@
ThreadPool thread_pool;
Monitor sync;
bool done = true;
- thread_pool.Run(new TestTask(&sync, &done));
+ thread_pool.Run<TestTask>(&sync, &done);
{
MonitorLocker ml(&sync);
done = false;
@@ -68,7 +68,7 @@
for (int i = 0; i < kTaskCount; i++) {
done[i] = true;
- thread_pool.Run(new TestTask(&sync[i], &done[i]));
+ thread_pool.Run<TestTask>(&sync[i], &done[i]);
}
for (int i = 0; i < kTaskCount; i++) {
MonitorLocker ml(&sync[i]);
@@ -124,7 +124,7 @@
// Run a single task.
for (int i = 0; i < kTaskCount; i++) {
- thread_pool->Run(new SleepTask(&sync, &started_count, &slept_count, 2));
+ thread_pool->Run<SleepTask>(&sync, &started_count, &slept_count, 2);
}
{
@@ -162,7 +162,7 @@
// Run a worker.
Monitor sync;
bool done = true;
- thread_pool.Run(new TestTask(&sync, &done));
+ thread_pool.Run<TestTask>(&sync, &done);
EXPECT_EQ(1U, thread_pool.workers_started());
EXPECT_EQ(0U, thread_pool.workers_stopped());
{
@@ -197,11 +197,10 @@
// Spawn 0-2 children.
if (todo_ > 0) {
- pool_->Run(
- new SpawnTask(pool_, sync_, todo_ - child_todo, total_, done_));
+ pool_->Run<SpawnTask>(pool_, sync_, todo_ - child_todo, total_, done_);
}
if (todo_ > 1) {
- pool_->Run(new SpawnTask(pool_, sync_, child_todo, total_, done_));
+ pool_->Run<SpawnTask>(pool_, sync_, child_todo, total_, done_);
}
{
@@ -226,8 +225,8 @@
Monitor sync;
const int kTotalTasks = 500;
int done = 0;
- thread_pool.Run(
- new SpawnTask(&thread_pool, &sync, kTotalTasks, kTotalTasks, &done));
+ thread_pool.Run<SpawnTask>(&thread_pool, &sync, kTotalTasks, kTotalTasks,
+ &done);
{
MonitorLocker ml(&sync);
while (done < kTotalTasks) {
diff --git a/runtime/vm/thread_test.cc b/runtime/vm/thread_test.cc
index 1030795..edbc632 100644
--- a/runtime/vm/thread_test.cc
+++ b/runtime/vm/thread_test.cc
@@ -180,8 +180,8 @@
isolate->heap()->DisableGrowthControl();
for (int i = 0; i < kTaskCount; i++) {
done[i] = false;
- Dart::thread_pool()->Run(
- new TaskWithZoneAllocation(isolate, &sync[i], &done[i], i));
+ Dart::thread_pool()->Run<TaskWithZoneAllocation>(isolate, &sync[i],
+ &done[i], i);
}
bool in_isolate = true;
for (int i = 0; i < kTaskCount; i++) {
@@ -302,8 +302,8 @@
EXPECT(isolate->heap()->GrowthControlState());
isolate->heap()->DisableGrowthControl();
for (intptr_t i = 0; i < kTaskCount; i++) {
- Dart::thread_pool()->Run(new SimpleTaskWithZoneAllocation(
- (i + 1), isolate, &threads[i], &sync, &monitor, &done_count, &wait));
+ Dart::thread_pool()->Run<SimpleTaskWithZoneAllocation>(
+ (i + 1), isolate, &threads[i], &sync, &monitor, &done_count, &wait);
}
// Wait until all spawned tasks finish their memory operations.
{
@@ -497,8 +497,8 @@
}
for (int i = 0; i < ICDataTestTask::kTaskCount; i++) {
- Dart::thread_pool()->Run(
- new ICDataTestTask(isolate, ic_datas, &monitor, &exited, &done));
+ Dart::thread_pool()->Run<ICDataTestTask>(isolate, ic_datas, &monitor,
+ &exited, &done);
}
for (int i = 0; i < 0x10000; i++) {
@@ -630,8 +630,8 @@
intptr_t total_done = 0;
intptr_t exited = 0;
for (int i = 0; i < SafepointTestTask::kTaskCount; i++) {
- Dart::thread_pool()->Run(new SafepointTestTask(
- isolate, &monitor, &expected_count, &total_done, &exited));
+ Dart::thread_pool()->Run<SafepointTestTask>(
+ isolate, &monitor, &expected_count, &total_done, &exited);
}
// Run Dart code on the main thread long enough to allow all helpers
// to get their verification done and exit. Use a specific UserTag
@@ -681,8 +681,8 @@
intptr_t total_done = 0;
intptr_t exited = 0;
for (int i = 0; i < SafepointTestTask::kTaskCount; i++) {
- Dart::thread_pool()->Run(new SafepointTestTask(
- isolate, &monitor, &expected_count, &total_done, &exited));
+ Dart::thread_pool()->Run<SafepointTestTask>(
+ isolate, &monitor, &expected_count, &total_done, &exited);
}
String& label = String::Handle(String::New("foo"));
UserTag& tag = UserTag::Handle(UserTag::New(label));
@@ -802,8 +802,8 @@
intptr_t total_done = 0;
intptr_t exited = 0;
for (int i = 0; i < SafepointTestTask::kTaskCount; i++) {
- Dart::thread_pool()->Run(new SafepointTestTask(
- isolate, &monitor, &expected_count, &total_done, &exited));
+ Dart::thread_pool()->Run<SafepointTestTask>(
+ isolate, &monitor, &expected_count, &total_done, &exited);
}
bool all_helpers = false;
do {
@@ -833,8 +833,8 @@
intptr_t total_done = 0;
intptr_t exited = 0;
for (int i = 0; i < SafepointTestTask::kTaskCount; i++) {
- Dart::thread_pool()->Run(new SafepointTestTask(
- isolate, &monitor, &expected_count, &total_done, &exited));
+ Dart::thread_pool()->Run<SafepointTestTask>(
+ isolate, &monitor, &expected_count, &total_done, &exited);
}
bool all_helpers = false;
do {
@@ -899,7 +899,7 @@
Monitor done_monitor;
bool done = false;
Isolate* isolate = thread->isolate();
- Dart::thread_pool()->Run(new AllocAndGCTask(isolate, &done_monitor, &done));
+ Dart::thread_pool()->Run<AllocAndGCTask>(isolate, &done_monitor, &done);
{
while (true) {
TransitionVMToBlocked transition(thread);
@@ -951,8 +951,8 @@
Isolate* isolate = thread->isolate();
for (int i = 0; i < NUMBER_TEST_THREADS; i++) {
done[i] = false;
- Dart::thread_pool()->Run(
- new AllocateGlobsOfMemoryTask(isolate, &done_monitor[i], &done[i]));
+ Dart::thread_pool()->Run<AllocateGlobsOfMemoryTask>(
+ isolate, &done_monitor[i], &done[i]);
}
for (int i = 0; i < NUMBER_TEST_THREADS; i++) {
