Skip to content

Commit

Permalink
Add a force pipelining option to load flags.
Browse files Browse the repository at this point in the history
Details:
- Add a HttpPipelinedHostForced class for connections with forced requests.
  + Forced requests get their own pipeline and there's only one per host.
  + They always try to pipeline and won't retry if evicted.
  + Only one HttpStreamFactoryImpl::Job runs for all requests to the same
    origin with forced pipelining. All requests will fail if that Job fails.
- Track HttpPipelinedHosts with a Key. Right now that's origin and
  force-pipelining, but it might be expanded to include content type.
- Add a BufferedWriteStreamSocket that wraps a normal socket. It buffers Write()
  calls until a task fires to dispatch the buffer to the underlying socket.

BUG=110794
TEST=net_unittests and unit_tests


Review URL: http://codereview.chromium.org/9433015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@124487 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
simonjam@chromium.org committed Mar 1, 2012
1 parent 271398a commit 5477d89
Show file tree
Hide file tree
Showing 32 changed files with 1,317 additions and 269 deletions.
19 changes: 17 additions & 2 deletions chrome/browser/net/http_pipelining_compatibility_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#include "base/stringprintf.h"
#include "net/base/load_flags.h"
#include "net/disk_cache/histogram_macros.h"
#include "net/http/http_network_layer.h"
#include "net/http/http_network_session.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_version.h"

Expand All @@ -25,10 +27,24 @@ void HttpPipeliningCompatibilityClient::Start(
std::vector<RequestInfo>& requests,
const net::CompletionCallback& callback,
net::URLRequestContext* url_request_context) {
net::HttpNetworkSession* old_session =
url_request_context->http_transaction_factory()->GetSession();
net::HttpNetworkSession::Params params = old_session->params();
params.force_http_pipelining = true;
scoped_refptr<net::HttpNetworkSession> session =
new net::HttpNetworkSession(params);
http_transaction_factory_.reset(
net::HttpNetworkLayer::CreateFactory(session.get()));

url_request_context_ = new net::URLRequestContext;
url_request_context_->CopyFrom(url_request_context);
url_request_context_->set_http_transaction_factory(
http_transaction_factory_.get());

finished_callback_ = callback;
for (size_t i = 0; i < requests.size(); ++i) {
requests_.push_back(new Request(i, base_url, requests[i], this,
url_request_context));
url_request_context_.get()));
}
}

