New issue
Advanced search Search tips

Issue 757535 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Delayable requests may get blocked behind 2 non-delayable hanging GETs

Project Member Reported by tbansal@chromium.org, Aug 21 2017

Issue description

If Chrome is in MaxDelayableRequestsNetworkOverride experiment, then it is possible that the max. number of delayable in-flight requests is lowered down to a value of say 4 from the default value of 10.

If Chrome also happens to be in NonDelayableThrottlesDelayable experiment, then the non-delayable requests may further aggresively throttle delayable requests. For example, if weight parameter is set to 3.0, then the max. number of delayable requests is lowered by 3 for each non-delayable request in-flight.

If Chrome happens to be in both experiments, then it is possible that at most 4 delayable in-flight requests are allowed, and if there are 2 non-delayable requests in-flight, then delayable requests would be completely throttled (since 4 - 2 * 3.0  = -2). This can cause the page load to block if the 2 non-delayable requests to be hanging GETs.

To reduce the chances of page load completely blocking, we should completely throttle delayable requests only when there are at least 4 requests in-flight.
 
Components: Internals>Network>NetworkQuality
Project Member

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

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

commit e45b03432be14890319d4f25029ea48e149c339b
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Aug 22 17:32:25 2017

Combine the two resource scheduler experiments for throttling delayable
requests.

Combine the two resource scheduler experiments
(MaxDelayableRequestsNetworkOverride and 
NonDelayableThrottlesDelayable) for throttling delayable
requests since the two experiments affect each other,
and can only be analysed together.

Bug:  757535 
Change-Id: I017823360905f48c4b42509d7953f60b3e9c3698
Reviewed-on: https://chromium-review.googlesource.com/622952
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496349}
[modify] https://crrev.com/e45b03432be14890319d4f25029ea48e149c339b/content/browser/loader/resource_scheduler.cc
[modify] https://crrev.com/e45b03432be14890319d4f25029ea48e149c339b/content/browser/loader/resource_scheduler.h
[modify] https://crrev.com/e45b03432be14890319d4f25029ea48e149c339b/content/browser/loader/resource_scheduler_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 23 2017

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

commit 81f5a4523b828bf706efa7ca876563da83b11e06
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Aug 23 03:56:23 2017

Merge the config for the two resource scheduler experiments

Bug:  757535 
Change-Id: I87a997cb848a948e674be9293f85aa0c70198fed
Reviewed-on: https://chromium-review.googlesource.com/627089
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496581}
[modify] https://crrev.com/81f5a4523b828bf706efa7ca876563da83b11e06/testing/variations/fieldtrial_testing_config.json

This is fixed in Chromium, but I still need to fix the configs.
Status: Fixed (was: Started)

Sign in to add a comment