New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 738104 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocking:
issue 659191



Sign in to add a comment

Allow the SchedulerWorkerPoolImpl in TaskScheduler to have a dynamic number of workers.

Project Member Reported by jeffreyhe@google.com, Jun 29 2017

Issue description

Currently, the number of SchedulerWorkers in TaskScheduler is fixed after initialization, creating a fixed maximum number of threads. This can cause deadlocks when that fixed maximum is low or low CPU usage when some threads become blocked.

The TaskScheduler will be changed to address these concerns:

- The first step is to make the list of SchedulerWorkers in SchedulerWorkerPoolImpl dynamic so that it can be grown or shrunk. This will involve protecting the workers_ list under a lock and adding workers as needed. This sets up the stage for adding mechanisms that grow or shrink the worker pool when tasks are blocked.

- Since workers can now be removed, the thread detachment mechanism will be removed.

- The StandbyThreadPolicy will be removed and StandbyThreadPolicy::ONE will be assumed. The lazy policy used to be useful when the SchedulerWorkerPool wasn’t heavily utilized so keeping a standby thread would’ve been wasteful. But, this is no longer the case and removing the StandbyThreadPolicy will simplify the code.

- A ScopedMayBlock class will be created that is used to mark blocks of code that may block. A ScopedMayBlock object will temporarily increase the capacity of the SchedulerWorkerPoolImpl if it is in scope long enough.
 
Project Member

Comment 1 by sheriffbot@chromium.org, Jul 11 2017

Labels: Hotlist-Google
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 12 2017

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

commit 65143c3c46c4c283996ebd6732e5dc955c540849
Author: Jeffrey He <jeffreyhe@google.com>
Date: Wed Jul 12 20:34:37 2017

Protect |workers_| under the |idle_workers_stack_lock_|

The SchedulerWorkerPoolImpl in TaskScheduler will be changed to have a
dynamic number of SchedulerWorkers, so |workers_| can be read and 
written to from multiple threads. The |idle_workers_stack_lock_|
will additionally cover |workers_| and be renamed to |lock_|.

Bug:  738104 
Change-Id: I23a49e18253a4a9a375be790be5a6735cb844323
Reviewed-on: https://chromium-review.googlesource.com/556341
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486074}
[modify] https://crrev.com/65143c3c46c4c283996ebd6732e5dc955c540849/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/65143c3c46c4c283996ebd6732e5dc955c540849/base/task_scheduler/scheduler_worker_pool_impl.h

Comment 3 by mark@chromium.org, Jul 12 2017

It looks like 65143c3c46c4 caused Android Builder and ios-device-xcode-clang to both fail their compile steps.

https://build.chromium.org/p/chromium.webkit/builders/Android%20Builder/builds/115478
https://build.chromium.org/p/chromium.mac/builders/ios-device-xcode-clang/builds/30361

[332/4577] CXX obj/base/base/scheduler_worker_pool_impl.o
FAILED: obj/base/base/scheduler_worker_pool_impl.o 
/b/c/goma_client/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ […] -c ../../base/task_scheduler/scheduler_worker_pool_impl.cc -o obj/base/base/scheduler_worker_pool_impl.o
../../base/task_scheduler/scheduler_worker_pool_impl.cc:130:6: error: unused function 'ContainsWorker' [-Werror,-Wunused-function]
bool ContainsWorker(const std::vector<scoped_refptr<SchedulerWorker>>& workers,
     ^
1 error generated.

I’m backing it out.
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 12 2017

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

commit 7e67623bd0d6d4c8b4857c9ca481138bda67ae63
Author: Mark Mentovai <mark@chromium.org>
Date: Wed Jul 12 20:54:02 2017

Revert "Protect |workers_| under the |idle_workers_stack_lock_|"

This reverts commit 65143c3c46c4c283996ebd6732e5dc955c540849.

Reason for revert:  https://crbug.com/738104#c3 

Original change's description:
> Protect |workers_| under the |idle_workers_stack_lock_|
> 
> The SchedulerWorkerPoolImpl in TaskScheduler will be changed to have a
> dynamic number of SchedulerWorkers, so |workers_| can be read and 
> written to from multiple threads. The |idle_workers_stack_lock_|
> will additionally cover |workers_| and be renamed to |lock_|.
> 
> Bug:  738104 
> Change-Id: I23a49e18253a4a9a375be790be5a6735cb844323
> Reviewed-on: https://chromium-review.googlesource.com/556341
> Commit-Queue: Jeffrey He <jeffreyhe@google.com>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Robert Liao <robliao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#486074}

TBR=gab@chromium.org,fdoray@chromium.org,robliao@chromium.org,jeffreyhe@google.com

Change-Id: I014492dfc9b16d3b3931dd3e2cd21ed565a96e4a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  738104 
Reviewed-on: https://chromium-review.googlesource.com/568878
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486079}
[modify] https://crrev.com/7e67623bd0d6d4c8b4857c9ca481138bda67ae63/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/7e67623bd0d6d4c8b4857c9ca481138bda67ae63/base/task_scheduler/scheduler_worker_pool_impl.h

