Commit 63db4c77 authored by alexclarke's avatar alexclarke Committed by Commit bot

Use WTF::Deque instead of std::queue in the blink scheduler

In theory this will reduce memory wasted by std::deque. It may also help diagnose
some puzzling crashes we've seen related to the immediate incomming and work queues.

BUG=674625, 674895
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2579773002
Cr-Commit-Position: refs/heads/master@{#443450}
parent 8c06f31f
......@@ -177,7 +177,7 @@ void TaskQueueImpl::UnregisterTaskQueue() {
any_thread().task_queue_manager = nullptr;
main_thread_only().task_queue_manager = nullptr;
main_thread_only().delayed_incoming_queue = std::priority_queue<Task>();
any_thread().immediate_incoming_queue = std::queue<Task>();
any_thread().immediate_incoming_queue.clear();
main_thread_only().immediate_work_queue.reset();
main_thread_only().delayed_work_queue.reset();
}
......@@ -339,9 +339,11 @@ void TaskQueueImpl::PushOntoImmediateIncomingQueueLocked(
any_thread().task_queue_manager->MaybeScheduleImmediateWork(FROM_HERE);
}
}
any_thread().immediate_incoming_queue.emplace(
posted_from, task, desired_run_time, sequence_number, nestable, sequence_number);
any_thread().task_queue_manager->DidQueueTask( any_thread().immediate_incoming_queue.back());
any_thread().immediate_incoming_queue.emplace_back(
posted_from, task, desired_run_time, sequence_number, nestable,
sequence_number);
any_thread().task_queue_manager->DidQueueTask(
any_thread().immediate_incoming_queue.back());
TraceQueueSize(true);
}
......@@ -691,18 +693,11 @@ EnqueueOrder TaskQueueImpl::GetFenceForTest() const {
}
// static
void TaskQueueImpl::QueueAsValueInto(const std::queue<Task>& queue,
void TaskQueueImpl::QueueAsValueInto(const WTF::Deque<Task>& queue,
base::trace_event::TracedValue* state) {
// Remove const to search |queue| in the destructive manner. Restore the
// content from |visited| later.
std::queue<Task>* mutable_queue = const_cast<std::queue<Task>*>(&queue);
std::queue<Task> visited;
while (!mutable_queue->empty()) {
TaskAsValueInto(mutable_queue->front(), state);
visited.push(std::move(mutable_queue->front()));
mutable_queue->pop();
for (const Task& task : queue) {
TaskAsValueInto(task, state);
}
*mutable_queue = std::move(visited);
}
// static
......
......@@ -18,6 +18,7 @@
#include "platform/scheduler/base/enqueue_order.h"
#include "platform/scheduler/base/intrusive_heap.h"
#include "public/platform/scheduler/base/task_queue.h"
#include "wtf/Deque.h"
namespace blink {
namespace scheduler {
......@@ -230,7 +231,7 @@ class BLINK_PLATFORM_EXPORT TaskQueueImpl final : public TaskQueue {
TaskQueueManager* task_queue_manager;
TimeDomain* time_domain;
std::queue<Task> immediate_incoming_queue;
WTF::Deque<Task> immediate_incoming_queue;
};
struct MainThreadOnly {
......@@ -294,7 +295,7 @@ class BLINK_PLATFORM_EXPORT TaskQueueImpl final : public TaskQueue {
bool BlockedByFenceLocked() const;
void TraceQueueSize(bool is_locked) const;
static void QueueAsValueInto(const std::queue<Task>& queue,
static void QueueAsValueInto(const WTF::Deque<Task>& queue,
base::trace_event::TracedValue* state);
static void QueueAsValueInto(const std::priority_queue<Task>& queue,
base::trace_event::TracedValue* state);
......
......@@ -18,17 +18,9 @@ WorkQueue::WorkQueue(TaskQueueImpl* task_queue, const char* name)
fence_(0) {}
void WorkQueue::AsValueInto(base::trace_event::TracedValue* state) const {
// Remove const to search |work_queue_| in the destructive manner. Restore the
// content from |visited| later.
std::queue<TaskQueueImpl::Task>* mutable_queue =
const_cast<std::queue<TaskQueueImpl::Task>*>(&work_queue_);
std::queue<TaskQueueImpl::Task> visited;
while (!mutable_queue->empty()) {
TaskQueueImpl::TaskAsValueInto(mutable_queue->front(), state);
visited.push(std::move(mutable_queue->front()));
mutable_queue->pop();
for (const TaskQueueImpl::Task& task : work_queue_) {
TaskQueueImpl::TaskAsValueInto(task, state);
}
*mutable_queue = std::move(visited);
}
WorkQueue::~WorkQueue() {
......@@ -77,7 +69,7 @@ void WorkQueue::Push(TaskQueueImpl::Task task) {
#endif
// Amoritized O(1).
work_queue_.push(std::move(task));
work_queue_.push_back(std::move(task));
if (!was_empty)
return;
......@@ -90,12 +82,12 @@ void WorkQueue::Push(TaskQueueImpl::Task task) {
void WorkQueue::PopTaskForTest() {
if (work_queue_.empty())
return;
work_queue_.pop();
work_queue_.pop_front();
}
void WorkQueue::SwapLocked(std::queue<TaskQueueImpl::Task>& incoming_queue) {
void WorkQueue::SwapLocked(WTF::Deque<TaskQueueImpl::Task>& incoming_queue) {
DCHECK(work_queue_.empty());
std::swap(work_queue_, incoming_queue);
work_queue_.swap(incoming_queue);
if (work_queue_.empty())
return;
// If we hit the fence, pretend to WorkQueueSets that we're empty.
......@@ -110,12 +102,10 @@ TaskQueueImpl::Task WorkQueue::TakeTaskFromWorkQueue() {
// Skip over canceled tasks, except for the last one since we always return
// something.
while (work_queue_.size() > 1u && work_queue_.front().task.IsCancelled()) {
work_queue_.pop();
work_queue_.pop_front();
}
TaskQueueImpl::Task pending_task =
std::move(const_cast<TaskQueueImpl::Task&>(work_queue_.front()));
work_queue_.pop();
TaskQueueImpl::Task pending_task = work_queue_.takeFirst();
work_queue_sets_->OnPopQueue(this);
task_queue_->TraceQueueSize(false);
return pending_task;
......
......@@ -66,7 +66,7 @@ class BLINK_PLATFORM_EXPORT WorkQueue {
// Swap the |work_queue_| with |incoming_queue| and if a fence hasn't been
// reached it informs the WorkQueueSets if the head changed. Assumes
// |task_queue_->any_thread_lock_| is locked.
void SwapLocked(std::queue<TaskQueueImpl::Task>& incoming_queue);
void SwapLocked(WTF::Deque<TaskQueueImpl::Task>& incoming_queue);
size_t Size() const { return work_queue_.size(); }
......@@ -113,7 +113,7 @@ class BLINK_PLATFORM_EXPORT WorkQueue {
bool BlockedByFence() const;
private:
std::queue<TaskQueueImpl::Task> work_queue_;
WTF::Deque<TaskQueueImpl::Task> work_queue_;
WorkQueueSets* work_queue_sets_; // NOT OWNED.
TaskQueueImpl* task_queue_; // NOT OWNED.
size_t work_queue_set_index_;
......
......@@ -32,7 +32,7 @@ class WorkQueueTest : public testing::Test {
work_queue_sets_.reset(new WorkQueueSets(1, "test"));
work_queue_sets_->AddQueue(work_queue_.get(), 0);
incoming_queue_.reset(new std::queue<TaskQueueImpl::Task>());
incoming_queue_.reset(new WTF::Deque<TaskQueueImpl::Task>());
}
void TearDown() override { work_queue_sets_->RemoveQueue(work_queue_.get()); }
......@@ -49,7 +49,7 @@ class WorkQueueTest : public testing::Test {
scoped_refptr<TaskQueueImpl> task_queue_;
std::unique_ptr<WorkQueue> work_queue_;
std::unique_ptr<WorkQueueSets> work_queue_sets_;
std::unique_ptr<std::queue<TaskQueueImpl::Task>> incoming_queue_;
std::unique_ptr<WTF::Deque<TaskQueueImpl::Task>> incoming_queue_;
};
TEST_F(WorkQueueTest, Empty) {
......@@ -124,9 +124,9 @@ TEST_F(WorkQueueTest, PushAfterFenceHit) {
}
TEST_F(WorkQueueTest, SwapLocked) {
incoming_queue_->push(FakeTaskWithEnqueueOrder(2));
incoming_queue_->push(FakeTaskWithEnqueueOrder(3));
incoming_queue_->push(FakeTaskWithEnqueueOrder(4));
incoming_queue_->push_back(FakeTaskWithEnqueueOrder(2));
incoming_queue_->push_back(FakeTaskWithEnqueueOrder(3));
incoming_queue_->push_back(FakeTaskWithEnqueueOrder(4));
WorkQueue* work_queue;
EXPECT_FALSE(work_queue_sets_->GetOldestQueueInSet(0, &work_queue));
......@@ -146,9 +146,9 @@ TEST_F(WorkQueueTest, SwapLocked) {
TEST_F(WorkQueueTest, SwapLockedAfterFenceHit) {
work_queue_->InsertFence(1);
incoming_queue_->push(FakeTaskWithEnqueueOrder(2));
incoming_queue_->push(FakeTaskWithEnqueueOrder(3));
incoming_queue_->push(FakeTaskWithEnqueueOrder(4));
incoming_queue_->push_back(FakeTaskWithEnqueueOrder(2));
incoming_queue_->push_back(FakeTaskWithEnqueueOrder(3));
incoming_queue_->push_back(FakeTaskWithEnqueueOrder(4));
WorkQueue* work_queue;
EXPECT_FALSE(work_queue_sets_->GetOldestQueueInSet(0, &work_queue));
......
......@@ -134,6 +134,27 @@ class Deque : public ConditionalDestructor<Deque<T, INLINE_CAPACITY, Allocator>,
void remove(iterator&);
void remove(const_iterator&);
// STL compatibility.
template <typename U>
void push_back(U&& u) {
append(std::forward<U>(u));
}
template <typename U>
void push_front(U&& u) {
prepend(std::forward<U>(u));
}
void pop_back() { removeLast(); }
void pop_front() { removeFirst(); }
bool empty() const { return isEmpty(); }
T& front() { return first(); }
const T& front() const { return first(); }
T& back() { return last(); }
const T& back() const { return last(); }
template <typename... Args>
void emplace_back(Args&&...);
template <typename... Args>
void emplace_front(Args&&...);
void clear();
template <typename VisitorDispatcher>
......@@ -485,11 +506,12 @@ template <typename T, size_t inlineCapacity, typename Allocator>
template <typename U>
inline void Deque<T, inlineCapacity, Allocator>::append(U&& value) {
expandCapacityIfNeeded();
new (NotNull, &m_buffer.buffer()[m_end]) T(std::forward<U>(value));
T* newElement = &m_buffer.buffer()[m_end];
if (m_end == m_buffer.capacity() - 1)
m_end = 0;
else
++m_end;
new (NotNull, newElement) T(std::forward<U>(value));
}
template <typename T, size_t inlineCapacity, typename Allocator>
......@@ -503,6 +525,29 @@ inline void Deque<T, inlineCapacity, Allocator>::prepend(U&& value) {
new (NotNull, &m_buffer.buffer()[m_start]) T(std::forward<U>(value));
}
template <typename T, size_t inlineCapacity, typename Allocator>
template <typename... Args>
inline void Deque<T, inlineCapacity, Allocator>::emplace_back(Args&&... args) {
expandCapacityIfNeeded();
T* newElement = &m_buffer.buffer()[m_end];
if (m_end == m_buffer.capacity() - 1)
m_end = 0;
else
++m_end;
new (NotNull, newElement) T(std::forward<Args>(args)...);
}
template <typename T, size_t inlineCapacity, typename Allocator>
template <typename... Args>
inline void Deque<T, inlineCapacity, Allocator>::emplace_front(Args&&... args) {
expandCapacityIfNeeded();
if (!m_start)
m_start = m_buffer.capacity() - 1;
else
--m_start;
new (NotNull, &m_buffer.buffer()[m_start]) T(std::forward<Args>(args)...);
}
template <typename T, size_t inlineCapacity, typename Allocator>
inline void Deque<T, inlineCapacity, Allocator>::removeFirst() {
DCHECK(!isEmpty());
......
......@@ -603,6 +603,36 @@ TEST(DequeTest, RemoveWhileIterating) {
}
}
struct Item {
Item(int value1, int value2) : value1(value1), value2(value2) {}
int value1;
int value2;
};
TEST(DequeTest, emplace_back) {
Deque<Item> deque;
deque.emplace_back(1, 2);
deque.emplace_back(3, 4);
EXPECT_EQ(2u, deque.size());
EXPECT_EQ(1, deque[0].value1);
EXPECT_EQ(2, deque[0].value2);
EXPECT_EQ(3, deque[1].value1);
EXPECT_EQ(4, deque[1].value2);
}
TEST(DequeTest, emplace_front) {
Deque<Item> deque;
deque.emplace_front(1, 2);
deque.emplace_front(3, 4);
EXPECT_EQ(2u, deque.size());
EXPECT_EQ(3, deque[0].value1);
EXPECT_EQ(4, deque[0].value2);
EXPECT_EQ(1, deque[1].value1);
EXPECT_EQ(2, deque[1].value2);
}
} // anonymous namespace
} // namespace WTF
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