Skip to content

Commit

Permalink
Remove experimental code to pick the "warmest" socket
Browse files Browse the repository at this point in the history
(based on age and bytes received) in favor of older
algorithm to pick the most recently used socket.

Tests showed no real performance difference, so
defaulting to the older, simpler, and more intuitive
algorithm.

This is basically a revert of
https://codereview.chromium.org/7251004

TBR=sergeyu@chromium.org
BUG=222090
Review URL: https://codereview.chromium.org/12886034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@191507 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
mmenke@chromium.org committed Mar 30, 2013
1 parent 36e5539 commit e86df8d
Show file tree
Hide file tree
Showing 58 changed files with 10 additions and 656 deletions.
61 changes: 2 additions & 59 deletions chrome/browser/chrome_browser_field_trials.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,31 +30,16 @@
#include "chrome/common/metrics/variations/uniformity_field_trials.h"
#include "chrome/common/metrics/variations/variations_util.h"
#include "chrome/common/pref_names.h"
#include "net/socket/client_socket_pool_base.h"
#include "net/spdy/spdy_session.h"
#include "ui/base/layout.h"

#if defined(OS_WIN)
#include "net/socket/tcp_client_socket_win.h"
#endif // defined(OS_WIN)

namespace {

void SetSocketReusePolicy(int warmest_socket_trial_group,
const int socket_policy[],
int num_groups) {
const int* result = std::find(socket_policy, socket_policy + num_groups,
warmest_socket_trial_group);
DCHECK_NE(result, socket_policy + num_groups)
<< "Not a valid socket reuse policy group";
net::SetSocketReusePolicy(result - socket_policy);
}

} // namespace

ChromeBrowserFieldTrials::ChromeBrowserFieldTrials(
const CommandLine& parsed_command_line) :
parsed_command_line_(parsed_command_line) {
const CommandLine& parsed_command_line)
: parsed_command_line_(parsed_command_line) {
}

