Remove unused histograms for QUIC

net/quic/chromium/quic_connection_logger.cc

Index: net/quic/chromium/quic_connection_logger.cc
diff --git a/net/quic/chromium/quic_connection_logger.cc b/net/quic/chromium/quic_connection_logger.cc
index df5c499b5b812c37b72fc3d8f39a4021d728d63f..df161207f34602e95c61bb68ee25ae5959e41dca 100644
--- a/net/quic/chromium/quic_connection_logger.cc
+++ b/net/quic/chromium/quic_connection_logger.cc
@@ -34,11 +34,6 @@ namespace net {
 
 namespace {
 
-// We have ranges-of-buckets in the cumulative histogram (covering 21 packet
-// sequences) of length 2, 3, 4, ... 22.
-// Hence the largest sample is bounded by the sum of those numbers.
-const int kBoundingSampleInCumulativeHistogram = ((2 + 22) * 21) / 2;
-
 std::unique_ptr<base::Value> NetLogQuicPacketCallback(
     const IPEndPoint* self_address,
     const IPEndPoint* peer_address,
@@ -347,7 +342,8 @@ QuicConnectionLogger::~QuicConnectionLogger() {
     }
   }
 
-  RecordLossHistograms();
+  if (largest_received_packet_number_ != 0)

[Ryan Hamilton, 2016/08/18 19:36:33]

Is this check needed, since the method already checks:

if (largest_received_packet_number_ <= 21)
  return;

[Zhongyi Shi, 2016/08/18 20:13:11]

Ah, yeah, this can be removed. Removed. 

+    RecordAggregatePacketLossRate();
 }
 
 void QuicConnectionLogger::OnFrameAddedToPacket(const QuicFrame& frame) {
@@ -685,15 +681,6 @@ void QuicConnectionLogger::OnCertificateVerified(
       base::Bind(&NetLogQuicCertificateVerifiedCallback, result.verified_cert));
 }
 
-base::HistogramBase* QuicConnectionLogger::GetPacketNumberHistogram(
-    const char* statistic_name) const {
-  string prefix("Net.QuicSession.PacketReceived_");
-  return base::LinearHistogram::FactoryGet(
-      prefix + statistic_name + connection_description_, 1,
-      received_packets_.size(), received_packets_.size() + 1,
-      base::HistogramBase::kUmaTargetedHistogramFlag);
-}
-
 base::HistogramBase* QuicConnectionLogger::Get6PacketHistogram(
     const char* which_6) const {
   // This histogram takes a binary encoding of the 6 consecutive packets
@@ -704,50 +691,6 @@ base::HistogramBase* QuicConnectionLogger::Get6PacketHistogram(
       base::HistogramBase::kUmaTargetedHistogramFlag);
 }
 
-base::HistogramBase* QuicConnectionLogger::Get21CumulativeHistogram(
-    const char* which_21) const {
-  // This histogram contains, for each sequence of 21 packets, the results from
-  // 21 distinct questions about that sequence.  Conceptually the histogtram is
-  // broken into 21 distinct ranges, and one sample is added into each of those
-  // ranges whenever we process a set of 21 packets.
-  // There is a little rendundancy, as each "range" must have the same number
-  // of samples, all told, but the histogram is a tad easier to read this way.
-  // The questions are:
-  // Was the first packet present (bucket 0==>no; bucket 1==>yes)
-  // Of the first two packets, how many were present? (bucket 2==> none;
-  //   bucket 3==> 1 of 2; bucket 4==> 2 of 2)
-  // Of the  first three packets, how many were present? (bucket 5==>none;
-  //   bucket 6==> 1 of 3; bucket 7==> 2 of 3; bucket 8==> 3 of 3).
-  // etc.
-  string prefix("Net.QuicSession.21CumulativePacketsReceived_");
-  return base::LinearHistogram::FactoryGet(
-      prefix + which_21 + connection_description_, 1,
-      kBoundingSampleInCumulativeHistogram,
-      kBoundingSampleInCumulativeHistogram + 1,
-      base::HistogramBase::kUmaTargetedHistogramFlag);
-}
-
-// static
-void QuicConnectionLogger::AddTo21CumulativeHistogram(
-    base::HistogramBase* histogram,
-    int bit_mask_of_packets,
-    int valid_bits_in_mask) {
-  DCHECK_LE(valid_bits_in_mask, 21);
-  DCHECK_LT(bit_mask_of_packets, 1 << 21);
-  const int blank_bits_in_mask = 21 - valid_bits_in_mask;
-  DCHECK_EQ(bit_mask_of_packets & ((1 << blank_bits_in_mask) - 1), 0);
-  bit_mask_of_packets >>= blank_bits_in_mask;
-  int bits_so_far = 0;
-  int range_start = 0;
-  for (int i = 1; i <= valid_bits_in_mask; ++i) {
-    bits_so_far += bit_mask_of_packets & 1;
-    bit_mask_of_packets >>= 1;
-    DCHECK_LT(range_start + bits_so_far, kBoundingSampleInCumulativeHistogram);
-    histogram->Add(range_start + bits_so_far);
-    range_start += i + 1;
-  }
-}
-
 float QuicConnectionLogger::ReceivedPacketLossRate() const {
   if (largest_received_packet_number_ <= num_packets_received_)
     return 0.0f;
@@ -785,75 +728,4 @@ void QuicConnectionLogger::RecordAggregatePacketLossRate() const {
       ReceivedPacketLossRate() * 1000));
 }
 
