Use TaskScheduler instead of WorkerPool in v8_platform.cc.

Use TaskScheduler instead of WorkerPool in v8_platform.cc.

The following traits are used:

Priority: Inherited (default)
  The priority is inherited from the calling context (i.e. TaskTraits
  are initialized with the priority of the current task).

Shutdown behavior: CONTINUE_ON_SHUTDOWN
  Tasks posted with this mode which have not started executing before
  shutdown is initiated will never run. Tasks with this mode running at
  shutdown will be ignored (the worker will not be joined).

  Note: Tasks that were previously posted to base::WorkerPool should
  use this shutdown behavior because this is how base::WorkerPool
  handles all its tasks.

MayBlock():
  The task may block.

BUG=659191
Review-Url: https://codereview.chromium.org/2610473002
Cr-Commit-Position: refs/heads/master@{#449976}
Committed: https://chromium.googlesource.com/chromium/src/+/b83be4ca0e00b50b17619adf7f7de275455e9852

[fdoray, 2016-12-31 21:26:51]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-31 21:27:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-31 21:58:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-31 21:58:29]

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_...)

[fdoray, 2017-01-10 15:38:59]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 15:39:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fdoray, 2017-01-10 15:48:57]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 15:49:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 15:53:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 15:53:33]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fdoray, 2017-01-10 16:01:32]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 16:02:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 16:38:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 16:38:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fdoray, 2017-01-10 20:07:47]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 20:08:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 20:14:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 20:14:39]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fdoray, 2017-01-11 14:48:16]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 14:48:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-11 15:27:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-11 15:27:19]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fdoray, 2017-01-12 16:18:07]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 16:18:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 17:24:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 17:24:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fdoray, 2017-01-12 18:18:05]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 18:18:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 19:10:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 19:10:49]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[fdoray, 2017-01-12 19:51:27]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 19:52:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 21:16:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 21:16:24]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[fdoray, 2017-01-13 20:17:42]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 20:19:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 21:04:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-13 21:04:20]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[fdoray, 2017-01-17 19:19:37]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-17 19:20:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fdoray, 2017-01-17 20:50:45]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-17 20:51:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-17 23:15:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-17 23:15:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fdoray, 2017-01-18 00:13:50]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 00:15:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-18 01:24:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-18 01:24:42]

Dry run: This issue passed the CQ dry run.

[fdoray, 2017-01-23 16:47:22]

fdoray@chromium.org changed reviewers:
+ jochen@chromium.org

[fdoray, 2017-01-23 16:47:23]

jochen@: PTAL

We are deprecating base::WorkerPool in favor of TaskScheduler. We don't have
extensive knowledge of v8, so your thoughts on this change are very appreciated.

[jochen (gone - plz use gerrit), 2017-01-24 12:35:20]

can you explain what this MayBlock() means?

How many threads will this end up using?

[fdoray, 2017-01-24 12:58:39]

On 2017/01/24 12:35:20, jochen wrote:
> can you explain what this MayBlock() means?
> 

Tasks posted to TaskScheduler are associated with traits. The MayBlock() trait
indicates that a task may block (e.g. reads a file synchronously). For more
details on how this is handled, see
https://docs.google.com/a/chromium.org/document/d/1ynRvQKah3Cx_eLqPS7KzIp8u73....
Note that this CL posts tasks *without* the MayBlock() trait
https://codereview.chromium.org/2610473002/diff/240001/gin/v8_platform.cc#new...

> How many threads will this end up using?

Tasks posted to TaskScheduler run in a dynamic thread pool. The maximum number
of threads available to non-MayBlock() tasks posted with a USER_VISIBLE priority
(default) is max(8, min(32, 0.3 * |num_cores|)). This may change.

[jochen (gone - plz use gerrit), 2017-01-24 14:30:13]

I assume it still works to post more background tasks from a background task,
right?

If I understood the doc correctly, those restrictions only apply if you'd use
primitives from base/?

Is there a mapping for the long-running-task vs short-running-task parameter
worker pools took?

https://codereview.chromium.org/2610473002/diff/240001/gin/v8_platform.cc
File gin/v8_platform.cc (right):

https://codereview.chromium.org/2610473002/diff/240001/gin/v8_platform.cc#new...
gin/v8_platform.cc:60: static_cast<size_t>(base::SysInfo::NumberOfProcessors());
can you update this to return the max. number of threads?

