For packet pacing and non-paced network connectivity tests, collect

chrome/browser/net/network_stats.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 "chrome/browser/net/network_stats.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/metrics/field_trial.h"
@@ skipping 66 matching lines @@
 static const uint32 kPacketNumberLength = 10;
 
 // This specifies the starting position of the <encoded_payload> in
 // "echo response".
 static const uint32 kEncodedPayloadStart = kKeyEnd;
 
 // HistogramPortSelector and kPorts should be kept in sync. Port 999999 is
 // used by the unit tests.
 static const int32 kPorts[] = {6121, 999999};
 
 // Maximum number of packets that can be sent to the server for packet loss

[jar (doing other things), 2012/09/14 22:34:54]

nit:  Change comment to:

Number of packets that are recorded in a packet-corelation histogram, which
shows exactly what seqence of packets were responded to.  We use this to deduce
specific packet loss correlation.


You might also change the name to: kCorrelatedLossPacketCount.

[ramant (doing other things), 2012/09/14 23:39:08]

Done.

 // correlation test.
 static const uint32 kMaximumCorrelationPackets = 6;
 
 // Maximum number of packets that can be sent to the server.
 static const uint32 kMaximumSequentialPackets = 21;
 
 // This specifies the maximum message (payload) size.
 static const uint32 kMaxMessage = kMaximumSequentialPackets * 2048;
 
 // NetworkStats methods and members.
@@ skipping 578 matching lines @@
     net::CompletionCallback callback = finished_callback_;
     finished_callback_.Reset();
     callback.Run(result);
   }
 }
 
 void NetworkStats::RecordHistograms(const ProtocolValue& protocol,
                                     const Status& status,
                                     int result) {
   if (packets_to_send_ != kMaximumSequentialPackets &&
       packets_to_send_ != kMaximumCorrelationPackets) {

[jar (doing other things), 2012/09/14 22:34:54]

This line can probably be deleted (you can leave 685).

[ramant (doing other things), 2012/09/14 23:39:08]

Done.

     return;
   }
 
   std::string load_size_string = base::StringPrintf("%dB", load_size_);
 
-  if (packets_to_send_ == kMaximumCorrelationPackets) {
-    RecordPacketLossSeriesHistograms(
-        protocol, load_size_string, status, result);
+  RecordPacketLossSeriesHistograms(protocol, load_size_string, status, result);
+  if (packets_to_send_ == kMaximumCorrelationPackets)
     return;

[jar (doing other things), 2012/09/14 22:34:54]

No longer needed.

[ramant (doing other things), 2012/09/14 23:39:08]

Done.

-  }
 
   for (uint32 i = 0; i < 3; i++)
     RecordRTTHistograms(protocol, load_size_string, i);
 
   RecordRTTHistograms(protocol, load_size_string, 9);
   RecordRTTHistograms(protocol, load_size_string, 19);
 
   RecordAcksReceivedHistograms(load_size_string);
 }
 
