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

Issue 6990036: Deciding best connection to schedule requests on based on cwnd and idle time (Closed)

Created:
9 years, 4 months ago by Gagan
Modified:
9 years, 2 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

Warmth of a connection (cwnd) is estimated by the amount of data written to the socket. Choosing the warmest connection would mean faster resource load times. idle time is the time a socket has remained idle (no http requests being served on it). Probability of server resetting a connection increases with idle time duration. Using a cost function that takes into account bytes transferred and idle time to pick best connection to schedule http requests on. Codereview CONTINUED here : http://codereview.chromium.org/7189055/

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Patch Set 6 : '' #

Patch Set 7 : Trying from linux #

Patch Set 8 : '' #

Patch Set 9 : '' #

Patch Set 10 : From linux #

Patch Set 11 : '' #

Patch Set 12 : '' #

Patch Set 13 : '' #

Patch Set 14 : '' #

Total comments: 12

Patch Set 15 : '' #

Patch Set 16 : '' #

Patch Set 17 : '' #

Total comments: 8

Patch Set 18 : '' #

Patch Set 19 : '' #

Patch Set 20 : Synced client #

Patch Set 21 : Changed bytes to int64 #

Patch Set 22 : Renaming bytes_read_vs_last_accessed_alpha to tcp_socket_estimated_cwnd_decay_coef #

Patch Set 23 : Fixing flag name #

Total comments: 43

Patch Set 24 : Addressing comments #

Patch Set 25 : logging #

Patch Set 26 : GetConnectTimeMicros #

Patch Set 27 : connect_time_micros_ #

Total comments: 6

Patch Set 28 : style comment #

Patch Set 29 : addressing comments #

Patch Set 30 : addressing comments #

Total comments: 22

Patch Set 31 : syncing head and addressing comments #

Total comments: 30

Patch Set 32 : addressing comments #

Patch Set 33 : addressing comments #

Total comments: 22

Patch Set 34 : addressing comments #

Patch Set 35 : small tweaks #

Patch Set 36 : synced to head #

Patch Set 37 : small tweaks #

Patch Set 38 : verified using tcpdump #

Patch Set 39 : removed logs #

Patch Set 40 : fixing release build policy check #

Total comments: 6

Patch Set 41 : Addressing wills comments #

Patch Set 42 : Sync to head #

Patch Set 43 : Reuploading from google.com account #

