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

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

[commit-bot: I haz the power, 2011-06-24 19:49:56]

No LGTM from valid reviewers yet.

[Sergey Ulanov, 2011-06-24 20:10:17]

LGTM with one nit.

http://codereview.chromium.org/7251004/diff/12009/net/socket/tcp_client_socke...
File net/socket/tcp_client_socket_win.cc (right):

http://codereview.chromium.org/7251004/diff/12009/net/socket/tcp_client_socke...
net/socket/tcp_client_socket_win.cc:327: connect_start_time_(),
nit: You don't need this, compiler invokes default constructor anyway.

[gagansingh, 2011-06-24 20:14:43]

Thanks for the quick review Sergey.

http://codereview.chromium.org/7251004/diff/12009/net/socket/tcp_client_socke...
File net/socket/tcp_client_socket_win.cc (right):

http://codereview.chromium.org/7251004/diff/12009/net/socket/tcp_client_socke...
net/socket/tcp_client_socket_win.cc:327: connect_start_time_(),
On 2011/06/24 20:10:17, sergeyu wrote:
> nit: You don't need this, compiler invokes default constructor anyway.

There was a valgrind error, because one of the class members was not
initialized: http://code.google.com/p/chromium/issues/detail?id=87423
so initializing all class members now

[commit-bot: I haz the power, 2011-06-24 20:15:59]

Presubmit check for 7251004-15072 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
net/spdy/spdy_proxy_client_socket.cc,net/socket/socket_test_util.h,net/spdy/spdy_proxy_client_socket.h,net/socket/tcp_client_socket_libevent.h,net/socket/ssl_server_socket_unittest.cc,net/socket/transport_client_socket_unittest.cc,net/socket/socks5_client_socket.h,net/socket/ssl_client_socket_mac.h,net/socket/client_socket_pool_base.h,net/socket/client_socket_pool_base.cc,net/socket/ssl_client_socket_nss.h,net/http/http_basic_stream.cc,net/socket/tcp_client_socket_win.cc,net/socket/socks5_client_socket.cc,net/socket/transport_client_socket_pool_unittest.cc,net/http/http_proxy_client_socket.h,net/socket/tcp_client_socket_libevent.cc,net/http/http_network_transaction_unittest.cc,net/socket/ssl_client_socket_mac.cc,net/socket/socks_client_socket.cc,net/http/http_stream.h,net/spdy/spdy_http_stream.h,net/socket/client_socket_pool_base_unittest.cc,net/socket/ssl_client_socket_win.h,net/socket/tcp_client_socket_win.h,net/socket/socket_test_util.cc,net/http/http_proxy_client_socket.cc,net/curvecp/curvecp_client_socket.h,net/socket/ssl_client_socket_win.cc,net/http/http_basic_stream.h,net/socket/ssl_client_socket_nss.cc,net/http/http_response_body_drainer_unittest.cc,net/socket/ssl_server_socket_nss.cc,net/http/http_network_transaction.cc,net/curvecp/curvecp_client_socket.cc,net/socket/ssl_server_socket_nss.h,net/socket/socks_client_socket.h,net/socket/stream_socket.h

Presubmit checks took 2.3s to calculate.

[willchan no longer on Chromium, 2011-06-27 08:51:32]

Next time to do this, upload the clean revert first, and then upload your
changes on top of that. That way, I can see the differences between the two
patchsets. Right now it's a PITA for me to do so. You'll save your reviewer a
lot of time if you do this in the future.

LGTM.

On 2011/06/24 20:15:59, I haz the power (commit-bot) wrote:
> Presubmit check for 7251004-15072 failed and returned exit status 1.
> 
> Running presubmit commit checks ...
> 
> ** Presubmit Messages **
> If this change has an associated bug, add BUG=[bug number].
> 
> If this change requires manual test instructions to QA team, add
> TEST=[instructions].
> 
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for:
>
net/spdy/spdy_proxy_client_socket.cc,net/socket/socket_test_util.h,net/spdy/spdy_proxy_client_socket.h,net/socket/tcp_client_socket_libevent.h,net/socket/ssl_server_socket_unittest.cc,net/socket/transport_client_socket_unittest.cc,net/socket/socks5_client_socket.h,net/socket/ssl_client_socket_mac.h,net/socket/client_socket_pool_base.h,net/socket/client_socket_pool_base.cc,net/socket/ssl_client_socket_nss.h,net/http/http_basic_stream.cc,net/socket/tcp_client_socket_win.cc,net/socket/socks5_client_socket.cc,net/socket/transport_client_socket_pool_unittest.cc,net/http/http_proxy_client_socket.h,net/socket/tcp_client_socket_libevent.cc,net/http/http_network_transaction_unittest.cc,net/socket/ssl_client_socket_mac.cc,net/socket/socks_client_socket.cc,net/http/http_stream.h,net/spdy/spdy_http_stream.h,net/socket/client_socket_pool_base_unittest.cc,net/socket/ssl_client_socket_win.h,net/socket/tcp_client_socket_win.h,net/socket/socket_test_util.cc,net/http/http_proxy_client_socket.cc,net/curvecp/curvecp_client_socket.h,net/socket/ssl_client_socket_win.cc,net/http/http_basic_stream.h,net/socket/ssl_client_socket_nss.cc,net/http/http_response_body_drainer_unittest.cc,net/socket/ssl_server_socket_nss.cc,net/http/http_network_transaction.cc,net/curvecp/curvecp_client_socket.cc,net/socket/ssl_server_socket_nss.h,net/socket/socks_client_socket.h,net/socket/stream_socket.h
> 
> Presubmit checks took 2.3s to calculate.