@@ skipping 64 matching lines @@
             base::Histogram::kUmaTargetedHistogramFlag);
     acks_received_count_histogram->Add(count);
   }
 }
 
 void NetworkStats::RecordPacketLossSeriesHistograms(
     const ProtocolValue& protocol,
     const std::string& load_size_string,
     const Status& status,
     int result) {
-  DCHECK_EQ(packets_to_send_, kMaximumCorrelationPackets);
-
   const char* test_name = TestName();
 
   // Build "NetConnectivity3.Send6.SeriesAcked.<port>.<load_size>" histogram
   // name. Total number of histograms are 5*2.
   std::string series_acked_histogram_name = base::StringPrintf(
       "NetConnectivity3.%s.Send6.SeriesAcked.%d.%s",
       test_name,
       kPorts[histogram_port_],
       load_size_string.c_str());
   // Build "NetConnectivity3.Send6.PacketsSent.<port>.<load_size>" histogram
   // name. Total number of histograms are 5*2.
   std::string packets_sent_histogram_name = base::StringPrintf(
       "NetConnectivity3.%s.Send6.PacketsSent.%d.%s",
       test_name,
       kPorts[histogram_port_],
       load_size_string.c_str());
 
+  uint32 six_packets_received_mask =

[jar (doing other things), 2012/09/14 22:34:54]

nit: suggest correlated_packet_mask

This way, the size is held in the kMaximumCorrelationPackets only.

[ramant (doing other things), 2012/09/14 23:39:08]

Done.

+    ((1 << kMaximumCorrelationPackets) - 1) & packets_received_mask_;
+
   // If we are running without a proxy, we'll generate 2 distinct histograms in
   // each case, one will have the ".NoProxy" suffix.
   size_t histogram_count = has_proxy_server_ ? 1 : 2;
   for (size_t i = 0; i < histogram_count; i++) {
     // For packet loss test, just record packet loss data.
     base::Histogram* series_acked_histogram = base::LinearHistogram::FactoryGet(
         series_acked_histogram_name,
         1,
         2 << kMaximumCorrelationPackets,

[jar (doing other things), 2012/09/14 22:34:54]

I think this and the next line can use (with plus 1 on next line):

1 << kMaximumCorrelationPackets

It will shrink the histograms.... but you weren't using the high end ones.

[ramant (doing other things), 2012/09/14 23:39:08]

Done.

         (2 << kMaximumCorrelationPackets) + 1,
         base::Histogram::kUmaTargetedHistogramFlag);
-    series_acked_histogram->Add(packets_received_mask_);
+    series_acked_histogram->Add(six_packets_received_mask);
     series_acked_histogram_name.append(".NoProxy");
 
     base::Histogram* packets_sent_histogram =
         base::Histogram::FactoryGet(
             packets_sent_histogram_name,
             1, kMaximumCorrelationPackets, kMaximumCorrelationPackets + 1,

[jar (doing other things), 2012/09/14 22:34:54]

We might not need this anymore, as we'll get it as part of the packets_acked
histograms (for the first 6? packets).

[ramant (doing other things), 2012/09/14 23:39:08]

Done.

             base::Histogram::kUmaTargetedHistogramFlag);
     packets_sent_histogram->Add(packets_sent_);
     packets_sent_histogram_name.append(".NoProxy");
   }
 }
 
 void NetworkStats::RecordRTTHistograms(const ProtocolValue& protocol,
                                        const std::string& load_size_string,
                                        uint32 index) {
   DCHECK_GE(index, 0u);
@@ skipping 179 matching lines @@
                            const net::HostPortPair& server_address,
                            NetworkStats::HistogramPortSelector histogram_port,
                            bool has_proxy_server) {
   int experiment_to_run = base::RandInt(1, 4);
   switch (experiment_to_run) {
     case 1:
       {
         NetworkStats* udp_stats_client = new NetworkStats();
         udp_stats_client->Start(
             host_resolver, server_address, histogram_port, has_proxy_server,
             kLargeTestBytesToSend, kMaximumCorrelationPackets,

[jar (doing other things), 2012/09/14 22:34:54]

We probably don't need this test anymore.  Line 1009 can reduce the choices as
well.

[ramant (doing other things), 2012/09/14 23:39:08]

Done.

             net::CompletionCallback());
       }
       break;
     case 2:
       {
         NetworkStats* udp_stats_client = new NetworkStats();
         udp_stats_client->Start(
             host_resolver, server_address, histogram_port, has_proxy_server,
             kSmallTestBytesToSend, kMaximumSequentialPackets,
             net::CompletionCallback());
@@ skipping 14 matching lines @@
         udp_stats_client->Start(
             host_resolver, server_address, histogram_port, has_proxy_server,
             kLargeTestBytesToSend, kMaximumSequentialPackets,
             net::CompletionCallback());
       }
       break;
   }
 }
 
 }  // namespace chrome_browser_net