New issue
Advanced search Search tips

Issue 756547 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Feature



Sign in to add a comment

Add a Windows thread pool backed SchedulerWorkerPool implementation.

Project Member Reported by jeffreyhe@google.com, Aug 17 2017

Issue description

A SchedulerWorkerPool implementation backed by the Windows thread block pool might perform better than the current SchedulerWorkerPoolImpl when there are many blocking threads. The Windows thread pool would have access to internal OS APIs that Chrome does not, so it can most likely make better decisions than the SchedulerWorkerPoolImpl.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12fba13663339074bcc5701290687d3b44c7b94c

commit 12fba13663339074bcc5701290687d3b44c7b94c
Author: Jeffrey He <jeffreyhe@google.com>
Date: Tue Aug 22 17:20:20 2017

Refactor SchedulerWorkerPoolImpl.

SchedulerWorkerPoolImpl contains a lot of logic that would be
common to other potential implementations of SchedulerWorkerPool
(especially logic that deals with how Sequences and delayed tasks work).
This logic is instead moved to SchedulerWorkerPool. This was done to
support a SchedulerWorkerPool implementation backed by the Windows
thread pool API.

Bug: 756547
Change-Id: I1dbb330fde94829a831256dff4c61aabdbab3037
Reviewed-on: https://chromium-review.googlesource.com/618800
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496344}
[modify] https://crrev.com/12fba13663339074bcc5701290687d3b44c7b94c/base/BUILD.gn
[add] https://crrev.com/12fba13663339074bcc5701290687d3b44c7b94c/base/task_scheduler/scheduler_worker_pool.cc
[modify] https://crrev.com/12fba13663339074bcc5701290687d3b44c7b94c/base/task_scheduler/scheduler_worker_pool.h
[modify] https://crrev.com/12fba13663339074bcc5701290687d3b44c7b94c/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/12fba13663339074bcc5701290687d3b44c7b94c/base/task_scheduler/scheduler_worker_pool_impl.h

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0dcf1c69a70a2f13e7a5672b56d98fa47e8e3ba9

commit 0dcf1c69a70a2f13e7a5672b56d98fa47e8e3ba9
Author: Jeffrey He <jeffreyhe@google.com>
Date: Tue Aug 22 21:01:47 2017

Add a Size() method to PriorityQueue in TaskScheduler.

The Size() method will return the number of Sequences in the
PriorityQueue. This will be used in implementing
SchedulerWorkerPoolWindowsImpl::Start() so that the pool knows how many
Sequences were posted before Start().

Bug: 756547
Change-Id: Ibc599ed94a6d93d60a91ce1bffc35eebac17ed4c
Reviewed-on: https://chromium-review.googlesource.com/621779
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496440}
[modify] https://crrev.com/0dcf1c69a70a2f13e7a5672b56d98fa47e8e3ba9/base/task_scheduler/priority_queue.cc
[modify] https://crrev.com/0dcf1c69a70a2f13e7a5672b56d98fa47e8e3ba9/base/task_scheduler/priority_queue.h

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0b3ba688cfdde7e0c1d432fcc876cc3e25a117df

commit 0b3ba688cfdde7e0c1d432fcc876cc3e25a117df
Author: Jeffrey He <jeffreyhe@google.com>
Date: Wed Aug 23 17:14:16 2017

Refactor out non-SchedulerWorkerPoolImpl specific tests.

SchedulerWorkerPoolImpl currently has unit tests that would be
applicable to any SchedulerWorkerPool implementation. This CL seperates
them out so that those unit tests will be able to reused when
adding SchedulerWorkerPoolWindowsImpl.

Bug: 756547
Change-Id: I9061e36ad779e1a4e69769cfb905613107c91dc3
Reviewed-on: https://chromium-review.googlesource.com/627112
Reviewed-by: Francois Doray <fdoray@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Cr-Commit-Position: refs/heads/master@{#496719}
[modify] https://crrev.com/0b3ba688cfdde7e0c1d432fcc876cc3e25a117df/base/BUILD.gn
[modify] https://crrev.com/0b3ba688cfdde7e0c1d432fcc876cc3e25a117df/base/task_scheduler/scheduler_worker_pool_impl_unittest.cc
[add] https://crrev.com/0b3ba688cfdde7e0c1d432fcc876cc3e25a117df/base/task_scheduler/scheduler_worker_pool_unittest.cc
[modify] https://crrev.com/0b3ba688cfdde7e0c1d432fcc876cc3e25a117df/base/task_scheduler/test_utils.cc
[modify] https://crrev.com/0b3ba688cfdde7e0c1d432fcc876cc3e25a117df/base/task_scheduler/test_utils.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f20648d590dee28d57ab688e142df3f1aba88bd0

commit f20648d590dee28d57ab688e142df3f1aba88bd0
Author: Jeffrey He <jeffreyhe@google.com>
Date: Thu Aug 24 19:54:13 2017

Add SchedulerWorkerPoolWindowsImpl.

SchedulerWorkerPoolWindowsImpl is an alternative worker pool that is
backed by the Windows thread pool. One of the benefits of using the
Windows thread pool is that it probably be more efficient with regards
to blocked threads since it most likely has access to internal
operating system APIs that Chrome does not.

Bug: 756547
Change-Id: If6743f809bb446b23cfed694951419297a2ad4d5
Reviewed-on: https://chromium-review.googlesource.com/615667
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497162}
[modify] https://crrev.com/f20648d590dee28d57ab688e142df3f1aba88bd0/base/BUILD.gn
[add] https://crrev.com/f20648d590dee28d57ab688e142df3f1aba88bd0/base/task_scheduler/platform_native_worker_pool_win.cc
[add] https://crrev.com/f20648d590dee28d57ab688e142df3f1aba88bd0/base/task_scheduler/platform_native_worker_pool_win.h
[modify] https://crrev.com/f20648d590dee28d57ab688e142df3f1aba88bd0/base/task_scheduler/scheduler_worker_pool.cc
[modify] https://crrev.com/f20648d590dee28d57ab688e142df3f1aba88bd0/base/task_scheduler/scheduler_worker_pool.h
[modify] https://crrev.com/f20648d590dee28d57ab688e142df3f1aba88bd0/base/task_scheduler/scheduler_worker_pool_unittest.cc

Project Member

Comment 5 by sheriffbot@chromium.org, Aug 29 2017

Labels: Hotlist-Recharge-BouncingOwner
Owner: ----
Status: Untriaged (was: Started)
The assigned owner "jeffreyhe@google.com" is not able to receive e-mails, please re-triage.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Recharge-BouncingOwner
Status: Available (was: Untriaged)
The main thing left here is to set up an experiment for this.
Project Member

Comment 7 by sheriffbot@chromium.org, Aug 30

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Recharge-Cold
Status: Available (was: Untriaged)

Sign in to add a comment