mac: Add a profiler for startup time.

chrome/browser/mac/mac_startup_profiler.cc

Index: chrome/browser/mac/mac_startup_profiler.cc
diff --git a/chrome/browser/mac/mac_startup_profiler.cc b/chrome/browser/mac/mac_startup_profiler.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ee160534d61e39a1cdfc58a96e9c85de8d31e4b7
--- /dev/null
+++ b/chrome/browser/mac/mac_startup_profiler.cc
@@ -0,0 +1,89 @@
+// Copyright 2014 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 "chrome/browser/mac/mac_startup_profiler.h"
+
+#include "base/logging.h"
+#include "base/metrics/histogram.h"
+
+// static
+MacStartupProfiler* MacStartupProfiler::GetInstance() {
+  return Singleton<MacStartupProfiler>::get();
+}
+
+MacStartupProfiler::MacStartupProfiler() : recorded_metrics_(false) {
+}
+
+MacStartupProfiler::~MacStartupProfiler() {
+}
+
+void MacStartupProfiler::RecordLaunchTime() {
+  launch_time_ = base::Time::Now();
+}
+
+void MacStartupProfiler::Profile(Location location) {
+  DCHECK(!launch_time_.is_null());
+  profiled_times_[location] = base::Time::Now();
+}
+
+void MacStartupProfiler::RecordMetrics() {
+  DCHECK(!launch_time_.is_null());
+  DCHECK(!recorded_metrics_);
+
+  recorded_metrics_ = true;
+
+  for (std::map<Location, base::Time>::const_iterator it =
+           profiled_times_.begin();
+       it != profiled_times_.end();
+       ++it) {
+    const base::Time& location_time = it->second;
+    base::TimeDelta delta = location_time - launch_time_;
+    RecordHistogram(it->first, delta);
+  }
+}
+
+const std::string MacStartupProfiler::HistogramName(Location location) {
+  std::string base("OSX.Startup.");

[Ilya Sherman, 2014/07/15 22:42:00]

Optional nit: Perhaps |prefix| would be a slightly clearer name for this
variable?

[erikchen, 2014/07/15 23:04:39]

Done.

+  const char* extension;

[Ilya Sherman, 2014/07/15 22:42:00]

Hmm, why did you decide to declare a variable for this, rather than keeping the
pattern of |return base + "AwakeFromNib";| from the previous version?

[erikchen, 2014/07/15 23:04:39]

Good question. I changed to the previous pattern.

+  switch (location) {
+    case PRE_MAIN_MESSAGE_LOOP_START:
+      extension = "PreMainMessageLoopStart";
+      break;
+    case AWAKE_FROM_NIB:
+      extension = "AwakeFromNib";
+      break;
+    case POST_MAIN_MESSAGE_LOOP_START:
+      extension = "PostMainMessageLoopStart";
+      break;
+    case PRE_PROFILE_INIT:
+      extension = "PreProfileInit";
+      break;
+    case POST_PROFILE_INIT:
+      extension = "PostProfileInit";
+      break;
+    case WILL_FINISH_LAUNCHING:
+      extension = "WillFinishLaunching";
+      break;
+    case DID_FINISH_LAUNCHING:
+      extension = "DidFinishLaunching";
+      break;
+  }
+  base += extension;
+  return base;

[Ilya Sherman, 2014/07/15 22:42:00]

nit: If you keep this structure, please shorten these two lines to "return base
+ extension;"

[erikchen, 2014/07/15 23:04:39]

N/A after previous comment.

+}
+
+void MacStartupProfiler::RecordHistogram(Location location,
+                                         const base::TimeDelta& delta) {
+  const char* name = HistogramName(location).c_str();

[Ilya Sherman, 2014/07/15 22:42:00]

nit: Why are you extracting out the C-string?  FactoryTimeGet accepts an
std::string.

[erikchen, 2014/07/15 23:04:39]

Right you are.

+  base::TimeDelta min = base::TimeDelta::FromMilliseconds(10);
+  base::TimeDelta max = base::TimeDelta::FromMinutes(1);
+  int bucket_count = 100;
+
+  // No need to cache the histogram pointers, since each invocation of this
+  // method will be the first and only usage of a histogram with that given
+  // name.
+  base::HistogramBase* histogram = base::Histogram::FactoryTimeGet(
+      name, min, max, bucket_count, base::HistogramBase::kNoFlags);
+  histogram->AddTime(delta);
+}