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

Issue 7251004: Deciding best connection to schedule requests on based on cwnd and idle time (Reopened after revert) (Closed)

Created:
8 years, 11 months ago by gagansingh
Modified:
8 years, 11 months ago
CC:
chromium-reviews, jamiewalch+watch_chromium.org, hclam+watch_chromium.org, cbentzel+watch_chromium.org, simonmorris+watch_chromium.org, wez+watch_chromium.org, jam, PaweĊ‚ Hajdan Jr., dmaclach+watch_chromium.org, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, darin-cc_chromium.org, ajwong+watch_chromium.org, brettw-cc_chromium.org, sergeyu+watch_chromium.org, joi+watch-content_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 done in http://codereview.chromium.org/6990036/ Contributed by gagansingh@google.com Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=90373 Reverted: http://codereview.chromium.org/7255002 :( Have fixed 2 things since: 1. Removed LOG(ERROR) from http_basic_stream.cc that was causing layout tests to fail. 2. Initialized class variables in http_basic_stream.cc that was causing uninitialized memory bugs in valgrind: http://code.google.com/p/chromium/issues/detail?id=87423 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=90601

Patch Set 1 #

Patch Set 2 : removing stupid LOG(ERROR) #

Patch Set 3 : fixing uninitialized memory because of class variables not initialized #

Patch Set 4 : fixing uninitialized memory because of class variables not initialized #

Total comments: 2

Patch Set 5 : adding chrome/browser/browser_main_unittest.cc #

Patch Set 6 : uploading again. stupid rietveld #

Patch Set 7 : addressing sergeys comment #