Comment 5 by mark@chromium.org, Jul 12 2017

Considering how every bot fell over, I’m surprised that the commit-queue didn’t catch this before it landed.
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 17 2017

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

commit 8e4c5d6e3f8187c71f8620ae2ef02b7a659478df
Author: Jeffrey He <jeffreyhe@google.com>
Date: Mon Jul 17 18:01:29 2017

Reland "Protect |workers_| under the |idle_workers_stack_lock_|"

This is a reland of 65143c3c46c4c283996ebd6732e5dc955c540849
Original change's description:
> Protect |workers_| under the |idle_workers_stack_lock_|
> 
> The SchedulerWorkerPoolImpl in TaskScheduler will be changed to have a
> dynamic number of SchedulerWorkers, so |workers_| can be read and 
> written to from multiple threads. The |idle_workers_stack_lock_|
> will additionally cover |workers_| and be renamed to |lock_|.
> 
> Bug:  738104 
> Change-Id: I23a49e18253a4a9a375be790be5a6735cb844323
> Reviewed-on: https://chromium-review.googlesource.com/556341
> Commit-Queue: Jeffrey He <jeffreyhe@google.com>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Robert Liao <robliao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#486074}

Bug:  738104 
Change-Id: Ib4eb5533577abaf190715961483da7550cc01ece
Reviewed-on: https://chromium-review.googlesource.com/574634
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487156}
[modify] https://crrev.com/8e4c5d6e3f8187c71f8620ae2ef02b7a659478df/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/8e4c5d6e3f8187c71f8620ae2ef02b7a659478df/base/task_scheduler/scheduler_worker_pool_impl.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 25 2017

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

commit b856d1a5c9e4426f51500eebe7dd9c2ad1699fc8
Author: Jeffrey He <jeffreyhe@google.com>
Date: Tue Jul 25 13:07:17 2017

Remove index from SchedulerWorker name

Removing the index of the SchedulerWorker within |workers_| will
simplify adding workers to |workers_|. Having the index currently provides
little value in traces since the index is usually truncated. In addition,
soon, a particular SchedulerWorker will always be associated with a
particular thread, so the thread id could be used instead to differentiate
between SchedulerWorkers.

Bug:  738104 
Change-Id: Ia9aa5dc4019dad505d86102d5d40c0327dedbad9
Reviewed-on: https://chromium-review.googlesource.com/559970
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489285}
[modify] https://crrev.com/b856d1a5c9e4426f51500eebe7dd9c2ad1699fc8/base/task_scheduler/scheduler_worker_pool_impl.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 26 2017

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

commit 12a3179cef9ba2f48e24463873604befe3575418
Author: Jeffrey He <jeffreyhe@google.com>
Date: Wed Jul 26 19:14:00 2017

Create SchedulerWorkers on Demand

Currently, all SchedulerWorkers are created in Start(). With this change, 
one SchedulerWorker in addition to the number required based off of tasks 
in flight will be created in Start().

After that, WakeUpOneWorker() will create SchedulerWorkers as needed.

