New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 870349 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-08-06
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Task



Sign in to add a comment

Scheduler: Extend resource fetch priorites experiment to the loading use case.

Project Member Reported by farahcharab@chromium.org, Aug 2

Issue description

Extend the resource fetch priorities experiment to support being enabled in the loading use case only.

 
Labels: Merge-Request-69
This is a non-risky change. The CL only adds more configuration for an experiment already existing on Beta. In particular, we want to extend this experiment to the loading phase as well, so we added some additional configs 
for that. 


Project Member

Comment 2 by sheriffbot@chromium.org, Aug 3

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Could you pls point the CL? How is the cl looking in canary?
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 3

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

commit bac05e51eb8937e213689a3e190acd697467eef6
Author: Farah Charab <farahcharab@google.com>
Date: Fri Aug 03 19:45:25 2018

Scheduler: Extend resource fetch priority experiment to loading only.

Extend resource fetch priority experiment to support being enabled
only in the loading use case.

Bug: 870349
Change-Id: I980f078f3a48bc7ab240e475869125d6dc9ffa55
Reviewed-on: https://chromium-review.googlesource.com/1159064
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Farah Charab <farahcharab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580629}
[modify] https://crrev.com/bac05e51eb8937e213689a3e190acd697467eef6/testing/variations/fieldtrial_testing_config.json
[modify] https://crrev.com/bac05e51eb8937e213689a3e190acd697467eef6/third_party/blink/renderer/platform/scheduler/child/features.h
[modify] https://crrev.com/bac05e51eb8937e213689a3e190acd697467eef6/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc
[modify] https://crrev.com/bac05e51eb8937e213689a3e190acd697467eef6/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl_unittest.cc
[modify] https://crrev.com/bac05e51eb8937e213689a3e190acd697467eef6/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
[modify] https://crrev.com/bac05e51eb8937e213689a3e190acd697467eef6/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h

NextAction: 2018-08-06
Pls update the bug with canary result on Monday.
Sure, Thank you!
Update: No Crashes has been reported yet, and this has been up and running since Friday. 


The NextAction date has arrived: 2018-08-06
Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #1, #7 and per offline chat with farahcharab@. Pls merge before 4:00 PM PT today, Monday so we can pick it up for this week M69 Beta release. Thank you.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 6

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/923afe907d243243d0b6511fe995b5c3fdbfff73

commit 923afe907d243243d0b6511fe995b5c3fdbfff73
Author: Alexander Timin <altimin@chromium.org>
Date: Mon Aug 06 15:42:59 2018

Scheduler: Extend resource fetch priority experiment to loading only.

Extend resource fetch priority experiment to support being enabled
only in the loading use case.

TBR=farahcharab@google.com

(cherry picked from commit d2bb65ad48bfd139afeedaaa06c61f13b77e1c8f)

Bug: 870349
Change-Id: I980f078f3a48bc7ab240e475869125d6dc9ffa55
Reviewed-on: https://chromium-review.googlesource.com/1159064
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Farah Charab <farahcharab@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#580629}
Reviewed-on: https://chromium-review.googlesource.com/1163712
Cr-Commit-Position: refs/branch-heads/3497@{#418}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/923afe907d243243d0b6511fe995b5c3fdbfff73/testing/variations/fieldtrial_testing_config.json
[modify] https://crrev.com/923afe907d243243d0b6511fe995b5c3fdbfff73/third_party/blink/renderer/platform/scheduler/child/features.h
[modify] https://crrev.com/923afe907d243243d0b6511fe995b5c3fdbfff73/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc
[modify] https://crrev.com/923afe907d243243d0b6511fe995b5c3fdbfff73/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl_unittest.cc
[modify] https://crrev.com/923afe907d243243d0b6511fe995b5c3fdbfff73/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
[modify] https://crrev.com/923afe907d243243d0b6511fe995b5c3fdbfff73/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 28

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

commit d2bade563869c5a429d129695ce754ca0f5e9507
Author: Farah Charab <farahcharab@google.com>
Date: Tue Aug 28 16:58:26 2018

Scheduler: WebURLLoader should notify Blink with the initial priority.

The loading stack only notifies Blink with any increase in the
request priority. Add support to notify the scheduler with the
initial priority as well.

Bug: 870349
Change-Id: Ia578c4ec1297f56ecd5c853fdda4b9fbd93e0e9b
Reviewed-on: https://chromium-review.googlesource.com/1185593
Commit-Queue: Farah Charab <farahcharab@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586752}
[modify] https://crrev.com/d2bade563869c5a429d129695ce754ca0f5e9507/content/renderer/loader/web_url_loader_impl.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 28

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