[commit-bot: I haz the power, 2011-06-27 08:55:52]

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
Hunk #3 FAILED at 341.
Hunk #4 succeeded at 441 (offset -1 lines).
Hunk #5 succeeded at 636 (offset -1 lines).
Hunk #6 succeeded at 726 (offset -1 lines).
1 out of 6 hunks FAILED -- saving rejects to file
net/socket/tcp_client_socket_libevent.cc.rej

[gagansingh, 2011-06-27 11:22:37]

Good point, sorry for the pain. I did try this but i think i messed up
somewhere. Will keep in mind next time.
Thanks for the review.

Thanks
Gagan




On Mon, Jun 27, 2011 at 2:21 PM, <willchan@chromium.org> wrote:

> Next time to do this, upload the clean revert first, and then upload your
> changes on top of that. That way, I can see the differences between the two
> patchsets. Right now it's a PITA for me to do so. You'll save your reviewer
> a
> lot of time if you do this in the future.
>
> LGTM.
>
>
> On 2011/06/24 20:15:59, I haz the power (commit-bot) wrote:
>
>> Presubmit check for 7251004-15072 failed and returned exit status 1.
>>
>
>  Running presubmit commit checks ...
>>
>
>  ** Presubmit Messages **
>> If this change has an associated bug, add BUG=[bug number].
>>
>
>  If this change requires manual test instructions to QA team, add
>> TEST=[instructions].
>>
>
>  ** Presubmit ERRORS **
>> Missing LGTM from an OWNER for:
>>
>
> net/spdy/spdy_proxy_client_**socket.cc,net/socket/socket_**
> test_util.h,net/spdy/spdy_**proxy_client_socket.h,net/**
> socket/tcp_client_socket_**libevent.h,net/socket/ssl_**
> server_socket_unittest.cc,net/**socket/transport_client_**
> socket_unittest.cc,net/socket/**socks5_client_socket.h,net/**
> socket/ssl_client_socket_mac.**h,net/socket/client_socket_**
> pool_base.h,net/socket/client_**socket_pool_base.cc,net/**
> socket/ssl_client_socket_nss.**h,net/http/http_basic_stream.**
> cc,net/socket/tcp_client_**socket_win.cc,net/socket/**
> socks5_client_socket.cc,net/**socket/transport_client_**
> socket_pool_unittest.cc,net/**http/http_proxy_client_socket.**
> h,net/socket/tcp_client_**socket_libevent.cc,net/http/**
> http_network_transaction_**unittest.cc,net/socket/ssl_**
> client_socket_mac.cc,net/**socket/socks_client_socket.cc,**
> net/http/http_stream.h,net/**spdy/spdy_http_stream.h,net/**
> socket/client_socket_pool_**base_unittest.cc,net/socket/**
> ssl_client_socket_win.h,net/**socket/tcp_client_socket_win.**
> h,net/socket/socket_test_util.**cc,net/http/http_proxy_client_**
> socket.cc,net/curvecp/curvecp_**client_socket.h,net/socket/**
> ssl_client_socket_win.cc,net/**http/http_basic_stream.h,net/**
> socket/ssl_client_socket_nss.**cc,net/http/http_response_**
> body_drainer_unittest.cc,net/**socket/ssl_server_socket_nss.**
> cc,net/http/http_network_**transaction.cc,net/curvecp/**
> curvecp_client_socket.cc,net/**socket/ssl_server_socket_nss.**
> h,net/socket/socks_client_**socket.h,net/socket/stream_**socket.h
>
>  Presubmit checks took 2.3s to calculate.
>>
>
>
>
>
http://codereview.chromium.**org/7251004/<http://codereview.chromium.org/7251...
>

[commit-bot: I haz the power, 2011-06-27 13:33:32]

Commit queue had an internal error.
Something went really wrong, probably a crash, a hickup or
simply the monkeys went out for diner.
The relevant dude was pinged already.

[commit-bot: I haz the power, 2011-06-27 17:26:43]

Change committed as 90601

[gagansingh, 2011-06-28 01:49:58]

Thanks for the reviews guys.

[wtc, 2011-07-01 16:27:35]

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.