Disable timing from chrome://profiler on Android.

base/profiler/tracked_time.cc

 // Copyright (c) 2012 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 "base/profiler/tracked_time.h"
 
+#include "base/base_switches.h"
+#include "base/command_line.h"
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <mmsystem.h>  // Declare timeGetTime()... after including build_config.
 #endif
 
+namespace {
+inline bool IsProfilerTimingEnabled() {
+  // This initialization is not thread safe. In particular, some early calls

[Mark Mentovai, 2013/12/05 23:54:04]

Nit: thread-safe (with dash).

[qsr, 2013/12/06 14:29:43]

Done.

+  // might see |timing_disabled| being false, even if this is incorrect. This is
+  // OK, as the timing is only disabled for performance reasons, and having a
+  // few calls to TimeTicks::Now() is acceptable, while needing to use a lock or
+  // a memory barrier would be more costly.
+  static bool timing_disabled =
+      CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kProfilerTiming) == "0";

[jar (doing other things), 2013/12/05 23:51:06]

You can indeed change the name to avoid negatives.

Instead of using a "function static," you should use a "file scope static." 
You'll then get linker initialization to any constant value of your choice, and
not need to use the negative naming convention.  

You could get fancy and use atomics here... but your original code was equally
or more susceptible to the race initialization issues (gcc might try to protect
these... but it is not guaranteed in most other implementations).  

For instance, the code could read:
static enum {  // You could rely on namespace, instead of static.
  UNDEFINED_TIMING,
  ENABLED_TIMING,
  DISABLED_TIMING,
} timing_enabled = UNDEFINED TIMING;

inline bool IsProfilerTimingEnabled() {
  if (timing_enabled == UNDEFINED_TIMING) {
    timing_enabled =
(CommandLine::ForCurrentProcess()->GetSwitchValueASCII(switches::kProfilerTiming)
!= "0")
        ? ENABLED_TIMING : DISABLED_TIMING;
  return timing_enabled == ENABLED_TIMING;
} 

[Mark Mentovai, 2013/12/05 23:56:05]

jar wrote:
C++11 requires thread-safe static initialization, and gcc and clang both have
had thread-safe statics by default for forever, but we have disabled this
feature globally for compatibility with our brain-dead Windows compiler.

Lazy instances and singletons are cheap-ish in that they have a fast path that
doesn’t use a lock, but there’s still a barrier.

[qsr, 2013/12/06 14:29:43]

Done.

[no sievers, 2013/12/09 18:59:29]

Or you could just use LazyInstance (or Singleton), which is thread-safe and
"will hopefully have fast access when the instance is already created"
(lazy_instance.h).

+  return !timing_disabled;
+}
+}  // namespace
+
 namespace tracked_objects {
 
 Duration::Duration() : ms_(0) {}
 Duration::Duration(int32 duration) : ms_(duration) {}
 
 Duration& Duration::operator+=(const Duration& other) {
   ms_ += other.ms_;
   return *this;
 }
 
@@ skipping 21 matching lines @@
 //------------------------------------------------------------------------------
 
 TrackedTime::TrackedTime() : ms_(0) {}
 TrackedTime::TrackedTime(int32 ms) : ms_(ms) {}
 TrackedTime::TrackedTime(const base::TimeTicks& time)
     : ms_((time - base::TimeTicks()).InMilliseconds()) {
 }
 
 // static
 TrackedTime TrackedTime::Now() {
+  if (IsProfilerTimingEnabled()) {

[jar (doing other things), 2013/12/05 23:51:06]

nit: Early return, inducing less indentation, etc., is generally the preferred
style.  It helps with readability. Please consider undoing the changes on lines
70-80.

[Mark Mentovai, 2013/12/05 23:54:04]

I’d rather flip the logic. Having the “real” definition of this function
indented inside a conditional makes it seem more like it’s the afterthought and
not the prime purpose here.

[qsr, 2013/12/06 14:29:43]

Done.

 #if defined(OS_WIN)
-  // Use lock-free accessor to 32 bit time.
-  // Note that TimeTicks::Now() is built on this, so we have "compatible"
-  // times when we down-convert a TimeTicks sample.
-  return TrackedTime(base::TimeTicks::UnprotectedNow());
+    // Use lock-free accessor to 32 bit time.
+    // Note that TimeTicks::Now() is built on this, so we have "compatible"
+    // times when we down-convert a TimeTicks sample.
+    return TrackedTime(base::TimeTicks::UnprotectedNow());
 #else
-  // Posix has nice cheap 64 bit times, so we just down-convert it.
-  return TrackedTime(base::TimeTicks::Now());
+    // Posix has nice cheap 64 bit times, so we just down-convert it.
+    return TrackedTime(base::TimeTicks::Now());
 #endif  // OS_WIN
+  }
+  return TrackedTime();
 }
 
 Duration TrackedTime::operator-(const TrackedTime& other) const {
   return Duration(ms_ - other.ms_);
 }
 
 TrackedTime TrackedTime::operator+(const Duration& other) const {
   return TrackedTime(ms_ + other.ms_);
 }
 
 bool TrackedTime::is_null() const { return ms_ == 0; }
 
 }  // namespace tracked_objects