Performance monitor stats gathering.

chrome/browser/performance_monitor/performance_monitor.cc

Index: chrome/browser/performance_monitor/performance_monitor.cc
diff --git a/chrome/browser/performance_monitor/performance_monitor.cc b/chrome/browser/performance_monitor/performance_monitor.cc
index f72b845c2277c69f6dfe1c6d9a8d1b9fa2786ccb..ca6833a3411534c8f6d86f1f994904b20763ed65 100644
--- a/chrome/browser/performance_monitor/performance_monitor.cc
+++ b/chrome/browser/performance_monitor/performance_monitor.cc
@@ -4,8 +4,16 @@
 
 #include "chrome/browser/performance_monitor/performance_monitor.h"
 
+#include <algorithm>
+#include <map>
+#include <string>
+#include <vector>
+
 #include "base/bind.h"
 #include "base/logging.h"
+#include "base/process.h"
+#include "base/process_util.h"
+#include "base/string_number_conversions.h"
 #include "base/threading/worker_pool.h"
 #include "base/time.h"
 #include "chrome/browser/performance_monitor/constants.h"
@@ -16,11 +24,28 @@
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
+#include "chrome/test/base/chrome_process_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 
 using extensions::Extension;
 
+namespace {
+
+template <class T>
+T GetMedian(std::vector<T> vec) {
+  typedef typename std::vector<T>::size_type vec_size;
+
+  vec_size size = vec.size();
+  DCHECK(size != 0);
+  std::sort(vec.begin(), vec.end());
+  vec_size mid = size / 2;
+
+  return size % 2 == 0 ? (vec[mid] + vec[mid - 1]) / 2 : vec[mid];
+}
+
+}  // namespace
+
 namespace performance_monitor {
 
 PerformanceMonitor::PerformanceMonitor() : database_(NULL) {
@@ -89,6 +114,23 @@ void PerformanceMonitor::AddEventOnBackgroundThread(scoped_ptr<Event> event) {
   database_->AddEvent(*event.get());
 }
 
+void PerformanceMonitor::AddMetric(const std::string& key,
+                                   const std::string& value) {
+  content::BrowserThread::PostBlockingPoolSequencedTask(
+      Database::kDatabaseSequenceToken,
+      FROM_HERE,
+      base::Bind(&PerformanceMonitor::AddMetricOnBackgroundThread,
+                 base::Unretained(this),
+                 key,
+                 value));
+}
+
+void PerformanceMonitor::AddMetricOnBackgroundThread(const std::string& key,
+                                                     const std::string& value) {
+  database_->AddMetric(key, value);

[Devlin, 2012/07/11 16:28:08]

These two functions can be combined, since they don't have to do any special
scoping operations like events do. Just bind the database task in AddMetric().

[mitchellwrosen, 2012/07/11 18:30:18]

Done.

+}
+
+
 void PerformanceMonitor::GetStateValueOnBackgroundThread(
     const std::string& key,
     const StateValueCallback& callback) {
@@ -144,6 +186,99 @@ void PerformanceMonitor::CheckForVersionUpdateHelper(
   }
 }
 
+void PerformanceMonitor::GatherStatistics() {

[Devlin, 2012/07/11 16:28:08]

If the method is public and runs on the background thread, it should be called
GatherStatisticsOnBackgroundThread().

[mitchellwrosen, 2012/07/11 18:30:18]

Done.

+  CHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

[Devlin, 2012/07/11 16:28:08]

Will gathering the stats on the background thread return the same as gathering
stats on the UI thread, or will it affect the pids? (Please double check).

+
+  // Because the CPU usage is gathered as an average since the last time the
+  // function was called, while the memory usage is gathered as an instantaneous
+  // usage, the CPU usage is gathered before the metrics map is wiped.
+  GatherCPUUsage();
+  UpdateMetricsMap();
+  GatherMemoryUsage();
+}
+
+void PerformanceMonitor::GatherCPUUsage() {
+  std::vector<double> cpu_usage_vec;
+
+  if (metrics_map_.size()) {
+    for (MetricsMap::const_iterator iter = metrics_map_.begin();
+         iter != metrics_map_.end(); ++iter) {
+      double cpu_usage = iter->second->GetCPUUsage();
+      DCHECK_NE(cpu_usage, 0) << "Gathered CPU usage of 0";
+      cpu_usage_vec.push_back(cpu_usage);
+    }
+
+    std::sort(cpu_usage_vec.begin(), cpu_usage_vec.end());
+    AddMetric(performance_monitor::kMetricCPUUsage,

[Devlin, 2012/07/11 16:28:08]

If you're on the background thread, you don't need to use the wrapper function.

[mitchellwrosen, 2012/07/11 18:30:18]

Done.

+              base::DoubleToString(GetMedian(cpu_usage_vec)));
+  }
+}
+
+void PerformanceMonitor::GatherMemoryUsage() {
+  std::vector<size_t> private_memory_vec;
+  std::vector<size_t> shared_memory_vec;
+
+  size_t private_memory = 0;
+  size_t shared_memory = 0;
+  for (MetricsMap::const_iterator iter = metrics_map_.begin();
+       iter != metrics_map_.end(); ++iter) {
+    if (iter->second->GetMemoryBytes(&private_memory, &shared_memory)) {
+      private_memory_vec.push_back(private_memory);
+      shared_memory_vec.push_back(shared_memory);
+    } else {
+      LOG(ERROR) << "GetMemoryBytes returned NULL (platform-specific error)";
+    }
+  }
+
+  std::sort(private_memory_vec.begin(), private_memory_vec.end());

[Devlin, 2012/07/11 16:28:08]

Nit: Check the size before sorting (even though sort will check, it makes the
code look cleaner). Perhaps just have
   if (!private_memory_vec.size()
     return;
right after you gather?

[mitchellwrosen, 2012/07/11 18:30:18]

Done.

+  std::sort(shared_memory_vec.begin(), shared_memory_vec.end());
+
+  if (private_memory_vec.size()) {
+    AddMetric(performance_monitor::kMetricPrivateMemoryUsage,
+              base::Uint64ToString(GetMedian(private_memory_vec)));
+
+    AddMetric(performance_monitor::kMetricSharedMemoryUsage,
+              base::Uint64ToString(GetMedian(shared_memory_vec)));
+  }
+}
+
+void PerformanceMonitor::UpdateMetricsMap() {
+  // Remove old process handles.
+  for (MetricsMap::iterator iter = metrics_map_.begin();
+       iter != metrics_map_.end(); ++iter) {
+    if (iter->first == base::kNullProcessHandle) {
+      // base::CloseProcessHandle(iter->first); // Necessary?
+      metrics_map_.erase(iter);
+    } else {
+      // Prime the CPUUsage to be gathered next time.
+      iter->second->GetCPUUsage();
+    }
+  }
+
+  // Add new process handles

[Devlin, 2012/07/11 16:28:08]

add a .

[mitchellwrosen, 2012/07/11 18:30:18]

Done.

+  base::ProcessId browser_pid = base::GetCurrentProcId();
+  ChromeProcessList chrome_processes(GetRunningChromeProcesses(browser_pid));

[Devlin, 2012/07/11 16:28:08]

If you are getting the list of running processes, should browser_pid be
base::GetParentProcessId on POSIX? (I'm not sure, but I seem to recall that it
might be.)

[mitchellwrosen, 2012/07/11 18:30:18]

I couldn't find anything to suggest otherwise. It's proven quite hard to track
down exactly how to get the browser process ID.

+  for (ChromeProcessList::const_iterator pid_iter = chrome_processes.begin();
+       pid_iter != chrome_processes.end(); ++pid_iter) {
+    base::ProcessHandle process_handle;
+    if (base::OpenProcessHandle(*pid_iter, &process_handle)) {
+      linked_ptr<base::ProcessMetrics> process_metrics(

[Yoyo Zhou, 2012/07/10 21:58:00]

This should be moved to inside the if statement.

[mitchellwrosen, 2012/07/11 18:30:18]

Done.

[Yoyo Zhou, 2012/07/19 21:35:35]

I did not mean the #if, I meant the if statement below it (at 261). But this is
probably a good idea too (keeping a complete statement within each ifdef block).

+#if !defined(OS_MACOSX)
+          base::ProcessMetrics::CreateProcessMetrics(process_handle));
+#else
+          base::ProcessMetrics::CreateProcessMetrics(process_handle,
+              content::BrowserChildProcessHost::GetPortProvider()));
+#endif
+      if (!ContainsKey(metrics_map_, process_handle)) {
+        // Prime the CPUUsage to be gathered next time.
+        process_metrics->GetCPUUsage();
+
+        metrics_map_[process_handle] = process_metrics;
+      }
+    }
+  }
+}
+
 void PerformanceMonitor::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {