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

Issue 746640 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Task

Blocked on:
issue 746642
issue 838443



Sign in to add a comment

Throttle delayable requests in the presence of non-delayable requests

Project Member Reported by devdeepray@chromium.org, Jul 19 2017

Issue description

Non-delayable requests in the later part of the page load process do not result in throttling of delayable requests. Create an experiment to throttle delayable requests in the presence of non-delayable requests after the layout blocking phase.
 
Blockedon: 746642
Separately from  Issue 746642 , we should also experiment with somehow accounting for the in-flight non-delayable requests when starting a delayable request. I believe you already have an in-progress CL for that.
Status: Started (was: Assigned)
Components: Speed>Metrics
Labels: -Performance Performance-Data
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 2 2017

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

commit ca9665cb956163fd3cee387f77312c2e45db30fc
Author: Devdeep Ray <devdeepray@chromium.org>
Date: Wed Aug 02 19:27:54 2017

Change kMaxNumDelayableRequestsPerClient variable name in unit tests

Change kMaxNumDelayableRequestsPerClient to
kDefaultMaxNumDelayableRequestsPerClient to be consistent with the
naming in cc.

Bug: 746640
Change-Id: I5d85e93099e92c437c61e87038a81d6296d327bd
Reviewed-on: https://chromium-review.googlesource.com/598493
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Devdeep Ray <devdeepray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491465}
[modify] https://crrev.com/ca9665cb956163fd3cee387f77312c2e45db30fc/content/browser/loader/resource_scheduler_unittest.cc

Comment 6 by bengr@chromium.org, Aug 4 2017

Labels: -Pri-1 Pri-3
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 8 2017

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

commit 86d04e2302d3c385815a8f53ca18ba3d1294a163
Author: Devdeep Ray <devdeepray@chromium.org>
Date: Tue Aug 08 05:50:57 2017

Throttle delayable requests when non-delayable requests are in-flight.

This CL implements the throttling of delayable requests whenever there
are non-delayable requests in-flight after the layout blocking phase.
Currently, a limit of 10 is imposed on delayable requests, but all
non-delayable requests are let through, which results in more than
10 requests being in flight simultaneously.

Design doc: https://docs.google.com/document/d/1pg5B4WoNfr1x2P9Xa3nfp4ggGNMBNnVjFxAGKbOhnDA/edit?usp=sharing

Bug: 746640
Change-Id: I22f8141281b05c2c849255d874e4dc6f3d51f94b
Reviewed-on: https://chromium-review.googlesource.com/585558
Commit-Queue: Devdeep Ray <devdeepray@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Ben Greenstein <bengr@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492544}
[modify] https://crrev.com/86d04e2302d3c385815a8f53ca18ba3d1294a163/content/browser/loader/resource_scheduler.cc
[modify] https://crrev.com/86d04e2302d3c385815a8f53ca18ba3d1294a163/content/browser/loader/resource_scheduler.h
[modify] https://crrev.com/86d04e2302d3c385815a8f53ca18ba3d1294a163/content/browser/loader/resource_scheduler_unittest.cc
[modify] https://crrev.com/86d04e2302d3c385815a8f53ca18ba3d1294a163/tools/metrics/histograms/histograms.xml

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 9 2017

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

commit fd6af04d6af3094509bd656b727e8d346698b9da
Author: Devdeep Ray <devdeepray@chromium.org>
Date: Wed Aug 09 16:38:48 2017

Add field trial test config for NonDelayableThrottlesDelayable.

Bug: 746640
Change-Id: Ib10b3c88c74d8807ae632bb60d544873d6971760
Reviewed-on: https://chromium-review.googlesource.com/602517
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Commit-Queue: Devdeep Ray <devdeepray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493020}
[modify] https://crrev.com/fd6af04d6af3094509bd656b727e8d346698b9da/testing/variations/fieldtrial_testing_config.json

Cc: -tbansal@chromium.org devdeepray@chromium.org
Owner: tbansal@chromium.org
Labels: -Performance-Data
Components: -Speed>Metrics
Labels: -Pri-3 M-62 Pri-2
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 20 2017

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

commit ddcdefdb64ce869b4c7e7cacc43869b854e98837
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Dec 20 23:43:51 2017

ResourceScheduler: Allow different params for different ECTs for ThrottleDelayable experiment

Currently, it is possible to set at most one value of Effective
Connection Type (ECT), and at most one value of loading
parameter (max. delayable requests) for one experiment group.

This CL changes it to allow multiple ranges of ECTs for one
experiment group.

The ability to limit the experiment to a range of BDPs
has been removed, and has been replaced by the ability
to replace the experiment to a range of ECTs.

Allow different params for different ECTs

Bug: 746640
Change-Id: I6e00813b69b81e05f55e67dc234433c8c7e18880
Reviewed-on: https://chromium-review.googlesource.com/820635
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525517}
[modify] https://crrev.com/ddcdefdb64ce869b4c7e7cacc43869b854e98837/content/browser/loader/resource_scheduler.cc
[modify] https://crrev.com/ddcdefdb64ce869b4c7e7cacc43869b854e98837/content/browser/loader/resource_scheduler.h
[modify] https://crrev.com/ddcdefdb64ce869b4c7e7cacc43869b854e98837/content/browser/loader/resource_scheduler_unittest.cc
[modify] https://crrev.com/ddcdefdb64ce869b4c7e7cacc43869b854e98837/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 13 2018

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

