Chrome Performance Monitor: Moved MetricDetails lookup to metric_details.*.

Database keys and MetricDetails lookups are now made through the MetricType enum.

BUG=130212
TEST=


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

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

chrome/browser/performance_monitor/constants.cc

@@ -11,7 +11,17 @@ namespace performance_monitor {
 const char kStateChromeVersion[] = "chrome_version";
 const char kMetricNotFoundError[] = "Mertic details not found.";
 const char kProcessChromeAggregate[] = "chrome_aggregate";
-const char kSampleMetricDescription[] = "A sample metric.";
-const char kSampleMetricName[] = "SAMPLE";
+
+const char kMetricCPUUsageName[] = "CPU Usage";
+const char kMetricCPUUsageDescription[] = "The CPU usage measured in percent.";
+const char kMetricCPUUsageUnits[] = "percent";
+const double kMetricCPUUsageTickSize = 100.0;
+
+const char kMetricPrivateMemoryUsageName[] = "Private Memory Usage";
+const char kMetricPrivateMemoryUsageDescription[] =
+    "The private memory usage measured in bytes.";
+const char kMetricPrivateMemoryUsageUnits[] = "percent";
+const double kMetricPrivateMemoryUsageTickSize = 10000000.0;
+
 
 }  // namespace performance_monitor

chrome/browser/performance_monitor/constants.h

@@ -7,18 +7,24 @@
 
 namespace performance_monitor {
 
-// TODO(chebert): i18n
+// TODO(chebert): i18n on all constants.
 extern const char kMetricNotFoundError[];
 
 // Any metric that is not associated with a specific activity will use this as
 // its activity.
 extern const char kProcessChromeAggregate[];
 
-// When you add a metric type, make sure to edit Database::InitMetricDetails
-// and add an entry to the MericType enum.
-// TODO(mwrosen): When you make real metrics, delete the sample metric.
-extern const char kSampleMetricDescription[];
-extern const char kSampleMetricName[];
+// Metrics keys for statistics gathering.
+extern const char kMetricCPUUsageName[];
+extern const char kMetricCPUUsageDescription[];
+extern const char kMetricCPUUsageUnits[];
+extern const double kMetricCPUUsageTickSize;
+
+extern const char kMetricPrivateMemoryUsageName[];
+extern const char kMetricPrivateMemoryUsageDescription[];
+extern const char kMetricPrivateMemoryUsageUnits[];
+extern const double kMetricPrivateMemoryUsageTickSize;
+
 extern const char kStateChromeVersion[];
 
 }  // namespace performance_monitor

chrome/browser/performance_monitor/database.h

@@ -141,55 +141,55 @@ class Database {
   }
 
   // Add a metric instance to the database.
