New issue
Advanced search Search tips

Issue 841172 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Break down TaskType::kNone by adding a 'default' task type to MainThreadTaskQueue

Project Member Reported by hajimehoshi@chromium.org, May 9 2018

Issue description

The current plan (we've not agreed yet though):

- Add a new values to distinguish task queues like v8, compositor to the existing TaskType enum.
- Add a comment to TaskType indicating not to use these values (except for MainThreadTaskQueue)
- Add DCHECK() to GetTaskRunner() in order not to accept the new values for task queues

 
https://chromium-review.googlesource.com/#/c/chromium/src/+/1051549

Created an experimental CL. I'll ask you after we agree the design.
s/ask you/ask your reviews/
Project Member

Comment 3 by bugdroid1@chromium.org, May 16 2018

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

commit c7761bb0a387dabf10ec04135d793def24b43353
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Wed May 16 09:14:25 2018

Add task types for MainThreadTaskQueue

Now tasks are posted with or without metadata (task type). Now UMA for
task durations per task types say >50% time is consumued by 'none' task
type, so we want to break down this 'none' task type usages.

We've already known that in most cases when tasks are posted without task
type, they are posted via MainTreadTaskQueue.

Bug:  841172 
Change-Id: I013a447dc5dd64776a9388523a24e658ff3b3146
Reviewed-on: https://chromium-review.googlesource.com/1051549
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559019}
[modify] https://crrev.com/c7761bb0a387dabf10ec04135d793def24b43353/third_party/blink/public/platform/task_type.h
[modify] https://crrev.com/c7761bb0a387dabf10ec04135d793def24b43353/third_party/blink/renderer/platform/scheduler/child/web_scheduler_impl.cc
[modify] https://crrev.com/c7761bb0a387dabf10ec04135d793def24b43353/third_party/blink/renderer/platform/scheduler/child/worker_scheduler.cc
[modify] https://crrev.com/c7761bb0a387dabf10ec04135d793def24b43353/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc
[modify] https://crrev.com/c7761bb0a387dabf10ec04135d793def24b43353/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
[modify] https://crrev.com/c7761bb0a387dabf10ec04135d793def24b43353/third_party/blink/renderer/platform/scheduler/renderer/renderer_web_scheduler_impl.cc
[modify] https://crrev.com/c7761bb0a387dabf10ec04135d793def24b43353/third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler.cc
[modify] https://crrev.com/c7761bb0a387dabf10ec04135d793def24b43353/tools/metrics/histograms/enums.xml

Project Member

Comment 4 by bugdroid1@chromium.org, May 17 2018

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

commit c70cdb314e37ae58980dd993433748d9c09e64dc
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu May 17 12:45:30 2018

Make MainThreadSchedulerImpl's *TaskRunner() return TaskQueueWithTaskType

This PR changes these functions to return TaskQueueWithTaskType
- MainThreadSchedulerImpl::DefaultTaskRunner()
- MainThreadSchedulerImpl::InputTaskRunner()
- MainThreadSchedulerImpl::IPCTaskRunner()
- IdleHelper::IdleTaskRunner()

so that we can know the task types posted to these main-thread task
queues, which didin't have any task-type information.

Bug:  841172 
Change-Id: I7c463437af1dd5478cbbfd39bdc1626ef2e5c3b3
Reviewed-on: https://chromium-review.googlesource.com/1063670
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559503}
[modify] https://crrev.com/c70cdb314e37ae58980dd993433748d9c09e64dc/third_party/blink/public/platform/task_type.h
[modify] https://crrev.com/c70cdb314e37ae58980dd993433748d9c09e64dc/third_party/blink/renderer/platform/scheduler/child/idle_helper.cc
[modify] https://crrev.com/c70cdb314e37ae58980dd993433748d9c09e64dc/third_party/blink/renderer/platform/scheduler/child/worker_scheduler.cc
[modify] https://crrev.com/c70cdb314e37ae58980dd993433748d9c09e64dc/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc
[modify] https://crrev.com/c70cdb314e37ae58980dd993433748d9c09e64dc/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
[modify] https://crrev.com/c70cdb314e37ae58980dd993433748d9c09e64dc/tools/metrics/histograms/enums.xml

