Commit f2d649a0 authored by ulan's avatar ulan Committed by Commit bot

Refactor CancelableTaskManager to use std::map.

BUG=

Review-Url: https://codereview.chromium.org/1963853004
Cr-Commit-Position: refs/heads/master@{#36142}
parent 8e48641c
...@@ -14,7 +14,6 @@ namespace internal { ...@@ -14,7 +14,6 @@ namespace internal {
Cancelable::Cancelable(CancelableTaskManager* parent) Cancelable::Cancelable(CancelableTaskManager* parent)
: parent_(parent), status_(kWaiting), id_(0), cancel_counter_(0) { : parent_(parent), status_(kWaiting), id_(0), cancel_counter_(0) {
id_ = parent->Register(this); id_ = parent->Register(this);
CHECK(id_ != 0);
} }
...@@ -27,49 +26,35 @@ Cancelable::~Cancelable() { ...@@ -27,49 +26,35 @@ Cancelable::~Cancelable() {
} }
} }
CancelableTaskManager::CancelableTaskManager() : task_id_counter_(0) {}
static bool ComparePointers(void* ptr1, void* ptr2) { return ptr1 == ptr2; }
CancelableTaskManager::CancelableTaskManager()
: task_id_counter_(0), cancelable_tasks_(ComparePointers) {}
uint32_t CancelableTaskManager::Register(Cancelable* task) { uint32_t CancelableTaskManager::Register(Cancelable* task) {
base::LockGuard<base::Mutex> guard(&mutex_); base::LockGuard<base::Mutex> guard(&mutex_);
uint32_t id = ++task_id_counter_; uint32_t id = ++task_id_counter_;
// The loop below is just used when task_id_counter_ overflows. // The loop below is just used when task_id_counter_ overflows.
while ((id == 0) || (cancelable_tasks_.Lookup(reinterpret_cast<void*>(id), while (cancelable_tasks_.count(id) > 0) ++id;
id) != nullptr)) { cancelable_tasks_[id] = task;
++id;
}
HashMap::Entry* entry =
cancelable_tasks_.LookupOrInsert(reinterpret_cast<void*>(id), id);
entry->value = task;
return id; return id;
} }
void CancelableTaskManager::RemoveFinishedTask(uint32_t id) { void CancelableTaskManager::RemoveFinishedTask(uint32_t id) {
base::LockGuard<base::Mutex> guard(&mutex_); base::LockGuard<base::Mutex> guard(&mutex_);
void* removed = cancelable_tasks_.Remove(reinterpret_cast<void*>(id), id); size_t removed = cancelable_tasks_.erase(id);
USE(removed); USE(removed);
DCHECK(removed != nullptr); DCHECK_NE(0, removed);
cancelable_tasks_barrier_.NotifyOne(); cancelable_tasks_barrier_.NotifyOne();
} }
bool CancelableTaskManager::TryAbort(uint32_t id) { bool CancelableTaskManager::TryAbort(uint32_t id) {
base::LockGuard<base::Mutex> guard(&mutex_); base::LockGuard<base::Mutex> guard(&mutex_);
HashMap::Entry* entry = auto entry = cancelable_tasks_.find(id);
cancelable_tasks_.Lookup(reinterpret_cast<void*>(id), id); if (entry != cancelable_tasks_.end()) {
if (entry != nullptr) { Cancelable* value = entry->second;
Cancelable* value = reinterpret_cast<Cancelable*>(entry->value);
if (value->Cancel()) { if (value->Cancel()) {
// Cannot call RemoveFinishedTask here because of recursive locking. // Cannot call RemoveFinishedTask here because of recursive locking.
void* removed = cancelable_tasks_.Remove(reinterpret_cast<void*>(id), id); cancelable_tasks_.erase(entry);
USE(removed);
DCHECK(removed != nullptr);
cancelable_tasks_barrier_.NotifyOne(); cancelable_tasks_barrier_.NotifyOne();
return true; return true;
} }
...@@ -85,27 +70,19 @@ void CancelableTaskManager::CancelAndWait() { ...@@ -85,27 +70,19 @@ void CancelableTaskManager::CancelAndWait() {
// started. // started.
base::LockGuard<base::Mutex> guard(&mutex_); base::LockGuard<base::Mutex> guard(&mutex_);
// HashMap does not support removing while iterating, hence keep a set of // Cancelable tasks could be running or could potentially register new
// entries that are to be removed. // tasks, requiring a loop here.
std::set<uint32_t> to_remove; while (!cancelable_tasks_.empty()) {
for (auto it = cancelable_tasks_.begin(); it != cancelable_tasks_.end();) {
// Cancelable tasks could potentially register new tasks, requiring a loop auto current = it;
// here. // We need to get to the next element before erasing the current.
while (cancelable_tasks_.occupancy() > 0) { ++it;
for (HashMap::Entry* p = cancelable_tasks_.Start(); p != nullptr; if (current->second->Cancel()) {
p = cancelable_tasks_.Next(p)) { cancelable_tasks_.erase(current);
if (reinterpret_cast<Cancelable*>(p->value)->Cancel()) {
to_remove.insert(reinterpret_cast<Cancelable*>(p->value)->id());
} }
} }
// Remove tasks that were successfully canceled. // Wait for already running background tasks.
for (auto id : to_remove) { if (!cancelable_tasks_.empty()) {
cancelable_tasks_.Remove(reinterpret_cast<void*>(id), id);
}
to_remove.clear();
// Finally, wait for already running background tasks.
if (cancelable_tasks_.occupancy() > 0) {
cancelable_tasks_barrier_.Wait(&mutex_); cancelable_tasks_barrier_.Wait(&mutex_);
} }
} }
......
...@@ -5,11 +5,12 @@ ...@@ -5,11 +5,12 @@
#ifndef V8_CANCELABLE_TASK_H_ #ifndef V8_CANCELABLE_TASK_H_
#define V8_CANCELABLE_TASK_H_ #define V8_CANCELABLE_TASK_H_
#include <map>
#include "include/v8-platform.h" #include "include/v8-platform.h"
#include "src/base/atomic-utils.h" #include "src/base/atomic-utils.h"
#include "src/base/macros.h" #include "src/base/macros.h"
#include "src/base/platform/condition-variable.h" #include "src/base/platform/condition-variable.h"
#include "src/hashmap.h"
namespace v8 { namespace v8 {
namespace internal { namespace internal {
...@@ -51,7 +52,7 @@ class CancelableTaskManager { ...@@ -51,7 +52,7 @@ class CancelableTaskManager {
uint32_t task_id_counter_; uint32_t task_id_counter_;
// A set of cancelable tasks that are currently registered. // A set of cancelable tasks that are currently registered.
HashMap cancelable_tasks_; std::map<uint32_t, Cancelable*> cancelable_tasks_;
// Mutex and condition variable enabling concurrent register and removing, as // Mutex and condition variable enabling concurrent register and removing, as
// well as waiting for background tasks on {CancelAndWait}. // well as waiting for background tasks on {CancelAndWait}.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment