Remove some unused proxy-related histograms.

net/proxy/proxy_resolver_v8_tracing.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/proxy/proxy_resolver_v8_tracing.h"
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop_proxy.h"
-#include "base/metrics/histogram.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/cancellation_flag.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/values.h"
 #include "net/base/address_list.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/dns/host_resolver.h"
@@ skipping 37 matching lines @@
 // Returns event parameters for a PAC error message (line number + message).
 base::Value* NetLogErrorCallback(int line_number,
                                  const base::string16* message,
                                  NetLog::LogLevel /* log_level */) {
   base::DictionaryValue* dict = new base::DictionaryValue();
   dict->SetInteger("line_number", line_number);
   dict->SetString("message", *message);
   return dict;
 }
 
-void IncrementWithoutOverflow(uint8* x) {
-  if (*x != 0xFF)
-    *x += 1;
-}
-
 }  // namespace
 
 // The Job class is responsible for executing GetProxyForURL() and
 // SetPacScript(), since both of these operations share similar code.
 //
 // The DNS for these operations can operate in either blocking or
 // non-blocking mode. Blocking mode is used as a fallback when the PAC script
 // seems to be misbehaving under the tracing optimization.
 //
 // Note that this class runs on both the origin thread and a worker
@@ skipping 54 matching lines @@
   ProxyResolverV8* v8_resolver();
   base::MessageLoop* worker_loop();
   HostResolver* host_resolver();
   ProxyResolverErrorObserver* error_observer();
   NetLog* net_log();
 
   // Invokes the user's callback.
   void NotifyCaller(int result);
   void NotifyCallerOnOriginLoop(int result);
 
-  void RecordMetrics() const;
-
   void Start(Operation op, bool blocking_dns,
              const CompletionCallback& callback);
 
   void ExecuteBlocking();
   void ExecuteNonBlocking();
   int ExecuteProxyResolver();
 
   // Implementation of ProxyResolverv8::JSBindings
   virtual bool ResolveDns(const std::string& host,
                           ResolveDnsOperation op,
@@ skipping 133 matching lines @@
   bool pending_dns_completed_synchronously_;
 
   // These are the inputs to DoDnsOperation(). Written on the worker thread,
   // read by the origin thread.
   std::string pending_dns_host_;
   ResolveDnsOperation pending_dns_op_;
 
   // This contains the resolved address list that DoDnsOperation() fills in.
   // Used exclusively on the origin thread.
   AddressList pending_dns_addresses_;
-
-  // ---------------------------------------------------------------------------
-  // Metrics for histograms
-  // ---------------------------------------------------------------------------
-  // These values are used solely for logging histograms. They do not affect
-  // the execution flow of requests.
-
-  // The time when the proxy resolve request started. Used exclusively on the
-  // origin thread.
-  base::TimeTicks metrics_start_time_;
-
-  // The time when the proxy resolve request completes on the worker thread.
-  // Written on the worker thread, read on the origin thread.
-  base::TimeTicks metrics_end_time_;
-
-  // The time when PostDnsOperationAndWait() was called. Written on the worker
-  // thread, read by the origin thread.
-  base::TimeTicks metrics_pending_dns_start_;
-
-  // The total amount of time that has been spent by the script waiting for
-  // DNS dependencies. This includes the time spent posting the task to
-  // the origin thread, up until the DNS result is found on the origin
-  // thread. It does not include any time spent waiting in the message loop
-  // for the worker thread, nor any time restarting or executing the
-  // script. Used exclusively on the origin thread.
-  base::TimeDelta metrics_dns_total_time_;
-
-  // The following variables are initialized on the origin thread,
-  // incremented on the worker thread, and then read upon completion on the
-  // origin thread. The values are not expected to exceed the range of a uint8.
-  // If they do, then they will be clamped to 0xFF.
-  uint8 metrics_num_executions_;
-  uint8 metrics_num_unique_dns_;
-  uint8 metrics_num_alerts_;
-  uint8 metrics_num_errors_;
-
-  // The time that the latest execution took (time spent inside of
-  // ExecuteProxyResolver(), which includes time spent in bindings too).
-  // Written on the worker thread, read on the origin thread.
-  base::TimeDelta metrics_execution_time_;
-
-  // The cumulative time spent in ExecuteProxyResolver() that was ultimately
-  // discarded work.
-  // Written on the worker thread, read on the origin thread.
-  base::TimeDelta metrics_abandoned_execution_total_time_;
-
-  // The duration that the worker thread was blocked waiting on DNS results from
-  // the origin thread, when operating in nonblocking mode.
-  // Written on the worker thread, read on the origin thread.
-  base::TimeDelta metrics_nonblocking_dns_wait_total_time_;
 };
 
 ProxyResolverV8Tracing::Job::Job(ProxyResolverV8Tracing* parent)
     : origin_loop_(base::MessageLoopProxy::current()),
       parent_(parent),
       event_(true, false),
       last_num_dns_(0),
