Report the duration of sampled tasks with histogram.

components/scheduler/renderer/task_time_reporter.cc

Index: components/scheduler/renderer/task_time_reporter.cc
diff --git a/components/scheduler/renderer/task_time_reporter.cc b/components/scheduler/renderer/task_time_reporter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e4ec861818640b4b5414e12fee43252ac5cd8e7b
--- /dev/null
+++ b/components/scheduler/renderer/task_time_reporter.cc
@@ -0,0 +1,26 @@
+// Copyright 2016 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 "components/scheduler/renderer/task_time_reporter.h"
+
+#include "base/metrics/histogram_macros.h"
+
+namespace scheduler {
+namespace {
+const int kSampleInterval = 100;
+}
+
+TaskTimeReporter::TaskTimeReporter() : sample_count_(0) {}
+
+TaskTimeReporter::~TaskTimeReporter() {}
+
+void TaskTimeReporter::ReportTaskTime(base::TimeDelta duration) {
+  ++sample_count_;
+  if (sample_count_ == kSampleInterval) {

[Sami, 2016/07/28 11:20:26]

+brianderson@, didn't we figure out that reducing the histogram sampling rate
doesn't actually reduce data volume? If so, we could just record the histogram
directly in the calling code.

[tdresser, 2016/07/28 12:15:50]

Yeah, sounds like that's the case.

[majidvp, 2016/07/28 13:30:11]

The other reason that we considered down-sampling was to avoid the CPU cost.
Looking at the histogram recording code it does a binary-search to find the
buckets and three atomic increments. This is compared to an increments & a
branch in down-sampled version.

To be honest I am not sure what the impact that difference makes so 
I suggest we check both against a micro-benchmark on Android and if we
don't see much difference we can just record every time.

+    UMA_HISTOGRAM_CUSTOM_COUNTS("RendererScheduler.TaskTime",
+                                duration.InMicroseconds(), 1, 1000000, 100);

[Ilya Sherman, 2016/07/28 00:31:30]

Do you really need 100 buckets, or would 50 suffice?

[Ilya Sherman, 2016/07/28 00:38:20]

(Expanding this comment, now that I've read through the review history some:)

What is your target granularity at which to see tasks?  What sort of questions
are you fundamentally trying to answer?  50 buckets tends to be a fairly good
default choice; and I didn't see any clear indication in the review discussion
so far as to what tradeoffs are being considered in choosing 100 buckets.  (Why
am I harping on this point?  Histogram buckets take up memory, both on the
client and on the server.  For any given histogram this isn't a big difference;
but across all of our histograms, it really does make a difference.  And, this
histogram will likely be reported with most buckets filled on most clients, so
it might actually impact the server-side storage quite a bit.)

FWIW,  I suspect that downsampling to 1 out of 100 tasks will not tend to affect
very much which buckets are filled on a given client ... but maybe you've
tested, and observed a difference?

[tdresser, 2016/07/28 12:15:50]

Nope, you're right, in terms of memory used, downsampling won't matter. I assume
that histogram recording is fast enough that we don't need to worry about the
performance of incrementing a histogram, even if we are doing it every task?

I think 100 buckets would be a reasonable choice here. Either that, or we'll
need multiple histograms, but I think this is more straight forward.

There are a variety of questions we want to answer here.
• What's the distribution of short tasks? 
  We want detail here in the 10 microsecond – 50ms range. 
  We could drop a few buckets off the bottom here I suppose.
• What's the distribution of long tasks?
  We want detail here in the 50ms – 1s range.
• How does the number of short tasks (<50ms) compare to the 
  number of long tasks (>50ms)?

Seeing the full distribution in detail will make us more sensitive to changes in
the distribution, as well as giving us more insight into the magnitude of tasks
that we see on the web.

[Ilya Sherman, 2016/07/28 17:47:41]

Let's be really concrete here.  With 50 buckets, you'd have these bucket
boundaries:

1us 2us 3us 4us 5us 7us 9us 12us 16us 21us 28us 37us 49us 65us 86us 113us 149us
196us 258us 340us 448us 590us 777us 1ms 1ms 1ms 2ms 3ms 4ms 5ms 7ms 9ms 12ms
16ms 21ms 27ms 36ms 48ms 63ms 83ms 110ms 145ms 191ms 252ms 332ms 437ms 576ms
759ms 1s

With 100 buckets, you'd have these:

1us 2us 3us 4us 5us 6us 7us 8us 9us 10us 11us 13us 15us 17us 19us 22us 25us 28us
32us 36us 41us 47us 53us 60us 68us 77us 88us 100us 114us 130us 148us 168us 191us
217us 247us 281us 320us 364us 414us 471us 536us 610us 695us 791us 901us 1ms 1ms
1ms 1ms 1ms 1ms 2ms 2ms 2ms 3ms 3ms 4ms 4ms 5ms 6ms 7ms 8ms 9ms 10ms 12ms 13ms
15ms 17ms 20ms 23ms 26ms 30ms 34ms 38ms 44ms 50ms 57ms 65ms 74ms 84ms 96ms 109ms
125ms 142ms 162ms 184ms 210ms 239ms 272ms 310ms 353ms 402ms 458ms 522ms 594ms
677ms 771ms 878ms 1s

With 75, you'd have these:

1us 2us 3us 4us 5us 6us 7us 8us 10us 12us 14us 17us 20us 24us 29us 35us 42us
50us 60us 72us 86us 103us 123us 147us 175us 209us 249us 297us 354us 422us 504us
601us 717us 855us 1ms 1ms 1ms 1ms 2ms 2ms 2ms 3ms 4ms 4ms 5ms 7ms 8ms 10ms 12ms
14ms 17ms 20ms 24ms 29ms 34ms 41ms 49ms 59ms 70ms 84ms 100ms 120ms 143ms 170ms
203ms 243ms 290ms 346ms 413ms 493ms 588ms 702ms 838ms 1000ms 

Where, specifically, do you not get quite enough resolution with 50 buckets? 
Are there values between 50 and 100 that might give you the desired resolution? 
Because this histogram is going to be emitted to very often, it will account for
a fairly sizable footprint of our data storage on the server side, so it's worth
trying to optimize the number of buckets somewhat.  That said, if you really
need the resolution that 100 buckets provides, then maybe that's the right
value.  I agree that it's better to have a single histogram that captures the
full range, rather than multiple histograms.

+    sample_count_ = 0;
+  }
+}
+}