Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(196)

Unified Diff: net/http/http_pipelined_host_impl.cc

Issue 8586015: Slow start pipelining. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Add missing files Created 9 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/http/http_pipelined_host_impl.cc
diff --git a/net/http/http_pipelined_host_impl.cc b/net/http/http_pipelined_host_impl.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f125ef7003b61f3cedbc96bc3eaf719f1a256cd3
--- /dev/null
+++ b/net/http/http_pipelined_host_impl.cc
@@ -0,0 +1,174 @@
+// Copyright (c) 2011 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_impl.h"
+
+#include "base/stl_util.h"
+#include "net/http/http_pipelined_connection_impl.h"
+#include "net/http/http_pipelined_stream.h"
+
+namespace net {
+
+// TODO(simonjam): Run experiments to see what value minimizes evictions without
+// costing too much performance. Until then, this is just a bad guess.
+static const int kNumKnownSuccessesThreshold = 3;
+
+class HttpPipelinedConnectionImplFactory :
+ public HttpPipelinedConnection::Factory {
+ public:
+ HttpPipelinedConnection* CreateNewPipeline(
+ ClientSocketHandle* connection,
+ HttpPipelinedConnection::Delegate* delegate,
+ const SSLConfig& used_ssl_config,
+ const ProxyInfo& used_proxy_info,
+ const BoundNetLog& net_log,
+ bool was_npn_negotiated) OVERRIDE {
+ return new HttpPipelinedConnectionImpl(connection, delegate,
+ used_ssl_config, used_proxy_info,
+ net_log, was_npn_negotiated);
+ }
+};
+
+HttpPipelinedHostImpl::HttpPipelinedHostImpl(
+ HttpPipelinedHost::Delegate* delegate,
+ const HostPortPair& origin,
+ HttpPipelinedConnection::Factory* factory,
+ Capability capability)
+ : delegate_(delegate),
+ origin_(origin),
+ factory_(factory),
+ capability_(capability) {
+ if (!factory) {
+ factory_.reset(new HttpPipelinedConnectionImplFactory());
+ }
+}
+
+HttpPipelinedHostImpl::~HttpPipelinedHostImpl() {
+ CHECK(pipelines_.empty());
+}
+
+HttpPipelinedStream* HttpPipelinedHostImpl::CreateStreamOnNewPipeline(
+ ClientSocketHandle* connection,
+ const SSLConfig& used_ssl_config,
+ const ProxyInfo& used_proxy_info,
+ const BoundNetLog& net_log,
+ bool was_npn_negotiated) {
+ HttpPipelinedConnection* pipeline = factory_->CreateNewPipeline(
+ connection, this, used_ssl_config, used_proxy_info, net_log,
+ was_npn_negotiated);
+ PipelineInfo info;
+ switch (capability_) {
+ case CAPABLE:
+ info.capacity = max_pipeline_depth();
+ break;
+
+ case INCAPABLE:
+ CHECK(false);
+
+ case UNKNOWN:
+ info.capacity = 1;
+ break;
+ }
+ pipelines_.insert(std::make_pair(pipeline, info));
+ return pipeline->CreateNewStream();
+}
+
+HttpPipelinedStream* HttpPipelinedHostImpl::CreateStreamOnExistingPipeline() {
+ HttpPipelinedConnection* available_pipeline = NULL;
+ for (PipelineInfoMap::iterator it = pipelines_.begin();
+ it != pipelines_.end(); ++it) {
+ if (it->first->usable() &&
+ it->first->active() &&
+ it->first->depth() < it->second.capacity &&
+ (!available_pipeline ||
+ it->first->depth() < available_pipeline->depth())) {
+ available_pipeline = it->first;
+ }
+ }
+ if (!available_pipeline) {
+ return NULL;
+ }
+ return available_pipeline->CreateNewStream();
+}
+
+bool HttpPipelinedHostImpl::IsExistingPipelineAvailable() {
+ for (PipelineInfoMap::iterator it = pipelines_.begin();
+ it != pipelines_.end(); ++it) {
+ if (it->first->usable() &&
+ it->first->active() &&
+ it->first->depth() < it->second.capacity) {
+ return true;
+ }
+ }
+ return false;
+}
+
+const HostPortPair& HttpPipelinedHostImpl::origin() const {
+ return origin_;
+}
+
+void HttpPipelinedHostImpl::OnPipelineEmpty(HttpPipelinedConnection* pipeline) {
+ CHECK(ContainsKey(pipelines_, pipeline));
+ if (capability_ == UNKNOWN &&
+ pipelines_[pipeline].num_successes >= kNumKnownSuccessesThreshold) {
+ capability_ = CAPABLE;
+ delegate_->OnHostDeterminedCapability(this, CAPABLE);
+ }
+ pipelines_.erase(pipeline);
+ delete pipeline;
+ if (pipelines_.empty()) {
+ delegate_->OnHostIdle(this);
+ // WARNING: We'll probably be deleted here.
+ }
+}
+
+void HttpPipelinedHostImpl::OnPipelineHasCapacity(
+ HttpPipelinedConnection* pipeline) {
+ CHECK(ContainsKey(pipelines_, pipeline));
+ if (pipeline->usable() &&
+ capability_ != INCAPABLE &&
+ pipeline->depth() < pipelines_[pipeline].capacity) {
+ delegate_->OnHostHasAdditionalCapacity(this);
+ }
+ if (!pipeline->depth()) {
+ OnPipelineEmpty(pipeline);
+ // WARNING: We might be deleted here.
+ }
+}
+
+void HttpPipelinedHostImpl::OnPipelineFeedback(
+ HttpPipelinedConnection* pipeline,
+ HttpPipelinedConnection::Feedback feedback) {
+ CHECK(ContainsKey(pipelines_, pipeline));
+ switch (feedback) {
+ case HttpPipelinedConnection::OK:
+ ++pipelines_[pipeline].num_successes;
+ if (pipelines_[pipeline].capacity != max_pipeline_depth()) {
+ pipelines_[pipeline].capacity = max_pipeline_depth();
+ OnPipelineHasCapacity(pipeline);
+ }
+ break;
+
+ case HttpPipelinedConnection::SOCKET_ERROR:
+ // TODO(simonjam): How often do we get spurious errors? Maybe we should
+ // just ignore these unless they're common. Until then, fall through...
mmenke 2011/11/17 22:25:37 Just think of the case of losing a network connect
James Simonsen 2011/11/18 00:03:56 Okay, I think a whitelist is the safest thing to d
mmenke 2011/11/21 14:51:39 Consider what servers that don't support pipelinin
+ case HttpPipelinedConnection::OLD_HTTP_VERSION:
+ // TODO(simonjam): I'd like to know if this ever happens after a HTTP/1.1.
+ // What's the best way to track that? If it does happen, I'd like to know
+ // which site, so I can debug it.
mmenke 2011/11/17 22:25:37 I'm no expert, but wouldn't surprise me if it did.
James Simonsen 2011/11/18 00:03:56 That's a good example. I guess I'll just take out
+ capability_ = INCAPABLE;
+ delegate_->OnHostDeterminedCapability(this, INCAPABLE);
+ break;
+
+ case HttpPipelinedConnection::MUST_CLOSE_CONNECTION:
+ break;
+ }
+}
+
+HttpPipelinedHostImpl::PipelineInfo::PipelineInfo()
+ : capacity(1),
mmenke 2011/11/17 22:25:37 nit: 4 space indent.
James Simonsen 2011/11/18 00:03:56 Done. I really need to figure out how to get vim t
+ num_successes(0) {
+}
+
+} // namespace net

Powered by Google App Engine
This is Rietveld 408576698