Unified diffs Side-by-side diffs Delta from patch set Stats (+377 lines, -3 lines) Patch
M chrome/browser/browser_main.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/browser_main.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 4 chunks +57 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +4 lines, -0 lines 0 comments Download
M jingle/glue/pseudotcp_adapter.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M jingle/glue/pseudotcp_adapter.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +10 lines, -0 lines 0 comments Download
M jingle/notifier/base/fake_ssl_client_socket.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M jingle/notifier/base/fake_ssl_client_socket.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +8 lines, -0 lines 0 comments Download
M jingle/notifier/base/proxy_resolving_client_socket.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M jingle/notifier/base/proxy_resolving_client_socket.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +14 lines, -0 lines 0 comments Download
M net/http/http_basic_stream.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 2 chunks +6 lines, -0 lines 0 comments Download
M net/http/http_basic_stream.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 5 chunks +48 lines, -0 lines 0 comments Download
M net/http/http_network_transaction.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +1 line, -0 lines 0 comments Download
M net/http/http_proxy_client_socket.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M net/http/http_proxy_client_socket.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +16 lines, -0 lines 0 comments Download
M net/http/http_response_body_drainer_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M net/http/http_stream.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +4 lines, -0 lines 0 comments Download
M net/socket/client_socket_pool_base.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 2 chunks +18 lines, -0 lines 0 comments Download
M net/socket/client_socket_pool_base.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 4 chunks +46 lines, -1 line 0 comments Download
M net/socket/socks5_client_socket.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M net/socket/socks5_client_socket.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +16 lines, -0 lines 0 comments Download
M net/socket/socks_client_socket.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M net/socket/socks_client_socket.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +16 lines, -0 lines 0 comments Download
M net/socket/ssl_client_socket_mac.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M net/socket/ssl_client_socket_mac.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +16 lines, -0 lines 0 comments Download
M net/socket/ssl_client_socket_nss.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M net/socket/ssl_client_socket_nss.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +16 lines, -0 lines 0 comments Download
M net/socket/stream_socket.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +6 lines, -0 lines 0 comments Download
M net/socket/tcp_client_socket_libevent.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 3 chunks +7 lines, -0 lines 0 comments Download
M net/socket/tcp_client_socket_libevent.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 5 chunks +8 lines, -1 line 0 comments Download
M net/socket/tcp_client_socket_win.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 3 chunks +7 lines, -0 lines 0 comments Download
M net/socket/tcp_client_socket_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 5 chunks +8 lines, -1 line 0 comments Download
M net/spdy/spdy_http_stream.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +1 line, -0 lines 0 comments Download
M net/spdy/spdy_proxy_client_socket.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M net/spdy/spdy_proxy_client_socket.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +8 lines, -0 lines 0 comments Download
M remoting/jingle_glue/ssl_socket_adapter.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +2 lines, -0 lines 0 comments Download
M remoting/jingle_glue/ssl_socket_adapter.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 27 (0 generated)
Gagan
9 years, 3 months ago (2011-06-06 07:40:46 UTC) #1
willchan no longer on Chromium
For cwnd calculations, the only bytes read that matters is the one for the underlying ...
9 years, 3 months ago (2011-06-06 10:57:36 UTC) #2
wtc
I only reviewed the API (.h files) changes in src/net in Patch Set 17. http://codereview.chromium.org/6990036/diff/31083/net/http/http_stream.h ...
9 years, 3 months ago (2011-06-06 17:24:05 UTC) #3
Gagan
Hi guys I addressed most of your comments, but i'm running into a weird problem. ...
9 years, 3 months ago (2011-06-06 20:27:09 UTC) #4
gagansingh
Hi Figured out what was going wrong: I had missed DidCompleteRead() method which amazingly also ...
9 years, 3 months ago (2011-06-07 16:32:01 UTC) #5
Gagan
http://codereview.chromium.org/6990036/diff/31083/net/socket/client_socket_pool_base.h File net/socket/client_socket_pool_base.h (right): http://codereview.chromium.org/6990036/diff/31083/net/socket/client_socket_pool_base.h#newcode292 net/socket/client_socket_pool_base.h:292: static double set_bytes_read_vs_last_accessed_alpha(double alpha); On 2011/06/06 17:24:05, wtc wrote: ...
9 years, 3 months ago (2011-06-08 16:26:37 UTC) #6
willchan no longer on Chromium
http://codereview.chromium.org/6990036/diff/47017/net/http/http_basic_stream.cc File net/http/http_basic_stream.cc (right): http://codereview.chromium.org/6990036/diff/47017/net/http/http_basic_stream.cc#newcode18 net/http/http_basic_stream.cc:18: std::string g_warm_connection_field_trial_group; This is not allowed by the style ...
9 years, 3 months ago (2011-06-09 15:08:54 UTC) #7
Mike Belshe
http://codereview.chromium.org/6990036/diff/47017/chrome/browser/browser_main.cc File chrome/browser/browser_main.cc (right): http://codereview.chromium.org/6990036/diff/47017/chrome/browser/browser_main.cc#newcode497 chrome/browser/browser_main.cc:497: const base::FieldTrial::Probability kWarmSocketProbability = 33; // 1% prob. The ...
9 years, 3 months ago (2011-06-09 16:08:51 UTC) #8
Gagan
http://codereview.chromium.org/6990036/diff/47017/chrome/browser/browser_main.cc File chrome/browser/browser_main.cc (right): http://codereview.chromium.org/6990036/diff/47017/chrome/browser/browser_main.cc#newcode497 chrome/browser/browser_main.cc:497: const base::FieldTrial::Probability kWarmSocketProbability = 33; // 1% prob. On ...
9 years, 3 months ago (2011-06-09 19:49:25 UTC) #9
willchan no longer on Chromium
http://codereview.chromium.org/6990036/diff/47017/net/http/http_basic_stream.cc File net/http/http_basic_stream.cc (right): http://codereview.chromium.org/6990036/diff/47017/net/http/http_basic_stream.cc#newcode18 net/http/http_basic_stream.cc:18: std::string g_warm_connection_field_trial_group; On 2011/06/09 19:49:26, gagan.goku wrote: > On ...
9 years, 3 months ago (2011-06-10 15:05:55 UTC) #10
Gagan
http://codereview.chromium.org/6990036/diff/47017/net/http/http_basic_stream.cc File net/http/http_basic_stream.cc (right): http://codereview.chromium.org/6990036/diff/47017/net/http/http_basic_stream.cc#newcode18 net/http/http_basic_stream.cc:18: std::string g_warm_connection_field_trial_group; On 2011/06/10 15:05:55, willchan wrote: > On ...
9 years, 3 months ago (2011-06-10 17:44:16 UTC) #11
Mike Belshe
Thanks for taking the time to go through all these comments. I realize we are ...
9 years, 3 months ago (2011-06-10 18:11:42 UTC) #12
Gagan
http://codereview.chromium.org/6990036/diff/47017/chrome/common/chrome_switches.h File chrome/common/chrome_switches.h (right): http://codereview.chromium.org/6990036/diff/47017/chrome/common/chrome_switches.h#newcode260 chrome/common/chrome_switches.h:260: extern const char kTcpSocketEstimatedCwndDecayCoef[]; On 2011/06/10 18:11:42, Mike Belshe ...
9 years, 3 months ago (2011-06-12 20:11:44 UTC) #13
Mike Belshe
Pretty close. If Will is ok with this, then lgtm. http://codereview.chromium.org/6990036/diff/48177/chrome/browser/browser_main.cc File chrome/browser/browser_main.cc (right): http://codereview.chromium.org/6990036/diff/48177/chrome/browser/browser_main.cc#newcode511 ...
9 years, 3 months ago (2011-06-12 22:10:35 UTC) #14
Mike Belshe
http://codereview.chromium.org/6990036/diff/48177/net/socket/client_socket_pool_base.cc File net/socket/client_socket_pool_base.cc (left): http://codereview.chromium.org/6990036/diff/48177/net/socket/client_socket_pool_base.cc#oldcode595 net/socket/client_socket_pool_base.cc:595: std::list<IdleSocket>::iterator j = group->mutable_idle_sockets()->begin(); BTW - do we need ...
9 years, 3 months ago (2011-06-12 22:17:03 UTC) #15
Gagan
Thanks for the review Mike. http://codereview.chromium.org/6990036/diff/48177/chrome/browser/browser_main.cc File chrome/browser/browser_main.cc (right): http://codereview.chromium.org/6990036/diff/48177/chrome/browser/browser_main.cc#newcode511 chrome/browser/browser_main.cc:511: "warm_socket", kWarmSocketProbability); On 2011/06/12 ...
9 years, 3 months ago (2011-06-13 03:52:47 UTC) #16
willchan no longer on Chromium
More nits, I need to look at how the algorithm worked out to make sure ...
9 years, 3 months ago (2011-06-13 15:06:07 UTC) #17
Gagan
>More nits, I need to look at how the algorithm worked out to make sure ...
9 years, 3 months ago (2011-06-13 16:45:58 UTC) #18
Gagan
>More nits, I need to look at how the algorithm worked out to make sure ...
9 years, 3 months ago (2011-06-13 16:46:54 UTC) #19
willchan no longer on Chromium
http://codereview.chromium.org/6990036/diff/48195/chrome/browser/browser_main.cc File chrome/browser/browser_main.cc (right): http://codereview.chromium.org/6990036/diff/48195/chrome/browser/browser_main.cc#newcode218 chrome/browser/browser_main.cc:218: net::HttpBasicStream::SetSocketReusePolicy(i); How about you don't call this one and ...
9 years, 3 months ago (2011-06-14 12:01:22 UTC) #20
Gagan
Removed all logging statements. http://codereview.chromium.org/6990036/diff/47017/net/socket/tcp_client_socket_libevent.cc File net/socket/tcp_client_socket_libevent.cc (right): http://codereview.chromium.org/6990036/diff/47017/net/socket/tcp_client_socket_libevent.cc#newcode440 net/socket/tcp_client_socket_libevent.cc:440: LOG(ERROR) << "this = " ...
9 years, 3 months ago (2011-06-14 18:25:02 UTC) #21
willchan no longer on Chromium
Last nits, otherwise it LGTM. http://codereview.chromium.org/6990036/diff/67103/net/http/http_basic_stream.cc File net/http/http_basic_stream.cc (right): http://codereview.chromium.org/6990036/diff/67103/net/http/http_basic_stream.cc#newcode160 net/http/http_basic_stream.cc:160: if (VLOG_IS_ON(2)) { Why ...
9 years, 3 months ago (2011-06-16 09:52:43 UTC) #22
Gagan
http://codereview.chromium.org/6990036/diff/67103/net/http/http_basic_stream.cc File net/http/http_basic_stream.cc (right): http://codereview.chromium.org/6990036/diff/67103/net/http/http_basic_stream.cc#newcode160 net/http/http_basic_stream.cc:160: if (VLOG_IS_ON(2)) { On 2011/06/16 09:52:44, willchan wrote: > ...
9 years, 3 months ago (2011-06-16 11:49:29 UTC) #23
commit-bot: I haz the power
Presubmit check for 6990036-79005 failed and returned exit status 1. Running presubmit commit checks ...
9 years, 3 months ago (2011-06-18 11:59:03 UTC) #24
willchan no longer on Chromium
+hclam,+ajwong for jingle/OWNERS and remoting/OWNERS. Hey guys, need approval for the small socket changes to ...
9 years, 3 months ago (2011-06-18 12:30:13 UTC) #25
Alpha Left Google
On 2011/06/18 12:30:13, willchan wrote: > +hclam,+ajwong for jingle/OWNERS and remoting/OWNERS. > > Hey guys, ...
9 years, 3 months ago (2011-06-21 19:06:51 UTC) #26
Gagan
9 years, 3 months ago (2011-06-22 08:41:58 UTC) #27
Hi Alpha

The codereview has been continued here :
http://codereview.chromium.org/7189055/ (because of CLA and other problems).
Requesting you to lgtm there.

Thanks
Gagan

On Wed, Jun 22, 2011 at 12:36 AM, <hclam@chromium.org> wrote:

> On 2011/06/18 12:30:13, willchan wrote:
>
>> +hclam,+ajwong for jingle/OWNERS and remoting/OWNERS.
>>
>
>  Hey guys, need approval for the small socket changes to jingle/ and
>> remoting/.
>> Cheers.
>>
>
> LGTM.
>
>
>
http://codereview.chromium.**org/6990036/<http://codereview.chromium.org/6990...
>

Powered by Google App Engine
This is Rietveld 408576698