-      pending_dns_(NULL),
-      metrics_num_executions_(0),
-      metrics_num_unique_dns_(0),
-      metrics_num_alerts_(0),
-      metrics_num_errors_(0) {
+      pending_dns_(NULL) {
   CheckIsOnOriginThread();
 }
 
 void ProxyResolverV8Tracing::Job::StartSetPacScript(
     const scoped_refptr<ProxyResolverScriptData>& script_data,
     const CompletionCallback& callback) {
   CheckIsOnOriginThread();
 
   script_data_ = script_data;
 
@@ skipping 110 matching lines @@
   return parent_->error_observer_.get();
 }
 
 NetLog* ProxyResolverV8Tracing::Job::net_log() {
   return parent_->net_log_;
 }
 
 void ProxyResolverV8Tracing::Job::NotifyCaller(int result) {
   CheckIsOnWorkerThread();
 
-  metrics_end_time_ = base::TimeTicks::Now();
-
   origin_loop_->PostTask(
       FROM_HERE,
       base::Bind(&Job::NotifyCallerOnOriginLoop, this, result));
 }
 
 void ProxyResolverV8Tracing::Job::NotifyCallerOnOriginLoop(int result) {
   CheckIsOnOriginThread();
 
   if (cancelled_.IsSet())
     return;
 
   DCHECK(!callback_.is_null());
   DCHECK(!pending_dns_);
 
   if (operation_ == GET_PROXY_FOR_URL) {
-    RecordMetrics();
     *user_results_ = results_;
   }
 
   // There is only ever 1 outstanding SET_PAC_SCRIPT job. It needs to be
   // tracked to support cancellation.
   if (operation_ == SET_PAC_SCRIPT) {
     DCHECK_EQ(parent_->set_pac_script_job_.get(), this);
     parent_->set_pac_script_job_ = NULL;
   }
 
   CompletionCallback callback = callback_;
   ReleaseCallback();
   callback.Run(result);
 
   owned_self_reference_ = NULL;
 }
 
-void ProxyResolverV8Tracing::Job::RecordMetrics() const {
-  CheckIsOnOriginThread();
-  DCHECK_EQ(GET_PROXY_FOR_URL, operation_);
-
-  base::TimeTicks now = base::TimeTicks::Now();
-
-  // Metrics are output for each completed request to GetProxyForURL()).
-  //
-  // Note that a different set of histograms is used to record the metrics for
-  // requests that completed in non-blocking mode versus blocking mode. The
-  // expectation is for requests to complete in non-blocking mode each time.
-  // If they don't then something strange is happening, and the purpose of the
-  // seprate statistics is to better understand that trend.
-#define UPDATE_HISTOGRAMS(base_name) \
-  do {\
-  UMA_HISTOGRAM_MEDIUM_TIMES(base_name "TotalTime", now - metrics_start_time_);\
-  UMA_HISTOGRAM_MEDIUM_TIMES(base_name "TotalTimeWorkerThread",\
-                             metrics_end_time_ - metrics_start_time_);\
-  UMA_HISTOGRAM_TIMES(base_name "OriginThreadLatency",\
-                      now - metrics_end_time_);\
-  UMA_HISTOGRAM_MEDIUM_TIMES(base_name "TotalTimeDNS",\
-                             metrics_dns_total_time_);\
-  UMA_HISTOGRAM_MEDIUM_TIMES(base_name "ExecutionTime",\
-                             metrics_execution_time_);\
-  UMA_HISTOGRAM_MEDIUM_TIMES(base_name "AbandonedExecutionTotalTime",\
-                             metrics_abandoned_execution_total_time_);\
-  UMA_HISTOGRAM_MEDIUM_TIMES(base_name "DnsWaitTotalTime",\
-                             metrics_nonblocking_dns_wait_total_time_);\
-  UMA_HISTOGRAM_CUSTOM_COUNTS(\
-      base_name "NumRestarts", metrics_num_executions_ - 1,\
-      1, kMaxUniqueResolveDnsPerExec, kMaxUniqueResolveDnsPerExec);\
-  UMA_HISTOGRAM_CUSTOM_COUNTS(\
-      base_name "UniqueDNS", metrics_num_unique_dns_,\
-      1, kMaxUniqueResolveDnsPerExec, kMaxUniqueResolveDnsPerExec);\
-  UMA_HISTOGRAM_COUNTS_100(base_name "NumAlerts", metrics_num_alerts_);\
-  UMA_HISTOGRAM_CUSTOM_COUNTS(\
-      base_name "NumErrors", metrics_num_errors_, 1, 10, 10);\
-  } while (false)
-
-  if (!blocking_dns_)
-    UPDATE_HISTOGRAMS("Net.ProxyResolver.");
-  else
-    UPDATE_HISTOGRAMS("Net.ProxyResolver.BlockingDNSMode.");
-
-#undef UPDATE_HISTOGRAMS
-
-  // Histograms to better understand http://crbug.com/240536 -- long
-  // URLs can cause a significant slowdown in PAC execution. Figure out how
-  // severe this is in the wild.
-  if (!blocking_dns_) {
-    size_t url_size = url_.spec().size();
-
-    UMA_HISTOGRAM_CUSTOM_COUNTS(
-        "Net.ProxyResolver.URLSize", url_size, 1, 200000, 50);
-
-    if (url_size > 2048) {
-      UMA_HISTOGRAM_MEDIUM_TIMES("Net.ProxyResolver.ExecutionTime_UrlOver2K",
-                                 metrics_execution_time_);
-    }
-
-    if (url_size > 4096) {
-      UMA_HISTOGRAM_MEDIUM_TIMES("Net.ProxyResolver.ExecutionTime_UrlOver4K",
-                                 metrics_execution_time_);
-    }
-
-    if (url_size > 8192) {
-      UMA_HISTOGRAM_MEDIUM_TIMES("Net.ProxyResolver.ExecutionTime_UrlOver8K",
-                                 metrics_execution_time_);
-    }
-
-    if (url_size > 131072) {
-      UMA_HISTOGRAM_MEDIUM_TIMES("Net.ProxyResolver.ExecutionTime_UrlOver128K",
-                                 metrics_execution_time_);
-    }
-  }
-}
-
-
 void ProxyResolverV8Tracing::Job::Start(Operation op, bool blocking_dns,
                                         const CompletionCallback& callback) {
   CheckIsOnOriginThread();
 
-  metrics_start_time_ = base::TimeTicks::Now();
   operation_ = op;
   blocking_dns_ = blocking_dns;
   SetCallback(callback);
 
   owned_self_reference_ = this;
 
   worker_loop()->PostTask(FROM_HERE,
       blocking_dns_ ? base::Bind(&Job::ExecuteBlocking, this) :
                       base::Bind(&Job::ExecuteNonBlocking, this));
 }
@@ skipping 17 matching lines @@
 
   // Reset state for the current execution.
   abandoned_ = false;
   num_dns_ = 0;
   alerts_and_errors_.clear();
   alerts_and_errors_byte_cost_ = 0;
   should_restart_with_blocking_dns_ = false;
 
   int result = ExecuteProxyResolver();
 
-  if (abandoned_)
-    metrics_abandoned_execution_total_time_ += metrics_execution_time_;
-
   if (should_restart_with_blocking_dns_) {
     DCHECK(!blocking_dns_);
     DCHECK(abandoned_);
     blocking_dns_ = true;
     ExecuteBlocking();
     return;
   }
 
   if (abandoned_)
     return;
 
   DispatchBufferedAlertsAndErrors();
   NotifyCaller(result);
 }
 
 int ProxyResolverV8Tracing::Job::ExecuteProxyResolver() {
-  IncrementWithoutOverflow(&metrics_num_executions_);
-
-  base::TimeTicks start = base::TimeTicks::Now();
-
   JSBindings* prev_bindings = v8_resolver()->js_bindings();
   v8_resolver()->set_js_bindings(this);
 
   int result = ERR_UNEXPECTED;  // Initialized to silence warnings.
 
   switch (operation_) {
     case SET_PAC_SCRIPT:
       result = v8_resolver()->SetPacScript(
           script_data_, CompletionCallback());
       break;
     case GET_PROXY_FOR_URL:
       result = v8_resolver()->GetProxyForURL(
         url_,
         // Important: Do not write directly into |user_results_|, since if the
         // request were to be cancelled from the origin thread, must guarantee
         // that |user_results_| is not accessed anymore.
         &results_,
         CompletionCallback(),
         NULL,
         bound_net_log_);
       break;
   }
 
   v8_resolver()->set_js_bindings(prev_bindings);
 
-  metrics_execution_time_ = base::TimeTicks::Now() - start;
-
   return result;
 }
 
 bool ProxyResolverV8Tracing::Job::ResolveDns(const std::string& host,
                                              ResolveDnsOperation op,
                                              std::string* output,
                                              bool* terminate) {
   if (cancelled_.IsSet()) {
     *terminate = true;
     return false;
@@ skipping 23 matching lines @@
                                                      std::string* output) {
   CheckIsOnWorkerThread();
 
   // Check if the DNS result for this host has already been cached.
   bool rv;
   if (GetDnsFromLocalCache(host, op, output, &rv)) {
     // Yay, cache hit!
     return rv;
   }
 
-  // If the host was not in the local cache, this is a new hostname.
-  IncrementWithoutOverflow(&metrics_num_unique_dns_);
-
   if (dns_cache_.size() >= kMaxUniqueResolveDnsPerExec) {
     // Safety net for scripts with unexpectedly many DNS calls.
     // We will continue running to completion, but will fail every
     // subsequent DNS request.
     return false;
   }
 
   if (!PostDnsOperationAndWait(host, op, NULL))
     return false;  // Was cancelled.
 
@@ skipping 15 matching lines @@
 
   num_dns_ += 1;
 
   // Check if the DNS result for this host has already been cached.
   bool rv;
   if (GetDnsFromLocalCache(host, op, output, &rv)) {
     // Yay, cache hit!
     return rv;
   }
 
-  // If the host was not in the local cache, then this is a new hostname.
-  IncrementWithoutOverflow(&metrics_num_unique_dns_);
-
   if (num_dns_ <= last_num_dns_) {
     // The sequence of DNS operations is different from last time!
     ScheduleRestartWithBlockingDns();
     *terminate = true;
     return false;
   }
 
   if (dns_cache_.size() >= kMaxUniqueResolveDnsPerExec) {
     // Safety net for scripts with unexpectedly many DNS calls.
     return false;
@@ skipping 16 matching lines @@
   abandoned_ = true;
   *terminate = true;
   last_num_dns_ = num_dns_;
   return false;
 }
 
 bool ProxyResolverV8Tracing::Job::PostDnsOperationAndWait(
     const std::string& host, ResolveDnsOperation op,
     bool* completed_synchronously) {
 
-  base::TimeTicks start = base::TimeTicks::Now();
-
   // Post the DNS request to the origin thread.
   DCHECK(!pending_dns_);
-  metrics_pending_dns_start_ = base::TimeTicks::Now();
   pending_dns_host_ = host;
   pending_dns_op_ = op;
   origin_loop_->PostTask(FROM_HERE, base::Bind(&Job::DoDnsOperation, this));
 
   event_.Wait();
   event_.Reset();
 
   if (cancelled_.IsSet())
     return false;
 
   if (completed_synchronously)
     *completed_synchronously = pending_dns_completed_synchronously_;
 
-  if (!blocking_dns_)
-    metrics_nonblocking_dns_wait_total_time_ += base::TimeTicks::Now() - start;
-
   return true;
 }
 
 void ProxyResolverV8Tracing::Job::DoDnsOperation() {
   CheckIsOnOriginThread();
   DCHECK(!pending_dns_);
 
   if (cancelled_.IsSet())
     return;
 
@@ skipping 35 matching lines @@
 void ProxyResolverV8Tracing::Job::OnDnsOperationComplete(int result) {
   CheckIsOnOriginThread();
 
   DCHECK(!cancelled_.IsSet());
   DCHECK(pending_dns_completed_synchronously_ == (pending_dns_ == NULL));
 
   SaveDnsToLocalCache(pending_dns_host_, pending_dns_op_, result,
                       pending_dns_addresses_);
   pending_dns_ = NULL;
 
-  metrics_dns_total_time_ +=
-      base::TimeTicks::Now() - metrics_pending_dns_start_;
-
   if (blocking_dns_) {
     event_.Signal();
     return;
   }
 
   if (!blocking_dns_ && !pending_dns_completed_synchronously_) {
     // Restart. This time it should make more progress due to having
     // cached items.
     worker_loop()->PostTask(FROM_HERE,
                             base::Bind(&Job::ExecuteNonBlocking, this));
@@ skipping 142 matching lines @@
   // If this happens, then some information will get written to the NetLog
   // needlessly, however the NetLog will still be alive so it shouldn't cause
   // problems.
   if (cancelled_.IsSet())
     return;
 
   if (is_alert) {
     // -------------------
     // alert
     // -------------------
-    IncrementWithoutOverflow(&metrics_num_alerts_);
     VLOG(1) << "PAC-alert: " << message;
 
     // Send to the NetLog.
     LogEventToCurrentRequestAndGlobally(
         NetLog::TYPE_PAC_JAVASCRIPT_ALERT,
         NetLog::StringCallback("message", &message));
   } else {
     // -------------------
     // error
     // -------------------
-    IncrementWithoutOverflow(&metrics_num_errors_);
     if (line_number == -1)
       VLOG(1) << "PAC-error: " << message;
     else
       VLOG(1) << "PAC-error: " << "line: " << line_number << ": " << message;
 
     // Send the error to the NetLog.
     LogEventToCurrentRequestAndGlobally(
         NetLog::TYPE_PAC_JAVASCRIPT_ERROR,
         base::Bind(&NetLogErrorCallback, line_number, &message));
 
@@ skipping 90 matching lines @@
   DCHECK(!set_pac_script_job_.get());
   CHECK_EQ(0, num_outstanding_callbacks_);
 
   set_pac_script_job_ = new Job(this);
   set_pac_script_job_->StartSetPacScript(script_data, callback);
 
   return ERR_IO_PENDING;
 }
 
 }  // namespace net