TaskScheduler: Avoid Sequence refcount bump in GetWork()
vmpstr@ discovered an implicit copy constructor resulting
in a refcount bump in https://codereview.chromium.org/1895903003/
and made it explicit in https://codereview.chromium.org/1901223003/
this CL removes the need for it via a couple changes:
1) Make SequenceAndSortKey move-only
- Allows it to be in std::priority_queue without std::unique_ptr's
- Less pointer chasing == yay?!
- Removing need for custom SequenceAndSortKeyComparator.
2) Add a take_sequence() method to it:
- Allows to take its Sequence without refbump on Pop()
3) Make it a private implementation detail and make Transaction's API:
- Push(sequence,sort_key), PeekSortKey(), PopSequence()
4) Do fdoray's TODO on adding IsEmpty() to Transaction's API:
- Removes need for null SequenceAndSortKey (required change to allow for PeekSortKey).
5) Make SequenceSortKey copyable.
- Required for SequenceAndSortKey's move constructor
- Means it turns into a class with operator== for tests instead of custom
test member verifiers.
Might have been able to split these up but they all line up fairly nicely
and don't amount to a crazy amount of code so here it is :-).
BUG=553459
Review-Url: https://codereview.chromium.org/1903133003
Cr-Commit-Position: refs/heads/master@{#390451}
diff --git a/base/task_scheduler/priority_queue.cc b/base/task_scheduler/priority_queue.cc
index 05a90b22..8fec497 100644
--- a/base/task_scheduler/priority_queue.cc
+++ b/base/task_scheduler/priority_queue.cc
@@ -12,15 +12,50 @@
namespace base {
namespace internal {
-PriorityQueue::SequenceAndSortKey::SequenceAndSortKey()
- : sort_key(TaskPriority::LOWEST, TimeTicks()) {}
+// A class combining a Sequence and the SequenceSortKey that determines its
+// position in a PriorityQueue. Instances are only mutable via take_sequence()
+// which can only be called once and renders its instance invalid after the
+// call.
+class PriorityQueue::SequenceAndSortKey {
+ public:
+ SequenceAndSortKey(scoped_refptr<Sequence>&& sequence,
+ const SequenceSortKey& sort_key)
+ : sequence_(sequence), sort_key_(sort_key) {
+ DCHECK(sequence_);
+ }
-PriorityQueue::SequenceAndSortKey::SequenceAndSortKey(
- scoped_refptr<Sequence> sequence,
- const SequenceSortKey& sort_key)
- : sequence(std::move(sequence)), sort_key(sort_key) {}
+ // Note: while |sequence_| should always be non-null post-move (i.e. we
+ // shouldn't be moving an invalid SequenceAndSortKey around), there can't be a
+ // DCHECK(sequence_) on moves as the Windows STL moves elements on pop instead
+ // of overwriting them: resulting in the move of a SequenceAndSortKey with a
+ // null |sequence_| in Transaction::Pop()'s implementation.
+ SequenceAndSortKey(SequenceAndSortKey&& other) = default;
+ SequenceAndSortKey& operator=(SequenceAndSortKey&& other) = default;
-PriorityQueue::SequenceAndSortKey::~SequenceAndSortKey() = default;
+ // Extracts |sequence_| from this object. This object is invalid after this
+ // call.
+ scoped_refptr<Sequence> take_sequence() {
+ DCHECK(sequence_);
+ return std::move(sequence_);
+ }
+
+ // Compares this SequenceAndSortKey to |other| based on their respective
+ // |sort_key_|.
+ bool operator<(const SequenceAndSortKey& other) const {
+ return sort_key_ < other.sort_key_;
+ }
+ bool operator>(const SequenceAndSortKey& other) const {
+ return other < *this;
+ }
+
+ const SequenceSortKey& sort_key() const { return sort_key_; }
+
+ private:
+ scoped_refptr<Sequence> sequence_;
+ SequenceSortKey sort_key_;
+
+ DISALLOW_COPY_AND_ASSIGN(SequenceAndSortKey);
+};
PriorityQueue::Transaction::Transaction(PriorityQueue* outer_queue)
: auto_lock_(outer_queue->container_lock_), outer_queue_(outer_queue) {
@@ -32,29 +67,36 @@
}
void PriorityQueue::Transaction::Push(
- std::unique_ptr<SequenceAndSortKey> sequence_and_sort_key) {
+ scoped_refptr<Sequence> sequence,
+ const SequenceSortKey& sequence_sort_key) {
DCHECK(CalledOnValidThread());
- DCHECK(!sequence_and_sort_key->is_null());
-
- outer_queue_->container_.push(std::move(sequence_and_sort_key));
+ outer_queue_->container_.emplace(std::move(sequence), sequence_sort_key);
}
-const PriorityQueue::SequenceAndSortKey& PriorityQueue::Transaction::Peek()
- const {
+const SequenceSortKey& PriorityQueue::Transaction::PeekSortKey() const {
DCHECK(CalledOnValidThread());
-
- // TODO(fdoray): Add an IsEmpty() method to Transaction and require Peek() to
- // be called on a non-empty PriorityQueue only.
- if (outer_queue_->container_.empty())
- return outer_queue_->empty_sequence_and_sort_key_;
-
- return *outer_queue_->container_.top();
+ DCHECK(!IsEmpty());
+ return outer_queue_->container_.top().sort_key();
}
-void PriorityQueue::Transaction::Pop() {
+scoped_refptr<Sequence> PriorityQueue::Transaction::PopSequence() {
DCHECK(CalledOnValidThread());
- DCHECK(!outer_queue_->container_.empty());
+ DCHECK(!IsEmpty());
+
+ // The const_cast on top() is okay since the SequenceAndSortKey is
+ // transactionally being popped from |container_| right after and taking its
+ // Sequence does not alter its sort order (a requirement for the Windows STL's
+ // consistency debug-checks for std::priority_queue::top()).
+ scoped_refptr<Sequence> sequence =
+ const_cast<PriorityQueue::SequenceAndSortKey&>(
+ outer_queue_->container_.top())
+ .take_sequence();
outer_queue_->container_.pop();
+ return sequence;
+}
+
+bool PriorityQueue::Transaction::IsEmpty() const {
+ return outer_queue_->container_.empty();
}
PriorityQueue::PriorityQueue() = default;
diff --git a/base/task_scheduler/priority_queue.h b/base/task_scheduler/priority_queue.h
index 8e0a532..ac1dbde 100644
--- a/base/task_scheduler/priority_queue.h
+++ b/base/task_scheduler/priority_queue.h
@@ -23,27 +23,6 @@
// A PriorityQueue holds Sequences of Tasks. This class is thread-safe.
class BASE_EXPORT PriorityQueue {
public:
- // An immutable struct combining a Sequence and the sort key that determines
- // its position in a PriorityQueue.
- struct BASE_EXPORT SequenceAndSortKey {
- // Constructs a null SequenceAndSortKey.
- SequenceAndSortKey();
-
- // Constructs a SequenceAndSortKey with the given |sequence| and |sort_key|.
- SequenceAndSortKey(scoped_refptr<Sequence> sequence,
- const SequenceSortKey& sort_key);
- ~SequenceAndSortKey();
-
- // Returns true if this is a null SequenceAndSortKey.
- bool is_null() const { return !sequence; }
-
- const scoped_refptr<Sequence> sequence;
- const SequenceSortKey sort_key;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(SequenceAndSortKey);
- };
-
// A Transaction can perform multiple operations atomically on a
// PriorityQueue. While a Transaction is alive, it is guaranteed that nothing
// else will access the PriorityQueue.
@@ -57,17 +36,25 @@
public:
~Transaction();
- // Inserts |sequence_and_sort_key| in the PriorityQueue.
- void Push(std::unique_ptr<SequenceAndSortKey> sequence_and_sort_key);
+ // Inserts |sequence| in the PriorityQueue with |sequence_sort_key|.
+ // Note: |sequence_sort_key| is required as a parameter instead of being
+ // extracted from |sequence| in Push() to avoid this Transaction having a
+ // lock interdependency with |sequence|.
+ void Push(scoped_refptr<Sequence> sequence,
+ const SequenceSortKey& sequence_sort_key);
- // Returns the SequenceAndSortKey with the highest priority or a null
- // SequenceAndSortKey if the PriorityQueue is empty. The reference becomes
- // invalid the next time that a Sequence is popped from the PriorityQueue.
- const SequenceAndSortKey& Peek() const;
+ // Returns a reference to the SequenceSortKey representing the priority of
+ // the highest pending task in this PriorityQueue. The reference becomes
+ // invalid the next time that this PriorityQueue is modified.
+ // Cannot be called on an empty PriorityQueue.
+ const SequenceSortKey& PeekSortKey() const;
- // Removes the SequenceAndSortKey with the highest priority from the
- // PriorityQueue. Cannot be called on an empty PriorityQueue.
- void Pop();
+ // Removes and returns the highest priority Sequence in this PriorityQueue.
+ // Cannot be called on an empty PriorityQueue.
+ scoped_refptr<Sequence> PopSequence();
+
+ // Returns true if the PriorityQueue is empty.
+ bool IsEmpty() const;
private:
friend class PriorityQueue;
@@ -100,26 +87,17 @@
const SchedulerLock* container_lock() const { return &container_lock_; }
private:
- struct SequenceAndSortKeyComparator {
- bool operator()(const std::unique_ptr<SequenceAndSortKey>& left,
- const std::unique_ptr<SequenceAndSortKey>& right) const {
- return left->sort_key < right->sort_key;
- }
- };
- using ContainerType =
- std::priority_queue<std::unique_ptr<SequenceAndSortKey>,
- std::vector<std::unique_ptr<SequenceAndSortKey>>,
- SequenceAndSortKeyComparator>;
+ // A class combining a Sequence and the SequenceSortKey that determines its
+ // position in a PriorityQueue.
+ class SequenceAndSortKey;
+
+ using ContainerType = std::priority_queue<SequenceAndSortKey>;
// Synchronizes access to |container_|.
SchedulerLock container_lock_;
ContainerType container_;
- // A null SequenceAndSortKey returned by Peek() when the PriorityQueue is
- // empty.
- const SequenceAndSortKey empty_sequence_and_sort_key_;
-
DISALLOW_COPY_AND_ASSIGN(PriorityQueue);
};
diff --git a/base/task_scheduler/priority_queue_unittest.cc b/base/task_scheduler/priority_queue_unittest.cc
index 84195cd..cf8d099 100644
--- a/base/task_scheduler/priority_queue_unittest.cc
+++ b/base/task_scheduler/priority_queue_unittest.cc
@@ -52,21 +52,6 @@
DISALLOW_COPY_AND_ASSIGN(ThreadBeginningTransaction);
};
-void ExpectSequenceAndSortKeyEq(
- const PriorityQueue::SequenceAndSortKey& expected,
- const PriorityQueue::SequenceAndSortKey& actual) {
- EXPECT_EQ(expected.sequence, actual.sequence);
- EXPECT_EQ(expected.sort_key.priority, actual.sort_key.priority);
- EXPECT_EQ(expected.sort_key.next_task_sequenced_time,
- actual.sort_key.next_task_sequenced_time);
-}
-
-#define EXPECT_SEQUENCE_AND_SORT_KEY_EQ(expected, actual) \
- do { \
- SCOPED_TRACE(""); \
- ExpectSequenceAndSortKeyEq(expected, actual); \
- } while (false)
-
} // namespace
TEST(TaskSchedulerPriorityQueueTest, PushPopPeek) {
@@ -98,66 +83,46 @@
// Create a PriorityQueue and a Transaction.
PriorityQueue pq;
auto transaction(pq.BeginTransaction());
- EXPECT_SEQUENCE_AND_SORT_KEY_EQ(PriorityQueue::SequenceAndSortKey(),
- transaction->Peek());
+ EXPECT_TRUE(transaction->IsEmpty());
// Push |sequence_a| in the PriorityQueue. It becomes the sequence with the
// highest priority.
- transaction->Push(WrapUnique(
- new PriorityQueue::SequenceAndSortKey(sequence_a, sort_key_a)));
- EXPECT_SEQUENCE_AND_SORT_KEY_EQ(
- PriorityQueue::SequenceAndSortKey(sequence_a, sort_key_a),
- transaction->Peek());
+ transaction->Push(sequence_a, sort_key_a);
+ EXPECT_EQ(sort_key_a, transaction->PeekSortKey());
// Push |sequence_b| in the PriorityQueue. It becomes the sequence with the
// highest priority.
- transaction->Push(WrapUnique(
- new PriorityQueue::SequenceAndSortKey(sequence_b, sort_key_b)));
- EXPECT_SEQUENCE_AND_SORT_KEY_EQ(
- PriorityQueue::SequenceAndSortKey(sequence_b, sort_key_b),
- transaction->Peek());
+ transaction->Push(sequence_b, sort_key_b);
+ EXPECT_EQ(sort_key_b, transaction->PeekSortKey());
// Push |sequence_c| in the PriorityQueue. |sequence_b| is still the sequence
// with the highest priority.
- transaction->Push(WrapUnique(
- new PriorityQueue::SequenceAndSortKey(sequence_c, sort_key_c)));
- EXPECT_SEQUENCE_AND_SORT_KEY_EQ(
- PriorityQueue::SequenceAndSortKey(sequence_b, sort_key_b),
- transaction->Peek());
+ transaction->Push(sequence_c, sort_key_c);
+ EXPECT_EQ(sort_key_b, transaction->PeekSortKey());
// Push |sequence_d| in the PriorityQueue. |sequence_b| is still the sequence
// with the highest priority.
- transaction->Push(WrapUnique(
- new PriorityQueue::SequenceAndSortKey(sequence_d, sort_key_d)));
- EXPECT_SEQUENCE_AND_SORT_KEY_EQ(
- PriorityQueue::SequenceAndSortKey(sequence_b, sort_key_b),
- transaction->Peek());
+ transaction->Push(sequence_d, sort_key_d);
+ EXPECT_EQ(sort_key_b, transaction->PeekSortKey());
// Pop |sequence_b| from the PriorityQueue. |sequence_c| becomes the sequence
// with the highest priority.
- transaction->Pop();
- EXPECT_SEQUENCE_AND_SORT_KEY_EQ(
- PriorityQueue::SequenceAndSortKey(sequence_c, sort_key_c),
- transaction->Peek());
+ EXPECT_EQ(sequence_b, transaction->PopSequence());
+ EXPECT_EQ(sort_key_c, transaction->PeekSortKey());
// Pop |sequence_c| from the PriorityQueue. |sequence_a| becomes the sequence
// with the highest priority.
- transaction->Pop();
- EXPECT_SEQUENCE_AND_SORT_KEY_EQ(
- PriorityQueue::SequenceAndSortKey(sequence_a, sort_key_a),
- transaction->Peek());
+ EXPECT_EQ(sequence_c, transaction->PopSequence());
+ EXPECT_EQ(sort_key_a, transaction->PeekSortKey());
// Pop |sequence_a| from the PriorityQueue. |sequence_d| becomes the sequence
// with the highest priority.
- transaction->Pop();
- EXPECT_SEQUENCE_AND_SORT_KEY_EQ(
- PriorityQueue::SequenceAndSortKey(sequence_d, sort_key_d),
- transaction->Peek());
+ EXPECT_EQ(sequence_a, transaction->PopSequence());
+ EXPECT_EQ(sort_key_d, transaction->PeekSortKey());
// Pop |sequence_d| from the PriorityQueue. It is now empty.
- transaction->Pop();
- EXPECT_SEQUENCE_AND_SORT_KEY_EQ(PriorityQueue::SequenceAndSortKey(),
- transaction->Peek());
+ EXPECT_EQ(sequence_d, transaction->PopSequence());
+ EXPECT_TRUE(transaction->IsEmpty());
}
// Check that creating Transactions on the same thread for 2 unrelated
@@ -215,14 +180,5 @@
thread_beginning_transaction.Join();
}
-TEST(TaskSchedulerPriorityQueueTest, SequenceAndSortKeyIsNull) {
- EXPECT_TRUE(PriorityQueue::SequenceAndSortKey().is_null());
-
- const PriorityQueue::SequenceAndSortKey non_null_sequence_and_sort_key(
- make_scoped_refptr(new Sequence),
- SequenceSortKey(TaskPriority::USER_VISIBLE, TimeTicks()));
- EXPECT_FALSE(non_null_sequence_and_sort_key.is_null());
-}
-
} // namespace internal
} // namespace base
diff --git a/base/task_scheduler/scheduler_thread_pool.h b/base/task_scheduler/scheduler_thread_pool.h
index 9793162..928ed3e 100644
--- a/base/task_scheduler/scheduler_thread_pool.h
+++ b/base/task_scheduler/scheduler_thread_pool.h
@@ -18,7 +18,7 @@
namespace internal {
class SchedulerWorkerThread;
-struct SequenceSortKey;
+class SequenceSortKey;
// Interface for a thread pool.
class BASE_EXPORT SchedulerThreadPool {
diff --git a/base/task_scheduler/scheduler_thread_pool_impl.cc b/base/task_scheduler/scheduler_thread_pool_impl.cc
index b64405d..1fc854b9 100644
--- a/base/task_scheduler/scheduler_thread_pool_impl.cc
+++ b/base/task_scheduler/scheduler_thread_pool_impl.cc
@@ -276,9 +276,8 @@
void SchedulerThreadPoolImpl::ReEnqueueSequence(
scoped_refptr<Sequence> sequence,
const SequenceSortKey& sequence_sort_key) {
- shared_priority_queue_.BeginTransaction()->Push(
- WrapUnique(new PriorityQueue::SequenceAndSortKey(std::move(sequence),
- sequence_sort_key)));
+ shared_priority_queue_.BeginTransaction()->Push(std::move(sequence),
+ sequence_sort_key);
// The thread calling this method just ran a Task from |sequence| and will
// soon try to get another Sequence from which to run a Task. If the thread
@@ -347,9 +346,8 @@
// - A worker thread is running a Task from |sequence|. It will insert
// |sequence| in a PriorityQueue once it's done running the Task.
const auto sequence_sort_key = sequence->GetSortKey();
- priority_queue->BeginTransaction()->Push(
- WrapUnique(new PriorityQueue::SequenceAndSortKey(std::move(sequence),
- sequence_sort_key)));
+ priority_queue->BeginTransaction()->Push(std::move(sequence),
+ sequence_sort_key);
// Wake up a worker thread to process |sequence|.
if (worker_thread)
@@ -395,15 +393,11 @@
{
std::unique_ptr<PriorityQueue::Transaction> shared_transaction(
outer_->shared_priority_queue_.BeginTransaction());
- const auto& shared_sequence_and_sort_key = shared_transaction->Peek();
-
std::unique_ptr<PriorityQueue::Transaction> single_threaded_transaction(
single_threaded_priority_queue_.BeginTransaction());
- const auto& single_threaded_sequence_and_sort_key =
- single_threaded_transaction->Peek();
- if (shared_sequence_and_sort_key.is_null() &&
- single_threaded_sequence_and_sort_key.is_null()) {
+ if (shared_transaction->IsEmpty() &&
+ single_threaded_transaction->IsEmpty()) {
single_threaded_transaction.reset();
// |shared_transaction| is kept alive while |worker_thread| is added to
@@ -425,20 +419,18 @@
// True if both PriorityQueues have Sequences and the Sequence at the top of
// the shared PriorityQueue is more important.
const bool shared_sequence_is_more_important =
- !shared_sequence_and_sort_key.is_null() &&
- !single_threaded_sequence_and_sort_key.is_null() &&
- shared_sequence_and_sort_key.sort_key >
- single_threaded_sequence_and_sort_key.sort_key;
+ !shared_transaction->IsEmpty() &&
+ !single_threaded_transaction->IsEmpty() &&
+ shared_transaction->PeekSortKey() >
+ single_threaded_transaction->PeekSortKey();
- if (single_threaded_sequence_and_sort_key.is_null() ||
+ if (single_threaded_transaction->IsEmpty() ||
shared_sequence_is_more_important) {
- sequence = shared_sequence_and_sort_key.sequence;
- shared_transaction->Pop();
+ sequence = shared_transaction->PopSequence();
last_sequence_is_single_threaded_ = false;
} else {
- DCHECK(!single_threaded_sequence_and_sort_key.is_null());
- sequence = single_threaded_sequence_and_sort_key.sequence;
- single_threaded_transaction->Pop();
+ DCHECK(!single_threaded_transaction->IsEmpty());
+ sequence = single_threaded_transaction->PopSequence();
last_sequence_is_single_threaded_ = true;
}
}
@@ -455,8 +447,7 @@
// PriorityQueue from which it was extracted.
const SequenceSortKey sequence_sort_key = sequence->GetSortKey();
single_threaded_priority_queue_.BeginTransaction()->Push(
- WrapUnique(new PriorityQueue::SequenceAndSortKey(std::move(sequence),
- sequence_sort_key)));
+ std::move(sequence), sequence_sort_key);
} else {
// |re_enqueue_sequence_callback_| will determine in which PriorityQueue
// |sequence| must be enqueued.
diff --git a/base/task_scheduler/scheduler_thread_pool_impl.h b/base/task_scheduler/scheduler_thread_pool_impl.h
index 13fba7e5..bb77967 100644
--- a/base/task_scheduler/scheduler_thread_pool_impl.h
+++ b/base/task_scheduler/scheduler_thread_pool_impl.h
@@ -31,7 +31,6 @@
namespace internal {
class DelayedTaskManager;
-struct SequenceSortKey;
class TaskTracker;
// A pool of threads that run Tasks. This class is thread-safe.
diff --git a/base/task_scheduler/sequence_sort_key.cc b/base/task_scheduler/sequence_sort_key.cc
index 758a411..e356c8b 100644
--- a/base/task_scheduler/sequence_sort_key.cc
+++ b/base/task_scheduler/sequence_sort_key.cc
@@ -9,19 +9,20 @@
SequenceSortKey::SequenceSortKey(TaskPriority priority,
TimeTicks next_task_sequenced_time)
- : priority(priority), next_task_sequenced_time(next_task_sequenced_time) {}
+ : priority_(priority),
+ next_task_sequenced_time_(next_task_sequenced_time) {}
bool SequenceSortKey::operator<(const SequenceSortKey& other) const {
// This SequenceSortKey is considered less important than |other| if it has a
// lower priority or if it has the same priority but its next task was posted
// later than |other|'s.
const int priority_diff =
- static_cast<int>(priority) - static_cast<int>(other.priority);
+ static_cast<int>(priority_) - static_cast<int>(other.priority_);
if (priority_diff < 0)
return true;
if (priority_diff > 0)
return false;
- return next_task_sequenced_time > other.next_task_sequenced_time;
+ return next_task_sequenced_time_ > other.next_task_sequenced_time_;
}
} // namespace internal
diff --git a/base/task_scheduler/sequence_sort_key.h b/base/task_scheduler/sequence_sort_key.h
index f2dd561c..0a03ed1 100644
--- a/base/task_scheduler/sequence_sort_key.h
+++ b/base/task_scheduler/sequence_sort_key.h
@@ -12,20 +12,33 @@
namespace base {
namespace internal {
-// An immutable representation of the priority of a Sequence.
-struct BASE_EXPORT SequenceSortKey final {
+// An immutable but assignable representation of the priority of a Sequence.
+class BASE_EXPORT SequenceSortKey final {
+ public:
SequenceSortKey(TaskPriority priority, TimeTicks next_task_sequenced_time);
bool operator<(const SequenceSortKey& other) const;
bool operator>(const SequenceSortKey& other) const { return other < *this; }
+ bool operator==(const SequenceSortKey& other) const {
+ return priority_ == other.priority_ &&
+ next_task_sequenced_time_ == other.next_task_sequenced_time_;
+ }
+ bool operator!=(const SequenceSortKey& other) const {
+ return !(other == *this);
+ };
+
+ private:
+ // The private section allows this class to keep its immutable property while
+ // being copy-assignable (i.e. instead of making its members const).
+
// Highest task priority in the sequence at the time this sort key was
// created.
- const TaskPriority priority;
+ TaskPriority priority_;
// Sequenced time of the next task to run in the sequence at the time this
// sort key was created.
- const TimeTicks next_task_sequenced_time;
+ TimeTicks next_task_sequenced_time_;
};
} // namespace internal
diff --git a/base/task_scheduler/sequence_sort_key_unittest.cc b/base/task_scheduler/sequence_sort_key_unittest.cc
index 5c6c917..2c1d80d 100644
--- a/base/task_scheduler/sequence_sort_key_unittest.cc
+++ b/base/task_scheduler/sequence_sort_key_unittest.cc
@@ -125,5 +125,119 @@
EXPECT_FALSE(key_f > key_f);
}
+TEST(TaskSchedulerSequenceSortKeyTest, OperatorEqual) {
+ SequenceSortKey key_a(TaskPriority::USER_BLOCKING,
+ TimeTicks::FromInternalValue(1000));
+ SequenceSortKey key_b(TaskPriority::USER_BLOCKING,
+ TimeTicks::FromInternalValue(2000));
+ SequenceSortKey key_c(TaskPriority::USER_VISIBLE,
+ TimeTicks::FromInternalValue(1000));
+ SequenceSortKey key_d(TaskPriority::USER_VISIBLE,
+ TimeTicks::FromInternalValue(2000));
+ SequenceSortKey key_e(TaskPriority::BACKGROUND,
+ TimeTicks::FromInternalValue(1000));
+ SequenceSortKey key_f(TaskPriority::BACKGROUND,
+ TimeTicks::FromInternalValue(2000));
+
+ EXPECT_EQ(key_a, key_a);
+ EXPECT_FALSE(key_b == key_a);
+ EXPECT_FALSE(key_c == key_a);
+ EXPECT_FALSE(key_d == key_a);
+ EXPECT_FALSE(key_e == key_a);
+ EXPECT_FALSE(key_f == key_a);
+
+ EXPECT_FALSE(key_a == key_b);
+ EXPECT_EQ(key_b, key_b);
+ EXPECT_FALSE(key_c == key_b);
+ EXPECT_FALSE(key_d == key_b);
+ EXPECT_FALSE(key_e == key_b);
+ EXPECT_FALSE(key_f == key_b);
+
+ EXPECT_FALSE(key_a == key_c);
+ EXPECT_FALSE(key_b == key_c);
+ EXPECT_EQ(key_c, key_c);
+ EXPECT_FALSE(key_d == key_c);
+ EXPECT_FALSE(key_e == key_c);
+ EXPECT_FALSE(key_f == key_c);
+
+ EXPECT_FALSE(key_a == key_d);
+ EXPECT_FALSE(key_b == key_d);
+ EXPECT_FALSE(key_c == key_d);
+ EXPECT_EQ(key_d, key_d);
+ EXPECT_FALSE(key_e == key_d);
+ EXPECT_FALSE(key_f == key_d);
+
+ EXPECT_FALSE(key_a == key_e);
+ EXPECT_FALSE(key_b == key_e);
+ EXPECT_FALSE(key_c == key_e);
+ EXPECT_FALSE(key_d == key_e);
+ EXPECT_EQ(key_e, key_e);
+ EXPECT_FALSE(key_f == key_e);
+
+ EXPECT_FALSE(key_a == key_f);
+ EXPECT_FALSE(key_b == key_f);
+ EXPECT_FALSE(key_c == key_f);
+ EXPECT_FALSE(key_d == key_f);
+ EXPECT_FALSE(key_e == key_f);
+ EXPECT_EQ(key_f, key_f);
+}
+
+TEST(TaskSchedulerSequenceSortKeyTest, OperatorNotEqual) {
+ SequenceSortKey key_a(TaskPriority::USER_BLOCKING,
+ TimeTicks::FromInternalValue(1000));
+ SequenceSortKey key_b(TaskPriority::USER_BLOCKING,
+ TimeTicks::FromInternalValue(2000));
+ SequenceSortKey key_c(TaskPriority::USER_VISIBLE,
+ TimeTicks::FromInternalValue(1000));
+ SequenceSortKey key_d(TaskPriority::USER_VISIBLE,
+ TimeTicks::FromInternalValue(2000));
+ SequenceSortKey key_e(TaskPriority::BACKGROUND,
+ TimeTicks::FromInternalValue(1000));
+ SequenceSortKey key_f(TaskPriority::BACKGROUND,
+ TimeTicks::FromInternalValue(2000));
+
+ EXPECT_FALSE(key_a != key_a);
+ EXPECT_NE(key_b, key_a);
+ EXPECT_NE(key_c, key_a);
+ EXPECT_NE(key_d, key_a);
+ EXPECT_NE(key_e, key_a);
+ EXPECT_NE(key_f, key_a);
+
+ EXPECT_NE(key_a, key_b);
+ EXPECT_FALSE(key_b != key_b);
+ EXPECT_NE(key_c, key_b);
+ EXPECT_NE(key_d, key_b);
+ EXPECT_NE(key_e, key_b);
+ EXPECT_NE(key_f, key_b);
+
+ EXPECT_NE(key_a, key_c);
+ EXPECT_NE(key_b, key_c);
+ EXPECT_FALSE(key_c != key_c);
+ EXPECT_NE(key_d, key_c);
+ EXPECT_NE(key_e, key_c);
+ EXPECT_NE(key_f, key_c);
+
+ EXPECT_NE(key_a, key_d);
+ EXPECT_NE(key_b, key_d);
+ EXPECT_NE(key_c, key_d);
+ EXPECT_FALSE(key_d != key_d);
+ EXPECT_NE(key_e, key_d);
+ EXPECT_NE(key_f, key_d);
+
+ EXPECT_NE(key_a, key_e);
+ EXPECT_NE(key_b, key_e);
+ EXPECT_NE(key_c, key_e);
+ EXPECT_NE(key_d, key_e);
+ EXPECT_FALSE(key_e != key_e);
+ EXPECT_NE(key_f, key_e);
+
+ EXPECT_NE(key_a, key_f);
+ EXPECT_NE(key_b, key_f);
+ EXPECT_NE(key_c, key_f);
+ EXPECT_NE(key_d, key_f);
+ EXPECT_NE(key_e, key_f);
+ EXPECT_FALSE(key_f != key_f);
+}
+
} // namespace internal
} // namespace base
diff --git a/base/task_scheduler/sequence_unittest.cc b/base/task_scheduler/sequence_unittest.cc
index 25baea4..6a15299 100644
--- a/base/task_scheduler/sequence_unittest.cc
+++ b/base/task_scheduler/sequence_unittest.cc
@@ -68,13 +68,6 @@
DISALLOW_COPY_AND_ASSIGN(TaskSchedulerSequenceTest);
};
-void ExpectSortKey(TaskPriority expected_priority,
- TimeTicks expected_sequenced_time,
- const SequenceSortKey& actual_sort_key) {
- EXPECT_EQ(expected_priority, actual_sort_key.priority);
- EXPECT_EQ(expected_sequenced_time, actual_sort_key.next_task_sequenced_time);
-}
-
} // namespace
TEST_F(TaskSchedulerSequenceTest, PushPopPeek) {
@@ -133,56 +126,63 @@
// Push task A in the sequence. The highest priority is from task A
// (BACKGROUND). Task A is in front of the sequence.
sequence->PushTask(std::move(task_a_owned_));
- ExpectSortKey(TaskPriority::BACKGROUND, task_a_->sequenced_time,
- sequence->GetSortKey());
+ EXPECT_EQ(SequenceSortKey(TaskPriority::BACKGROUND, task_a_->sequenced_time),
+ sequence->GetSortKey());
// Push task B in the sequence. The highest priority is from task B
// (USER_VISIBLE). Task A is still in front of the sequence.
sequence->PushTask(std::move(task_b_owned_));
- ExpectSortKey(TaskPriority::USER_VISIBLE, task_a_->sequenced_time,
- sequence->GetSortKey());
+ EXPECT_EQ(
+ SequenceSortKey(TaskPriority::USER_VISIBLE, task_a_->sequenced_time),
+ sequence->GetSortKey());
// Push task C in the sequence. The highest priority is from task C
// (USER_BLOCKING). Task A is still in front of the sequence.
sequence->PushTask(std::move(task_c_owned_));
- ExpectSortKey(TaskPriority::USER_BLOCKING, task_a_->sequenced_time,
- sequence->GetSortKey());
+ EXPECT_EQ(
+ SequenceSortKey(TaskPriority::USER_BLOCKING, task_a_->sequenced_time),
+ sequence->GetSortKey());
// Push task D in the sequence. The highest priority is from tasks C/D
// (USER_BLOCKING). Task A is still in front of the sequence.
sequence->PushTask(std::move(task_d_owned_));
- ExpectSortKey(TaskPriority::USER_BLOCKING, task_a_->sequenced_time,
- sequence->GetSortKey());
+ EXPECT_EQ(
+ SequenceSortKey(TaskPriority::USER_BLOCKING, task_a_->sequenced_time),
+ sequence->GetSortKey());
// Pop task A. The highest priority is still USER_BLOCKING. The task in front
// of the sequence is now task B.
sequence->PopTask();
- ExpectSortKey(TaskPriority::USER_BLOCKING, task_b_->sequenced_time,
- sequence->GetSortKey());
+ EXPECT_EQ(
+ SequenceSortKey(TaskPriority::USER_BLOCKING, task_b_->sequenced_time),
+ sequence->GetSortKey());
// Pop task B. The highest priority is still USER_BLOCKING. The task in front
// of the sequence is now task C.
sequence->PopTask();
- ExpectSortKey(TaskPriority::USER_BLOCKING, task_c_->sequenced_time,
- sequence->GetSortKey());
+ EXPECT_EQ(
+ SequenceSortKey(TaskPriority::USER_BLOCKING, task_c_->sequenced_time),
+ sequence->GetSortKey());
// Pop task C. The highest priority is still USER_BLOCKING. The task in front
// of the sequence is now task D.
sequence->PopTask();
- ExpectSortKey(TaskPriority::USER_BLOCKING, task_d_->sequenced_time,
- sequence->GetSortKey());
+ EXPECT_EQ(
+ SequenceSortKey(TaskPriority::USER_BLOCKING, task_d_->sequenced_time),
+ sequence->GetSortKey());
// Push task E in the sequence. The highest priority is still USER_BLOCKING.
// The task in front of the sequence is still task D.
sequence->PushTask(std::move(task_e_owned_));
- ExpectSortKey(TaskPriority::USER_BLOCKING, task_d_->sequenced_time,
- sequence->GetSortKey());
+ EXPECT_EQ(
+ SequenceSortKey(TaskPriority::USER_BLOCKING, task_d_->sequenced_time),
+ sequence->GetSortKey());
// Pop task D. The highest priority is now from task E (BACKGROUND). The
// task in front of the sequence is now task E.
sequence->PopTask();
- ExpectSortKey(TaskPriority::BACKGROUND, task_e_->sequenced_time,
- sequence->GetSortKey());
+ EXPECT_EQ(SequenceSortKey(TaskPriority::BACKGROUND, task_e_->sequenced_time),
+ sequence->GetSortKey());
}
} // namespace internal