[fdoray, 2017-02-01 00:24:27]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-01 00:25:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 00:36:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-01 00:36:12]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fdoray, 2017-02-03 14:58:33]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-03 14:58:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-03 16:09:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-03 16:09:23]

Dry run: This issue passed the CQ dry run.

[fdoray, 2017-02-03 17:20:01]

https://codereview.chromium.org/2610473002/diff/240001/gin/v8_platform.cc
File gin/v8_platform.cc (right):

https://codereview.chromium.org/2610473002/diff/240001/gin/v8_platform.cc#new...
gin/v8_platform.cc:60: static_cast<size_t>(base::SysInfo::NumberOfProcessors());
On 2017/01/24 14:30:12, jochen (travelling til Feb 4) wrote:
> can you update this to return the max. number of threads?

Done.

[fdoray, 2017-02-03 17:27:11]

PTAnL

On 2017/01/24 14:30:13, jochen (travelling til Feb 4) wrote:
> I assume it still works to post more background tasks from a background task,
> right?

Yes.

> 
> If I understood the doc correctly, those restrictions only apply if you'd use
> primitives from base/?

Tasks that block on base/ sync primitives must be posted with
.WithBaseSyncPrimitives().
Tasks that block on anything else (except low-contention locks) must be posted
with .MayBlock().
These traits are used by TaskScheduler to make good scheduling decisions. When
possible, we DCHECK if you block without using the appropriate traits.

> 
> Is there a mapping for the long-running-task vs short-running-task parameter
> worker pools took?

No. TaskScheduler doesn't need this to make scheduling decisions [1]. Knowing if
a task blocks is more useful (we can schedule tasks on a new thread when a
blocking task is running, to maximize CPU utilization).

[1] base::WorkerPool doesn't need it either
https://cs.chromium.org/chromium/src/base/threading/worker_pool_posix.cc?sq=p...
:)

> 
> https://codereview.chromium.org/2610473002/diff/240001/gin/v8_platform.cc
> File gin/v8_platform.cc (right):
> 
>
https://codereview.chromium.org/2610473002/diff/240001/gin/v8_platform.cc#new...
> gin/v8_platform.cc:60:
static_cast<size_t>(base::SysInfo::NumberOfProcessors());
> can you update this to return the max. number of threads?

[jochen (gone - plz use gerrit), 2017-02-03 17:46:03]

On 2017/02/03 at 17:27:11, fdoray wrote:
> PTAnL
> 
> On 2017/01/24 14:30:13, jochen (travelling til Feb 4) wrote:
> > I assume it still works to post more background tasks from a background
task,
> > right?
> 
> Yes.
> 
> > 
> > If I understood the doc correctly, those restrictions only apply if you'd
use
> > primitives from base/?
> 
> Tasks that block on base/ sync primitives must be posted with
.WithBaseSyncPrimitives().
> Tasks that block on anything else (except low-contention locks) must be posted
with .MayBlock().

I think we need MayBlock() then (regardless of the task is slow flag).

> These traits are used by TaskScheduler to make good scheduling decisions. When
possible, we DCHECK if you block without using the appropriate traits.
> 
> > 
> > Is there a mapping for the long-running-task vs short-running-task parameter
> > worker pools took?
> 
> No. TaskScheduler doesn't need this to make scheduling decisions [1]. Knowing
if a task blocks is more useful (we can schedule tasks on a new thread when a
blocking task is running, to maximize CPU utilization).
> 
> [1] base::WorkerPool doesn't need it either
https://cs.chromium.org/chromium/src/base/threading/worker_pool_posix.cc?sq=p...
:)
> 

https://cs.chromium.org/chromium/src/base/threading/worker_pool_win.cc?rcl=9b...

> > 
> > https://codereview.chromium.org/2610473002/diff/240001/gin/v8_platform.cc
> > File gin/v8_platform.cc (right):
> > 
> >
https://codereview.chromium.org/2610473002/diff/240001/gin/v8_platform.cc#new...
> > gin/v8_platform.cc:60:
static_cast<size_t>(base::SysInfo::NumberOfProcessors());
> > can you update this to return the max. number of threads?

[fdoray, 2017-02-03 17:52:06]

