Scheduler: Extend resource fetch priorites experiment to the loading use case. |
|||||
Issue descriptionExtend the resource fetch priorities experiment to support being enabled in the loading use case only.
,
Aug 3
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
,
Aug 3
Could you pls point the CL? How is the cl looking in canary?
,
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
,
Aug 3
Pls update the bug with canary result on Monday.
,
Aug 3
Sure, Thank you!
,
Aug 6
Update: No Crashes has been reported yet, and this has been up and running since Friday.
,
Aug 6
The NextAction date has arrived: 2018-08-06
,
Aug 6
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.
,
Aug 6
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
,
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
,
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
,
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
,
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
,
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
,
Sep 26
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 |
|||||
Comment 1 by farahcharab@chromium.org
, Aug 3