TaskScheduler [3/9] Task and Sequence

base/task_scheduler/sequence_sort_key.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_TASK_SCHEDULER_SEQUENCE_SORT_KEY_H_
+#define BASE_TASK_SCHEDULER_SEQUENCE_SORT_KEY_H_
+
+#include "base/base_export.h"
+#include "base/task_scheduler/task_traits.h"
+#include "base/time/time.h"
+
+namespace base {
+namespace internal {
+
+// A SequenceSortKey determines the position of a Sequence in a PriorityQueue.
+// Contrary to a Sequence, which can be modified when it is inside a

[robliao, 2016/02/19 02:33:43]

// An immutable representation of the priority of a Sequence.

[fdoray, 2016/02/19 14:12:14]

Done.

+// PriorityQueue, a SequenceSortKey is immutable.
+struct BASE_EXPORT SequenceSortKey {

[robliao, 2016/02/19 02:33:43]

Nit: Add final.

[fdoray, 2016/02/19 14:12:14]

Done.

+ public:
+  SequenceSortKey(TaskPriority priority, TimeTicks next_task_sequenced_time);
+
+  bool operator<(const SequenceSortKey& other) const;

[robliao, 2016/02/19 02:33:43]

Do we need to support operator reflexivity?
a < b is the same as b > a

[fdoray, 2016/02/19 14:12:14]

We need operator< to sort sequences in PriorityQueue. I didn't implement
operator> because we don't need it. Is it OK?

[gab, 2016/02/19 16:50:47]

Good catch Rob.

The style @
https://google.github.io/styleguide/cppguide.html#Operator_Overloading says "If
you define an operator, also define any related operators that make sense, and
make sure they are defined consistently. For example, if you overload <,
overload all the comparison operators, and make sure < and > never return true
for the same arguments."

An alternative approach would be to give a custom comparator (e.g. a Compare()
method defined on this class) to priority_queue and not even worry about
operator overloading?
I think I prefer that alternate option, WDYT?

[gab, 2016/02/19 16:52:56]

Note: it may also make sense to remove comparison altogether in this CL and only
bring it in the PQ CL when there is context for it? Up to you.

[fdoray, 2016/02/19 22:28:29]

As we discussed, the comparison is required outside of the PQ: we need it to
compare the sequences extracted from a single-threaded and a shared PQ.

+  bool operator==(const SequenceSortKey& other) const;
+
+  // Highest task priority in the sequence at the time this sort key was
+  // created.
+  const TaskPriority priority_;

[robliao, 2016/02/19 02:33:43]

Will this immutability cause problems in STL containers? (e.g., we can never
have a vector of SequenceSortKeys).

[fdoray, 2016/02/19 14:12:14]

You're right, we won't be able to create STL containers of SequenceSortKeys.
PriorityQueue will have an std::priority_queue<SequenceAndSortKeyPair*> instead
of an std::priority_queue<SequenceAndSortKeyPair>. See gab's comment here
https://codereview.chromium.org/1709713002/diff/1/base/task_scheduler/priorit...

[robliao, 2016/02/22 21:34:12]

Acknowledged.

+
+  // 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_;
+};
+
+}  // namespace internal
+}  // namespace base
+
+#endif  // BASE_TASK_SCHEDULER_SEQUENCE_SORT_KEY_H_