Separate the create and start phases in SchedulerSingleThreadTaskRunnerManager.
With this CL, single-thread TaskRunners created from a newly
instantiated SchedulerSingleThreadTaskRunnerManager aren't backed
by a thread (and hence their tasks can't run right away). Thread
creation is delayed until
SchedulerSingleThreadTaskRunnerManager::Start().
BUG=690706
Review-Url: https://codereview.chromium.org/2806413002
Cr-Commit-Position: refs/heads/master@{#465439}
Committed: https://chromium.googlesource.com/chromium/src/+/b64df4b2b6feeebc0ae622fb2fc012cfa6b58c1a
3 years, 8 months ago
(2017-04-10 17:40:08 UTC)
#4
PTAL
gab
Nice https://codereview.chromium.org/2806413002/diff/20001/base/task_scheduler/scheduler_single_thread_task_runner_manager.h File base/task_scheduler/scheduler_single_thread_task_runner_manager.h (right): https://codereview.chromium.org/2806413002/diff/20001/base/task_scheduler/scheduler_single_thread_task_runner_manager.h#newcode38 base/task_scheduler/scheduler_single_thread_task_runner_manager.h:38: class BASE_EXPORT SchedulerSingleThreadTaskRunnerManager final { This class is ...
3 years, 8 months ago
(2017-04-10 18:04:56 UTC)
#5
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/builds/190660)
3 years, 8 months ago
(2017-04-10 18:10:25 UTC)
#7
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/426132)
3 years, 8 months ago
(2017-04-10 19:35:48 UTC)
#16
https://codereview.chromium.org/2806413002/diff/60001/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc File base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc (right): https://codereview.chromium.org/2806413002/diff/60001/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc#newcode117 base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc:117: single_thread_task_runner_manager_->Start(); Given the majority of the tests start the ...
3 years, 8 months ago
(2017-04-10 20:52:48 UTC)
#17
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/359591) linux_chromium_tsan_rel_ng on ...
3 years, 8 months ago
(2017-04-11 16:03:37 UTC)
#22
3 years, 8 months ago
(2017-04-13 16:42:30 UTC)
#27
https://codereview.chromium.org/2806413002/diff/60001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker.h (right):
https://codereview.chromium.org/2806413002/diff/60001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker.h:118: bool Start(InitialState
initial_state = InitialState::ALIVE);
On 2017/04/11 15:27:24, fdoray wrote:
> On 2017/04/10 20:52:48, robliao wrote:
> > Initial state seems better placed at the constructor. Start just allows this
> > object to start doing work. Callers then don't have to remember the initial
> > state after they've created the SchedulerWorker.
>
> Did you have in mind a solution that doesn't involve adding a |initial_state_|
> member to SchedulerWorker?
In this CL, we create the SchedulerWorker and then immediately start it in both
SchedulerWorkerPoolImpl and SchedulerSingleThreadTaskRunnerManager. Why can't
initial_state_ be in the constructor?
robliao
3 years, 8 months ago
(2017-04-13 16:42:33 UTC)
#28
3 years, 8 months ago
(2017-04-13 19:53:15 UTC)
#29
ptanl
https://codereview.chromium.org/2806413002/diff/60001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker.h (right):
https://codereview.chromium.org/2806413002/diff/60001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker.h:118: bool Start(InitialState
initial_state = InitialState::ALIVE);
On 2017/04/13 16:42:30, robliao wrote:
> On 2017/04/11 15:27:24, fdoray wrote:
> > On 2017/04/10 20:52:48, robliao wrote:
> > > Initial state seems better placed at the constructor. Start just allows
this
> > > object to start doing work. Callers then don't have to remember the
initial
> > > state after they've created the SchedulerWorker.
> >
> > Did you have in mind a solution that doesn't involve adding a
|initial_state_|
> > member to SchedulerWorker?
>
> In this CL, we create the SchedulerWorker and then immediately start it in
both
> SchedulerWorkerPoolImpl and SchedulerSingleThreadTaskRunnerManager. Why can't
> initial_state_ be in the constructor?
No. SchedulerWorkers created by SchedulerSingleThreadTaskRunnerManager before
SchedulerSingleThreadTaskRunnerManager::Start() are started here
https://codereview.chromium.org/2806413002/diff/100001/base/task_scheduler/sc...
(not in the method from which they were instantiated).
fdoray
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
3 years, 8 months ago
(2017-04-18 12:02:40 UTC)
#30
Dry run: Try jobs failed on following builders: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xcode-clang/builds/81973)
3 years, 8 months ago
(2017-04-18 12:10:46 UTC)
#33
CQ is committing da patch. Bot data: {"patchset_id": 200001, "attempt_start_ts": 1492556692926440, "parent_rev": "4cb75df89a5fabfb4a83717cdc0f9289eb3b0596", "commit_rev": "b64df4b2b6feeebc0ae622fb2fc012cfa6b58c1a"}
3 years, 8 months ago
(2017-04-19 00:37:31 UTC)
#50
CQ is committing da patch.
Bot data: {"patchset_id": 200001, "attempt_start_ts": 1492556692926440,
"parent_rev": "4cb75df89a5fabfb4a83717cdc0f9289eb3b0596", "commit_rev":
"b64df4b2b6feeebc0ae622fb2fc012cfa6b58c1a"}
commit-bot: I haz the power
Description was changed from ========== Separate the create and start phases in SchedulerSingleThreadTaskRunnerManager. With this ...
3 years, 8 months ago
(2017-04-19 00:37:40 UTC)
#51
Message was sent while issue was closed.
Description was changed from
==========
Separate the create and start phases in SchedulerSingleThreadTaskRunnerManager.
With this CL, single-thread TaskRunners created from a newly
instantiated SchedulerSingleThreadTaskRunnerManager aren't backed
by a thread (and hence their tasks can't run right away). Thread
creation is delayed until
SchedulerSingleThreadTaskRunnerManager::Start().
BUG=690706
==========
to
==========
Separate the create and start phases in SchedulerSingleThreadTaskRunnerManager.
With this CL, single-thread TaskRunners created from a newly
instantiated SchedulerSingleThreadTaskRunnerManager aren't backed
by a thread (and hence their tasks can't run right away). Thread
creation is delayed until
SchedulerSingleThreadTaskRunnerManager::Start().
BUG=690706
Review-Url: https://codereview.chromium.org/2806413002
Cr-Commit-Position: refs/heads/master@{#465439}
Committed:
https://chromium.googlesource.com/chromium/src/+/b64df4b2b6feeebc0ae622fb2fc0...
==========
commit-bot: I haz the power
Committed patchset #11 (id:200001) as https://chromium.googlesource.com/chromium/src/+/b64df4b2b6feeebc0ae622fb2fc012cfa6b58c1a
3 years, 8 months ago
(2017-04-19 00:37:41 UTC)
#52
Issue 2806413002: Separate the create and start phases in SchedulerSingleThreadTaskRunnerManager.
(Closed)
Created 3 years, 8 months ago by fdoray
Modified 3 years, 8 months ago
Reviewers: gab, robliao
Base URL:
Comments: 20