[scheduler] Support adding one queue to multiple budget pools

[scheduler] Support adding one queue to multiple budget pools

This refactoring prepares for wakeup-based throttling and introduction of the WakeupBudgetPool.

R=skyostil@chromium.org,alexclarke@chromium.org
BUG=699541
Review-Url: https://codereview.chromium.org/2754853003
Cr-Commit-Position: refs/heads/master@{#457784}
Committed: https://chromium.googlesource.com/chromium/src/+/e6967d7ed906839db8acfdc287bfd209e481062c

[altimin, 2017-03-16 18:16:53]

Description was changed from

==========
[scheduler] Support one queue to multiple budget pools

R=skyostil@chromium.org,alexclarke@chromium.org
BUG=699541
==========

to

==========
[scheduler] Support adding one queue to multiple budget pools

R=skyostil@chromium.org,alexclarke@chromium.org
BUG=699541
==========

[altimin, 2017-03-16 18:17:09]

PTAL

[altimin, 2017-03-16 18:17:13]

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

[commit-bot: I haz the power, 2017-03-16 18:18:03]

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

[alex clarke (OOO till 29th), 2017-03-16 19:12:01]

lgtm

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

https://codereview.chromium.org/2754853003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc:329:
auto find_it = queue_details_.find(task_queue);
I tend to write exactly that too but I wonder if this should really be const
auto& find_it = ...  or we may get unexpected copy operations.

[altimin, 2017-03-16 19:14:26]

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

https://codereview.chromium.org/2754853003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc:329:
auto find_it = queue_details_.find(task_queue);
On 2017/03/16 19:12:01, alex clarke wrote:
> I tend to write exactly that too but I wonder if this should really be const
> auto& find_it = ...  or we may get unexpected copy operations.

It's an iterator, copying them is cheap.

[commit-bot: I haz the power, 2017-03-16 19:56:56]

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

[commit-bot: I haz the power, 2017-03-16 19:56:57]

Dry run: This issue passed the CQ dry run.

[Sami, 2017-03-17 11:18:39]

Btw the description could say a bit more about *why* we want to do this.

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

https://codereview.chromium.org/2754853003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc:179:
for (BudgetPool* budget_pool : find_it->second.budget_pools) {
budget_pools is being modified while we iterate it -- is that okay?

[altimin, 2017-03-17 14:20:35]

Description was changed from

==========
[scheduler] Support adding one queue to multiple budget pools

R=skyostil@chromium.org,alexclarke@chromium.org
BUG=699541
==========

to

==========
[scheduler] Support adding one queue to multiple budget pools

This refactoring prepares for wakeup-based throttling and introduction of the
WakeupBudgetPool.

R=skyostil@chromium.org,alexclarke@chromium.org
BUG=699541
==========

[altimin, 2017-03-17 14:33:55]

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

[commit-bot: I haz the power, 2017-03-17 14:34:14]

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

[altimin, 2017-03-17 14:35:42]

PTAL

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

https://codereview.chromium.org/2754853003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc:179:
for (BudgetPool* budget_pool : find_it->second.budget_pools) {
On 2017/03/17 11:18:39, Sami wrote:
> budget_pools is being modified while we iterate it -- is that okay?

Very well spotted, thanks. (Bigger problem is that iterator may have been
invalidated after the cycle).

[Sami, 2017-03-17 14:54:51]

Thanks, lgtm.

[altimin, 2017-03-17 14:55:22]

The CQ bit was unchecked by altimin@chromium.org

[altimin, 2017-03-17 14:55:22]

The CQ bit was checked by altimin@chromium.org

[altimin, 2017-03-17 14:55:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
alexclarke@chromium.org
Link to the patchset: https://codereview.chromium.org/2754853003/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2017-03-17 14:55: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, 2017-03-17 16:05:26]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489762522790220,
"parent_rev": "f074360f3b524bf3e0902d788910842a42aa78e6", "commit_rev":
"e6967d7ed906839db8acfdc287bfd209e481062c"}

[commit-bot: I haz the power, 2017-03-17 16:06:07]

Description was changed from

==========
[scheduler] Support adding one queue to multiple budget pools

This refactoring prepares for wakeup-based throttling and introduction of the
WakeupBudgetPool.

R=skyostil@chromium.org,alexclarke@chromium.org
BUG=699541
==========

to

==========
[scheduler] Support adding one queue to multiple budget pools

This refactoring prepares for wakeup-based throttling and introduction of the
WakeupBudgetPool.

R=skyostil@chromium.org,alexclarke@chromium.org
BUG=699541

Review-Url: https://codereview.chromium.org/2754853003
Cr-Commit-Position: refs/heads/master@{#457784}
Committed:
https://chromium.googlesource.com/chromium/src/+/e6967d7ed906839db8acfdc287bf...
==========

[commit-bot: I haz the power, 2017-03-17 16:06:08]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/e6967d7ed906839db8acfdc287bf...