New issue
Advanced search Search tips

Issue 813857 link

Starred by 1 user
Status: Archived
Owner: ----
Closed: May 2018
Cc:
robliao@chromium.org
gab@chromium.org
fdoray@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 749310



Sign in to add a comment

Reconsider SchedulerWorkerPoolImpl Capacity Tracking

Project Member Reported by robliao@chromium.org, Feb 20 2018 
SchedulerWorkerPoolImpl could stand to some simplification:
1) At a few times, the priority queue lock and SchedulerWorkerPoolImpl lock are acquired at the same time. Maybe they should be merged?
2) The Service Thread is a MessageLoop thread so it was easy to get Async IO support. This seems strange with MessageLoop redirection to the task scheduler (and thus impacts capacity monitoring). Maybe the Service Thread should have a custom loop instead. (This has a side benefit of better coalescing and better delayed task handling)
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 22 2018 

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

commit 858c0ce43fc61f9bc7b2dab867293430e853ebb1
Author: Robert Liao <robliao@chromium.org>
Date: Thu Feb 22 04:47:55 2018

Move the AdjustWorkerCapacity PostDelayedTask Out of the SchedulerLock

It is unsafe to hold the SchedulerLock and call PostDelayedTask() on a
task runner as the PostDelayedTask() may itself acquire a
SchedulerLock.

BUG=749312, 813857 

Change-Id: Ie908fc34c0cbf0487de5e7991084e38007f0021d
Reviewed-on: https://chromium-review.googlesource.com/924386
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538349}
[modify] https://crrev.com/858c0ce43fc61f9bc7b2dab867293430e853ebb1/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/858c0ce43fc61f9bc7b2dab867293430e853ebb1/base/task_scheduler/scheduler_worker_pool_impl.h

Comment 2 by gab@chromium.org, May 3 2018

Blocking: 749310
Status: Archived (was: Available)
Re. locking : I think we should wait for lock annotations making it to base::Lock (double-locking is fine when properly stacked).

Re. ServiceThread being an MLForIO. Indeed we'll probably need to change that but I'll coalesce this as a task for MessageLoop integration in TaskScheduler
►

Sign in to add a comment