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

Issue 600839 link

Starred by 4 users

Issue metadata

Status: WontFix
Owner: ----
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Move throttling done by ResourceScheduler down into the network stack

Project Member Reported by rdsmith@chromium.org, Apr 5 2016

Issue description

The network stack has much more information about how much throttling is required, and may (eventually) adaptively throttle requests based on network resources available.  Putting throttling in the network stack also allows it to be applied after the cache is probed for the presence of any resources.

See https://docs.google.com/document/d/1p-XGz-BFFehs5j2olbkDkjrXzxzT3n45PVcLSZzOV3c/edit for more details on this effort.

 

Comment 1 by allada@chromium.org, Apr 23 2016

Cc: allada@chromium.org
This issue is related to:  http://crbug.com/563644 
Would you say how?  I suspect I should be taking that issue into account in my work, and at the moment I'm not clear as to what the right way to do that is.  Is there a design doc for the work described in  issue 563644 ?

A straw-person argument against the relationship: As I understand devtools throttling, it's increasing latency and limiting bandwidth on a per-request level to whatever the target network is estimated to have.  That doesn't currently affect the choices that are made as to whether or not to dispatch network requests.  Eventually I want to look at how to adaptively throttle based on experienced network bandwidth, and I should make sure that that adaptive throttle includes devtools throttling, but that's work not covered by this bug.

Comment 3 by allada@chromium.org, Apr 23 2016

I tagged this to make sure the people working on this issue knew devtools has needs that are related to netstack and throttling and maybe our needs align at certain points.

A quick overview of devtools problems with throttling:
   We are wrapping HTTP factory in browser and throttling the data at the browser level. The problem with this is that we cannot throttle things that are handled on the netstack level like handshakes, WebSockets, RTC, exc.

See: go/devtoolsthrottling (internal doc, use chromium account)
Project Member

Comment 4 by sheriffbot@chromium.org, Jun 1 2016

Labels: -M-52 M-53 MovedFrom-52
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by sheriffbot@chromium.org, Jul 14 2016

Labels: -M-53 MovedFrom-53
This issue has been moved once and is lower than Pri-1. Removing the milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 21 2016

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

commit 5eb6fbc3c9488ec131081ac9090696af47125ae7
Author: rdsmith <rdsmith@chromium.org>
Date: Fri Oct 21 17:36:08 2016

Add a new priority level, THROTTLED, below IDLE.

THROTTLED is used when the consumer is expecting that low priority
requests will soon be followed by higher priority ones.  It directs the
network stack to reserve resources to handle the coming requests.  In
practice this will mean that only a very small number of THROTTLED requests
may be outstanding at any given time.

BUG= 600839 

