base/task_scheduler/delayed_task_manager.cc - Issue 2405243003: TaskScheduler: Replace the SchedulerServiceThread with a base::Thread.

Side by Side Diff: base/task_scheduler/delayed_task_manager.cc

Issue 2405243003: TaskScheduler: Replace the SchedulerServiceThread with a base::Thread. (Closed)

Patch Set: self-review Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« no previous file with comments | « base/task_scheduler/delayed_task_manager.h ('k') | base/task_scheduler/delayed_task_manager_unittest.cc » ('j') | base/task_scheduler/scheduler_worker_pool_impl.cc » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
	(Empty)
1 // Copyright 2016 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.

4

5 #include "base/task_scheduler/delayed_task_manager.h"

6

7 #include <utility>

8

9 #include "base/logging.h"

10 #include "base/task_scheduler/scheduler_worker_pool.h"

11

12 namespace base {

13 namespace internal {

14

15 struct DelayedTaskManager::DelayedTask {

16 DelayedTask(std::unique_ptr<Task> task,

17 scoped_refptr<Sequence> sequence,

18 SchedulerWorker* worker,

19 SchedulerWorkerPool* worker_pool,

20 uint64_t index)

21 : task(std::move(task)),

22 sequence(std::move(sequence)),

23 worker(worker),

24 worker_pool(worker_pool),

25 index(index) {}

26

27 DelayedTask(DelayedTask&& other) = default;

28

29 ~DelayedTask() = default;

30

31 DelayedTask& operator=(DelayedTask&& other) = default;

32

33 // \|task\| will be posted to \|worker_pool\| with \|sequence\| and \|worker\|

34 // when it becomes ripe for execution.

35 std::unique_ptr<Task> task;

36 scoped_refptr<Sequence> sequence;

37 SchedulerWorker* worker;

38 SchedulerWorkerPool* worker_pool;

39

40 // Ensures that tasks that have the same \|delayed_run_time\| are sorted

41 // according to the order in which they were added to the DelayedTaskManager.

42 uint64_t index;

43

44 private:

45 DISALLOW_COPY_AND_ASSIGN(DelayedTask);

46 };

47

48 DelayedTaskManager::DelayedTaskManager(

49 const Closure& on_delayed_run_time_updated)

50 : on_delayed_run_time_updated_(on_delayed_run_time_updated) {

51 DCHECK(!on_delayed_run_time_updated_.is_null());

52 }

53

54 DelayedTaskManager::~DelayedTaskManager() = default;

55

56 void DelayedTaskManager::AddDelayedTask(std::unique_ptr<Task> task,
robliao 2016/10/12 20:55:45 An alternative implementation would be to route th An alternative implementation would be to route the service thread here and keep the Delayed Task Manager. That way the SchedulerWorkerPools only have to know about their own threads. This has the benefit of keeping all of the policy about delayed tasks here (and makes it an easy place to calculate metrics over the delayed tasks. Thoughts? fdoray 2016/10/13 13:51:29 Instead of passing a TaskRunner to SchedulerWorker Show quoted text On 2016/10/12 20:55:45, robliao wrote: > An alternative implementation would be to route the service thread here and keep > the Delayed Task Manager. That way the SchedulerWorkerPools only have to know > about their own threads. > > This has the benefit of keeping all of the policy about delayed tasks here (and > makes it an easy place to calculate metrics over the delayed tasks. > > Thoughts? Instead of passing a TaskRunner to SchedulerWorkerPools, we could pass an opaque Callback that posts a delayed task to the service thread TaskRunner? But SchedulerWorkerPools will have to know about the service thread TaskRunner anyways to support FileDescriptorWatcher::WatchReadable/Writable [1]. I'm hesitant to keep DelayedTaskManager (153 + 88 LOC, extra lock, extra priority queue) just to have all the policy about delayed tasks in one place. I think we should delete it for now and restore it later if necessary (e.g. if we introduce a new metric or a new policy about delayed tasks). [1] FileDescriptorWatcher relies on a MessageLoopForIO pointer stored in TLS (I intend to replace this with a TaskRunner pointer). robliao 2016/10/13 17:03:39 We wouldn't keep around everything in the DelayedT Show quoted text On 2016/10/13 13:51:29, fdoray wrote: > On 2016/10/12 20:55:45, robliao wrote: > > An alternative implementation would be to route the service thread here and > keep > > the Delayed Task Manager. That way the SchedulerWorkerPools only have to know > > about their own threads. > > > > This has the benefit of keeping all of the policy about delayed tasks here > (and > > makes it an easy place to calculate metrics over the delayed tasks. > > > > Thoughts? > > Instead of passing a TaskRunner to SchedulerWorkerPools, we could pass an opaque > Callback that posts a delayed task to the service thread TaskRunner? > > But SchedulerWorkerPools will have to know about the service thread TaskRunner > anyways to support FileDescriptorWatcher::WatchReadable/Writable [1]. > > I'm hesitant to keep DelayedTaskManager (153 + 88 LOC, extra lock, extra > priority queue) just to have all the policy about delayed tasks in one place. I > think we should delete it for now and restore it later if necessary (e.g. if we > introduce a new metric or a new policy about delayed tasks). > > [1] FileDescriptorWatcher relies on a MessageLoopForIO pointer stored in TLS (I > intend to replace this with a TaskRunner pointer). We wouldn't keep around everything in the DelayedTaskManager. It would simply forward tasks to the service thread task runner. The locks and queue goes away. We would simply use the abstraction we have a little longer until we determine it's no longer necessary. I suspect that its probably we would like to gather data about the sorts of delayed tasks going through the system, so we would need a central gathering point somewhere.
57 scoped_refptr<Sequence> sequence,

58 SchedulerWorker* worker,

59 SchedulerWorkerPool* worker_pool) {

60 DCHECK(task);

61 DCHECK(sequence);

62 DCHECK(worker_pool);

63

64 const TimeTicks new_task_delayed_run_time = task->delayed_run_time;

65 TimeTicks current_delayed_run_time;

66

67 {

68 AutoSchedulerLock auto_lock(lock_);

69

70 if (!delayed_tasks_.empty())

71 current_delayed_run_time = delayed_tasks_.top().task->delayed_run_time;

72

73 delayed_tasks_.emplace(std::move(task), std::move(sequence), worker,

74 worker_pool, ++delayed_task_index_);

75 }

76

77 if (current_delayed_run_time.is_null() \|\|

78 new_task_delayed_run_time < current_delayed_run_time) {

79 on_delayed_run_time_updated_.Run();

80 }

81 }

82

83 void DelayedTaskManager::PostReadyTasks() {

84 const TimeTicks now = Now();

85

86 // Move delayed tasks that are ready for execution into \|ready_tasks\|. Don't

87 // post them right away to avoid imposing an unecessary lock dependency on

88 // PostTaskNowHelper.

89 std::vector<DelayedTask> ready_tasks;

90

91 {

92 AutoSchedulerLock auto_lock(lock_);

93 while (!delayed_tasks_.empty() &&

94 delayed_tasks_.top().task->delayed_run_time <= now) {

95 // The const_cast for std::move is okay since we're immediately popping

96 // the task from \|delayed_tasks_\|. See DelayedTaskComparator::operator()

97 // for minor debug-check implications.

98 ready_tasks.push_back(

99 std::move(const_cast<DelayedTask&>(delayed_tasks_.top())));

100 delayed_tasks_.pop();

101 }

102 }

103

104 // Post delayed tasks that are ready for execution.

105 for (auto& delayed_task : ready_tasks) {

106 delayed_task.worker_pool->PostTaskWithSequenceNow(

107 std::move(delayed_task.task), std::move(delayed_task.sequence),

108 delayed_task.worker);

109 }

110 }

111

112 TimeTicks DelayedTaskManager::GetDelayedRunTime() const {

113 AutoSchedulerLock auto_lock(lock_);

114

115 if (delayed_tasks_.empty())

116 return TimeTicks();

117

118 return delayed_tasks_.top().task->delayed_run_time;

119 }

120

121 // In std::priority_queue, the largest element is on top. Therefore, this

122 // comparator returns true if the delayed run time of \|right\| is earlier than

123 // the delayed run time of \|left\|.

124 bool DelayedTaskManager::DelayedTaskComparator::operator()(

125 const DelayedTask& left,

126 const DelayedTask& right) const {

127 #ifndef NDEBUG

128 // Due to STL consistency checks in Windows and const_cast'ing right before

129 // popping the DelayedTask, a null task can be passed to this comparator in

130 // Debug builds. To satisfy these consistency checks, this comparator

131 // considers null tasks to be the larger than anything.

132 DCHECK(left.task \|\| right.task);

133 if (!left.task)

134 return false;

135 if (!right.task)

136 return true;

137 #else

138 DCHECK(left.task);

139 DCHECK(right.task);

140 #endif // NDEBUG

141 if (left.task->delayed_run_time > right.task->delayed_run_time)

142 return true;

143 if (left.task->delayed_run_time < right.task->delayed_run_time)

144 return false;

145 return left.index > right.index;

146 }

147

148 TimeTicks DelayedTaskManager::Now() const {

149 return TimeTicks::Now();

150 }

151

152 } // namespace internal

153 } // namespace base

OLD	NEW