[platform] Add NumberOfAvailableBackgroundThreads

[platform] Add NumberOfAvailableBackgroundThreads

The call can be used by the embedder to provide information on the workers
executing background tasks.

BUG=chromium:524425
LOG=N

Committed: https://crrev.com/f222240374169c69186f5b59ea50101841642109
Cr-Commit-Position: refs/heads/master@{#33788}

[Michael Lippautz, 2016-02-04 17:27:48]

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

[Michael Lippautz, 2016-02-04 17:27:48]

wdyt?

The outcome of a short brainstorming with scheduler people today was that the
easiest method would probably be to pipe through some limit.

Chromium has a cached version (sandbox) of # processors, but it would probably
better to use their worker pool limit, if they have one. The one within v8
(base::SysInfo::NumberOfProcessors) is not safe to use within the sandbox.

[jochen (gone - plz use gerrit), 2016-02-05 08:27:07]

https://codereview.chromium.org/1664203004/diff/1/include/v8-platform.h
File include/v8-platform.h (right):

https://codereview.chromium.org/1664203004/diff/1/include/v8-platform.h#newco...
include/v8-platform.h:60: * be used to limit parallelism in V8.
I'd remove the "Can be used to limit..." sentence (because it's not true, e.g.,
if the method returns 0, we still post tasks).

You could say that this is used to estimate into how many work packages certain
tasks will be split up.

https://codereview.chromium.org/1664203004/diff/1/include/v8-platform.h#newco...
include/v8-platform.h:62: virtual int NumberOfAvailableBackgroundThreads() {
return 0; }
size_t

[jochen (gone - plz use gerrit), 2016-02-05 08:27:53]

https://codereview.chromium.org/1664203004/diff/1/include/v8-platform.h
File include/v8-platform.h (right):

https://codereview.chromium.org/1664203004/diff/1/include/v8-platform.h#newco...
include/v8-platform.h:60: * be used to limit parallelism in V8.
and maybe explain what 0 means. I'd guess it means ignore this value, and just
post what you feel is right. The platform always has to be able to execute
background tasks.

[Michael Lippautz, 2016-02-05 09:11:53]

Patchset #2 (id:20001) has been deleted

[Michael Lippautz, 2016-02-05 09:13:40]

Patchset #2 (id:40001) has been deleted

[Michael Lippautz, 2016-02-05 09:15:34]

Reworded the API description, ptal.

https://codereview.chromium.org/1664203004/diff/1/include/v8-platform.h
File include/v8-platform.h (right):

https://codereview.chromium.org/1664203004/diff/1/include/v8-platform.h#newco...
include/v8-platform.h:60: * be used to limit parallelism in V8.
On 2016/02/05 08:27:53, jochen wrote:
> and maybe explain what 0 means. I'd guess it means ignore this value, and just
> post what you feel is right. The platform always has to be able to execute
> background tasks.

Done.

https://codereview.chromium.org/1664203004/diff/1/include/v8-platform.h#newco...
include/v8-platform.h:60: * be used to limit parallelism in V8.
On 2016/02/05 08:27:07, jochen wrote:
> I'd remove the "Can be used to limit..." sentence (because it's not true,
e.g.,
> if the method returns 0, we still post tasks).
> 
> You could say that this is used to estimate into how many work packages
certain
> tasks will be split up.

Done.

https://codereview.chromium.org/1664203004/diff/1/include/v8-platform.h#newco...
include/v8-platform.h:62: virtual int NumberOfAvailableBackgroundThreads() {
return 0; }
On 2016/02/05 08:27:07, jochen wrote:
> size_t

Done.

[jochen (gone - plz use gerrit), 2016-02-05 15:03:47]

lgtm with comment

https://codereview.chromium.org/1664203004/diff/80001/src/libplatform/default...
File src/libplatform/default-platform.cc (right):

https://codereview.chromium.org/1664203004/diff/80001/src/libplatform/default...
src/libplatform/default-platform.cc:198: return
static_cast<size_t>(DefaultPlatform::kMaxThreadPoolSize);
return thread_pool_size_?

[Michael Lippautz, 2016-02-05 15:13:31]

https://codereview.chromium.org/1664203004/diff/80001/src/libplatform/default...
File src/libplatform/default-platform.cc (right):

https://codereview.chromium.org/1664203004/diff/80001/src/libplatform/default...
src/libplatform/default-platform.cc:198: return
static_cast<size_t>(DefaultPlatform::kMaxThreadPoolSize);
On 2016/02/05 15:03:47, jochen (OOO) wrote:
> return thread_pool_size_?

Whoops, done.

[Michael Lippautz, 2016-02-05 15:13:38]

The CQ bit was checked by mlippautz@chromium.org

[Michael Lippautz, 2016-02-05 15:13:39]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1664203004/#ps100001
(title: "Addressed comment")

[commit-bot: I haz the power, 2016-02-05 15:13:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1664203004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1664203004/100001

[Michael Lippautz, 2016-02-05 15:17:05]

Description was changed from

==========
[platform] Add NumberOfAvailableBackgroundThreads

The call can be used by the embedder to provide information on the workers
executing background tasks.

BUG=
==========

to

==========
[platform] Add NumberOfAvailableBackgroundThreads

The call can be used by the embedder to provide information on the workers
executing background tasks.

BUG=chromium:524425
LOG=N
==========

[Hannes Payer (out of office), 2016-02-05 15:24:16]

hpayer@chromium.org changed reviewers:
+ hpayer@chromium.org

[Hannes Payer (out of office), 2016-02-05 15:24:17]

Can NumberOfAvailableBackgroundThreads be bigger than number of cores?

[Michael Lippautz, 2016-02-05 15:29:43]

On 2016/02/05 15:24:17, Hannes Payer wrote:
> Can NumberOfAvailableBackgroundThreads be bigger than number of cores?

The embedder can provide any value here, so yes. It would not make too much
sense though.

* DefaultPlatform sets it to std::max(std::min(thread_pool_size,
kMaxThreadPoolSize), 1), where thread_pool size is either #cores or some
user-defined value. 
* Chromium could provide us with the number of threads in the pool, if
available. Otherwise #cores-1 would make sense if threads are created
dynamically.

[commit-bot: I haz the power, 2016-02-05 15:37:07]

Description was changed from

==========
[platform] Add NumberOfAvailableBackgroundThreads

The call can be used by the embedder to provide information on the workers
executing background tasks.

BUG=chromium:524425
LOG=N
==========

to

==========
[platform] Add NumberOfAvailableBackgroundThreads

The call can be used by the embedder to provide information on the workers
executing background tasks.

BUG=chromium:524425
LOG=N
==========

[commit-bot: I haz the power, 2016-02-05 15:37:07]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-02-05 15:37:43]

Description was changed from

==========
[platform] Add NumberOfAvailableBackgroundThreads

The call can be used by the embedder to provide information on the workers
executing background tasks.

BUG=chromium:524425
LOG=N
==========

to

==========
[platform] Add NumberOfAvailableBackgroundThreads

The call can be used by the embedder to provide information on the workers
executing background tasks.

BUG=chromium:524425
LOG=N

Committed: https://crrev.com/f222240374169c69186f5b59ea50101841642109
Cr-Commit-Position: refs/heads/master@{#33788}
==========

[commit-bot: I haz the power, 2016-02-05 15:37:44]

Patchset 4 (id:??) landed as
https://crrev.com/f222240374169c69186f5b59ea50101841642109
Cr-Commit-Position: refs/heads/master@{#33788}