Record the TaskLatency Histogram in Microseconds Instead of Milliseconds

base/task_scheduler/task_tracker.cc

Index: base/task_scheduler/task_tracker.cc
diff --git a/base/task_scheduler/task_tracker.cc b/base/task_scheduler/task_tracker.cc
index da21d5ea8ef510c33636cc993b34e3501aa09101..5dad684e1805e96c06e47de8a01cbdff09342552 100644
--- a/base/task_scheduler/task_tracker.cc
+++ b/base/task_scheduler/task_tracker.cc
@@ -74,11 +74,11 @@ const char kQueueFunctionName[] = "base::PostTask";
 const char kRunFunctionName[] = "TaskSchedulerRunTask";
 
 HistogramBase* GetTaskLatencyHistogram(const char* suffix) {
-  // Mimics the UMA_HISTOGRAM_TIMES macro.
-  return Histogram::FactoryTimeGet(
-      std::string("TaskScheduler.TaskLatency.") + suffix,
-      TimeDelta::FromMilliseconds(1), TimeDelta::FromSeconds(10), 50,
-      HistogramBase::kUmaTargetedHistogramFlag);
+  // Mimics the UMA_HISTOGRAM_TIMES macro except we don't specify bounds with
+  // TimeDeltas as FactoryTimeGet assumes millisecond granularity.
+  return Histogram::FactoryGet(
+      std::string("TaskScheduler.TaskLatencyMicroseconds.") + suffix, 1, 2000,
+      50, HistogramBase::kUmaTargetedHistogramFlag);

[Ilya Sherman, 2017/04/27 21:24:01]

While you're here, would you mind updating this code to use UmaHistogramTimes()
rather than getting into the FactoryGet calls directly?

[robliao, 2017/04/27 21:42:22]

UmaHistogramTimes starts at 1ms minimum time sample, no?
https://cs.chromium.org/chromium/src/base/metrics/histogram_functions.h?l=87

[Ilya Sherman, 2017/04/27 21:50:35]

Yes, sorry, you're right.  I meant UmaHistogramCustomTimes().

[robliao, 2017/04/27 21:54:49]

UmaHistogramCustomTimes -> 
HistogramBase* histogram = Histogram::FactoryTimeGet(
      name, min, max, buckets, HistogramBase::kUmaTargetedHistogramFlag);

This leads to
  return FactoryGet(name, static_cast<Sample(minimum.InMilliseconds()),
static_cast<Sample>(maximum.InMilliseconds()), bucket_count, flags);

I wasn't able to find a Uma* Time Histogram function that handled microseconds,
which is why I went directly to FactoryGet.

[Ilya Sherman, 2017/04/27 21:56:45]

Ah, I see.  How about UmaHistogramCustomCounts?

It's not really super important, tbh.  Someday we might want to hide
kUmaTargetedHistogramFlag as a non-leaky implementation detail; but if you
prefer FactoryGet for now, that's fine too.

[robliao, 2017/04/27 22:07:44]

Gotcha. We also keep the histogram around so we don't have to call FactoryGet()
so I will leave it as is.

The UmaHistogram* calls requery for the histogram every time.

 }
 
 // Upper bound for the
@@ -495,7 +495,7 @@ void TaskTracker::RecordTaskLatencyHistogram(Task* task) {
                                    task->traits.with_base_sync_primitives()
                                ? 1
                                : 0]
-                              ->AddTime(task_latency);
+                              ->Add(task_latency.InMicroseconds());
 }
 
 }  // namespace internal