Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(196)

Issue 10479014: Increase Chrome SPDY/3 stream receive window to 10MB. (Closed)

Created:
8 years, 6 months ago by Ryan Hamilton
Modified:
8 years, 6 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

Increase Chrome SPDY/3 stream receive window to 10MB. The goal is to set the SPDY window high enough that we'd never realistically hit it, since we'd hit TCP flow control first. The buffers are kept in the Chrome browser process, which should ship things over to WebKit immediately, so the buffers should almost always be small, unless the renderer process goes out to lunch or we're getting a ton of server pushed stuff (unless it goes to browser cache, but I don't think we do that yet, but I forget). 10MB per stream prevents us from completely hosing the browser process in these exceptional situations Change SpdySession so that if the initial receive window size is larger than the protocol default, notify the server. Rename SendSettings to SendInitialSettings to be more clear. BUG=131055 TEST=SpdySessionSpdy3Test.SendInitialWindowSizeSettingsOnNewSession Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=140433

Patch Set 1 #

Patch Set 2 : Actually incease the window size. *sigh* #

Total comments: 6

Patch Set 3 : Send two SETTINGS frames since the SettingsMap does not a allow a single frame with identical IDs (… #

Total comments: 3

Patch Set 4 : Fix raman's comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+105 lines, -7 lines) Patch
M net/spdy/spdy_session.h View 1 2 3 2 chunks +7 lines, -1 line 0 comments Download
M net/spdy/spdy_session.cc View 1 2 7 chunks +33 lines, -6 lines 0 comments Download
M net/spdy/spdy_session_spdy3_unittest.cc View 1 2 3 1 chunk +61 lines, -0 lines 0 comments Download
M net/spdy/spdy_test_util_spdy3.cc View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Ryan Hamilton
8 years, 6 months ago (2012-06-04 19:06:15 UTC) #1
ramant (doing other things)
lgtm http://codereview.chromium.org/10479014/diff/2001/net/spdy/spdy_session.cc File net/spdy/spdy_session.cc (right): http://codereview.chromium.org/10479014/diff/2001/net/spdy/spdy_session.cc#newcode1714 net/spdy/spdy_session.cc:1714: // initial window size. nit: could we update ...
8 years, 6 months ago (2012-06-04 20:22:35 UTC) #2
Ryan Hamilton
Thanks for the lgtm. Since I ended up having to make a code change (to ...
8 years, 6 months ago (2012-06-04 20:48:50 UTC) #3
ramant (doing other things)
lgtm http://codereview.chromium.org/10479014/diff/6001/net/spdy/spdy_session.h File net/spdy/spdy_session.h (right): http://codereview.chromium.org/10479014/diff/6001/net/spdy/spdy_session.h#newcode235 net/spdy/spdy_session.h:235: // Sets the initial recieve window size for ...
8 years, 6 months ago (2012-06-04 22:29:00 UTC) #4
Ryan Hamilton
Thanks! http://codereview.chromium.org/10479014/diff/6001/net/spdy/spdy_session.h File net/spdy/spdy_session.h (right): http://codereview.chromium.org/10479014/diff/6001/net/spdy/spdy_session.h#newcode235 net/spdy/spdy_session.h:235: // Sets the initial recieve window size for ...
8 years, 6 months ago (2012-06-04 22:34:44 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/10479014/10001
8 years, 6 months ago (2012-06-04 22:36:07 UTC) #6
commit-bot: I haz the power
8 years, 6 months ago (2012-06-04 23:49:58 UTC) #7
Change committed as 140433

Powered by Google App Engine
This is Rietveld 408576698