Set receive buffer size, and comment about possible setting of send buffer.

net/quic/quic_stream_factory.cc

 // 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/quic/quic_stream_factory.h"
 
 #include <set>
 
+#include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/values.h"
 #include "net/base/net_errors.h"
 #include "net/dns/host_resolver.h"
 #include "net/dns/single_request_host_resolver.h"
 #include "net/quic/crypto/quic_random.h"
 #include "net/quic/quic_client_session.h"
@@ skipping 348 matching lines @@
     const AddressList& address_list,
     const BoundNetLog& net_log) {
   QuicGuid guid = random_generator_->RandUint64();
   IPEndPoint addr = *address_list.begin();
   DatagramClientSocket* socket =
       client_socket_factory_->CreateDatagramClientSocket(
           DatagramSocket::DEFAULT_BIND, base::Bind(&base::RandInt),
           net_log.net_log(), net_log.source());
   socket->Connect(addr);
 
+  // We should adaptively set this buffer size, but for now, we'll use a size
+  // that is more than large enough for a 100 packet congestion window, and yet
+  // does not consume "too much" memory.  If we see bursty packet loss, we may
+  // revisit this setting and test for its impact.
+  int32 const kSocketBufferSize(1200 * 100);  // Support 100 packets.

[Ryan Hamilton, 2013/06/20 03:42:23]

nit: in quic_protocol.h, we have kMaxPacketSize = 1200.  so how 'bout:

const int32 kSocketReceiveBufferSize = kMaxPacketSize * 100;

Also we tend to define such constants at the top of the file, so I recommend

namespace {

const int32 kSocketReceiveBufferSize = kMaxPacketSize * 100;

}  // namespace

[jar (doing other things), 2013/06/20 21:45:51]

Done.

+  DLOG(INFO) << "QUIC socket receive buffer set to " << kSocketBufferSize;

[Ryan Hamilton, 2013/06/20 03:42:23]

super minor nit:  Do we really need this log line?  It's going to log the exact
same value each time.  Seems a bit unnecessary.  But clearly, this is not a big
deal :>

[jar (doing other things), 2013/06/20 21:45:51]

Mixed feeelings.... but since it bothered you, it is out-a-here ;-).

+  socket->SetReceiveBufferSize(kSocketBufferSize);

[Ryan Hamilton, 2013/06/20 03:42:23]

I don't think this is worth doing, but for the sake of completeness...  If you
were super excited about testing, you could add a bit of logic to
MockUDPClientSocket which would allow you to verify that the QuicStreamFactory
sets the correct value for the receive buffer size.  This doesn't seem terribly
important, though in some sense this CL is a bug fix so perhaps a test would be
a good idea?

[jar (doing other things), 2013/06/20 21:45:51]

Yeah... probably a good idea... I'd rather get this in simply, and not have to
extend the mock socket etc. now.

+  // What should the send buffer be set to?  If the send buffer is too large,

[Ryan Hamilton, 2013/06/20 03:42:23]

nit: Add TODO(jar):  (or TODO(rch)?) to this comment.

[jar (doing other things), 2013/06/20 21:45:51]

Done.

+  // then we might(?) wastefully queue packets in the OS, when we'd rather
+  // construct packets just in time.  If this is too small, then we will starve
+  // our outgoing link on a fast connection, because we won't respond fast
+  // enough to the many async callbacks to get data from us.  We also buy a bit
+  // of adhoc-pacing by requiring the application to refill this OS buffer
+  // (ensuring that we don't blast a pile of packets at the kernel's max egress
+  // rate).
+  // socket->SetSendBufferSize(kSocketBufferSize?? other??);
+
   QuicConnectionHelper* helper = new QuicConnectionHelper(
       base::MessageLoop::current()->message_loop_proxy().get(),
       clock_.get(),
       random_generator_,
       socket);
 
   QuicConnection* connection = new QuicConnection(guid, addr, helper, false);
 
   QuicCryptoClientConfig* crypto_config =
       GetOrCreateCryptoConfig(host_port_proxy_pair);
@@ skipping 29 matching lines @@
     DCHECK(crypto_config);
   } else {
     crypto_config = new QuicCryptoClientConfig();
     crypto_config->SetDefaults();
     all_crypto_configs_[host_port_proxy_pair] = crypto_config;
   }
   return crypto_config;
 }
 
 }  // namespace net