Project Member

Comment 5 by bugdroid1@chromium.org, May 18 2018

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

commit 38a1c0c283dbfaab2ef08495710313c71ef091a2
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Fri May 18 09:50:35 2018

Use MainThreadSchedulerImpl::DefaultTaskRunner instead of DefaultTaskQueue at WebThreadImplForRendererScheduler

Now DefaultTaskRunner() returns TaskQueueWithTaskType, this should be
used for the task type annotation.

Bug:  841172 
Change-Id: If55175b5083094863f13bca52ae277e164418865
Reviewed-on: https://chromium-review.googlesource.com/1065534
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559849}
[modify] https://crrev.com/38a1c0c283dbfaab2ef08495710313c71ef091a2/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h
[modify] https://crrev.com/38a1c0c283dbfaab2ef08495710313c71ef091a2/third_party/blink/renderer/platform/scheduler/renderer/webthread_impl_for_renderer_scheduler.cc

Project Member

Comment 6 by bugdroid1@chromium.org, May 18 2018

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

commit 47827ec56830179ce0ccf3be7b7886488555b579
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Fri May 18 12:43:51 2018

Add TaskType::kMainThreadTaskQueueControl and use it for MainThreadScheduelrImpl::control_task_queue_

This adds TaskType information to post the control task queue usages in
DeadlineTaskRunner.

For the direct usage of |control_task_queue_| in MainThreadSchedulerImpl,
the task type is not added yet.

Bug:  841172 
Change-Id: I865336e0fce3988f476aa3bb3651e837d13eddaa
Reviewed-on: https://chromium-review.googlesource.com/1065951
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559872}
[modify] https://crrev.com/47827ec56830179ce0ccf3be7b7886488555b579/third_party/blink/public/platform/task_type.h
[modify] https://crrev.com/47827ec56830179ce0ccf3be7b7886488555b579/third_party/blink/renderer/platform/scheduler/child/worker_scheduler.cc
[modify] https://crrev.com/47827ec56830179ce0ccf3be7b7886488555b579/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc
[modify] https://crrev.com/47827ec56830179ce0ccf3be7b7886488555b579/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
[modify] https://crrev.com/47827ec56830179ce0ccf3be7b7886488555b579/tools/metrics/histograms/enums.xml

Project Member

Comment 7 by bugdroid1@chromium.org, May 24 2018

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

commit 212bdb7850c4cd1a0646fd21ae0d661ed2f1e4b3
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu May 24 10:25:37 2018

Re-add TaskType information for the main thread's V8/compositor task queues

It looks like the CL https://chromium-review.googlesource.com/c/chromium/src/+/1058990
lost the information. This CL adds the infromation again.

Bug:  841172 
Change-Id: I9eda1527dedb50c99c742140ad05c1848f37de6b
Reviewed-on: https://chromium-review.googlesource.com/1071159
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561452}
[modify] https://crrev.com/212bdb7850c4cd1a0646fd21ae0d661ed2f1e4b3/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc

Project Member

Comment 8 by bugdroid1@chromium.org, May 24 2018

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

commit 54209cdb4db9fc7c518ff89e55c2912da4de8c9a
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu May 24 11:32:45 2018

Add task types for compositor/worker default task runners

This CL adds TaskType information to post the compositor/worker task
runners.

Bug:  841172 
Change-Id: If1da5fefa86f18fb6523539eb953c1f769d202bd
Reviewed-on: https://chromium-review.googlesource.com/1068898
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561463}
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/public/platform/task_type.h
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/child/idle_helper_unittest.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/child/webthread_impl_for_worker_scheduler.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/child/webthread_impl_for_worker_scheduler.h
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/child/worker_scheduler.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/common/scheduler_helper.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/common/scheduler_helper.h
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/common/scheduler_helper_unittest.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/main_thread/auto_advancing_virtual_time_domain_unittest.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_helper.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/worker/compositor_thread_scheduler.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/worker/compositor_thread_scheduler.h
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler_helper.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler_helper.h
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.cc
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.h
[modify] https://crrev.com/54209cdb4db9fc7c518ff89e55c2912da4de8c9a/tools/metrics/histograms/enums.xml

