Improve download performance statistics.

I've removed the questionable Download.BandwidthNetwork measurement, and
replaced it with measurements designed to answer the question "What is the
bottleneck for a download?"  Specfically, this includes two new statistics: 

* Download.DiskBandwidthUsedPercentage: The percentage of the
  available disk bandwith that was actually used.  100% implies that
  the disk was the performance bottleneck.
* Download.ResourceHandlerBlockedPercentage: The percentage of the
  time it was alive that the DownloadResourceHandler was throttled for
  performance reasons.  0% implies that the network was the
  performance bottleneck.

R=benjhayden@chromium.org


Review URL: https://chromiumcodereview.appspot.com/10695062

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@145320 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 22 insertions, 25 deletions

diff --git a/content/browser/download/download_file_impl.cc b/content/browser/download/download_file_impl.cc
index 9441580..48157ea 100644
--- a/content/browser/download/download_file_impl.cc
+++ b/content/browser/download/download_file_impl.cc
@@ -186,6 +186,7 @@ void DownloadFileImpl::StreamActive() {
           result = AppendDataToFile(
               incoming_data.get()->data(), incoming_data_size);
           disk_writes_time_ += (base::TimeTicks::Now() - write_start);
+          bytes_seen_ += incoming_data_size;
           total_incoming_data_size += incoming_data_size;
           reason = content::ConvertNetErrorToInterruptReason(
               result, content::DOWNLOAD_INTERRUPT_FROM_DISK);
@@ -224,7 +225,6 @@ void DownloadFileImpl::StreamActive() {
 
   if (total_incoming_data_size)
     download_stats::RecordFileThreadReceiveBuffers(num_buffers);
-  bytes_seen_ += total_incoming_data_size;
 
   download_stats::RecordContiguousWriteTime(now - start);
 
diff --git a/content/browser/download/download_resource_handler.cc b/content/browser/download/download_resource_handler.cc
index cec28ff..cc2bc40 100644
--- a/content/browser/download/download_resource_handler.cc
+++ b/content/browser/download/download_resource_handler.cc
@@ -342,6 +342,8 @@ bool DownloadResourceHandler::OnResponseCompleted(
   }
 
   download_stats::RecordAcceptsRanges(accept_ranges_, bytes_read_);
+  download_stats::RecordNetworkBlockage(
+      base::TimeTicks::Now() - download_start_time_, total_pause_time_);
 
   CallStartedCB(DownloadId(), error_code);
 
@@ -353,11 +355,6 @@ bool DownloadResourceHandler::OnResponseCompleted(
   stream_writer_.reset();  // We no longer need the stream.
   read_buffer_ = NULL;
 
-  // Stats
-  download_stats::RecordNetworkBandwidth(
-      bytes_read_, base::TimeTicks::Now() - download_start_time_,
-      total_pause_time_);
-
   return true;
 }
 
diff --git a/content/browser/download/download_stats.cc b/content/browser/download/download_stats.cc
index 6332993..f1492dd 100644
--- a/content/browser/download/download_stats.cc
+++ b/content/browser/download/download_stats.cc
@@ -306,21 +306,18 @@ void RecordContiguousWriteTime(base::TimeDelta time_blocked) {
   UMA_HISTOGRAM_TIMES("Download.FileThreadBlockedTime", time_blocked);
 }
 
-void RecordNetworkBandwidth(size_t length,
-                            base::TimeDelta elapsed_time,
-                            base::TimeDelta paused_time) {
-  size_t non_pause_time_ms = (elapsed_time - paused_time).InMilliseconds();
-  if (0u == non_pause_time_ms)
-    non_pause_time_ms = 1;
-
-  // Note that this will be somewhat higher than sustainable network
-  // bandwidth because of buffering in the kernel during pauses.
-  // Using Bytes/s rather than bits/s so that this value is easily
-  // comparable with BandwidthOverall and BandwidthDisk.
-  UMA_HISTOGRAM_CUSTOM_COUNTS(
-      "Download.BandwidthNetworkBytesPerSecond",
-      (1000 * length / non_pause_time_ms),
-      1, 100000000, 50);
+// Record what percentage of the time we have the network flow controlled.
+void RecordNetworkBlockage(base::TimeDelta resource_handler_lifetime,
+                           base::TimeDelta resource_handler_blocked_time) {
+  int percentage = 0;
+  // Avoid division by zero errors.
+  if (resource_handler_blocked_time != base::TimeDelta()) {
+    percentage =
+        resource_handler_blocked_time * 100 / resource_handler_lifetime;
+  }
+
+  UMA_HISTOGRAM_COUNTS_100("Download.ResourceHandlerBlockedPercentage",
+                           percentage);
 }
 
 void RecordFileBandwidth(size_t length,
@@ -339,6 +336,8 @@ void RecordFileBandwidth(size_t length,
   UMA_HISTOGRAM_CUSTOM_COUNTS(
       "Download.BandwidthDiskBytesPerSecond",
       (1000 * length / disk_write_time_ms), 1, 50000000, 50);
+  UMA_HISTOGRAM_COUNTS_100("Download.DiskBandwidthUsedPercentage",
+                           disk_write_time_ms * 100 / elapsed_time_ms);
 }
 
 void RecordSavePackageEvent(SavePackageEvent event) {
diff --git a/content/browser/download/download_stats.h b/content/browser/download/download_stats.h
index 7633e51..91a6822 100644
--- a/content/browser/download/download_stats.h
+++ b/content/browser/download/download_stats.h
@@ -141,10 +141,11 @@ void RecordOpensOutstanding(int size);
 // Record how long we block the file thread at a time.
 void RecordContiguousWriteTime(base::TimeDelta time_blocked);
 
-// Record overall bandwidth stats at the network end.
-void RecordNetworkBandwidth(size_t length,
-                            base::TimeDelta elapsed_time,
-                            base::TimeDelta paused_time);
+// Record the percentage of time we had to block the network (i.e.
+// how often, for each download, something other than the network
+// was the bottleneck).
+void RecordNetworkBlockage(base::TimeDelta resource_handler_lifetime,
+                           base::TimeDelta resource_handler_blocked_time);
 
 // Record overall bandwidth stats at the file end.
 void RecordFileBandwidth(size_t length,