Patch Set 8 : Syncing to head #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+678 lines, -9 lines) Patch
M chrome/browser/browser_main.h View 1 2 3 4 5 6 7 3 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/browser_main.cc View 1 2 3 4 5 6 7 4 chunks +54 lines, -0 lines 0 comments Download
A chrome/browser/browser_main_unittest.cc View 1 2 3 4 1 chunk +68 lines, -0 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/renderer_host/p2p/socket_host_test_utils.h View 1 2 3 4 5 6 7 2 chunks +10 lines, -0 lines 0 comments Download
M jingle/glue/pseudotcp_adapter.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M jingle/glue/pseudotcp_adapter.cc View 1 2 3 4 5 6 7 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 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 1 chunk +8 lines, -0 lines 0 comments Download
M jingle/notifier/base/fake_ssl_client_socket_unittest.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M jingle/notifier/base/proxy_resolving_client_socket.h View 1 2 3 4 5 6 7 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 1 chunk +14 lines, -0 lines 0 comments Download
M net/curvecp/curvecp_client_socket.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M net/curvecp/curvecp_client_socket.cc View 1 2 3 4 5 6 7 1 chunk +8 lines, -0 lines 0 comments Download
M net/http/http_basic_stream.h View 1 2 3 4 5 6 7 2 chunks +6 lines, -0 lines 0 comments Download
M net/http/http_basic_stream.cc View 1 2 3 4 5 6 7 6 chunks +48 lines, -1 line 0 comments Download
M net/http/http_network_transaction.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M net/http/http_network_transaction_unittest.cc View 1 2 3 4 5 6 7 2 chunks +29 lines, -0 lines 0 comments Download
M net/http/http_proxy_client_socket.h View 1 2 3 4 5 6 7 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 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 1 chunk +2 lines, -0 lines 0 comments Download
M net/http/http_stream.h View 1 2 3 4 5 6 7 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 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 4 chunks +43 lines, -1 line 0 comments Download
M net/socket/client_socket_pool_base_unittest.cc View 1 2 3 4 5 6 7 3 chunks +80 lines, -4 lines 0 comments Download
M net/socket/socket_test_util.h View 1 2 3 4 5 6 7 4 chunks +7 lines, -0 lines 0 comments Download
M net/socket/socket_test_util.cc View 1 2 3 4 5 6 7 5 chunks +29 lines, -0 lines 0 comments Download
M net/socket/socks5_client_socket.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M net/socket/socks5_client_socket.cc View 1 2 3 4 5 6 7 1 chunk +16 lines, -0 lines 0 comments Download
M net/socket/socks_client_socket.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M net/socket/socks_client_socket.cc View 1 2 3 4 5 6 7 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 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 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 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 1 chunk +16 lines, -0 lines 0 comments Download
M net/socket/ssl_client_socket_win.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M net/socket/ssl_client_socket_win.cc View 1 2 3 4 5 6 7 1 chunk +16 lines, -0 lines 0 comments Download
M net/socket/ssl_server_socket_nss.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M net/socket/ssl_server_socket_nss.cc View 1 2 3 4 5 6 7 1 chunk +8 lines, -0 lines 0 comments Download
M net/socket/ssl_server_socket_unittest.cc View 1 2 3 4 5 6 7 1 chunk +8 lines, -0 lines 0 comments Download
M net/socket/stream_socket.h View 1 2 3 4 5 6 7 2 chunks +7 lines, -0 lines 1 comment Download
M net/socket/tcp_client_socket_libevent.h View 1 2 3 4 5 6 7 2 chunks +6 lines, -0 lines 0 comments Download
M net/socket/tcp_client_socket_libevent.cc View 1 2 3 4 5 6 7 6 chunks +14 lines, -1 line 0 comments Download
M net/socket/tcp_client_socket_win.h View 1 2 3 4 5 6 7 2 chunks +6 lines, -0 lines 0 comments Download
M net/socket/tcp_client_socket_win.cc View 1 2 3 4 5 6 7 6 chunks +14 lines, -1 line 0 comments Download
M net/socket/transport_client_socket_pool_unittest.cc View 1 2 3 4 5 6 7 3 chunks +12 lines, -0 lines 0 comments Download
M net/socket/transport_client_socket_unittest.cc View 1 2 3 4 5 6 7 3 chunks +8 lines, -1 line 0 comments Download
M net/spdy/spdy_http_stream.h View 1 2 3 4 5 6 7 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 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 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 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 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
gagansingh
8 years, 11 months ago (2011-06-24 19:32:53 UTC) #1
commit-bot: I haz the power
No LGTM from valid reviewers yet.
8 years, 11 months ago (2011-06-24 19:49:56 UTC) #2
Sergey Ulanov
LGTM with one nit. http://codereview.chromium.org/7251004/diff/12009/net/socket/tcp_client_socket_win.cc File net/socket/tcp_client_socket_win.cc (right): http://codereview.chromium.org/7251004/diff/12009/net/socket/tcp_client_socket_win.cc#newcode327 net/socket/tcp_client_socket_win.cc:327: connect_start_time_(), nit: You don't need ...
8 years, 11 months ago (2011-06-24 20:10:17 UTC) #3
gagansingh
Thanks for the quick review Sergey. http://codereview.chromium.org/7251004/diff/12009/net/socket/tcp_client_socket_win.cc File net/socket/tcp_client_socket_win.cc (right): http://codereview.chromium.org/7251004/diff/12009/net/socket/tcp_client_socket_win.cc#newcode327 net/socket/tcp_client_socket_win.cc:327: connect_start_time_(), On 2011/06/24 ...
8 years, 11 months ago (2011-06-24 20:14:43 UTC) #4
commit-bot: I haz the power
Presubmit check for 7251004-15072 failed and returned exit status 1. Running presubmit commit checks ...
8 years, 11 months ago (2011-06-24 20:15:59 UTC) #5
willchan no longer on Chromium
Next time to do this, upload the clean revert first, and then upload your changes ...
8 years, 11 months ago (2011-06-27 08:51:32 UTC) #6
commit-bot: I haz the power
Can't apply patch for file net/socket/tcp_client_socket_libevent.cc. While running patch -p0 --forward --force; patching file net/socket/tcp_client_socket_libevent.cc ...
8 years, 11 months ago (2011-06-27 08:55:52 UTC) #7
gagansingh
Good point, sorry for the pain. I did try this but i think i messed ...
8 years, 11 months ago (2011-06-27 11:22:37 UTC) #8
commit-bot: I haz the power
Commit queue had an internal error. Something went really wrong, probably a crash, a hickup ...
8 years, 11 months ago (2011-06-27 13:33:32 UTC) #9
commit-bot: I haz the power
Change committed as 90601
8 years, 11 months ago (2011-06-27 17:26:43 UTC) #10
gagansingh
Thanks for the reviews guys.
8 years, 11 months ago (2011-06-28 01:49:58 UTC) #11
wtc
8 years, 11 months ago (2011-07-01 16:27:35 UTC) #12
http://codereview.chromium.org/7251004/diff/13028/net/socket/stream_socket.h
File net/socket/stream_socket.h (right):

http://codereview.chromium.org/7251004/diff/13028/net/socket/stream_socket.h#...
net/socket/stream_socket.h:87: virtual base::TimeDelta GetConnectTimeMicros()
const = 0;
Can you rename this method GetConnectTime?

I don't know what "Micros" means.

Powered by Google App Engine
This is Rietveld 408576698