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..0e6a3d38e5239d026b55f3b86406bdcacbc991b4
--- /dev/null
+++ b/chrome/browser/mac/mac_startup_profiler.cc
@@ -0,0 +1,100 @@
+// 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), launch_time_valid_(false) {
+}
+
+MacStartupProfiler::~MacStartupProfiler() {
+}
+
+void MacStartupProfiler::RecordLaunchTime() {
+  launch_time_ = base::Time::Now();
+  launch_time_valid_ = true;
+}
+
+void MacStartupProfiler::Profile(Location location) {
+  DCHECK(launch_time_valid_);
+  map_.insert(std::make_pair(location, base::Time::Now()));

[Ilya Sherman, 2014/07/15 18:14:24]

nit: IMO "map_[location] = base::Time::Now();" is clearer.

[erikchen, 2014/07/15 21:39:42]

Done.

+}
+
+void MacStartupProfiler::RecordMetrics() {
+  DCHECK(launch_time_valid_);
+  DCHECK(!recorded_metrics_);
+
+  recorded_metrics_ = true;
+
+  for (std::map<Location, base::Time>::iterator it = map_.begin();

[Ilya Sherman, 2014/07/15 18:14:24]

nit: Can this be a const_iterator?

[erikchen, 2014/07/15 21:39:42]

yes, done

+       it != map_.end();
+       ++it) {
+    const base::Time& location_time = it->second;
+    base::TimeDelta delta = location_time - launch_time_;
+    RecordHistogram(it->first, delta);
+  }
+}
+
+const char* MacStartupProfiler::HistogramName(Location location) {
+#define HISTOGRAM_PREFIX "OSX.Startup."

[Ilya Sherman, 2014/07/15 18:14:24]

Please return a string and use normal string concatenation operations rather
than defining a preprocessor macro.

[erikchen, 2014/07/15 21:39:42]

The preprocessor macro uses fewer lines of code, and is more performant. *shrug*
I've done as you've suggested.

+  switch (location) {
+    case PRE_MAIN_MESSAGE_LOOP_START:
+      return HISTOGRAM_PREFIX "PreMainMessageLoopStart";
+    case AWAKE_FROM_NIB:
+      return HISTOGRAM_PREFIX "AwakeFromNib";
+    case POST_MAIN_MESSAGE_LOOP_START:
+      return HISTOGRAM_PREFIX "PostMainMessageLoopStart";
+    case PRE_PROFILE_INIT:
+      return HISTOGRAM_PREFIX "PreProfileInit";
+    case POST_PROFILE_INIT:
+      return HISTOGRAM_PREFIX "PostProfileInit";
+    case WILL_FINISH_LAUNCHING:
+      return HISTOGRAM_PREFIX "WillFinishLaunching";
+    case DID_FINISH_LAUNCHING:
+      return HISTOGRAM_PREFIX "DidFinishLaunching";
+  }
+#undef HISTOGRAM_PREFIX
+}
+
+void MacStartupProfiler::RecordHistogram(Location location,
+                                         const base::TimeDelta& delta) {
+  const char* name = HistogramName(location);
+  base::TimeDelta min = base::TimeDelta::FromMilliseconds(10);
+  base::TimeDelta max = base::TimeDelta::FromMinutes(1);
+  int buckets = 100;

[Ilya Sherman, 2014/07/15 18:14:24]

Are you sure that the parameters for, say, UMA_HISTOGRAM_MEDIUM_TIMES aren't
matched closely enough to your needs?

[erikchen, 2014/07/15 21:39:42]

This is no longer relevant, since I'm not using a MACRO anymore.

+
+  // The UMA_HISTOGRAM_CUSTOM_TIMES macro requires a separate declaration for
+  // each different name.

[Ilya Sherman, 2014/07/15 18:14:24]

Given that you do this below, why not just inline the HistogramName() method? 
Alternately, you could avoid using the macro to avoid needing the switch stmt
below.

[erikchen, 2014/07/15 21:39:42]

inlining HistogramName() does not actually work (there's still only 1 pointer
made from the UMA_HISTOGRAM_CUSTOM_TIMES macro). I've gone ahead and removed the
macros entirely.

[Ilya Sherman, 2014/07/15 22:41:59]

FWIW, by inlining HistogramName(), I meant not having a |name| variable, and
instead just listing the exact histogram name at each spot.  But, yes, this is
fine too.

+  switch (location) {
+    case PRE_MAIN_MESSAGE_LOOP_START:
+      UMA_HISTOGRAM_CUSTOM_TIMES(name, delta, min, max, buckets);
+      return;
+    case AWAKE_FROM_NIB:
+      UMA_HISTOGRAM_CUSTOM_TIMES(name, delta, min, max, buckets);
+      return;
+    case POST_MAIN_MESSAGE_LOOP_START:
+      UMA_HISTOGRAM_CUSTOM_TIMES(name, delta, min, max, buckets);
+      return;
+    case PRE_PROFILE_INIT:
+      UMA_HISTOGRAM_CUSTOM_TIMES(name, delta, min, max, buckets);
+      return;
+    case POST_PROFILE_INIT:
+      UMA_HISTOGRAM_CUSTOM_TIMES(name, delta, min, max, buckets);
+      return;
+    case WILL_FINISH_LAUNCHING:
+      UMA_HISTOGRAM_CUSTOM_TIMES(name, delta, min, max, buckets);
+      return;
+    case DID_FINISH_LAUNCHING:
+      UMA_HISTOGRAM_CUSTOM_TIMES(name, delta, min, max, buckets);
+      return;
+  }
+}