Enable to call ExecuteAfterCurrentTask for NonMainThreadSchedulerImpl [chromium/src : main]
Hajime Hoshi (Gerrit)
Attention is currently required from: Alexander Timin.
Hajime Hoshi would like Alexander Timin to review this change.
Enable to call ExecuteAfterCurrentTask for NonMainThreadSchedulerImpl
This CL enables NonMainThreadSchedulerImpl to batch notifying feature
usages from workers, like what FrameSchedulerImpl and
MainThreadSchedulerImpl do.
This is a preparation to detect features not only from a frame but also
a dedicated worker.
Design Doc: https://docs.google.com/document/d/1CJfDNqA2tEp_dmxK5kaBHHqGjx7cK2_jhTpsCwSCa3A/edit?usp=sharing
Bug: 1146955
Change-Id: I8fea584e5924de26ef8f10e5905a38dbd650ce10
---
M third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler_impl.cc
M third_party/blink/renderer/platform/scheduler/common/thread_scheduler_impl.h
M third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
M third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h
M third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.cc
M third_party/blink/renderer/platform/scheduler/worker/compositor_thread_scheduler.cc
M third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler_impl.h
7 files changed, 67 insertions(+), 13 deletions(-)
To view, visit change 3233543. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Alexander Timin.
1 comment:
Patchset:
PTAL
To view, visit change 3233543. To unsubscribe, or for help writing mail filters, visit settings.
Alexander Timin (Gerrit)
Attention is currently required from: Hajime Hoshi, Scott Haseley.
Alexander Timin would like Scott Haseley to review this change authored by Hajime Hoshi.
Enable to call ExecuteAfterCurrentTask for NonMainThreadSchedulerImpl
This CL enables NonMainThreadSchedulerImpl to batch notifying feature
usages from workers, like what FrameSchedulerImpl and
MainThreadSchedulerImpl do.
This is a preparation to detect features not only from a frame but also
a dedicated worker.
Design Doc: https://docs.google.com/document/d/1CJfDNqA2tEp_dmxK5kaBHHqGjx7cK2_jhTpsCwSCa3A/edit?usp=sharing
Bug: 1146955
Change-Id: I8fea584e5924de26ef8f10e5905a38dbd650ce10
---
M third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler_impl.cc
M third_party/blink/renderer/platform/scheduler/common/thread_scheduler_impl.h
M third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
M third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h
M third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.cc
M third_party/blink/renderer/platform/scheduler/worker/compositor_thread_scheduler.cc
M third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler_impl.h
7 files changed, 67 insertions(+), 13 deletions(-)
To view, visit change 3233543. To unsubscribe, or for help writing mail filters, visit settings.
Alexander Timin (Gerrit)
Attention is currently required from: Hajime Hoshi, Scott Haseley.
1 comment:
Patchset:
+Scott here as well.
To view, visit change 3233543. To unsubscribe, or for help writing mail filters, visit settings.
Scott Haseley (Gerrit)
Attention is currently required from: Hajime Hoshi.
3 comments:
File third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h:
Patch Set #4, Line 1017:
WTF::Vector<base::OnceClosure> on_task_completion_callbacks_;Moving this outside of main_thread_only() removes the thread check. Is that safe to do?
File third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler_impl.cc:
Patch Set #4, Line 135:
void NonMainThreadSchedulerImpl::ExecuteAfterCurrentTask(I wonder if this, DispatchOnTaskCompletionCallbacks, and |on_task_completion_callbacks_| should be defined in thread_scheduer_impl.cc (doesn't yet exist)? This code looks duplicated between here and MainThreadSchedulerImpl.
File third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.cc:
Patch Set #4, Line 207:
NonMainThreadSchedulerImpl::OnTaskCompleted(task_queue, task, task_timing,MainThreadSchedulerImpl runs after-task callbacks after the microtask checkpoint and recording the task end time. Should they be the same? I think we'd want to have this after task_timing->RecordTaskEnd() since changes the task_timing, which is passed to the callbacks.
To view, visit change 3233543. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Scott Haseley.
4 comments:
Patchset:
Thank you!
File third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h:
Patch Set #4, Line 1017:
WTF::Vector<base::OnceClosure> on_task_completion_callbacks_;
Moving this outside of main_thread_only() removes the thread check. […]
Done (Move this back to main_thread_onlly())
File third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler_impl.cc:
Patch Set #4, Line 135:
void NonMainThreadSchedulerImpl::ExecuteAfterCurrentTask(
I wonder if this, DispatchOnTaskCompletionCallbacks, and |on_task_completion_callbacks_| should be d […]
Done
File third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.cc:
Patch Set #4, Line 207:
NonMainThreadSchedulerImpl::OnTaskCompleted(task_queue, task, task_timing,
MainThreadSchedulerImpl runs after-task callbacks after the microtask checkpoint and recording the t […]
Done
To view, visit change 3233543. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Scott Haseley.
Patch set 8:Commit-Queue +1
Scott Haseley (Gerrit)
Attention is currently required from: Hajime Hoshi.
Patch set 8:Code-Review +1
2 comments:
Patchset:
lgtm w/ 1 nit. Thanks!
File third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h:
Patch Set #8, Line 471:
WTF::Vector<base::OnceClosure>& GetOnTaskCompletionCallbacks() override;nit: should this be protected to match parent class and non_main_thread_scheduler_impl? Not sure if this is intentional. I don't think it matters in practice because this class isn't subclassed.
To view, visit change 3233543. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
1 comment:
File third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h:
Patch Set #8, Line 471:
WTF::Vector<base::OnceClosure>& GetOnTaskCompletionCallbacks() override;
nit: should this be protected to match parent class and non_main_thread_scheduler_impl? Not sure if […]
Done
To view, visit change 3233543. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Hajime Hoshi.
Patch set 14:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
8 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h
Insertions: 3, Deletions: 3.
The diff is too large to show. Please review the diff.
```
Enable to call ExecuteAfterCurrentTask for NonMainThreadSchedulerImpl
This CL enables NonMainThreadSchedulerImpl to batch notifying feature
usages from workers, like what FrameSchedulerImpl and
MainThreadSchedulerImpl do.
This is a preparation to detect features not only from a frame but also
a dedicated worker.
Design Doc: https://docs.google.com/document/d/1CJfDNqA2tEp_dmxK5kaBHHqGjx7cK2_jhTpsCwSCa3A/edit?usp=sharing
Bug: 1146955
Change-Id: I8fea584e5924de26ef8f10e5905a38dbd650ce10
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3233543
Commit-Queue: Hajime Hoshi <hajim...@chromium.org>
Reviewed-by: Scott Haseley <shas...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#935808}
---
M third_party/blink/renderer/platform/scheduler/BUILD.gn
M third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler_impl.cc
A third_party/blink/renderer/platform/scheduler/common/thread_scheduler_impl.cc
M third_party/blink/renderer/platform/scheduler/common/thread_scheduler_impl.h
M third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
M third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h
M third_party/blink/renderer/platform/scheduler/worker/worker_thread_scheduler.cc
M third_party/blink/renderer/platform/scheduler/worker/compositor_thread_scheduler.cc
M third_party/blink/renderer/platform/scheduler/worker/non_main_thread_scheduler_impl.h
9 files changed, 81 insertions(+), 22 deletions(-)