> > > If I understood the doc correctly, those restrictions only apply if you'd
> use
> > > primitives from base/?
> > 
> > Tasks that block on base/ sync primitives must be posted with
> .WithBaseSyncPrimitives().
> > Tasks that block on anything else (except low-contention locks) must be
posted
> with .MayBlock().
> 
> I think we need MayBlock() then (regardless of the task is slow flag).
> 

Done. Out of curiosity, can you point me to a place where a
CallOnBackgroundThread() task blocks?

[jochen (gone - plz use gerrit), 2017-02-03 17:55:08]

the last place I added is
https://cs.chromium.org/chromium/src/v8/src/compiler-dispatcher/compiler-disp...

[jochen (gone - plz use gerrit), 2017-02-03 17:55:33]

lgtm with updated CL description

[fdoray, 2017-02-03 17:57:05]

Description was changed from

==========
Use TaskScheduler instead of WorkerPool in v8_platform.cc.

The following traits are used:

Priority: Inherited (default)
  The priority is inherited from the calling context (i.e. TaskTraits
  are initialized with the priority of the current task).

Shutdown behavior: CONTINUE_ON_SHUTDOWN
  Tasks posted with this mode which have not started executing before
  shutdown is initiated will never run. Tasks with this mode running at
  shutdown will be ignored (the worker will not be joined).

  Note: Tasks that were previously posted to base::WorkerPool should
  use this shutdown behavior because this is how base::WorkerPool
  handles all its tasks.

Does Not Block (default):
  Tasks without the MayBlock() and WithBaseSyncPrimitives() traits
  may not block.

BUG=659191
==========

to

==========
Use TaskScheduler instead of WorkerPool in v8_platform.cc.

The following traits are used:

Priority: Inherited (default)
  The priority is inherited from the calling context (i.e. TaskTraits
  are initialized with the priority of the current task).

Shutdown behavior: CONTINUE_ON_SHUTDOWN
  Tasks posted with this mode which have not started executing before
  shutdown is initiated will never run. Tasks with this mode running at
  shutdown will be ignored (the worker will not be joined).

  Note: Tasks that were previously posted to base::WorkerPool should
  use this shutdown behavior because this is how base::WorkerPool
  handles all its tasks.

MayBlock():
  The task may block.

BUG=659191
==========

[fdoray, 2017-02-03 18:25:34]

fdoray@chromium.org changed reviewers:
+ jam@chromium.org

[fdoray, 2017-02-03 18:25:35]

jam@: Please review changes in mojo/.

[jam, 2017-02-06 18:48:33]

lgtm

[fdoray, 2017-02-07 13:32:28]

On 2017/02/03 17:55:08, jochen wrote:
> the last place I added is
>
https://cs.chromium.org/chromium/src/v8/src/compiler-dispatcher/compiler-disp...

jochen@: Since this is only in tests, it doesn't justify the use of MayBlock().
Do you have examples of v8 tasks blocking in production?
Note: Low-contention mutexes don't justify the use of MayBlock() either.

[jochen (gone - plz use gerrit), 2017-02-07 15:15:13]

On 2017/02/07 at 13:32:28, fdoray wrote:
> On 2017/02/03 17:55:08, jochen wrote:
> > the last place I added is
> >
https://cs.chromium.org/chromium/src/v8/src/compiler-dispatcher/compiler-disp...
> 
> jochen@: Since this is only in tests, it doesn't justify the use of
MayBlock(). Do you have examples of v8 tasks blocking in production?
> Note: Low-contention mutexes don't justify the use of MayBlock() either.

not off the top of my head. What happens if a task blocks in production?

[fdoray, 2017-02-07 15:20:39]

On 2017/02/07 15:15:13, jochen wrote:
> On 2017/02/07 at 13:32:28, fdoray wrote:
> > On 2017/02/03 17:55:08, jochen wrote:
> > > the last place I added is
> > >
>
https://cs.chromium.org/chromium/src/v8/src/compiler-dispatcher/compiler-disp...
> > 
> > jochen@: Since this is only in tests, it doesn't justify the use of
> MayBlock(). Do you have examples of v8 tasks blocking in production?
> > Note: Low-contention mutexes don't justify the use of MayBlock() either.
> 
> not off the top of my head. What happens if a task blocks in production?

