Deciding best connection to schedule requests on based on cwnd and idle time

net/socket/client_socket_pool_base.cc

 // 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/socket/client_socket_pool_base.h"
 
+#include <math.h>
+#include <utility>
+#include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/format_macros.h"
+#include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/metrics/stats_counters.h"
 #include "base/stl_util-inl.h"
+#include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/time.h"
 #include "base/values.h"
 #include "net/base/net_log.h"
 #include "net/base/net_errors.h"
 #include "net/socket/client_socket_handle.h"
 
 using base::TimeDelta;
 
 namespace {
 
 // The timeout value, in seconds, used to clean up idle sockets that can't be
 // reused.
 //
 // Note: It's important to close idle sockets that have received data as soon
 // as possible because the received data may cause BSOD on Windows XP under
 // some conditions.  See http://crbug.com/4606.
 const int kCleanupInterval = 10;  // DO NOT INCREASE THIS TIMEOUT.
 
 // Indicate whether or not we should establish a new transport layer connection
 // after a certain timeout has passed without receiving an ACK.
 bool g_connect_backup_jobs_enabled = true;
 
+// Decay coef.
+double g_tcp_socket_estimated_cwnd_decay_coef = 1000000;
+
 }  // namespace
 
 namespace net {
 
 ConnectJob::ConnectJob(const std::string& group_name,
                        base::TimeDelta timeout_duration,
                        Delegate* delegate,
                        const BoundNetLog& net_log)
     : group_name_(group_name),
       timeout_duration_(timeout_duration),
@@ skipping 311 matching lines @@
     }
   }
 
   return rv;
 }
 
 bool ClientSocketPoolBaseHelper::AssignIdleSocketToGroup(
     const Request* request, Group* group) {
   std::list<IdleSocket>* idle_sockets = group->mutable_idle_sockets();
   std::list<IdleSocket>::iterator idle_socket_it = idle_sockets->end();
+  double max_benefit = -100000000;
+  std::pair<int, int64> best_pair;
+
+  std::string debug_log(request->handle()->group_name());
+  std::string candidates;
+  double rtt_ms = 0;
 
   // Iterate through the idle sockets forwards (oldest to newest)
   //   * Delete any disconnected ones.
   //   * If we find a used idle socket, assign to |idle_socket|.  At the end,
   //   the |idle_socket_it| will be set to the newest used idle socket.
   for (std::list<IdleSocket>::iterator it = idle_sockets->begin();
        it != idle_sockets->end();) {
     if (!it->socket->IsConnectedAndIdle()) {
       DecrementIdleCount();
       delete it->socket;
       it = idle_sockets->erase(it);
       continue;
     }
 
     if (it->socket->WasEverUsed()) {
       // We found one we can reuse!
-      idle_socket_it = it;
+      int64 bytes_read = it->socket->NumBytesRead();
+      double B = static_cast<double>(bytes_read) / 1024.0;

[willchan no longer on Chromium, 2011/06/09 15:08:54]

nit: fix your variable naming, they should follow
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Variab....

as well as:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Genera...

It seems like you're trying to follow some function. I am not familiar with the
function, so using abbreviations like this are just confusing. Please use full
names.

You also need way more comments to explain what the calculations you are doing
mean and why you are doing this.

[Gagan, 2011/06/09 19:49:26]

Changed the variable names to follow convention.
 
Nothing complicated, simply num_kbs - (coef ^ idle_time_in_mins)
coef = 1 prefers warmest connection
coef = 1.83 simulates cwnd decay of 20KB in 5 minutes.
This function partly simulates cwnd validation and does not penalize cwnd for
smaller idle times.

+
+      int64 idle_time = (base::TimeTicks::Now() - it->start_time)
+          .InMicroseconds();
+      double idle_time_mins = static_cast<double>(idle_time) / 60000000.0;
+      double T = pow(g_tcp_socket_estimated_cwnd_decay_coef, idle_time_mins);

[Mike Belshe, 2011/06/09 16:08:52]

I believe this math is broken and we have to find a different algorithm.

The units of B are bytes.
The units of T are minutes (squared).

I don't know of any way to do interesting addition/subtraction when units are
different.  This is not a rate (like B / T (bytes/sec)).  It seems like you've
found a heuristic which kinda sorta seems to work, but it's meaningless
mathematically.

I realize you're just trying to implement a damper.  Perhaps you can explain why
you think this works?

[Gagan, 2011/06/09 19:49:26]

Yes, i want a damping function.
Im choosing coef ^ idle_time because it simulates cwnd validation, reduction of
cwnd by a factor of coef every RTO. Plus, this simulates a non linear
probability increase of connection close as idle time increases.
I thought a lot about what kind of function to choose, and this one kind of felt
right. Would love to hear your ideas.

20 KB / 5 mins was chosen conservatively. I think warmest connection first is
the way to go, but a warm with smaller idle time is also good. Hence choosing
one more value. If there are many cases where missing FIN packet causes
problems, then the stat for this should be better than warmest first.

You are right that there is a mismatch in units, thats because i don't have any
estimate of bandwidth or a good rate function.

+
+      debug_log = StringPrintf("%s\n%f\t%f\t%f",

[Mike Belshe, 2011/06/09 16:08:52]

this debug_log appears unused (or overridden below)

[Gagan, 2011/06/09 19:49:26]

its appending to its previous value

+                               debug_log.data(), B, idle_time_mins, T);
+      double benefit = B - T;
+
+      // Equality to prefer recently used connection.
+      if (benefit >= max_benefit) {
+        idle_socket_it = it;
+        max_benefit = benefit;
+        best_pair = std::make_pair(bytes_read, idle_time);
+        rtt_ms = it->socket->GetRTTInMs();
+      }
     }
 
     ++it;
   }
+  debug_log = StringPrintf("%s\nChose socket <%d %ld>\nRtt = %f",
+                           debug_log.data(),
+                           best_pair.first, best_pair.second,
+                           rtt_ms);
+  LOG(ERROR) << "\n\n----------------------------------------" << debug_log;
 
   // If we haven't found an idle socket, that means there are no used idle
   // sockets.  Pick the oldest (first) idle socket (FIFO).
 
   if (idle_socket_it == idle_sockets->end() && !idle_sockets->empty())
     idle_socket_it = idle_sockets->begin();
 
   if (idle_socket_it != idle_sockets->end()) {
     DecrementIdleCount();
     base::TimeDelta idle_time =
@@ skipping 245 matching lines @@
   return g_connect_backup_jobs_enabled;
 }
 
 // static
 bool ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(bool enabled) {
   bool old_value = g_connect_backup_jobs_enabled;
   g_connect_backup_jobs_enabled = enabled;
   return old_value;
 }
 
+// static
+double ClientSocketPoolBaseHelper::set_tcp_socket_estimated_cwnd_decay_coef(
+    double coef) {
+  LOG(ERROR) << "Setting g_tcp_socket_estimated_cwnd_decay_coef = " << coef;

[Mike Belshe, 2011/06/09 16:08:52]

nit:  this isn't a LOG(ERROR), LOG(DEBUG) or remove?

[Gagan, 2011/06/09 19:49:26]

Changed to LOG(DEBUG)

+  double old_value = g_tcp_socket_estimated_cwnd_decay_coef;
+  g_tcp_socket_estimated_cwnd_decay_coef = coef;
+  return old_value;
+}
+
 void ClientSocketPoolBaseHelper::EnableConnectBackupJobs() {
   connect_backup_jobs_enabled_ = g_connect_backup_jobs_enabled;
 }
 
 void ClientSocketPoolBaseHelper::IncrementIdleCount() {
   if (++idle_socket_count_ == 1)
     timer_.Start(TimeDelta::FromSeconds(kCleanupInterval), this,
                  &ClientSocketPoolBaseHelper::OnCleanupTimerFired);
 }
 
@@ skipping 419 matching lines @@
   // Delete active jobs.
   STLDeleteElements(&jobs_);
 
   // Cancel pending backup job.
   method_factory_.RevokeAll();
 }
 
 }  // namespace internal
 
 }  // namespace net