[scheduler] Add type to scheduler::TaskQueue

[scheduler] Add type to scheduler::TaskQueue

Match each TaskQueue to one of few hardcoded types for metric
collection.

This patch also adds metric to track number of renderer tasks per
task queue type.

BUG=

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

[altimin, 2016-10-20 10:26:07]

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

[commit-bot: I haz the power, 2016-10-20 10:26:20]

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

[altimin, 2016-10-20 10:28:14]

Description was changed from

==========
Add metric for number of tasks per queue type


[scheduler] Add types for scheduler::TaskQueue.

Match each TaskQueue to one of few hardcoded types for metric
collection.

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

to

==========
[scheduler] Add type to scheduler::TaskQueue 

Match each TaskQueue to one of few hardcoded types for metric
collection.

This patch also adds metric to track number of renderer tasks per 
task queue type.



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

[altimin, 2016-10-20 10:28:34]

altimin@chromium.org changed reviewers:
+ alexclarke@chromium.org

[altimin, 2016-10-20 10:30:34]

PTAL

Alex, if you have a bug for removing default task queue in mind, could you
please let me know the id (and cc me to it?)

[commit-bot: I haz the power, 2016-10-20 10:38:40]

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

[commit-bot: I haz the power, 2016-10-20 10:38:40]

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