Review-Url: https://chromiumcodereview.appspot.com/1866483002
Cr-Commit-Position: refs/heads/master@{#426831}

[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/chrome/browser/predictors/resource_prefetch_predictor.proto
[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/chrome/browser/predictors/resource_prefetch_predictor_tables.h
[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/content/browser/loader/async_revalidation_manager.cc
[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/net/base/prioritized_dispatcher_unittest.cc
[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/net/base/request_priority.cc
[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/net/base/request_priority.h
[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/net/spdy/spdy_http_utils.cc
[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/net/spdy/spdy_http_utils_unittest.cc
[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/net/spdy/spdy_network_transaction_unittest.cc
[modify] https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7/net/url_request/url_request_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 21 2016

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

commit 1d343be5a6202b9f5acaa8c04131fa159d0627c0
Author: rdsmith <rdsmith@chromium.org>
Date: Fri Oct 21 20:37:50 2016

Implementation of network level throttler.

NetworkThrottleManager serves throttle objects to consumers (currently
HttpNetworkTransaction) that inform the consumer of their throttled state
and let the throttler track their lifetime.

BUG= 600839 
R=mmenke@chromium.org

Review-Url: https://chromiumcodereview.appspot.com/1901533002
Cr-Commit-Position: refs/heads/master@{#426879}

[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/chrome/app/generated_resources.grd
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/chrome/browser/ui/tab_contents/core_tab_helper.cc
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/base/load_states_list.h
[add] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/base/network_throttle_manager.cc
[add] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/base/network_throttle_manager.h
[add] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/base/network_throttle_manager_unittest.cc
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/http/http_network_session.cc
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/http/http_network_session.h
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/http/http_network_session_peer.cc
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/http/http_network_session_peer.h
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/http/http_network_transaction.cc
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/http/http_network_transaction.h
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/http/http_network_transaction_unittest.cc
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/http/http_stream_factory_impl.h
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/log/net_log_event_type_list.h
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/net.gypi
[modify] https://crrev.com/1d343be5a6202b9f5acaa8c04131fa159d0627c0/net/quic/chromium/quic_network_transaction_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 26 2016

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

commit 2f23b702967fc351f90a8274c6df03054fa23234
Author: mmenke <mmenke@chromium.org>
Date: Wed Oct 26 23:26:42 2016

Fix DNS logging macro that was broken by adding another net priority.

This was broken by https://codereview.chromium.org/1866483002/

BUG= 600839 , 659338 

Review-Url: https://codereview.chromium.org/2452863003
Cr-Commit-Position: refs/heads/master@{#427874}

[modify] https://crrev.com/2f23b702967fc351f90a8274c6df03054fa23234/net/dns/host_resolver_impl.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 16 2016

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

commit a3ea24ae1987d24b536859dad3d18537dd917242
Author: rdsmith <rdsmith@chromium.org>
Date: Wed Nov 16 18:20:28 2016

Implement THROTTLED priority semantics.

BUG= 600839 
R=mmenke@chromium.org

Review-Url: https://codereview.chromium.org/2130493002
Cr-Commit-Position: refs/heads/master@{#432558}

[modify] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/content/browser/android/url_request_content_job_unittest.cc
[modify] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/base/network_throttle_manager.h
[add] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/base/network_throttle_manager_impl.cc
[add] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/base/network_throttle_manager_impl.h
[add] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/base/network_throttle_manager_impl_unittest.cc
[add] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/base/percentile_estimator.cc
[add] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/base/percentile_estimator.h
[add] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/base/percentile_estimator_unittest.cc
[modify] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/http/http_network_session.cc
[modify] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/http/http_network_transaction.cc
[modify] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/http/http_network_transaction.h
[modify] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/http/http_network_transaction_unittest.cc
[modify] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/net.gypi
[modify] https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242/net/url_request/url_request_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 16 2016

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

commit 62767ec185ed08d25b60402c8c848d935da40d43
Author: rdsmith <rdsmith@chromium.org>
Date: Wed Nov 16 19:19:50 2016

Revert of Implement THROTTLED priority semantics. (patchset #54 id:1090001 of https://codereview.chromium.org/2130493002/ )

Reason for revert:
Crash on Android Cronet Lollipop bots.

Original issue's description:
> Implement THROTTLED priority semantics.
>
> BUG= 600839 
> R=mmenke@chromium.org
>
> Committed: https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242
> Cr-Commit-Position: refs/heads/master@{#432558}

TBR=mmenke@chromium.org,csharrison@chromium.org,rsleevi@chromium.org,yfriedman@chromium.org,tedchoc@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 600839 

Review-Url: https://codereview.chromium.org/2511493002
Cr-Commit-Position: refs/heads/master@{#432585}

[modify] https://crrev.com/62767ec185ed08d25b60402c8c848d935da40d43/content/browser/android/url_request_content_job_unittest.cc
[modify] https://crrev.com/62767ec185ed08d25b60402c8c848d935da40d43/net/base/network_throttle_manager.h
[delete] https://crrev.com/3e256ad41fdb6711f19c85cc80a7c860304a9b45/net/base/network_throttle_manager_impl.cc
[delete] https://crrev.com/3e256ad41fdb6711f19c85cc80a7c860304a9b45/net/base/network_throttle_manager_impl.h
[delete] https://crrev.com/3e256ad41fdb6711f19c85cc80a7c860304a9b45/net/base/network_throttle_manager_impl_unittest.cc
[delete] https://crrev.com/3e256ad41fdb6711f19c85cc80a7c860304a9b45/net/base/percentile_estimator.cc
[delete] https://crrev.com/3e256ad41fdb6711f19c85cc80a7c860304a9b45/net/base/percentile_estimator.h
[delete] https://crrev.com/3e256ad41fdb6711f19c85cc80a7c860304a9b45/net/base/percentile_estimator_unittest.cc
[modify] https://crrev.com/62767ec185ed08d25b60402c8c848d935da40d43/net/http/http_network_session.cc
[modify] https://crrev.com/62767ec185ed08d25b60402c8c848d935da40d43/net/http/http_network_transaction.cc
[modify] https://crrev.com/62767ec185ed08d25b60402c8c848d935da40d43/net/http/http_network_transaction.h
[modify] https://crrev.com/62767ec185ed08d25b60402c8c848d935da40d43/net/http/http_network_transaction_unittest.cc
[modify] https://crrev.com/62767ec185ed08d25b60402c8c848d935da40d43/net/net.gypi
[modify] https://crrev.com/62767ec185ed08d25b60402c8c848d935da40d43/net/url_request/url_request_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 18 2016

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

commit bf8c3c188888638ef161b51f3918cbcd0f2cc3e0
Author: rdsmith <rdsmith@chromium.org>
Date: Fri Nov 18 18:16:24 2016

Implement THROTTLED priority semantics.

BUG= 600839 
R=mmenke@chromium.org

Committed: https://crrev.com/a3ea24ae1987d24b536859dad3d18537dd917242
Review-Url: https://codereview.chromium.org/2130493002
Cr-Original-Commit-Position: refs/heads/master@{#432558}
Cr-Commit-Position: refs/heads/master@{#433240}

[modify] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/chrome/browser/ssl/chrome_expect_ct_reporter_unittest.cc
[modify] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/components/cronet/url_request_context_config_unittest.cc
[modify] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/content/browser/android/url_request_content_job_unittest.cc
[modify] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/base/network_throttle_manager.h
[add] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/base/network_throttle_manager_impl.cc
[add] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/base/network_throttle_manager_impl.h
[add] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/base/network_throttle_manager_impl_unittest.cc
[add] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/base/percentile_estimator.cc
[add] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/base/percentile_estimator.h
[add] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/base/percentile_estimator_unittest.cc
[modify] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/http/http_network_session.cc
[modify] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/http/http_network_transaction.cc
[modify] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/http/http_network_transaction.h
[modify] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/http/http_network_transaction_unittest.cc
[modify] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/net.gypi
[modify] https://crrev.com/bf8c3c188888638ef161b51f3918cbcd0f2cc3e0/net/url_request/url_request_unittest.cc

Cc: allanwoj@chromium.org
Project Member

Comment 13 by bugdroid1@chromium.org, Feb 17 2017

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

commit 29dbad12b55c8d2d520b26987735354c25b9590c
Author: rdsmith <rdsmith@chromium.org>
Date: Fri Feb 17 02:22:18 2017

Add reprioritization to socket pools.

BUG= 600839 
BUG=166689

Review-Url: https://codereview.chromium.org/1898133002
Cr-Commit-Position: refs/heads/master@{#451185}

[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/http/http_network_transaction.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/http/http_proxy_client_socket_pool.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/http/http_proxy_client_socket_pool.h
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/http/http_stream_factory_impl_job.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/http/http_stream_factory_impl_unittest.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/client_socket_handle.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/client_socket_handle.h
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/client_socket_pool.h
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/client_socket_pool_base.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/client_socket_pool_base.h
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/client_socket_pool_base_unittest.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/socket_test_util.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/socket_test_util.h
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/socks_client_socket_pool.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/socks_client_socket_pool.h
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/ssl_client_socket_pool.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/ssl_client_socket_pool.h
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/transport_client_socket_pool.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/transport_client_socket_pool.h
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/websocket_transport_client_socket_pool.cc
[modify] https://crrev.com/29dbad12b55c8d2d520b26987735354c25b9590c/net/socket/websocket_transport_client_socket_pool.h

Cc: csharrison@chromium.org jkarlin@chromium.org rdsmith@chromium.org
Owner: ----
Status: Available (was: Assigned)
It is, sadly, unlikely I'll get back to this in the short term.  CCing some folks who have interest in this area.

Cc: -rdsmith@chromium.org
Status: WontFix (was: Available)
Going to close this.  We may want to revisit this, but don't think this bug is doing us any good.

Sign in to add a comment