Bug:  738104 
Change-Id: I9a0e2fdcfc7338e10a38e1021f22fdb7ecadfa10
Reviewed-on: https://chromium-review.googlesource.com/556385
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489719}
[modify] https://crrev.com/12a3179cef9ba2f48e24463873604befe3575418/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/12a3179cef9ba2f48e24463873604befe3575418/base/task_scheduler/scheduler_worker_pool_impl.h
[modify] https://crrev.com/12a3179cef9ba2f48e24463873604befe3575418/base/task_scheduler/scheduler_worker_pool_impl_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 26 2017

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

commit fbf000f4e9c0bdc7d02fa4685a0115f664e72229
Author: Jeffrey He <jeffreyhe@google.com>
Date: Wed Jul 26 22:44:45 2017

Remove StandbyThreadPolicy.

Since the TaskScheduler is now actively being used, StandbyThreadPolicy::LAZY is no 
longer useful as a thread will at some point always be created.

Bug:  738104 
Change-Id: Ib18779420d867fcbce091c6bd1161b9fbb863c0e
Reviewed-on: https://chromium-review.googlesource.com/562017
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Scott Byer <scottbyer@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489782}
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/base/task_scheduler/scheduler_worker_pool_impl_unittest.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/base/task_scheduler/scheduler_worker_pool_params.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/base/task_scheduler/scheduler_worker_pool_params.h
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/base/task_scheduler/task_scheduler.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/base/task_scheduler/task_scheduler_impl_unittest.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/base/test/scoped_async_task_scheduler.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/base/test/scoped_task_environment.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/base/threading/sequenced_worker_pool_unittest.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/chrome/service/service_process.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/components/task_scheduler_util/common/variations_util.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/components/task_scheduler_util/common/variations_util_unittest.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/content/browser/browser_main_loop.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/content/renderer/render_process_impl.cc
[modify] https://crrev.com/fbf000f4e9c0bdc7d02fa4685a0115f664e72229/ios/web/public/global_state/ios_global_state.mm

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 27 2017

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

commit 14b28d779f3857e6bff0a454c24e8445fb222969
Author: Jeffrey He <jeffreyhe@google.com>
Date: Thu Jul 27 16:11:10 2017

Maintain an idle active worker in WakeUpOneWorker

StandbyThreadPolicy was removed in 
https://chromium-review.googlesource.com/c/562017 and
StandbyThreadPolicy::ONE was assumed. But, this policy was not always
followed. This CL completes implementation of that policy by ensuring
that an idle worker gets created, if needed, after a task is posted.

Bug:  738104 
Change-Id: Ib6cb2a530f068ea9daacc3fcfc484a4ddaace2d3
Reviewed-on: https://chromium-review.googlesource.com/563639
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489954}
[modify] https://crrev.com/14b28d779f3857e6bff0a454c24e8445fb222969/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/14b28d779f3857e6bff0a454c24e8445fb222969/base/task_scheduler/scheduler_worker_pool_impl_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 8 2017

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

commit a9a085dd2b2d0c3f838d6341571099baaeb0c99b
Author: Jeffrey He <jeffreyhe@google.com>
Date: Tue Aug 08 15:05:01 2017

Remove worker detachment.

After SchedulerWorker::Start(), a SchedulerWorker will always contain a
thread until the thread exits and the SchedulerWorker is removed from 
the SchedulerWorkerPoolImpl.

The SchedulerWorker no longer has a notion of worker detachment. 
Instead, the SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl will
determine within GetWork() if the worker should cleanup. If so, the
pool delegate will call Cleanup() on the worker and also remove the
SchedulerWorker from the SchedulerWorkerPoolImpl. This maintains the
invariant the the pool only contains workers with threads. 