Project Member

Comment 9 by bugdroid1@chromium.org, May 25 2018

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

commit b8531a72488de4eb7d9c6396472a3f88b7ab3e69
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Fri May 25 08:02:17 2018

Add TaskType::kWorkerThreadTaskQueueV8 and use it for workers

This CL fixes worker's isolate to use V8 task runner instead of worker
thread's default task runner.

Bug:  841172 ,  846255 
Change-Id: I3d462d0188f1860cad8b30ce083e24f9161afc84
Reviewed-on: https://chromium-review.googlesource.com/1071310
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561805}
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/public/platform/task_type.h
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/renderer/core/workers/worker_backing_thread.cc
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/renderer/platform/scheduler/child/worker_scheduler.cc
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/renderer/platform/scheduler/public/non_main_thread_scheduler.h
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/renderer/platform/scheduler/worker/compositor_thread_scheduler.cc
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/renderer/platform/scheduler/worker/compositor_thread_scheduler.h
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler.cc
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.cc
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.h
[modify] https://crrev.com/b8531a72488de4eb7d9c6396472a3f88b7ab3e69/tools/metrics/histograms/enums.xml

Project Member

Comment 10 by bugdroid1@chromium.org, May 25 2018

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

commit d5bd60f820f57b8c3326ee5993652100780013d1
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Fri May 25 13:32:04 2018

Add TaskType::kWorkerThreadTaskQueueCompositor and use it for compositors

Before this CL, null was passed to Mojo binding as a compositor task
runner in offscreen canvas on worker thread as NonMainThreadTaskRunner::
CompositorTaskRunner() returns null. In this case,
SingleThreadTaskRunner::Get() was used internally Mojo.

This CL implements WorkerThreadScheduler::CompositorTaskRunner() with
a new task type kWorkerThreadTaskQueueCompositor so that we can know the
task runner's usage on UMA for task duration per task type.

Bug:  841172 
Change-Id: I8caf3244169300aed45eeab4fcd01999bc823976
Reviewed-on: https://chromium-review.googlesource.com/1073169
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561847}
[modify] https://crrev.com/d5bd60f820f57b8c3326ee5993652100780013d1/third_party/blink/public/platform/task_type.h
[modify] https://crrev.com/d5bd60f820f57b8c3326ee5993652100780013d1/third_party/blink/renderer/platform/scheduler/child/worker_scheduler.cc
[modify] https://crrev.com/d5bd60f820f57b8c3326ee5993652100780013d1/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc
[modify] https://crrev.com/d5bd60f820f57b8c3326ee5993652100780013d1/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
[modify] https://crrev.com/d5bd60f820f57b8c3326ee5993652100780013d1/third_party/blink/renderer/platform/scheduler/public/non_main_thread_scheduler.h
[modify] https://crrev.com/d5bd60f820f57b8c3326ee5993652100780013d1/third_party/blink/renderer/platform/scheduler/worker/compositor_thread_scheduler.cc
[modify] https://crrev.com/d5bd60f820f57b8c3326ee5993652100780013d1/third_party/blink/renderer/platform/scheduler/worker/compositor_thread_scheduler.h
[modify] https://crrev.com/d5bd60f820f57b8c3326ee5993652100780013d1/third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler.cc
[modify] https://crrev.com/d5bd60f820f57b8c3326ee5993652100780013d1/third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.cc
[modify] https://crrev.com/d5bd60f820f57b8c3326ee5993652100780013d1/third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.h

Status: Fixed (was: Untriaged)
I think now all the thread-local task runner should have task types :-)

Sign in to add a comment