-void QuicConnectionLogger::RecordLossHistograms() const {
-  if (largest_received_packet_number_ == 0)
-    return;  // Connection was never used.
-  RecordAggregatePacketLossRate();
-
-  base::HistogramBase* is_not_ack_histogram =
-      GetPacketNumberHistogram("IsNotAck_");
-  base::HistogramBase* is_an_ack_histogram =
-      GetPacketNumberHistogram("IsAnAck_");
-  base::HistogramBase* packet_arrived_histogram =
-      GetPacketNumberHistogram("Ack_");
-  base::HistogramBase* packet_missing_histogram =
-      GetPacketNumberHistogram("Nack_");
-  base::HistogramBase* ongoing_cumulative_packet_histogram =
-      Get21CumulativeHistogram("Some21s_");
-  base::HistogramBase* first_cumulative_packet_histogram =
-      Get21CumulativeHistogram("First21_");
-  base::HistogramBase* six_packet_histogram = Get6PacketHistogram("Some6s_");
-
-  DCHECK_EQ(received_packets_.size(), received_acks_.size());
-  const QuicPacketNumber last_index = std::min<QuicPacketNumber>(
-      received_packets_.size() - 1, largest_received_packet_number_);
-  const QuicPacketNumber index_of_first_21_contribution =
-      std::min<QuicPacketNumber>(21, last_index);
-  // Bit pattern of consecutively received packets that is maintained as we scan
-  // through the received_packets_ vector. Less significant bits correspond to
-  // less recent packets, and only the low order 21 bits are ever defined.
-  // Bit is 1 iff corresponding packet was received.
-  int packet_pattern_21 = 0;
-  // Zero is an invalid packet sequence number.
-  DCHECK(!received_packets_[0]);
-  for (size_t i = 1; i <= last_index; ++i) {
-    if (received_acks_[i])
-      is_an_ack_histogram->Add(i);
-    else
-      is_not_ack_histogram->Add(i);
-
-    packet_pattern_21 >>= 1;
-    if (received_packets_[i]) {
-      packet_arrived_histogram->Add(i);
-      packet_pattern_21 |= (1 << 20);  // Turn on the 21st bit.
-    } else {
-      packet_missing_histogram->Add(i);
-    }
-
-    if (i == index_of_first_21_contribution) {
-      AddTo21CumulativeHistogram(first_cumulative_packet_histogram,
-                                 packet_pattern_21, i);
-    }
-    // We'll just record for non-overlapping ranges, to reduce histogramming
-    // cost for now.  Each call does 21 separate histogram additions.
-    if (i > 21 || i % 21 == 0) {
-      AddTo21CumulativeHistogram(ongoing_cumulative_packet_histogram,
-                                 packet_pattern_21, 21);
-    }
-
-    if (i < 6)
-      continue;  // Not enough packets to do any pattern recording.
-    int recent_6_mask = packet_pattern_21 >> 15;
-    DCHECK_LT(recent_6_mask, 64);
-    if (i == 6) {
-      Get6PacketHistogram("First6_")->Add(recent_6_mask);
-      continue;
-    }
-    // Record some overlapping patterns, to get a better picture, since this is
-    // not very expensive.
-    if (i % 3 == 0)
-      six_packet_histogram->Add(recent_6_mask);
-  }
-}
-
 }  // namespace net