Bug:  738104 
Change-Id: Ie99b47694014d5638d6be46a48459a5847807a44
Reviewed-on: https://chromium-review.googlesource.com/568579
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Cr-Commit-Position: refs/heads/master@{#492622}
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/task_scheduler/scheduler_worker.cc
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/task_scheduler/scheduler_worker.h
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/task_scheduler/scheduler_worker_pool_impl.h
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/task_scheduler/scheduler_worker_pool_impl_unittest.cc
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/task_scheduler/scheduler_worker_stack_unittest.cc
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/task_scheduler/scheduler_worker_unittest.cc
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/task_scheduler/task_scheduler_impl.cc
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/task_scheduler/task_scheduler_impl_unittest.cc
[modify] https://crrev.com/a9a085dd2b2d0c3f838d6341571099baaeb0c99b/base/threading/sequenced_worker_pool_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 9 2017

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

commit 1471bcd67cd24cee282f0b25550820ac732545f9
Author: Jeffrey He <jeffreyhe@google.com>
Date: Wed Aug 09 14:47:32 2017

Add ScopedMayBlock and BlockingObserver classes

The ScopedMayBlock class is used to annotate code that may block. It
informs the BlockingObserver object for the current thread in TLS by
calling BlockingObserver::BlockingScopeEntered() when ScopedMayBlock 
is created and BlockingObserver::BlockingScopeExited() when destroyed.

Bug:  738104 
Change-Id: I965932a058a0ac22e465e2afa2f85b515c35a439
Reviewed-on: https://chromium-review.googlesource.com/587390
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Cr-Commit-Position: refs/heads/master@{#492976}
[modify] https://crrev.com/1471bcd67cd24cee282f0b25550820ac732545f9/base/BUILD.gn
[add] https://crrev.com/1471bcd67cd24cee282f0b25550820ac732545f9/base/threading/scoped_may_block.cc
[add] https://crrev.com/1471bcd67cd24cee282f0b25550820ac732545f9/base/threading/scoped_may_block.h

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 18 2017

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

commit dfade29f26a2e96d30ec134403f7bd8a58652d96
Author: Jeffrey He <jeffreyhe@google.com>
Date: Fri Aug 18 16:33:42 2017

Change ScopedMayBlock to be ScopedBlockingCall.

ScopedBlockingCall will be added instead of ScopedMayBlock to force
users to consider if a call will definitely block or is just likely to
block.

Bug:  738104 
Change-Id: I8c7c70408b2dafec2eee8255c6a5b7edf247ad1e
Reviewed-on: https://chromium-review.googlesource.com/611123
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495575}
[modify] https://crrev.com/dfade29f26a2e96d30ec134403f7bd8a58652d96/base/BUILD.gn
[add] https://crrev.com/dfade29f26a2e96d30ec134403f7bd8a58652d96/base/threading/scoped_blocking_call.cc
[add] https://crrev.com/dfade29f26a2e96d30ec134403f7bd8a58652d96/base/threading/scoped_blocking_call.h
[delete] https://crrev.com/ad7cf5304bb4b0ad3ecdf0b647d37a658ad588c5/base/threading/scoped_may_block.cc
[delete] https://crrev.com/ad7cf5304bb4b0ad3ecdf0b647d37a658ad588c5/base/threading/scoped_may_block.h

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 18 2017

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

commit ccc775ee6a7269cd3225e067019a1251efafeedc
Author: Jeffrey He <jeffreyhe@google.com>
Date: Fri Aug 18 20:53:03 2017

Add |is_on_idle_workers_stack_| member to SchedulerWorkerPoolDelegateImpl.

The |is_on_idle_workers_stack_| member will track whether a worker is 
on the |SchedulerWorkerPoolImpl::idle_workers_stack_|. This member 
allows us to determine what caused a worker to call GetWork():
  * Its WaitableEvent timed out (|is_on_idle_workers_stack_| is true)
  * It woke up (|is_on_idle_workers_stack_| is false)

Bug:  738104 
Change-Id: Ieaf57d97fbf9634bcaee3ba381760682735fe024
Reviewed-on: https://chromium-review.googlesource.com/611958
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Cr-Commit-Position: refs/heads/master@{#495675}
[modify] https://crrev.com/ccc775ee6a7269cd3225e067019a1251efafeedc/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/ccc775ee6a7269cd3225e067019a1251efafeedc/base/task_scheduler/scheduler_worker_pool_impl.h

Project Member

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

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

commit 4da1205d60e6f000ba318bf188b5604f1dbb192f
Author: Jeffrey He <jeffreyhe@google.com>
Date: Tue Aug 22 22:32:49 2017

Implement BlockingObserver for SchedulerWorkerPoolImpl

In this CL, SchedulerWorkerPoolImpl::SchedulerWorker::Delegate now
implements the BlockingObserver interface. Note: In this CL, only
BlockingType::WILL_BLOCK will supported.

When ScopedBlockingCall is instantiated with WILL_BLOCK, 
SchedulerWorker::Delegate::BlockingScopeEntered will
increment |worker_capacity_| and create a new worker if needed. When
the ScopedBlockingCall object is destroyed, |worker_capacity_| is 
decremented back.

Bug:  738104 
Change-Id: I19e1a2ba6cc11cf77cd8cc3de487b2f4a2bbc351
Reviewed-on: https://chromium-review.googlesource.com/612502
Commit-Queue: Jeffrey He <jeffreyhe@google.com>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496477}
[modify] https://crrev.com/4da1205d60e6f000ba318bf188b5604f1dbb192f/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/4da1205d60e6f000ba318bf188b5604f1dbb192f/base/task_scheduler/scheduler_worker_pool_impl.h
[modify] https://crrev.com/4da1205d60e6f000ba318bf188b5604f1dbb192f/base/task_scheduler/scheduler_worker_pool_impl_unittest.cc

Project Member

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

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

commit d628419b4d5e14f51841978b95e434996d98dc05
Author: Jeffrey He <jeffreyhe@google.com>
Date: Wed Aug 23 18:51:51 2017

Rename SchedulerWorkerPoolImpl::GetMaxConcurrentTasksDeprecated.

SchedulerWorkerPoolImpl::GetMaxConcurrentTasksDeprecated is renamed to
GetMaxConcurrentNonBlockedTasksDeprecated and
TaskSchedueler::GetMaxConcurrentTasksWithTraitsDeprecated is
renamed to GetMaxConcurrentNonBlockedTasksWithTraitsDeprecated.

The function is used to determine the number of available background 
threads in v8. But, with the advent of
ScopedBlockingCall, the max number of concurrent tasks actually changes
with threads being blocked. However, the maximum number of non-blocked
threads stays the same and is more applicable for current use cases
of the function.

TBR=jochen@chromium.org

Bug:  738104 
Change-Id: I1b465f7155c85852ab4a361674755afc27b19817
Reviewed-on: https://chromium-review.googlesource.com/594208
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@{#496749}
[modify] https://crrev.com/d628419b4d5e14f51841978b95e434996d98dc05/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/d628419b4d5e14f51841978b95e434996d98dc05/base/task_scheduler/scheduler_worker_pool_impl.h
[modify] https://crrev.com/d628419b4d5e14f51841978b95e434996d98dc05/base/task_scheduler/task_scheduler.h
[modify] https://crrev.com/d628419b4d5e14f51841978b95e434996d98dc05/base/task_scheduler/task_scheduler_impl.cc
[modify] https://crrev.com/d628419b4d5e14f51841978b95e434996d98dc05/base/task_scheduler/task_scheduler_impl.h
[modify] https://crrev.com/d628419b4d5e14f51841978b95e434996d98dc05/base/task_scheduler/task_scheduler_impl_unittest.cc
[modify] https://crrev.com/d628419b4d5e14f51841978b95e434996d98dc05/content/browser/browser_main_loop_unittest.cc
[modify] https://crrev.com/d628419b4d5e14f51841978b95e434996d98dc05/gin/v8_platform.cc

Project Member

Comment 17 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: -Pri-3 -Hotlist-Recharge-BouncingOwner Pri-2
Status: fdoraychromium.org (was: Untriaged)
fdoray has picked up the flag here.
Project Member

Comment 19 by bugdroid1@chromium.org, Sep 11 2017

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

commit adf582e2773d1d5cce9e403a493e47b2276a6565
Author: Francois Doray <fdoray@chromium.org>
Date: Mon Sep 11 17:27:50 2017

Support nested ScopedBlockingCalls.

Nested ScopedBlockingCalls are useful when Chrome code makes a
blocking call to an external library which may reenter Chrome, and
the reentrant code may itself make another blocking call.

A WILL_BLOCK ScopedBlockingCall within the scope of another
WILL_BLOCK ScopedBlockingCall has no effect. A MAY_BLOCK within
the scope of another ScopedBlockingCall (of any type) has no effect.
A WILL_BLOCK ScopedBlockingCall within the scope of a MAY_BLOCK
ScopedBlockingCall is equivalent to causing the timeout of the
MAY_BLOCK ScopedBlockingCall to expire immediately.

Bug:  738104 
Change-Id: I1f19f626ebb602d0366258bc74d3127e636eeb99
Reviewed-on: https://chromium-review.googlesource.com/653595
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500956}
[modify] https://crrev.com/adf582e2773d1d5cce9e403a493e47b2276a6565/base/BUILD.gn
[modify] https://crrev.com/adf582e2773d1d5cce9e403a493e47b2276a6565/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/adf582e2773d1d5cce9e403a493e47b2276a6565/base/task_scheduler/scheduler_worker_pool_impl.h
[modify] https://crrev.com/adf582e2773d1d5cce9e403a493e47b2276a6565/base/task_scheduler/scheduler_worker_pool_impl_unittest.cc
[modify] https://crrev.com/adf582e2773d1d5cce9e403a493e47b2276a6565/base/threading/scoped_blocking_call.cc
[modify] https://crrev.com/adf582e2773d1d5cce9e403a493e47b2276a6565/base/threading/scoped_blocking_call.h
[add] https://crrev.com/adf582e2773d1d5cce9e403a493e47b2276a6565/base/threading/scoped_blocking_call_unittest.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Sep 11 2017

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

commit 3abc0c0cc1e7ae9c5629f417609ae478d98b09ab
Author: Francois Doray <fdoray@chromium.org>
Date: Mon Sep 11 19:53:29 2017

Add MAY_BLOCK ScopedBlockingCall in WaitableEvent::*Wait*.

Care has been taken to avoid changing worker capacity when the
TaskScheduler code itself waits on a WaitableEvent.

Bug:  738104 
Change-Id: If0c7220c6a5602ba2969fbc867e4bc7b81469137
Reviewed-on: https://chromium-review.googlesource.com/654058
Commit-Queue: Francois Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501001}
[modify] https://crrev.com/3abc0c0cc1e7ae9c5629f417609ae478d98b09ab/base/synchronization/waitable_event_mac.cc
[modify] https://crrev.com/3abc0c0cc1e7ae9c5629f417609ae478d98b09ab/base/synchronization/waitable_event_posix.cc
[modify] https://crrev.com/3abc0c0cc1e7ae9c5629f417609ae478d98b09ab/base/synchronization/waitable_event_win.cc
[modify] https://crrev.com/3abc0c0cc1e7ae9c5629f417609ae478d98b09ab/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/3abc0c0cc1e7ae9c5629f417609ae478d98b09ab/base/task_scheduler/scheduler_worker_pool_impl_unittest.cc
[modify] https://crrev.com/3abc0c0cc1e7ae9c5629f417609ae478d98b09ab/base/threading/scoped_blocking_call.cc
[modify] https://crrev.com/3abc0c0cc1e7ae9c5629f417609ae478d98b09ab/base/threading/scoped_blocking_call.h

Project Member

Comment 21 by bugdroid1@chromium.org, Sep 11 2017

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

commit ac15286d789bf0d9b7634dfc6f229d83de59affe
Author: Francois Doray <fdoray@chromium.org>
Date: Mon Sep 11 22:31:19 2017

Add MAY_BLOCK ScopedBlockingCall in ConditionVariable::*Wait.

Bug:  738104 
Change-Id: I9a2330ee38533bdfd5cd63012c6610265a85a1a5
Reviewed-on: https://chromium-review.googlesource.com/657897
Commit-Queue: Francois Doray <fdoray@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501066}
[modify] https://crrev.com/ac15286d789bf0d9b7634dfc6f229d83de59affe/base/synchronization/condition_variable_posix.cc
[modify] https://crrev.com/ac15286d789bf0d9b7634dfc6f229d83de59affe/base/synchronization/condition_variable_win.cc

Comment 22 by gab@chromium.org, Sep 26 2017

Blocking: 659191
Owner: fdoray@chromium.org
Status: Fixed (was: fdoraychromium.org)

Comment 23 by gab@chromium.org, Sep 26 2017

Cc: jeffreyhe@google.com

Sign in to add a comment