-  bool AddMetric(const std::string& activity, const std::string& metric,
+  bool AddMetric(const std::string& activity,
+                 MetricType metric_type,
                  const std::string& value);
 
-  bool AddMetric(const std::string& metric, const std::string& value) {
-    return AddMetric(kProcessChromeAggregate, metric, value);
+  bool AddMetric(MetricType metric_type, const std::string& value) {
+    return AddMetric(kProcessChromeAggregate, metric_type, value);
   }
 
-  void AddMetricDetails(const MetricDetails& details);
-
   // Get the metrics that are active for the given process between |start|
   // (inclusive) and |end| (exclusive).
-  std::vector<MetricDetails> GetActiveMetrics(const base::Time& start,
-                                              const base::Time& end);
+  std::vector<const MetricDetails*> GetActiveMetrics(const base::Time& start,
+                                                     const base::Time& end);
 
   // Get the activities that are active for the given metric after |start|.
-  std::vector<std::string> GetActiveActivities(const std::string& metric,
+  std::vector<std::string> GetActiveActivities(MetricType metric_type,
                                                const base::Time& start);
 
-  // Query given metric and activity.
+  // Query given |metric_type| and |activity|.
   MetricInfoVector GetStatsForActivityAndMetric(const std::string& activity,
-                                                const std::string& metric,
+                                                MetricType metric_type,
                                                 const base::Time& start,
                                                 const base::Time& end);
 
-  MetricInfoVector GetStatsForActivityAndMetric(const std::string& metric,
+  MetricInfoVector GetStatsForActivityAndMetric(MetricType metric_type,
                                                 const base::Time& start,
                                                 const base::Time& end) {
-    return GetStatsForActivityAndMetric(kProcessChromeAggregate, metric,
+    return GetStatsForActivityAndMetric(kProcessChromeAggregate, metric_type,
                                         start, end);
   }
 
   MetricInfoVector GetStatsForActivityAndMetric(const std::string& activity,
-                                                const std::string& metric) {
-    return GetStatsForActivityAndMetric(activity, metric, base::Time(),
+                                                MetricType metric_type) {
+    return GetStatsForActivityAndMetric(activity, metric_type, base::Time(),
                                         clock_->GetTime());
   }
 
-  MetricInfoVector GetStatsForActivityAndMetric(const std::string& metric) {
-    return GetStatsForActivityAndMetric(kProcessChromeAggregate, metric,
+  MetricInfoVector GetStatsForActivityAndMetric(MetricType metric_type) {
+    return GetStatsForActivityAndMetric(kProcessChromeAggregate, metric_type,
                                         base::Time(), clock_->GetTime());
   }
 
-  // Query given metric. The returned map is keyed by activity.
-  MetricVectorMap GetStatsForMetricByActivity(const std::string& metric,
+  // Query given |metric_type|. The returned map is keyed by activity.
+  MetricVectorMap GetStatsForMetricByActivity(MetricType metric_type,
                                               const base::Time& start,
                                               const base::Time& end);
 
-  MetricVectorMap GetStatsForMetricByActivity(const std::string& metric) {
-    return GetStatsForMetricByActivity(metric, base::Time(), clock_->GetTime());
+  MetricVectorMap GetStatsForMetricByActivity(MetricType metric_type) {
+    return GetStatsForMetricByActivity(
+        metric_type, base::Time(), clock_->GetTime());
   }
 
   // Returns the active time intervals that overlap with the time interval
@@ -208,7 +208,6 @@ class Database {
   FRIEND_TEST_ALL_PREFIXES(PerformanceMonitorDatabaseSetupTest, ActiveInterval);
 
   typedef std::map<std::string, std::string> RecentMap;
-  typedef std::map<std::string, MetricDetails> MetricDetailsMap;
 
   // By default, the database uses a clock that simply returns the current time.
   class SystemClock : public Clock {
@@ -222,8 +221,6 @@ class Database {
 
   void InitDBs();
 
-  void InitMetricDetails();
-
   bool Close();
 
   // Load recent info from the db into recent_map_.
@@ -237,10 +234,6 @@ class Database {
   // insert.
   RecentMap recent_map_;
 
-  // A mapping of a metric key to details about that metric. New metrics should
-  // modify InitMetricDetails to add an entry in this map.
-  MetricDetailsMap metric_details_map_;
-
   // The directory where all the databases will reside.
   FilePath path_;
 

chrome/browser/performance_monitor/database_unittest.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <algorithm>
 #include <string>
 #include <vector>
 
@@ -11,6 +12,7 @@
 #include "base/scoped_temp_dir.h"
 #include "base/time.h"
 #include "chrome/browser/performance_monitor/database.h"
+#include "chrome/browser/performance_monitor/metric_details.h"
 #include "chrome/browser/performance_monitor/performance_monitor_util.h"
 #include "chrome/common/extensions/extension.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -93,12 +95,6 @@ class PerformanceMonitorDatabaseMetricTest : public ::testing::Test {
     CHECK(db_.get());
     db_->set_clock(scoped_ptr<Database::Clock>(clock_));
     activity_ = std::string("A");
-    cpu_metric_ = std::string("CPU");
-    mem_metric_ = std::string("MEM");
-    fun_metric_ = std::string("FUN");
-    cpu_metric_details_ = MetricDetails(cpu_metric_, "Testing CPU metric.");
-    mem_metric_details_ = MetricDetails(mem_metric_, "Testing MEM metric.");
-    fun_metric_details_ = MetricDetails(fun_metric_, "Testing FUN metric.");
   }
 
   void SetUp() {
@@ -107,27 +103,20 @@ class PerformanceMonitorDatabaseMetricTest : public ::testing::Test {
   }
 
   void PopulateDB() {
-    db_->AddMetricDetails(cpu_metric_details_);
-    db_->AddMetricDetails(mem_metric_details_);
-    db_->AddMetricDetails(fun_metric_details_);
-    db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("50.5"));
-    db_->AddMetric(activity_, cpu_metric_, std::string("13.1"));
-    db_->AddMetric(kProcessChromeAggregate, mem_metric_, std::string("100000"));
-    db_->AddMetric(activity_, mem_metric_, std::string("20000"));
-    db_->AddMetric(kProcessChromeAggregate, fun_metric_, std::string("3.14"));
-    db_->AddMetric(activity_, fun_metric_, std::string("1.68"));
+    db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE,
+                   std::string("50.5"));
+    db_->AddMetric(activity_, METRIC_CPU_USAGE, std::string("13.1"));
+
+    db_->AddMetric(kProcessChromeAggregate, METRIC_PRIVATE_MEMORY_USAGE,
+                   std::string("1000000"));
+    db_->AddMetric(activity_, METRIC_PRIVATE_MEMORY_USAGE,
+                   std::string("3000000"));
   }
 
   scoped_ptr<Database> db_;
   Database::Clock* clock_;
   ScopedTempDir temp_dir_;
   std::string activity_;
-  std::string cpu_metric_;
-  std::string mem_metric_;
-  std::string fun_metric_;
-  MetricDetails cpu_metric_details_;
-  MetricDetails mem_metric_details_;
-  MetricDetails fun_metric_details_;
 };
 
 ////// PerformanceMonitorDatabaseSetupTests ////////////////////////////////////
@@ -285,20 +274,15 @@ TEST_F(PerformanceMonitorDatabaseEventTest, GetEventsTimeRange) {
 
 ////// PerformanceMonitorDatabaseMetricTests ///////////////////////////////////
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetActiveMetrics) {
-  std::vector<MetricDetails> metrics =
+  std::vector<const MetricDetails*> active_metrics =
     db_->GetActiveMetrics(base::Time(), clock_->GetTime());
-  ASSERT_EQ(3u, metrics.size());
-  std::vector<MetricDetails>::iterator it;
-  for (it = metrics.begin(); it != metrics.end(); ++it) {
-    if (it->name == cpu_metric_details_.name)
-      EXPECT_EQ(it->description, cpu_metric_details_.description);
-    else if (it->name == mem_metric_details_.name)
-      EXPECT_EQ(it->description, mem_metric_details_.description);
-    else if (it->name == fun_metric_details_.name)
-      EXPECT_EQ(it->description, fun_metric_details_.description);
-    else
-      NOTREACHED();
-  }
+  std::sort(active_metrics.begin(), active_metrics.end());
+
+  std::vector<const MetricDetails*> expected_metrics;
+  expected_metrics.push_back(GetMetricDetails(METRIC_CPU_USAGE));
+  expected_metrics.push_back(GetMetricDetails(METRIC_PRIVATE_MEMORY_USAGE));
+  std::sort(expected_metrics.begin(), expected_metrics.end());
+  EXPECT_EQ(expected_metrics, active_metrics);
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetState) {
@@ -319,23 +303,23 @@ TEST_F(PerformanceMonitorDatabaseMetricTest, GetStateOverride) {
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForActivityAndMetric) {
   Database::MetricInfoVector stats = db_->GetStatsForActivityAndMetric(
-      activity_, cpu_metric_, base::Time(), clock_->GetTime());
+      activity_, METRIC_CPU_USAGE, base::Time(), clock_->GetTime());
   ASSERT_EQ(1u, stats.size());
   EXPECT_EQ(13.1, stats[0].value);
   base::Time before = clock_->GetTime();
-  db_->AddMetric(activity_, cpu_metric_, std::string("18"));
-  stats = db_->GetStatsForActivityAndMetric(activity_, cpu_metric_,
+  db_->AddMetric(activity_, METRIC_CPU_USAGE, std::string("18"));
+  stats = db_->GetStatsForActivityAndMetric(activity_, METRIC_CPU_USAGE,
                                             before, clock_->GetTime());
   ASSERT_EQ(1u, stats.size());
   EXPECT_EQ(18, stats[0].value);
-  stats = db_->GetStatsForActivityAndMetric(activity_, cpu_metric_);
+  stats = db_->GetStatsForActivityAndMetric(activity_, METRIC_CPU_USAGE);
   ASSERT_EQ(2u, stats.size());
   EXPECT_EQ(13.1, stats[0].value);
   EXPECT_EQ(18, stats[1].value);
-  stats = db_->GetStatsForActivityAndMetric(mem_metric_);
+  stats = db_->GetStatsForActivityAndMetric(METRIC_PRIVATE_MEMORY_USAGE);
   ASSERT_EQ(1u, stats.size());
-  EXPECT_EQ(100000, stats[0].value);
-  stats = db_->GetStatsForActivityAndMetric(activity_, cpu_metric_,
+  EXPECT_EQ(1000000, stats[0].value);
+  stats = db_->GetStatsForActivityAndMetric(activity_, METRIC_CPU_USAGE,
                                             clock_->GetTime(),
                                             clock_->GetTime());
   EXPECT_TRUE(stats.empty());
@@ -343,7 +327,7 @@ TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForActivityAndMetric) {
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForMetricByActivity) {
   Database::MetricVectorMap stats_map = db_->GetStatsForMetricByActivity(
-      cpu_metric_, base::Time(), clock_->GetTime());
+      METRIC_CPU_USAGE, base::Time(), clock_->GetTime());
   ASSERT_EQ(2u, stats_map.size());
   linked_ptr<Database::MetricInfoVector> stats = stats_map[activity_];
   ASSERT_EQ(1u, stats->size());
@@ -351,12 +335,12 @@ TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForMetricByActivity) {
   stats = stats_map[kProcessChromeAggregate];
   ASSERT_EQ(1u, stats->size());
   EXPECT_EQ(50.5, stats->at(0).value);
-  stats_map = db_->GetStatsForMetricByActivity(cpu_metric_, clock_->GetTime(),
-                                               clock_->GetTime());
+  stats_map = db_->GetStatsForMetricByActivity(
+      METRIC_CPU_USAGE, clock_->GetTime(), clock_->GetTime());
   EXPECT_EQ(0u, stats_map.size());
   base::Time before = clock_->GetTime();
-  db_->AddMetric(activity_, cpu_metric_, std::string("18"));
-  stats_map = db_->GetStatsForMetricByActivity(cpu_metric_, before,
+  db_->AddMetric(activity_, METRIC_CPU_USAGE, std::string("18"));
+  stats_map = db_->GetStatsForMetricByActivity(METRIC_CPU_USAGE, before,
                                                clock_->GetTime());
   ASSERT_EQ(1u, stats_map.size());
   stats = stats_map[activity_];
@@ -365,10 +349,10 @@ TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForMetricByActivity) {
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetFullRange) {
-  db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("3.4"));
-  db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("21"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("3.4"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("21"));
   Database::MetricInfoVector stats =
-      db_->GetStatsForActivityAndMetric(cpu_metric_);
+      db_->GetStatsForActivityAndMetric(METRIC_CPU_USAGE);
   ASSERT_EQ(3u, stats.size());
   ASSERT_EQ(50.5, stats[0].value);
   ASSERT_EQ(3.4, stats[1].value);
@@ -377,12 +361,12 @@ TEST_F(PerformanceMonitorDatabaseMetricTest, GetFullRange) {
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetRange) {
   base::Time start = clock_->GetTime();
-  db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("3"));
-  db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("9"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("3"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("9"));
   base::Time end = clock_->GetTime();
-  db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("21"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("21"));
   Database::MetricInfoVector stats =
-      db_->GetStatsForActivityAndMetric(cpu_metric_, start, end);
+      db_->GetStatsForActivityAndMetric(METRIC_CPU_USAGE, start, end);
   ASSERT_EQ(2u, stats.size());
   ASSERT_EQ(3, stats[0].value);
   ASSERT_EQ(9, stats[1].value);

chrome/browser/performance_monitor/metric_details.cc

@@ -12,29 +12,28 @@ namespace {
 
 // Keep this array synced with MetricTypes in the header file.
 // TODO(mtytel): i18n.
-const char* kMetricTypeNames[] = {
-  kSampleMetricName
+const MetricDetails kMetricDetailsList[] = {
+  {
+    kMetricCPUUsageName,
+    kMetricCPUUsageDescription,
+    kMetricCPUUsageUnits,
+    kMetricCPUUsageTickSize,
+  },
+  {
+    kMetricPrivateMemoryUsageName,
+    kMetricPrivateMemoryUsageDescription,
+    kMetricPrivateMemoryUsageUnits,
+    kMetricPrivateMemoryUsageTickSize
+  },
 };
-COMPILE_ASSERT(ARRAYSIZE_UNSAFE(kMetricTypeNames) == METRIC_NUMBER_OF_METRICS,
+COMPILE_ASSERT(ARRAYSIZE_UNSAFE(kMetricDetailsList) == METRIC_NUMBER_OF_METRICS,
                metric_names_incorrect_size);
 
 }  // namespace
 
-const char* MetricTypeToString(MetricType metric_type) {
+const MetricDetails* GetMetricDetails(MetricType metric_type) {
   DCHECK_GT(METRIC_NUMBER_OF_METRICS, metric_type);
-  return kMetricTypeNames[metric_type];
-}
-
-MetricDetails::MetricDetails() {
-}
-
-MetricDetails::MetricDetails(const std::string& metric_name,
-                             const std::string& metric_description)
-    : name(metric_name),
-      description(metric_description) {
-}
-
-MetricDetails::~MetricDetails() {
+  return &kMetricDetailsList[metric_type];
 }
 
 }  // namespace performance_monitor

chrome/browser/performance_monitor/metric_details.h

@@ -9,23 +9,22 @@
 
 namespace performance_monitor {
 
+// Keep this enum synced with kMetricDetailsList in the cc file.
 enum MetricType {
-  METRIC_SAMPLE,
+  METRIC_CPU_USAGE,
+  METRIC_PRIVATE_MEMORY_USAGE,
   METRIC_NUMBER_OF_METRICS
 };
 
-const char* MetricTypeToString(MetricType event_type);
-
 struct MetricDetails {
-  MetricDetails();
-  MetricDetails(const std::string& metric_name,
-                const std::string& metric_description);
-  ~MetricDetails();
-
-  std::string name;
-  std::string description;
+  const char* const name;
+  const char* const description;
+  const char* const units;
+  const double tick_size;
 };
 
+const MetricDetails* GetMetricDetails(MetricType event_type);
+
 }  // namespace performance_monitor
 
 #endif  // CHROME_BROWSER_PERFORMANCE_MONITOR_METRIC_DETAILS_H_

chrome/browser/ui/webui/performance_monitor/web_ui_handler.cc

@@ -58,8 +58,8 @@ void DoGetMetric(ListValue* results,
                  const base::Time& start, const base::Time& end,
                  const base::TimeDelta& resolution) {
   Database* db = PerformanceMonitor::GetInstance()->database();
-  Database::MetricVectorMap metric_vector_map = db->GetStatsForMetricByActivity(
-      MetricTypeToString(metric_type), start, end);
+  Database::MetricVectorMap metric_vector_map =
+      db->GetStatsForMetricByActivity(metric_type, start, end);
 
   linked_ptr<Database::MetricInfoVector> metric_vector =
       metric_vector_map[kProcessChromeAggregate];
@@ -174,10 +174,14 @@ void WebUIHandler::HandleGetAllMetricTypes(const ListValue* args) {
   ListValue results;
   for (int i = 0; i < METRIC_NUMBER_OF_METRICS; ++i) {
     MetricType metric_type = static_cast<MetricType>(i);
+    const MetricDetails* metric_details = GetMetricDetails(metric_type);
+
     DictionaryValue* metric_type_info = new DictionaryValue();
     metric_type_info->SetInteger("metricType", metric_type);
     metric_type_info->SetString("shortDescription",
-                                MetricTypeToString(metric_type));
+                                metric_details->description);
+    metric_type_info->SetDouble("tickSize", metric_details->tick_size);
+    metric_type_info->SetString("units", metric_details->units);
     results.Append(metric_type_info);
   }