Remove unused histograms for QUIC

net/quic/chromium/quic_connection_logger.cc

 // Copyright (c) 2013 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/chromium/quic_connection_logger.h"
 
 #include <algorithm>
 #include <limits>
 #include <memory>
 #include <utility>
@@ skipping 16 matching lines @@
 #include "net/quic/core/quic_socket_address_coder.h"
 #include "net/quic/core/quic_time.h"
 
 using base::StringPiece;
 using std::string;
 
 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,
     size_t packet_size,
     NetLogCaptureMode /* capture_mode */) {
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
   dict->SetString("self_address", self_address->ToString());
   dict->SetString("peer_address", peer_address->ToString());
   dict->SetInteger("size", packet_size);
   return std::move(dict);
@@ skipping 288 matching lines @@
       UMA_HISTOGRAM_CUSTOM_COUNTS(
           "Net.QuicSession.StreamFrameDuplicatedShortConnection",
           duplicate_stream_frame_per_thousand, 1, 1000, 75);
     } else {
       UMA_HISTOGRAM_CUSTOM_COUNTS(
           "Net.QuicSession.StreamFrameDuplicatedLongConnection",
           duplicate_stream_frame_per_thousand, 1, 1000, 75);
     }
   }
 
-  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) {
   switch (frame.type) {
     case PADDING_FRAME:
       break;
     case STREAM_FRAME:
       net_log_.AddEvent(
           NetLog::TYPE_QUIC_SESSION_STREAM_FRAME_SENT,
           base::Bind(&NetLogQuicStreamFrameCallback, frame.stream_frame));
@@ skipping 317 matching lines @@
     const CertVerifyResult& result) {
   if (result.cert_status == CERT_STATUS_INVALID) {
     net_log_.AddEvent(NetLog::TYPE_QUIC_SESSION_CERTIFICATE_VERIFY_FAILED);
     return;
   }
   net_log_.AddEvent(
       NetLog::TYPE_QUIC_SESSION_CERTIFICATE_VERIFIED,
       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
   // received.  As a result, there are 64 possible sample-patterns.
   string prefix("Net.QuicSession.6PacketsPatternsReceived_");
   return base::LinearHistogram::FactoryGet(
       prefix + which_6 + connection_description_, 1, 64, 65,
       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;
   float num_received = largest_received_packet_number_ - num_packets_received_;
   return num_received / largest_received_packet_number_;
 }
 
 void QuicConnectionLogger::OnRttChanged(QuicTime::Delta rtt) const {
   // Notify socket performance watcher of the updated RTT value.
   if (!socket_performance_watcher_)
@@ skipping 17 matching lines @@
     return;
 
   string prefix("Net.QuicSession.PacketLossRate_");
   base::HistogramBase* histogram = base::Histogram::FactoryGet(
       prefix + connection_description_, 1, 1000, 75,
       base::HistogramBase::kUmaTargetedHistogramFlag);
   histogram->Add(static_cast<base::HistogramBase::Sample>(
       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