commit 02472a209454a042e6a248de62f324e0b8c353ad
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Mar 13 07:04:07 2018

Resource Scheduler: Enable ThrottleDelayable experiment

This experiment throttles low priority requests based on the
count of high priority requests and the current network quality.
The experiment changes the scheduling behavior only when
the current effective connection type is Slow2G or 2G.

The code to adapt the scheduling behavior based on ECT
is still there in the ResourceScheduler in case we need to
revert it or update the params using finch.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I77ba950344e6ff8e05a83ebb0dc5b48cec6ff3c8
Bug: 746640
Reviewed-on: https://chromium-review.googlesource.com/955703
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542742}
[modify] https://crrev.com/02472a209454a042e6a248de62f324e0b8c353ad/services/network/resource_scheduler.cc
[modify] https://crrev.com/02472a209454a042e6a248de62f324e0b8c353ad/services/network/resource_scheduler_unittest.cc
[modify] https://crrev.com/02472a209454a042e6a248de62f324e0b8c353ad/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 22 2018

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

commit ff70408ded42ef11319a1079fa544129b2ceb6c4
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Mar 22 20:56:24 2018

Enable ThrottleDelayable experiment by default

This experiment throttles low priority requests based on the
count of high priority requests and the current network quality.
The experiment changes the scheduling behavior only when
the current effective connection type is Slow2G or 2G.

The code to adapt the scheduling behavior based on ECT
is still there in the ResourceScheduler in case we need to
revert it or update the params using finch.

A previous CL had set the default parameters for the field
trial, but did not enable the feature. This CL enables
the feature by default so that the field trial
configs can be removed.

Bug: 746640
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ib65c03e7fe8cdbbfd2d6b1a1d8772f8837870152
Reviewed-on: https://chromium-review.googlesource.com/969105
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545243}
[modify] https://crrev.com/ff70408ded42ef11319a1079fa544129b2ceb6c4/services/network/resource_scheduler.cc
[modify] https://crrev.com/ff70408ded42ef11319a1079fa544129b2ceb6c4/services/network/resource_scheduler_unittest.cc

Status: Fixed (was: Started)
This experiment is now enabled for all users on Slow2G and 2G connections.
Components: -Internals>Network Internals>Network>NetworkQuality
Labels: -M-62 M-65
Status: Started (was: Fixed)
Need to do this for 4G ECT now.
Project Member

Comment 19 by bugdroid1@chromium.org, Apr 23 2018

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

commit 69c98ffced8e38364bf9c1356f12799269b8e7d5
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Apr 23 18:37:40 2018

Add ThrottleDelayable 4G params to fieldtrial_testing_config.json

Bug: 746640
Change-Id: Ie312a7756f5e6917a4d7c2bd107951f4928e97d0
Reviewed-on: https://chromium-review.googlesource.com/1024054
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552765}
[modify] https://crrev.com/69c98ffced8e38364bf9c1356f12799269b8e7d5/testing/variations/fieldtrial_testing_config.json

Blockedon: 838443
tbansal, this bug has had many CLs land and it's not clear what work is left, could you write a comment for what is outstanding here? And update the milestone if needed.
Project Member

Comment 22 by bugdroid1@chromium.org, Jun 27 2018

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

commit 5796dc835bab03d84b1c96a465ab23944a21cdc9
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Jun 27 20:02:30 2018

Revert "Add ThrottleDelayable 4G params to fieldtrial_testing_config.json"

This reverts commit 69c98ffced8e38364bf9c1356f12799269b8e7d5.

Reason for revert: The field trial experiment was turned down since
it did not show any improvement (see http://shortn/_3N4YH2mKyq). Besides, the perf bots showed a
regression.

Original change's description:
> Add ThrottleDelayable 4G params to fieldtrial_testing_config.json
> 
> Bug: 746640
> Change-Id: Ie312a7756f5e6917a4d7c2bd107951f4928e97d0
> Reviewed-on: https://chromium-review.googlesource.com/1024054
> Reviewed-by: Jesse Doherty <jwd@chromium.org>
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#552765}

TBR=jwd@chromium.org,tbansal@chromium.org

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

Bug: 746640,  838443 
Change-Id: I349ed4bdafedeadb8e80af332472937babb7b820
Reviewed-on: https://chromium-review.googlesource.com/1117220
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570875}
[modify] https://crrev.com/5796dc835bab03d84b1c96a465ab23944a21cdc9/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 23 by bugdroid1@chromium.org, Jul 6

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

commit c5994471a7f8e7328318d5bdae37e144a18fe4d0
Author: Tarun Bansal <tbansal@chromium.org>
Date: Fri Jul 06 23:03:25 2018

Update field trial params for throttle delayable experiment.

Update field trial params for throttle delayable experiment
for 3G effective connection type.

Bug: 746640
Change-Id: Ia492ec4ed512b9b99014228b15496f2e2068c00f
Reviewed-on: https://chromium-review.googlesource.com/1128242
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573107}
[modify] https://crrev.com/c5994471a7f8e7328318d5bdae37e144a18fe4d0/testing/variations/fieldtrial_testing_config.json

Any updates?
Refreshed during triage
Refreshed during triage.
Labels: -Pri-2 Pri-3
Refreshed during triage.

Sign in to add a comment