Slow start pipelining.

net/http/http_pipelined_host_impl.h

 // 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.
 
-#ifndef NET_HTTP_HTTP_PIPELINED_HOST_H_
-#define NET_HTTP_HTTP_PIPELINED_HOST_H_
+#ifndef NET_HTTP_HTTP_PIPELINED_HOST_IMPL_H_
+#define NET_HTTP_HTTP_PIPELINED_HOST_IMPL_H_
 #pragma once
 
-#include <set>
+#include <map>
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #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.h"
 
 namespace net {
 
 class BoundNetLog;
 class ClientSocketHandle;
 class HttpPipelinedStream;
 class ProxyInfo;
 struct SSLConfig;
 
 // Manages all of the pipelining state for specific host with active pipelined
 // HTTP requests. Manages connection jobs, constructs pipelined streams, and
 // assigns requests to the least loaded pipelined connection.
-class NET_EXPORT_PRIVATE HttpPipelinedHost
-    : public HttpPipelinedConnection::Delegate {
+class NET_EXPORT_PRIVATE HttpPipelinedHostImpl
+    : public HttpPipelinedHost,
+      public HttpPipelinedConnection::Delegate {
  public:
-  class Delegate {
-   public:
-    // Called when a pipelined host has no outstanding requests on any of its
-    // pipelined connections.
-    virtual void OnHostIdle(HttpPipelinedHost* host) = 0;
+  HttpPipelinedHostImpl(HttpPipelinedHost::Delegate* delegate,
+                        const HostPortPair& origin,
+                        HttpPipelinedConnection::Factory* factory,
+                        Capability capability);
+  virtual ~HttpPipelinedHostImpl();
 
-    // Called when a pipelined host has newly available pipeline capacity, like
-    // when a request completes.
-    virtual void OnHostHasAdditionalCapacity(HttpPipelinedHost* host) = 0;
-  };
-
-  HttpPipelinedHost(Delegate* delegate, const HostPortPair& origin,
-                    HttpPipelinedConnection::Factory* factory);
-  virtual ~HttpPipelinedHost();
-
-  // Constructs a new pipeline on |connection| and returns a new
-  // HttpPipelinedStream that uses it.
-  HttpPipelinedStream* CreateStreamOnNewPipeline(
+  // HttpPipelinedHost Interface

[mmenke, 2011/11/21 14:51:39]

nit:  interface is generally not capitalized in lines like this, elsewhere in
the code.

[James Simonsen, 2011/12/01 01:17:10]

Done.

+  virtual HttpPipelinedStream* CreateStreamOnNewPipeline(
       ClientSocketHandle* connection,
       const SSLConfig& used_ssl_config,
       const ProxyInfo& used_proxy_info,
       const BoundNetLog& net_log,
-      bool was_npn_negotiated);
+      bool was_npn_negotiated) OVERRIDE;
 
-  // Tries to find an existing pipeline with capacity for a new request. If
-  // successful, returns a new stream on that pipeline. Otherwise, returns NULL.
-  HttpPipelinedStream* CreateStreamOnExistingPipeline();
+  virtual HttpPipelinedStream* CreateStreamOnExistingPipeline() OVERRIDE;
 
-  // Returns true if we have a pipelined connection that can accept new
-  // requests.
-  bool IsExistingPipelineAvailable();
+  virtual bool IsExistingPipelineAvailable() OVERRIDE;

[mmenke, 2011/11/21 14:51:39]

I believe this can be const.

[James Simonsen, 2011/12/01 01:17:10]

Done.

 
-  // Callbacks for HttpPipelinedConnection.
+  // HttpPipelinedConnection::Delegate Interface

[mmenke, 2011/11/21 14:51:39]

lowercase "interface"

[James Simonsen, 2011/12/01 01:17:10]

Done.

 
   // Called when a pipelined connection completes a request. Adds a pending
   // request to the pipeline if the pipeline is still usable.
   virtual void OnPipelineHasCapacity(
       HttpPipelinedConnection* pipeline) OVERRIDE;
 
-  const HostPortPair& origin() const { return origin_; }
+  virtual void OnPipelineFeedback(
+      HttpPipelinedConnection* pipeline,
+      HttpPipelinedConnection::Feedback feedback) OVERRIDE;
+
+  virtual const HostPortPair& origin() const OVERRIDE;
 
  private:
+  struct PipelineInfo {
+    PipelineInfo();
+
+    int capacity;

[mmenke, 2011/11/28 23:05:28]

Having capacity here strikes me as a little weird.  It seems more natural to me
to either make it a property of the PipelinedHost, and apply it to all its
streams, or to make it a property of the pipeline itself (Maybe along with
num_successes), rather than to have it here.

The one complexity of the first case is that we'd need to call
OnPipelineHasCapacity on all a host's streams once we decide a host supports
pipelining, but that may give us better performance, anyways.

What do you think?

[James Simonsen, 2011/12/01 01:17:10]

Good idea. I like the performance win. Done.

+    int num_successes;
+  };
+  typedef std::map<HttpPipelinedConnection*, PipelineInfo> PipelineInfoMap;
+
   // Called when a pipeline is empty and there are no pending requests. Closes
   // the connection.
   void OnPipelineEmpty(HttpPipelinedConnection* pipeline);
 
   // Adds the next pending request to the pipeline if it's still usuable.
   void AddRequestToPipeline(HttpPipelinedConnection* connection);
 
   int max_pipeline_depth() const { return 3; }
 
-  Delegate* delegate_;
+  HttpPipelinedHost::Delegate* delegate_;
   const HostPortPair origin_;
-  std::set<HttpPipelinedConnection*> pipelines_;
+  PipelineInfoMap pipelines_;
   scoped_ptr<HttpPipelinedConnection::Factory> factory_;
+  Capability capability_;
 
-  DISALLOW_COPY_AND_ASSIGN(HttpPipelinedHost);
+  DISALLOW_COPY_AND_ASSIGN(HttpPipelinedHostImpl);
 };
 
 }  // namespace net
 
-#endif  // NET_HTTP_HTTP_PIPELINED_HOST_H_
+#endif  // NET_HTTP_HTTP_PIPELINED_HOST_IMPL_H_