Expand Down Expand Up @@ -74,7 +90,6 @@ HttpPipeliningCompatibilityClient::Request::Request(
client_(client),
finished_(false) {
request_.set_context(url_request_context);
// TODO(simonjam): Force pipelining.
request_.set_load_flags(net::LOAD_BYPASS_CACHE |
net::LOAD_DISABLE_CACHE |
net::LOAD_DO_NOT_SAVE_COOKIES |
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/net/http_pipelining_compatibility_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "net/base/completion_callback.h"
#include "net/base/io_buffer.h"
#include "net/url_request/url_request.h"
#include "net/url_request/url_request_context.h"

namespace chrome_browser_net {

Expand Down Expand Up @@ -118,6 +119,8 @@ class HttpPipeliningCompatibilityClient {
ScopedVector<Request> requests_;
net::CompletionCallback finished_callback_;
size_t num_finished_;
scoped_ptr<net::HttpTransactionFactory> http_transaction_factory_;
scoped_refptr<net::URLRequestContext> url_request_context_;
};

} // namespace chrome_browser_net
Expand Down
41 changes: 30 additions & 11 deletions chrome/browser/net/http_pipelining_compatibility_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,32 @@ class HttpPipeliningCompatibilityClientTest : public testing::Test {
private:
base::Histogram::SampleSet GetHistogram(const char* name) {
base::Histogram::SampleSet sample;
base::Histogram* histogram;
base::Histogram* current_histogram = NULL;
base::Histogram* cached_histogram = NULL;
base::StatisticsRecorder::FindHistogram(name, &current_histogram);
if (ContainsKey(histograms_, name)) {
histogram = histograms_[name];
histogram->SnapshotSample(&sample);
} else if (base::StatisticsRecorder::FindHistogram(name, &histogram)) {
histograms_[name] = histogram;
histogram->SnapshotSample(&sample);
cached_histogram = histograms_[name];
}

// This is to work around the CACHE_HISTOGRAM_* macros caching the last used
// histogram by name. So, even though we throw out the StatisticsRecorder
// between tests, the CACHE_HISTOGRAM_* might still write into the old
// Histogram if it has the same name as the last run. We keep a cache of the
// last used Histogram and then update the cache if it's different than the
// current Histogram.
if (cached_histogram && current_histogram) {
cached_histogram->SnapshotSample(&sample);
if (cached_histogram != current_histogram) {
base::Histogram::SampleSet current_sample;
current_histogram->SnapshotSample(&current_sample);
sample.Add(current_sample);
histograms_[name] = current_histogram;
}
} else if (current_histogram) {
current_histogram->SnapshotSample(&sample);
histograms_[name] = current_histogram;
} else if (cached_histogram) {
cached_histogram->SnapshotSample(&sample);
}
return sample;
}
Expand Down Expand Up @@ -416,7 +435,7 @@ TEST_F(HttpPipeliningCompatibilityClientTest, MultipleRequests) {
base::Histogram::SampleSet network_sample2 =
GetHistogramValue(1, FIELD_NETWORK_ERROR);
EXPECT_EQ(1, network_sample2.TotalCount());
EXPECT_EQ(1, network_sample2.counts(-net::ERR_EMPTY_RESPONSE));
EXPECT_EQ(1, network_sample2.counts(-net::ERR_PIPELINE_EVICTION));

base::Histogram::SampleSet response_sample2 =
GetHistogramValue(1, FIELD_RESPONSE_CODE);
Expand All @@ -426,16 +445,16 @@ TEST_F(HttpPipeliningCompatibilityClientTest, MultipleRequests) {
GetHistogramValue(2, FIELD_STATUS);
EXPECT_EQ(1, status_sample3.TotalCount());
EXPECT_EQ(1, status_sample3.counts(
HttpPipeliningCompatibilityClient::BAD_RESPONSE_CODE));
HttpPipeliningCompatibilityClient::NETWORK_ERROR));

base::Histogram::SampleSet network_sample3 =
GetHistogramValue(2, FIELD_NETWORK_ERROR);
EXPECT_EQ(0, network_sample3.TotalCount());
EXPECT_EQ(1, network_sample3.TotalCount());
EXPECT_EQ(1, network_sample3.counts(-net::ERR_PIPELINE_EVICTION));

base::Histogram::SampleSet response_sample3 =
GetHistogramValue(2, FIELD_RESPONSE_CODE);
EXPECT_EQ(1, response_sample3.TotalCount());
EXPECT_EQ(1, response_sample3.counts(401));
EXPECT_EQ(0, response_sample3.TotalCount());
}

} // anonymous namespace
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/resources/net_internals/http_pipeline_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ var HttpPipelineView = (function() {
function createConnectionTablePrinter(httpPipelinedConnectionInfo) {
var tablePrinter = new TablePrinter();
tablePrinter.addHeaderCell('Host');
tablePrinter.addHeaderCell('Forced');
tablePrinter.addHeaderCell('Depth');
tablePrinter.addHeaderCell('Capacity');
tablePrinter.addHeaderCell('Usable');
Expand All @@ -134,6 +135,8 @@ var HttpPipelineView = (function() {
tablePrinter.addRow();

tablePrinter.addCell(connection.host);
tablePrinter.addCell(
connection.forced === undefined ? false : connection.forced);
tablePrinter.addCell(connection.depth);
tablePrinter.addCell(connection.capacity);
tablePrinter.addCell(connection.usable);
Expand Down
6 changes: 4 additions & 2 deletions net/http/http_network_session.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand Down Expand Up @@ -29,6 +29,7 @@ HttpNetworkSession::HttpNetworkSession(const Params& params)
http_server_properties_(params.http_server_properties),
cert_verifier_(params.cert_verifier),
http_auth_handler_factory_(params.http_auth_handler_factory),
force_http_pipelining_(params.force_http_pipelining),
proxy_service_(params.proxy_service),
ssl_config_service_(params.ssl_config_service),
socket_pool_manager_(
Expand All @@ -49,7 +50,8 @@ HttpNetworkSession::HttpNetworkSession(const Params& params)
params.ssl_config_service,
params.http_server_properties),
ALLOW_THIS_IN_INITIALIZER_LIST(http_stream_factory_(
new HttpStreamFactoryImpl(this))) {
new HttpStreamFactoryImpl(this))),
params_(params) {
DCHECK(proxy_service_);
DCHECK(ssl_config_service_);
CHECK(http_server_properties_);
Expand Down
14 changes: 12 additions & 2 deletions net/http/http_network_session.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand Down Expand Up @@ -62,7 +62,8 @@ class NET_EXPORT HttpNetworkSession
http_auth_handler_factory(NULL),
network_delegate(NULL),
http_server_properties(NULL),
net_log(NULL) {}
net_log(NULL),
force_http_pipelining(false) {}

ClientSocketFactory* client_socket_factory;
HostResolver* host_resolver;
Expand All @@ -77,6 +78,7 @@ class NET_EXPORT HttpNetworkSession
NetworkDelegate* network_delegate;
HttpServerProperties* http_server_properties;
NetLog* net_log;
bool force_http_pipelining;
};

explicit HttpNetworkSession(const Params& params);
Expand Down Expand Up @@ -138,6 +140,11 @@ class NET_EXPORT HttpNetworkSession
void CloseAllConnections();
void CloseIdleConnections();

bool force_http_pipelining() const { return force_http_pipelining_; }

// Returns the original Params used to construct this session.
const Params& params() const { return params_; }

private:
friend class base::RefCounted<HttpNetworkSession>;
friend class HttpNetworkSessionPeer;
Expand All @@ -149,6 +156,7 @@ class NET_EXPORT HttpNetworkSession
HttpServerProperties* const http_server_properties_;
CertVerifier* const cert_verifier_;
HttpAuthHandlerFactory* const http_auth_handler_factory_;
bool force_http_pipelining_;

// Not const since it's modified by HttpNetworkSessionPeer for testing.
ProxyService* proxy_service_;
Expand All @@ -160,6 +168,8 @@ class NET_EXPORT HttpNetworkSession
SpdySessionPool spdy_session_pool_;
scoped_ptr<HttpStreamFactory> http_stream_factory_;
std::set<HttpResponseBodyDrainer*> response_drainers_;

Params params_;
};

} // namespace net
Expand Down
8 changes: 8 additions & 0 deletions net/http/http_network_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1209,6 +1209,14 @@ int HttpNetworkTransaction::HandleIOError(int error) {
}
break;
case ERR_PIPELINE_EVICTION:
if (!session_->force_http_pipelining()) {
net_log_.AddEvent(
NetLog::TYPE_HTTP_TRANSACTION_RESTART_AFTER_ERROR,
make_scoped_refptr(new NetLogIntegerParameter("net_error", error)));
ResetConnectionAndRequestForResend();
error = OK;
}
break;
case ERR_SPDY_PING_FAILED:
case ERR_SPDY_SERVER_REFUSED_STREAM:
net_log_.AddEvent(
Expand Down
20 changes: 19 additions & 1 deletion net/http/http_pipelined_connection_impl.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand Down Expand Up @@ -46,6 +46,24 @@ class SSLInfo;
class NET_EXPORT_PRIVATE HttpPipelinedConnectionImpl
: public HttpPipelinedConnection {
public:
class Factory : public HttpPipelinedConnection::Factory {
public:
virtual HttpPipelinedConnection* CreateNewPipeline(
ClientSocketHandle* connection,
HttpPipelinedConnection::Delegate* delegate,
const HostPortPair& origin,
const SSLConfig& used_ssl_config,
const ProxyInfo& used_proxy_info,
const BoundNetLog& net_log,
bool was_npn_negotiated,
SSLClientSocket::NextProto protocol_negotiated) OVERRIDE {
return new HttpPipelinedConnectionImpl(connection, delegate, origin,
used_ssl_config, used_proxy_info,
net_log, was_npn_negotiated,
protocol_negotiated);
}
};

HttpPipelinedConnectionImpl(ClientSocketHandle* connection,
Delegate* delegate,
const HostPortPair& origin,
Expand Down
17 changes: 17 additions & 0 deletions net/http/http_pipelined_host.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "net/http/http_pipelined_host.h"

namespace net {

HttpPipelinedHost::Key::Key(const HostPortPair& origin)
: origin_(origin) {
}

bool HttpPipelinedHost::Key::operator<(const Key& rhs) const {
return origin_ < rhs.origin_;
}

} // namespace net
26 changes: 20 additions & 6 deletions net/http/http_pipelined_host.h
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef NET_HTTP_HTTP_PIPELINED_HOST_H_
#define NET_HTTP_HTTP_PIPELINED_HOST_H_
#pragma once

#include "net/base/host_port_pair.h"
#include "net/base/net_export.h"
#include "net/http/http_pipelined_connection.h"
#include "net/http/http_pipelined_host_capability.h"
Expand All @@ -28,6 +29,19 @@ struct SSLConfig;
// assigns requests to the least loaded pipelined connection.
class NET_EXPORT_PRIVATE HttpPipelinedHost {
public:
class NET_EXPORT_PRIVATE Key {
public:
Key(const HostPortPair& origin);

// The host and port associated with this key.
const HostPortPair& origin() const { return origin_; }

bool operator<(const Key& rhs) const;

private:
const HostPortPair origin_;
};

class Delegate {
public:
// Called when a pipelined host has no outstanding requests on any of its
Expand All @@ -50,9 +64,10 @@ class NET_EXPORT_PRIVATE HttpPipelinedHost {

// Returns a new HttpPipelinedHost.
virtual HttpPipelinedHost* CreateNewHost(
Delegate* delegate, const HostPortPair& origin,
Delegate* delegate, const Key& key,
HttpPipelinedConnection::Factory* factory,
HttpPipelinedHostCapability capability) = 0;
HttpPipelinedHostCapability capability,
bool force_pipelining) = 0;
};

virtual ~HttpPipelinedHost() {}
Expand All @@ -75,13 +90,12 @@ class NET_EXPORT_PRIVATE HttpPipelinedHost {
// requests.
virtual bool IsExistingPipelineAvailable() const = 0;

// Returns the host and port associated with this class.
virtual const HostPortPair& origin() const = 0;
// Returns a Key that uniquely identifies this host.
virtual const Key& GetKey() const = 0;

// Creates a Value summary of this host's pipelines. Caller assumes
// ownership of the returned Value.
virtual base::Value* PipelineInfoToValue() const = 0;

};

} // namespace net
Expand Down
Loading

0 comments on commit 5477d89

Please sign in to comment.