ChromeBrowserFieldTrials::~ChromeBrowserFieldTrials() {
Expand All @@ -75,7 +60,6 @@ void ChromeBrowserFieldTrials::SetupDesktopFieldTrials(
PrefService* local_state) {
prerender::ConfigurePrefetchAndPrerender(parsed_command_line_);
SpdyFieldTrial();
WarmConnectionFieldTrial();
AutoLaunchChromeFieldTrial();
gpu_util::InitializeCompositingFieldTrial();
OmniboxFieldTrial::ActivateStaticTrials();
Expand Down Expand Up @@ -125,47 +109,6 @@ void ChromeBrowserFieldTrials::SpdyFieldTrial() {
trial->AppendGroup("cwndMin10", kSpdyCwndMin10);
}

// If --socket-reuse-policy is not specified, run an A/B test for choosing the
// warmest socket.
void ChromeBrowserFieldTrials::WarmConnectionFieldTrial() {
const CommandLine& command_line = parsed_command_line_;
if (command_line.HasSwitch(switches::kSocketReusePolicy)) {
std::string socket_reuse_policy_str = command_line.GetSwitchValueASCII(
switches::kSocketReusePolicy);
int policy = -1;
base::StringToInt(socket_reuse_policy_str, &policy);

const int policy_list[] = { 0, 1, 2 };
VLOG(1) << "Setting socket_reuse_policy = " << policy;
SetSocketReusePolicy(policy, policy_list, arraysize(policy_list));
return;
}

const base::FieldTrial::Probability kWarmSocketDivisor = 100;
const base::FieldTrial::Probability kWarmSocketProbability = 33;

// Default value is USE_LAST_ACCESSED_SOCKET.
int last_accessed_socket = -1;

// After January 30, 2013 builds, it will always be in default group.
scoped_refptr<base::FieldTrial> warmest_socket_trial(
base::FieldTrialList::FactoryGetFieldTrial(
"WarmSocketImpact", kWarmSocketDivisor, "last_accessed_socket",
2013, 1, 30, &last_accessed_socket));

const int warmest_socket = warmest_socket_trial->AppendGroup(
"warmest_socket", kWarmSocketProbability);
const int warm_socket = warmest_socket_trial->AppendGroup(
"warm_socket", kWarmSocketProbability);

const int warmest_socket_trial_group = warmest_socket_trial->group();

const int policy_list[] = { warmest_socket, warm_socket,
last_accessed_socket };
SetSocketReusePolicy(warmest_socket_trial_group, policy_list,
arraysize(policy_list));
}

void ChromeBrowserFieldTrials::AutoLaunchChromeFieldTrial() {
std::string brand;
google_util::GetBrand(&brand);
Expand Down
9 changes: 0 additions & 9 deletions chrome/browser/chrome_browser_field_trials.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include "base/basictypes.h"
#include "base/command_line.h"
#include "base/gtest_prod_util.h"
#include "base/time.h"

class PrefService;
Expand All @@ -22,11 +21,6 @@ class ChromeBrowserFieldTrials {
void SetupFieldTrials(PrefService* local_state);

private:
FRIEND_TEST_ALL_PREFIXES(BrowserMainTest,
WarmConnectionFieldTrial_WarmestSocket);
FRIEND_TEST_ALL_PREFIXES(BrowserMainTest, WarmConnectionFieldTrial_Random);
FRIEND_TEST_ALL_PREFIXES(BrowserMainTest, WarmConnectionFieldTrial_Invalid);

// Sets up common desktop-only field trials.
// Add an invocation of your field trial init function to this method, or to
// SetupFieldTrials if it is for all platforms.
Expand All @@ -41,9 +35,6 @@ class ChromeBrowserFieldTrials {
// A/B test for spdy when --use-spdy not set.
void SpdyFieldTrial();

// A/B test for warmest socket vs. most recently used socket.
void WarmConnectionFieldTrial();

// Field trial to see what disabling DNS pre-resolution does to
// latency of page loads.
void PredictorFieldTrial();
Expand Down
7 changes: 0 additions & 7 deletions chrome/browser/chrome_browser_main.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#define CHROME_BROWSER_CHROME_BROWSER_MAIN_H_

#include "base/basictypes.h"
#include "base/gtest_prod_util.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/scoped_vector.h"
#include "base/metrics/field_trial.h"
#include "base/tracked_objects.h"
#include "chrome/browser/chrome_browser_field_trials.h"
Expand Down Expand Up @@ -199,11 +197,6 @@ class ChromeBrowserMainParts : public content::BrowserMainParts {
// network stack, as this can only be done once.
static bool disable_enforcing_cookie_policies_for_tests_;

friend class BrowserMainTest;
FRIEND_TEST_ALL_PREFIXES(BrowserMainTest,
WarmConnectionFieldTrial_WarmestSocket);
FRIEND_TEST_ALL_PREFIXES(BrowserMainTest, WarmConnectionFieldTrial_Random);
FRIEND_TEST_ALL_PREFIXES(BrowserMainTest, WarmConnectionFieldTrial_Invalid);
DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainParts);
};

Expand Down
96 changes: 0 additions & 96 deletions chrome/browser/chrome_browser_main_unittest.cc

This file was deleted.

1 change: 0 additions & 1 deletion chrome/chrome_tests_unit.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,6 @@
'browser/captive_portal/testing_utils.cc',
'browser/captive_portal/testing_utils.h',
'browser/chrome_browser_application_mac_unittest.mm',
'browser/chrome_browser_main_unittest.cc',
'browser/chrome_page_zoom_unittest.cc',
'browser/chromeos/accessibility/magnification_manager_unittest.cc',
'browser/chromeos/contacts/contact_database_unittest.cc',
Expand Down
4 changes: 0 additions & 4 deletions chrome/common/chrome_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1271,10 +1271,6 @@ const char kSimulateOutdated[] = "simulate-outdated";
// Replaces the buffered data source for <audio> and <video> with a simplified
// resource loader that downloads the entire resource into memory.

// Socket reuse policy. The value should be of type enum
// ClientSocketReusePolicy.
const char kSocketReusePolicy[] = "socket-reuse-policy";

// Origin for which SpdyProxy authentication is supported.
const char kSpdyProxyAuthOrigin[] = "spdy-proxy-auth-origin";

Expand Down
1 change: 0 additions & 1 deletion chrome/common/chrome_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,6 @@ extern const char kSilentDumpOnDCHECK[];
extern const char kSimulateUpgrade[];
extern const char kSimulateCriticalUpdate[];
extern const char kSimulateOutdated[];
extern const char kSocketReusePolicy[];
extern const char kSpeculativeResourcePrefetching[];
extern const char kSpeculativeResourcePrefetchingDisabled[];
extern const char kSpeculativeResourcePrefetchingLearning[];
Expand Down
10 changes: 0 additions & 10 deletions content/browser/renderer_host/p2p/socket_host_test_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ class FakeSocket : public net::StreamSocket {
virtual void SetOmniboxSpeculation() OVERRIDE;
virtual bool WasEverUsed() const OVERRIDE;
virtual bool UsingTCPFastOpen() const OVERRIDE;
virtual int64 NumBytesRead() const OVERRIDE;
virtual base::TimeDelta GetConnectTimeMicros() const OVERRIDE;
virtual bool WasNpnNegotiated() const OVERRIDE;
virtual net::NextProto GetNegotiatedProtocol() const OVERRIDE;
virtual bool GetSSLInfo(net::SSLInfo* ssl_info) OVERRIDE;
Expand Down Expand Up @@ -213,14 +211,6 @@ bool FakeSocket::UsingTCPFastOpen() const {
return false;
}

int64 FakeSocket::NumBytesRead() const {
return -1;
}

base::TimeDelta FakeSocket::GetConnectTimeMicros() const {
return base::TimeDelta::FromMicroseconds(-1);
}

bool FakeSocket::WasNpnNegotiated() const {
return false;
}
Expand Down
8 changes: 0 additions & 8 deletions jingle/glue/fake_ssl_client_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -330,14 +330,6 @@ bool FakeSSLClientSocket::UsingTCPFastOpen() const {
return transport_socket_->UsingTCPFastOpen();
}

int64 FakeSSLClientSocket::NumBytesRead() const {
return transport_socket_->NumBytesRead();
}

base::TimeDelta FakeSSLClientSocket::GetConnectTimeMicros() const {
return transport_socket_->GetConnectTimeMicros();
}

bool FakeSSLClientSocket::WasNpnNegotiated() const {
return transport_socket_->WasNpnNegotiated();
}
Expand Down
2 changes: 0 additions & 2 deletions jingle/glue/fake_ssl_client_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ class FakeSSLClientSocket : public net::StreamSocket {
virtual void SetOmniboxSpeculation() OVERRIDE;
virtual bool WasEverUsed() const OVERRIDE;
virtual bool UsingTCPFastOpen() const OVERRIDE;
virtual int64 NumBytesRead() const OVERRIDE;
virtual base::TimeDelta GetConnectTimeMicros() const OVERRIDE;
virtual bool WasNpnNegotiated() const OVERRIDE;
virtual net::NextProto GetNegotiatedProtocol() const OVERRIDE;
virtual bool GetSSLInfo(net::SSLInfo* ssl_info) OVERRIDE;
Expand Down
14 changes: 0 additions & 14 deletions jingle/glue/proxy_resolving_client_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -373,20 +373,6 @@ bool ProxyResolvingClientSocket::UsingTCPFastOpen() const {
return false;
}

int64 ProxyResolvingClientSocket::NumBytesRead() const {
if (transport_.get() && transport_->socket())
return transport_->socket()->NumBytesRead();
NOTREACHED();
return -1;
}

base::TimeDelta ProxyResolvingClientSocket::GetConnectTimeMicros() const {
if (transport_.get() && transport_->socket())
return transport_->socket()->GetConnectTimeMicros();
NOTREACHED();
return base::TimeDelta::FromMicroseconds(-1);
}

bool ProxyResolvingClientSocket::WasNpnNegotiated() const {
return false;
}
Expand Down
2 changes: 0 additions & 2 deletions jingle/glue/proxy_resolving_client_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ class ProxyResolvingClientSocket : public net::StreamSocket {
virtual void SetOmniboxSpeculation() OVERRIDE;
virtual bool WasEverUsed() const OVERRIDE;
virtual bool UsingTCPFastOpen() const OVERRIDE;
virtual int64 NumBytesRead() const OVERRIDE;
virtual base::TimeDelta GetConnectTimeMicros() const OVERRIDE;
virtual bool WasNpnNegotiated() const OVERRIDE;
virtual net::NextProto GetNegotiatedProtocol() const OVERRIDE;
virtual bool GetSSLInfo(net::SSLInfo* ssl_info) OVERRIDE;
Expand Down
10 changes: 0 additions & 10 deletions jingle/glue/pseudotcp_adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -563,16 +563,6 @@ bool PseudoTcpAdapter::UsingTCPFastOpen() const {
return false;
}

int64 PseudoTcpAdapter::NumBytesRead() const {
DCHECK(CalledOnValidThread());
return -1;
}

base::TimeDelta PseudoTcpAdapter::GetConnectTimeMicros() const {
DCHECK(CalledOnValidThread());
return base::TimeDelta::FromMicroseconds(-1);
}

bool PseudoTcpAdapter::WasNpnNegotiated() const {
DCHECK(CalledOnValidThread());
return false;
Expand Down
2 changes: 0 additions & 2 deletions jingle/glue/pseudotcp_adapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ class PseudoTcpAdapter : public net::StreamSocket, base::NonThreadSafe {
virtual void SetOmniboxSpeculation() OVERRIDE;
virtual bool WasEverUsed() const OVERRIDE;
virtual bool UsingTCPFastOpen() const OVERRIDE;
virtual int64 NumBytesRead() const OVERRIDE;
virtual base::TimeDelta GetConnectTimeMicros() const OVERRIDE;
virtual bool WasNpnNegotiated() const OVERRIDE;
virtual net::NextProto GetNegotiatedProtocol() const OVERRIDE;
virtual bool GetSSLInfo(net::SSLInfo* ssl_info) OVERRIDE;
Expand Down
Loading

0 comments on commit e86df8d

Please sign in to comment.