Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(376)

Issue 2686593003: DESIGN DISCUSSION ONLY Task Scheduler Single Thread Task Runner Manager for Dedicated Threads per S… (Closed)

Created:
3 years, 10 months ago by robliao
Modified:
3 years, 9 months ago
Reviewers:
gab, fdoray
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

DESIGN DISCUSSION ONLY Task Scheduler Single Thread Task Runner Manager for Dedicated Threads per SingleThreadTaskRunner BUG=684080

Patch Set 1 #

Patch Set 2 : Wait for Detached Thread to Complete #

Total comments: 38
Unified diffs Side-by-side diffs Delta from patch set Stats (+685 lines, -67 lines) Patch
M base/BUILD.gn View 2 chunks +3 lines, -0 lines 0 comments Download
M base/task_scheduler/delayed_task_manager.h View 2 chunks +9 lines, -15 lines 0 comments Download
M base/task_scheduler/delayed_task_manager.cc View 2 chunks +4 lines, -10 lines 0 comments Download
M base/task_scheduler/delayed_task_manager_unittest.cc View 4 chunks +26 lines, -5 lines 2 comments Download
A base/task_scheduler/scheduler_single_thread_task_runner_manager.h View 1 chunk +80 lines, -0 lines 2 comments Download
A base/task_scheduler/scheduler_single_thread_task_runner_manager.cc View 1 chunk +266 lines, -0 lines 14 comments Download
A base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc View 1 chunk +210 lines, -0 lines 10 comments Download
M base/task_scheduler/scheduler_worker.h View 1 chunk +4 lines, -0 lines 0 comments Download
M base/task_scheduler/scheduler_worker.cc View 3 chunks +21 lines, -0 lines 6 comments Download
M base/task_scheduler/scheduler_worker_pool_impl.cc View 1 chunk +9 lines, -2 lines 0 comments Download
M base/task_scheduler/scheduler_worker_pool_params.h View 3 chunks +2 lines, -5 lines 0 comments Download
M base/task_scheduler/scheduler_worker_pool_params.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M base/task_scheduler/scheduler_worker_unittest.cc View 1 7 chunks +38 lines, -26 lines 4 comments Download
M base/task_scheduler/task_scheduler_impl.h View 2 chunks +3 lines, -0 lines 0 comments Download
M base/task_scheduler/task_scheduler_impl.cc View 3 chunks +8 lines, -2 lines 0 comments Download

Messages

Total messages: 17 (10 generated)
robliao
The manager is small enough that I can live with the small duplication of logic ...
3 years, 10 months ago (2017-02-08 02:13:23 UTC) #4
fdoray
This is excellent! I think you could get rid of WaitForAllWorkersIdleForTesting(). See comments in scheduler_single_thread_task_runner_manager_unittest.cc. ...
3 years, 10 months ago (2017-02-08 17:59:02 UTC) #11
robliao
I've responded to some of the comments. It sounds like we're okay with the general ...
3 years, 10 months ago (2017-02-10 00:04:05 UTC) #12
robliao
Adding notes https://codereview.chromium.org/2686593003/diff/20001/base/task_scheduler/scheduler_worker.cc File base/task_scheduler/scheduler_worker.cc (right): https://codereview.chromium.org/2686593003/diff/20001/base/task_scheduler/scheduler_worker.cc#newcode101 base/task_scheduler/scheduler_worker.cc:101: } On 2017/02/08 17:59:01, fdoray wrote: > ...
3 years, 10 months ago (2017-02-11 02:13:34 UTC) #13
robliao
Adding some more notes. https://codereview.chromium.org/2686593003/diff/20001/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/2686593003/diff/20001/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc#newcode111 base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc:111: single_thread_task_runner_manager_->WaitForAllWorkersIdleForTesting(); On 2017/02/08 17:59:01, fdoray ...
3 years, 10 months ago (2017-02-14 21:30:07 UTC) #14
robliao
On 2017/02/14 21:30:07, robliao (PST plus 17h) wrote: > Adding some more notes. > > ...
3 years, 9 months ago (2017-02-27 07:40:27 UTC) #16
Theresa
3 years, 9 months ago (2017-03-02 18:36:20 UTC) #17
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2732503002/ by twellington@chromium.org.

The reason for reverting is:
TaskSchedulerSingleThreadTaskRunnerManagerTest.PrioritySetCorrectly is failing
on numerous bots. See crbug.com/697697.

Powered by Google App Engine
This is Rietveld 408576698