commit 02a6a20203357b6d445e34a63c5547d9b42b2a35
Author: Farah Charab <farahcharab@google.com>
Date: Tue Aug 28 18:01:59 2018

UMA: Records the number of resource loading tasks split by net priority.

Records the number of resource loading tasks split by net priority. Recorded
each time a resource's request priority changes.

Bug: 870349
Change-Id: Idef96f2e9e0a08045387b8bf78874727b2deeb91
Reviewed-on: https://chromium-review.googlesource.com/1185596
Reviewed-by: Misha Efimov <mef@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Farah Charab <farahcharab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586783}
[modify] https://crrev.com/02a6a20203357b6d445e34a63c5547d9b42b2a35/net/base/request_priority.h
[modify] https://crrev.com/02a6a20203357b6d445e34a63c5547d9b42b2a35/third_party/blink/renderer/platform/scheduler/main_thread/DEPS
[modify] https://crrev.com/02a6a20203357b6d445e34a63c5547d9b42b2a35/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc
[modify] https://crrev.com/02a6a20203357b6d445e34a63c5547d9b42b2a35/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_metrics_helper.cc
[modify] https://crrev.com/02a6a20203357b6d445e34a63c5547d9b42b2a35/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_task_queue.cc
[modify] https://crrev.com/02a6a20203357b6d445e34a63c5547d9b42b2a35/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_task_queue.h
[modify] https://crrev.com/02a6a20203357b6d445e34a63c5547d9b42b2a35/third_party/blink/renderer/platform/scheduler/main_thread/resource_loading_task_runner_handle_impl.cc
[modify] https://crrev.com/02a6a20203357b6d445e34a63c5547d9b42b2a35/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/02a6a20203357b6d445e34a63c5547d9b42b2a35/tools/metrics/histograms/histograms.xml

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 30

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

commit 4603b56a8a9bf3b9e57d168d7ffbd71609bace34
Author: Farah Charab <farahcharab@google.com>
Date: Thu Aug 30 23:23:03 2018

UMA Metric: Record the number of resource loading tasks split by priority.

Records the number of resource loading tasks split by priority. Recorded
each time a task is executed.

Bug: 870349
Change-Id: Ic643cf1c5a578017c5cc8a78471c3ea7ebfc617d
Reviewed-on: https://chromium-review.googlesource.com/1185199
Commit-Queue: Farah Charab <farahcharab@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587836}
[modify] https://crrev.com/4603b56a8a9bf3b9e57d168d7ffbd71609bace34/base/task/sequence_manager/task_queue.h
[modify] https://crrev.com/4603b56a8a9bf3b9e57d168d7ffbd71609bace34/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_metrics_helper.cc
[modify] https://crrev.com/4603b56a8a9bf3b9e57d168d7ffbd71609bace34/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/4603b56a8a9bf3b9e57d168d7ffbd71609bace34/tools/metrics/histograms/histograms.xml

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 11

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

commit 3ce3af5c962d5692f2979846628f3c4b8ceb3e30
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Tue Sep 11 06:49:33 2018

Revert "Scheduler: WebURLLoader should notify Blink with the initial priority."

This reverts commit d2bade563869c5a429d129695ce754ca0f5e9507.

Reason for revert: Speculative revert for test failure.
https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVy1QELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCKeAWNocm9taXVtLm1hYy9NYWMxMC4xMSBUZXN0cy8yODg5My9icm93c2VyX3Rlc3RzIG9uIChub25lKSBHUFUgb24gTWFjIG9uIE1hYy0xMC4xMS9UbUYyYVdkaGRHbHVaMFY0ZEdWdWMybHZibEJ2Y0hWd1FuSnZkM05sY2xSbGMzUXVVR0ZuWlVsdVUyRnRaVVY0ZEdWdWMybHZiZz09DAsSE01hc3RlckZsYWtlQW5hbHlzaXMYAQw