[alex clarke (OOO till 29th), 2016-10-20 10:47:21]

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
(right):

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:42:
DCHECK(false);
s/TaskQueue::QueueType:COUNT:/default:

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
(right):

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc:1497:
void RendererSchedulerImpl::ReportTaskTime(TaskQueue* task_queue,
Would you mind measuring how long this method takes on non-trivial sites like
the verge?

Hopefully it's nice and fast.

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/scheduler/base/task_queue.h (right):

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/scheduler/base/task_queue.h:57: enum class
QueueType {
// Keep TaskQueue::NameForQueueType in sync.

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/scheduler/base/task_queue.h:138: virtual
QueueType GetType() const = 0;
s/GetType/GetQueueType

[altimin, 2016-10-20 11:29:40]

altimin@chromium.org changed reviewers:
+ jochen@chromium.org, jwd@chromium.org

[altimin, 2016-10-20 11:29:40]

PTAL

+jochen@ for changes in webkit tests.
+jwd@ for histograms.xml

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
(right):

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:42:
DCHECK(false);
On 2016/10/20 10:47:21, alex clarke wrote:
> s/TaskQueue::QueueType:COUNT:/default:

When using QueueType::COUNT we will get a compilation error here when we add a
new type to QueueType (something like 'enumeration value 'NEW_TYPE' not handled
in switch').

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
(right):

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc:1497:
void RendererSchedulerImpl::ReportTaskTime(TaskQueue* task_queue,
On 2016/10/20 10:47:21, alex clarke wrote:
> Would you mind measuring how long this method takes on non-trivial sites like
> the verge?
> 
> Hopefully it's nice and fast.

It takes several microseconds to call this function (0.003ms on average), same
as DidProcessTask. This is fine for now, but we should keep this in mind.

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/scheduler/base/task_queue.h (right):

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/scheduler/base/task_queue.h:57: enum class
QueueType {
On 2016/10/20 10:47:21, alex clarke wrote:
> // Keep TaskQueue::NameForQueueType in sync.

Done.

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/scheduler/base/task_queue.h:138: virtual
QueueType GetType() const = 0;
On 2016/10/20 10:47:21, alex clarke wrote:
> s/GetType/GetQueueType

Done.

[alex clarke (OOO till 29th), 2016-10-20 12:36:13]

scheduler/ LGTM

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
(right):

https://codereview.chromium.org/2437003002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:42:
DCHECK(false);
On 2016/10/20 11:29:40, altimin wrote:
> On 2016/10/20 10:47:21, alex clarke wrote:
> > s/TaskQueue::QueueType:COUNT:/default:
> 
> When using QueueType::COUNT we will get a compilation error here when we add a
> new type to QueueType (something like 'enumeration value 'NEW_TYPE' not
handled
> in switch').

Ah yes it's an enum class. OK then lets leave it like this.

[altimin, 2016-10-21 09:39:15]

altimin@chromium.org changed reviewers:
+ haraken@chromium.org

[altimin, 2016-10-21 09:39:16]

+haraken@ — could you please approve changes in TimerTest.cpp?

[haraken, 2016-10-21 10:39:32]

On 2016/10/21 09:39:16, altimin wrote:
> +haraken@ — could you please approve changes in TimerTest.cpp?

LGTM

[jochen (gone - plz use gerrit), 2016-10-21 11:08:29]

sorry for the delay. I guess Kentaro's review covers everything..

[jwd, 2016-10-21 15:47:00]

https://codereview.chromium.org/2437003002/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/scheduler/base/task_queue.h (right):

https://codereview.chromium.org/2437003002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/scheduler/base/task_queue.h:58: // Keep
TaskQueue::NameForQueueType in sync.
Please mention that this enum is used for a histogram and shouldn't be
re-numbered.

https://codereview.chromium.org/2437003002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2437003002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:50259: +<histogram
name="RendererScheduler.NumberOfTasksPerQueueType" units="">
Please add an enum instead of units, and define the enum with the same values as
QueueType.

[altimin, 2016-10-21 16:41:34]

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

[commit-bot: I haz the power, 2016-10-21 16:41:50]

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

[altimin, 2016-10-21 16:52:44]

PTAL

https://codereview.chromium.org/2437003002/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/scheduler/base/task_queue.h (right):

https://codereview.chromium.org/2437003002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/scheduler/base/task_queue.h:58: // Keep
TaskQueue::NameForQueueType in sync.
On 2016/10/21 15:47:00, jwd wrote:
> Please mention that this enum is used for a histogram and shouldn't be
> re-numbered.

Done.

https://codereview.chromium.org/2437003002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2437003002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:50259: +<histogram
name="RendererScheduler.NumberOfTasksPerQueueType" units="">
On 2016/10/21 15:47:00, jwd wrote:
> Please add an enum instead of units, and define the enum with the same values
as
> QueueType.

Done.

[jwd, 2016-10-21 17:20:30]

lgtm

https://codereview.chromium.org/2437003002/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/scheduler/base/task_queue.h (right):

https://codereview.chromium.org/2437003002/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/platform/scheduler/base/task_queue.h:60: // be
re-numbered.
Can this comment be on one line?

[altimin, 2016-10-21 17:23:51]

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

[commit-bot: I haz the power, 2016-10-21 17:24:39]

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

[altimin, 2016-10-21 17:26:56]

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

[commit-bot: I haz the power, 2016-10-21 17:27:40]

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

[altimin, 2016-10-21 19:09:17]

The CQ bit was checked by altimin@chromium.org

[altimin, 2016-10-21 19:09:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, alexclarke@chromium.org, jwd@chromium.org
Link to the patchset: https://codereview.chromium.org/2437003002/#ps80001
(title: "fix windows compilation")

[altimin, 2016-10-21 19:09:20]

The CQ bit was unchecked by altimin@chromium.org

[altimin, 2016-10-21 19:10:09]

The CQ bit was unchecked by altimin@chromium.org

[altimin, 2016-10-21 19:11:18]

The CQ bit was checked by altimin@chromium.org

[commit-bot: I haz the power, 2016-10-21 19:12:02]

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-10-21 19:29:14]

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

[commit-bot: I haz the power, 2016-10-21 19:29:16]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)

[altimin, 2016-10-21 19:29:54]

The CQ bit was checked by altimin@chromium.org

[commit-bot: I haz the power, 2016-10-21 19:30:33]

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-10-21 20:12:46]

Description was changed from

==========
[scheduler] Add type to scheduler::TaskQueue 

Match each TaskQueue to one of few hardcoded types for metric
collection.

This patch also adds metric to track number of renderer tasks per 
task queue type.



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

to

==========
[scheduler] Add type to scheduler::TaskQueue 

Match each TaskQueue to one of few hardcoded types for metric
collection.

This patch also adds metric to track number of renderer tasks per 
task queue type.



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

[commit-bot: I haz the power, 2016-10-21 20:12:47]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-21 20:14:05]

Description was changed from

==========
[scheduler] Add type to scheduler::TaskQueue 

Match each TaskQueue to one of few hardcoded types for metric
collection.

This patch also adds metric to track number of renderer tasks per 
task queue type.



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

to

==========
[scheduler] Add type to scheduler::TaskQueue

Match each TaskQueue to one of few hardcoded types for metric
collection.

This patch also adds metric to track number of renderer tasks per
task queue type.

BUG=

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

[commit-bot: I haz the power, 2016-10-21 20:14:06]

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