Chromium Code Reviews
Help | Chromium Project | Sign in
(12)

Issue 2778123003: [scheduler] Add WakeupBudgetPool.

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month ago by altimin
Modified:
5 hours, 27 minutes ago
Reviewers:
Sami, alex clarke
CC:
chromium-reviews, blink-reviews, kinuko+watch, scheduler-bugs_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[scheduler] Add WakeupBudgetPool. WakeupBudgetPool allows a short chain of fast continuation tasks to when immediately even when throttled (old throttling mechanism introduced a second delay between each pair). Please note that full task queue throttler integration is still missing (task queue throttler still inserts fences unconditionally) and will be added in a later patch. R=skyostil@chromium.org,alexclarke@chromium.org BUG=699541

Patch Set 1 #

Patch Set 2 : WIP #

Patch Set 3 : First meaningful version #

Total comments: 49

Patch Set 4 : Rebased #

Patch Set 5 : Second iteration. #

Total comments: 38

Patch Set 6 : addressed comments #

Total comments: 10

Patch Set 7 : Addressed comments from alexclarke@ #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+762 lines, -575 lines) Patch
M third_party/WebKit/Source/platform/BUILD.gn View 1 2 3 4 5 6 2 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scheduler/base/task_queue.h View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h View 1 2 3 4 2 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc View 1 2 5 6 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h View 1 2 3 4 5 6 5 chunks +29 lines, -102 lines 0 comments Download
M third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.cc View 1 2 3 4 5 6 4 chunks +15 lines, -168 lines 0 comments Download
M third_party/WebKit/Source/platform/scheduler/renderer/budget_pool_unittest.cc View 1 2 3 4 5 6 5 chunks +105 lines, -43 lines 0 comments Download
A + third_party/WebKit/Source/platform/scheduler/renderer/cpu_time_budget_pool.h View 1 2 3 4 5 6 3 chunks +17 lines, -79 lines 0 comments Download
A + third_party/WebKit/Source/platform/scheduler/renderer/cpu_time_budget_pool.cc View 1 2 3 4 5 6 7 chunks +29 lines, -86 lines 1 comment Download
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h View 1 2 3 4 5 3 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc View 1 2 3 4 5 5 chunks +20 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h View 1 2 3 4 5 6 6 chunks +32 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc View 1 2 3 4 5 6 8 chunks +164 lines, -82 lines 0 comments Download
M third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler_unittest.cc View 1 2 3 4 2 chunks +127 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.h View 1 2 3 4 5 2 chunks +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.cc View 1 2 3 4 5 1 chunk +7 lines, -0 lines 1 comment Download
A third_party/WebKit/Source/platform/scheduler/renderer/wake_up_budget_pool.h View 1 2 3 4 5 6 1 chunk +68 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/platform/scheduler/renderer/wake_up_budget_pool.cc View 1 2 3 4 5 6 1 chunk +125 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 28 (16 generated)
alex clarke
I think my thought is he TimeDomain::Observer interface is wrong. The Throttler is the only ...
1 month ago (2017-03-30 14:05:20 UTC) #1
altimin
PTAL. (I'll add some more tests tomorrow)
1 week, 2 days ago (2017-04-20 19:13:49 UTC) #2
alex clarke
https://codereview.chromium.org/2778123003/diff/40001/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h (right): https://codereview.chromium.org/2778123003/diff/40001/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h#newcode265 third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:265: base::Optional<EnqueueOrder> current_enqueue_order; This doesn't seem to be used? https://codereview.chromium.org/2778123003/diff/40001/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.cc ...
1 week, 1 day ago (2017-04-21 09:14:20 UTC) #7
altimin
PTAL. Please note that due to the current mechanism we can't block immediate task (InsertFencePosition::NOW ...
4 days, 9 hours ago (2017-04-25 13:22:36 UTC) #8
Sami
The wake up logic seems good to me. Sorry for all the dumb questions below ...
3 days, 10 hours ago (2017-04-26 13:09:08 UTC) #13
alex clarke
https://codereview.chromium.org/2778123003/diff/80001/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc File third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc (right): https://codereview.chromium.org/2778123003/diff/80001/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc#newcode1901 third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc:1901: task_queue_throttler()->CreateWakeUpBudgetPool("renderer-wake-up-pool"); I wonder if we should use a builder ...
3 days, 9 hours ago (2017-04-26 13:31:55 UTC) #14
altimin
PTAL https://codereview.chromium.org/2778123003/diff/80001/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.cc File third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.cc (right): https://codereview.chromium.org/2778123003/diff/80001/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.cc#newcode311 third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.cc:311: base::TimeDelta::FromMicroseconds(1); On 2017/04/26 13:09:08, Sami wrote: > Would ...
3 days, 8 hours ago (2017-04-26 14:31:30 UTC) #15
alex clarke
https://codereview.chromium.org/2778123003/diff/40001/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h File third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h (right): https://codereview.chromium.org/2778123003/diff/40001/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h#newcode220 third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h:220: class BLINK_PLATFORM_EXPORT WakeupBudgetPool : public BudgetPool { On 2017/04/25 ...
2 days, 12 hours ago (2017-04-27 10:28:14 UTC) #20
altimin
PTAL https://codereview.chromium.org/2778123003/diff/40001/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h File third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h (right): https://codereview.chromium.org/2778123003/diff/40001/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h#newcode220 third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h:220: class BLINK_PLATFORM_EXPORT WakeupBudgetPool : public BudgetPool { On ...
2 days, 12 hours ago (2017-04-27 11:07:41 UTC) #21
Sami
Thanks, couple of more questions below. Maybe I just need some ASCII art or a ...
2 days, 10 hours ago (2017-04-27 12:12:30 UTC) #22
altimin
https://codereview.chromium.org/2778123003/diff/80001/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc File third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc (right): https://codereview.chromium.org/2778123003/diff/80001/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc#newcode339 third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc:339: if (CanRunTasksUntil(queue, now, next_desired_run_time.value())) { On 2017/04/27 12:12:30, Sami ...
2 days, 10 hours ago (2017-04-27 12:23:51 UTC) #24
Sami
5 hours, 27 minutes ago (2017-04-29 17:43:02 UTC) #28
So it sounded like you guys converged on this approach after all? I think lg2me
now with some nits, but Alex please approve too.

https://codereview.chromium.org/2778123003/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/platform/scheduler/renderer/cpu_time_budget_pool.cc
(right):

https://codereview.chromium.org/2778123003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/renderer/cpu_time_budget_pool.cc:92:
return CanRunTasksAt(now, false);
I think this is slightly misleading: I would expect CanRanTasksUntil to return
true until the point at which the budget becomes negative. To do that we would
need to look at |moment| which we're not doing here. Should we do that?

https://codereview.chromium.org/2778123003/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.cc
(right):

https://codereview.chromium.org/2778123003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.cc:30:
next_task_ = run_time;
nit: next_task_run_time_
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46