Original change's description:
> Scheduler: WebURLLoader should notify Blink with the initial priority.
>
> The loading stack only notifies Blink with any increase in the
> request priority. Add support to notify the scheduler with the
> initial priority as well.
>
> Bug: 870349
> Change-Id: Ia578c4ec1297f56ecd5c853fdda4b9fbd93e0e9b
> Reviewed-on: https://chromium-review.googlesource.com/1185593
> Commit-Queue: Farah Charab <farahcharab@chromium.org>
> Reviewed-by: Alexander Timin <altimin@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#586752}

TBR=kinuko@chromium.org,altimin@chromium.org,farahcharab@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 870349, 882200
Change-Id: Id010e1ce5f2c4374f3f89208567fe4e344399051
Reviewed-on: https://chromium-review.googlesource.com/1218263
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590218}
[modify] https://crrev.com/3ce3af5c962d5692f2979846628f3c4b8ceb3e30/content/renderer/loader/web_url_loader_impl.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Sep 12

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

commit 9fbd6bbc97324bc72d8fcb7bb7a846399517fdee
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Wed Sep 12 02:22:52 2018

Reland "Scheduler: WebURLLoader should notify Blink with the initial priority."

This reverts commit 3ce3af5c962d5692f2979846628f3c4b8ceb3e30.

Reason for revert: Revert didn't fix the flakiness.

Original change's description:
> Revert "Scheduler: WebURLLoader should notify Blink with the initial priority."
> 
> This reverts commit d2bade563869c5a429d129695ce754ca0f5e9507.
> 
> Reason for revert: Speculative revert for test failure.
> https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVy1QELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCKeAWNocm9taXVtLm1hYy9NYWMxMC4xMSBUZXN0cy8yODg5My9icm93c2VyX3Rlc3RzIG9uIChub25lKSBHUFUgb24gTWFjIG9uIE1hYy0xMC4xMS9UbUYyYVdkaGRHbHVaMFY0ZEdWdWMybHZibEJ2Y0hWd1FuSnZkM05sY2xSbGMzUXVVR0ZuWlVsdVUyRnRaVVY0ZEdWdWMybHZiZz09DAsSE01hc3RlckZsYWtlQW5hbHlzaXMYAQw
> 
> Original change's description:
> > Scheduler: WebURLLoader should notify Blink with the initial priority.
> >
> > The loading stack only notifies Blink with any increase in the
> > request priority. Add support to notify the scheduler with the
> > initial priority as well.
> >
> > Bug: 870349
> > Change-Id: Ia578c4ec1297f56ecd5c853fdda4b9fbd93e0e9b
> > Reviewed-on: https://chromium-review.googlesource.com/1185593
> > Commit-Queue: Farah Charab <farahcharab@chromium.org>
> > Reviewed-by: Alexander Timin <altimin@chromium.org>
> > Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#586752}
> 
> TBR=kinuko@chromium.org,altimin@chromium.org,farahcharab@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 870349, 882200
> Change-Id: Id010e1ce5f2c4374f3f89208567fe4e344399051
> Reviewed-on: https://chromium-review.googlesource.com/1218263
> Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#590218}

TBR=kinuko@chromium.org,yhirano@chromium.org,altimin@chromium.org,farahcharab@chromium.org

Change-Id: I1b363f0f4c7396ccf1cdc7b67b436602e8738df3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 870349, 882200
Reviewed-on: https://chromium-review.googlesource.com/1220409
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590576}
[modify] https://crrev.com/9fbd6bbc97324bc72d8fcb7bb7a846399517fdee/content/renderer/loader/web_url_loader_impl.cc

Hi, we recently observed that this feature could improve FCP of loading.desktop/TheVerge_warm on multiple ChromeOS devices by ~70%, which is really great, but it's not enabled in official build yet. I was wondering if you have a schedule to turn it on in official build? 
Thank you!

Sign in to add a comment