Nothing bad will happen. MayBlock() is just a hint to help TaskScheduler make
scheduling decisions (TaskScheduler tries to always have |num cores| tasks ready
to use the CPU. If a MayBlock() tasks takes some time to run, TaskScheduler
assumes that it is not using the CPU and may start scheduling tasks on a new
thread.)

[fdoray, 2017-02-08 13:59:51]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[fdoray, 2017-02-08 14:00:02]

On 2017/02/07 15:20:39, fdoray wrote:
> On 2017/02/07 15:15:13, jochen wrote:
> > On 2017/02/07 at 13:32:28, fdoray wrote:
> > > On 2017/02/03 17:55:08, jochen wrote:
> > > > the last place I added is
> > > >
> >
>
https://cs.chromium.org/chromium/src/v8/src/compiler-dispatcher/compiler-disp...
> > > 
> > > jochen@: Since this is only in tests, it doesn't justify the use of
> > MayBlock(). Do you have examples of v8 tasks blocking in production?
> > > Note: Low-contention mutexes don't justify the use of MayBlock() either.
> > 
> > not off the top of my head. What happens if a task blocks in production?
> 
> Nothing bad will happen. MayBlock() is just a hint to help TaskScheduler make
> scheduling decisions (TaskScheduler tries to always have |num cores| tasks
ready
> to use the CPU. If a MayBlock() tasks takes some time to run, TaskScheduler
> assumes that it is not using the CPU and may start scheduling tasks on a new
> thread.)

Can I land this without MayBlock()?

[commit-bot: I haz the power, 2017-02-08 14:00:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 15:36:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 15:36:41]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2017-02-09 13:19:02]

lgtm

[fdoray, 2017-02-13 15:41:34]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2017-02-13 15:41:34]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2610473002/#ps340001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-13 15:41:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-13 16:39:56]

CQ is committing da patch.

Bot data: {"patchset_id": 340001, "attempt_start_ts": 1487000494086870,
"parent_rev": "efe430dcec0b2909047a8c48074a93036ce1d47c", "commit_rev":
"b83be4ca0e00b50b17619adf7f7de275455e9852"}

[commit-bot: I haz the power, 2017-02-13 16:40:30]

Description was changed from

==========
Use TaskScheduler instead of WorkerPool in v8_platform.cc.

The following traits are used:

Priority: Inherited (default)
  The priority is inherited from the calling context (i.e. TaskTraits
  are initialized with the priority of the current task).

Shutdown behavior: CONTINUE_ON_SHUTDOWN
  Tasks posted with this mode which have not started executing before
  shutdown is initiated will never run. Tasks with this mode running at
  shutdown will be ignored (the worker will not be joined).

  Note: Tasks that were previously posted to base::WorkerPool should
  use this shutdown behavior because this is how base::WorkerPool
  handles all its tasks.

MayBlock():
  The task may block.

BUG=659191
==========

to

==========
Use TaskScheduler instead of WorkerPool in v8_platform.cc.

The following traits are used:

Priority: Inherited (default)
  The priority is inherited from the calling context (i.e. TaskTraits
  are initialized with the priority of the current task).

Shutdown behavior: CONTINUE_ON_SHUTDOWN
  Tasks posted with this mode which have not started executing before
  shutdown is initiated will never run. Tasks with this mode running at
  shutdown will be ignored (the worker will not be joined).

  Note: Tasks that were previously posted to base::WorkerPool should
  use this shutdown behavior because this is how base::WorkerPool
  handles all its tasks.

MayBlock():
  The task may block.

BUG=659191

Review-Url: https://codereview.chromium.org/2610473002
Cr-Commit-Position: refs/heads/master@{#449976}
Committed:
https://chromium.googlesource.com/chromium/src/+/b83be4ca0e00b50b17619adf7f7d...
==========

[commit-bot: I haz the power, 2017-02-13 16:40:32]

Committed patchset #18 (id:340001) as
https://chromium.googlesource.com/chromium/src/+/b83be4ca0e00b50b17619adf7f7d...

[jam, 2017-02-22 00:26:25]

A revert of this CL (patchset #18 id:340001) has been created in
https://codereview.chromium.org/2711703002/ by jam@chromium.org.

The reason for reverting is: Causing extensions_unittests to hang and take 15
minutes.

BUG=694828.