Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(36)

Issue 18541: Add a constraint on how many requests can be outstanding for any given render (browser-side). (Closed)

Created:
10 years, 6 months ago by eroman
Modified:
8 years, 2 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Add a constraint on how many requests can be outstanding for any given render process (browser-side). Once the constraint is reached, subsequent requests will fail with net::ERR_INSUFFICIENT_RESOURCES. The bound is defined as "25 MB", which represents the amount of private bytes we expect the pending requests to consume in the browser. This number translates into around 6000 typical requests. Note that the upload data of a request is not currently considered part of the request's in-memory cost -- more data is needed on the average/maximum upload sizes of users before deciding what a compatible limit is. BUG=5688 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=9298

Patch Set 1 #

Total comments: 14

Patch Set 2 : '' #

Total comments: 8

Patch Set 3 : '' #

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+343 lines, -30 lines) Patch
M chrome/browser/renderer_host/resource_dispatcher_host.h View 1 2 6 chunks +49 lines, -0 lines 0 comments Download
M chrome/browser/renderer_host/resource_dispatcher_host.cc View 1 2 6 chunks +106 lines, -2 lines 0 comments Download
M chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc View 1 2 17 chunks +173 lines, -17 lines 0 comments Download
M net/base/net_error_list.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M net/url_request/url_request.h View 2 chunks +5 lines, -0 lines 0 comments Download
M net/url_request/url_request.cc View 1 2 3 chunks +5 lines, -5 lines 0 comments Download
M net/url_request/url_request_test_job.cc View 1 2 1 chunk +2 lines, -6 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
eroman
http://codereview.chromium.org/18541/diff/1/6 File chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc (right): http://codereview.chromium.org/18541/diff/1/6#newcode197 Line 197: EXPECT_EQ(0, host_.GetOutstandingRequestsCount(0)); The reason for sprinkling all these ...
10 years, 6 months ago (2009-01-23 02:16:11 UTC) #1
darin (slow to review)
http://codereview.chromium.org/18541/diff/1/7 File chrome/browser/renderer_host/resource_dispatcher_host.cc (right): http://codereview.chromium.org/18541/diff/1/7#newcode74 Line 74: static const int kMaxOutstandingRequestsPerProcess = 100; 100 seems ...
10 years, 6 months ago (2009-01-23 16:43:13 UTC) #2
Peter Kasting
http://codereview.chromium.org/18541/diff/1/7 File chrome/browser/renderer_host/resource_dispatcher_host.cc (right): http://codereview.chromium.org/18541/diff/1/7#newcode74 Line 74: static const int kMaxOutstandingRequestsPerProcess = 100; On 2009/01/23 ...
10 years, 6 months ago (2009-01-23 18:39:26 UTC) #3
eroman
Thanks for the feedback. > 100 seems too small. i could easily imagine normal web ...
10 years, 6 months ago (2009-01-23 21:20:35 UTC) #4
eroman
> I'm probably not understanding the proposal... what happens for very > large downloads? (3 ...
10 years, 6 months ago (2009-01-23 21:51:51 UTC) #5
eroman
The bound is now defined as a "25MB per render process host". Given average pending ...
10 years, 6 months ago (2009-01-27 01:55:31 UTC) #6
eroman
ping.
10 years, 6 months ago (2009-01-30 10:58:32 UTC) #7
darin (slow to review)
LGTM http://codereview.chromium.org/18541/diff/202/29 File chrome/browser/renderer_host/resource_dispatcher_host.cc (right): http://codereview.chromium.org/18541/diff/202/29#newcode868 Line 868: if (request->has_upload()) { it could be large... ...
10 years, 6 months ago (2009-01-30 16:56:43 UTC) #8
eroman
http://codereview.chromium.org/18541/diff/202/29 File chrome/browser/renderer_host/resource_dispatcher_host.cc (right): http://codereview.chromium.org/18541/diff/202/29#newcode868 Line 868: if (request->has_upload()) { On 2009/01/30 16:56:43, darin wrote: ...
10 years, 6 months ago (2009-01-31 02:31:27 UTC) #9
darin (slow to review)
10 years, 6 months ago (2009-02-05 06:50:02 UTC) #10
LGTM w/ this change:

http://codereview.chromium.org/18541/diff/202/29
File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/18541/diff/202/29#newcode868
Line 868: if (request->has_upload()) {
histogramming sounds like the better idea to me too.  +1 :)

Powered by Google App Engine
This is Rietveld 408576698